Amos Jeffries [Fri, 17 Jun 2011 13:26:58 +0000 (07:26 -0600)]
Fix squidclient -V option and allow non-HTTP protocols to be tested
The "-" case is for old style HTTP (called 0.9) where there is no version
string. The "-V 0.9" is for testing servers with broken version number
tag "HTTP/0.9". Do not mix these up!
This also adds the ability to send non-HTTP version tags for testing.
ie "-V ICAP/1.0" or "-V ICY/1.0"
Fixed bypass of SSL certificate validation errors.
The bypass code was calling ACLChecklist::fastCheck() multiple times
if multiple certificate errors were found. That method should not be
called multiple times because it changes the internal ACLChecklist
state, producing wrong answers for repeated calls.
This patch fixes the ACLChecklist::fastCheck() method so it can be called
multiple times. Each fastCheck() call results in an independent access
list check.
Alex Rousskov [Fri, 17 Jun 2011 13:21:15 +0000 (07:21 -0600)]
Bug 3153: Prevent ICAP RESPMOD transactions getting stuck with the adapted body.
Part 1.
Server is expected to receive adapted response headers and then consume the
adapted response body, if any. If the server receives the headers and then
aborts, it must notify the ICAP side that nobody will consume the body.
Otherwise, the ICAP transaction will fill the BodyPipe buffer and get stuck
waiting for the consumer to free some space.
Part 2:
This fix still leaves one potential race condition unhandled: The ICAP
Initiatee disappears right after sending the adapted headers to the Server
(because there is nothing else for that initiatee to do). After the
noteAdaptationAnswer() call is scheduled by ICAP and before it is received by
the Server job, there is no usable link between Server and ICAP. There is no
way for the Server to notify the ICAP transaction that the Server job is
aborting during that time (and there is no Server job at all after it aborts,
naturally).
The solutions is to develop a custom AsyncCall which will call the
expectNoConsumption() on the message pipe if the call cannot be dialed (i.e.,
the message cannot be delivered to Server).
Bug 3214: "helperHandleRead: unexpected read from ssl_crtd" errors.
Squid would read the beginning of a crtd response split across multiple
read operations and treat it as a complete response, causing various
certificate-related errors.
This patch:
- allow the use of other than the '\n' character as the end of message mark
for helper responses.
- Use the '\1' char as end-of-message char for crtd helper. This char looks
safe because the crtd messages are clear text only messages.
Amos Jeffries [Sun, 29 May 2011 04:40:52 +0000 (22:40 -0600)]
URL re-writer handling bug fixes
This patch includes two bug fixes in URL handling which were uncovered
during testing of the URL logging update:
* URL re-write handling was not correctly creating its adapted request
copy. The code here is much reduced by using the clone() method. Still
not completely satisfactory (marked with XXX) since on invalid URL
there is a wasted cycles cloning and deleting almost immediately.
Future cleanups moving the URL parts outside HttpRequest will fix that.
* URL parsing needs to set the canonical field to unset whenever the URI
is re-parsed into a request. This field is an optimization for later
display speed-ups. This has been causing incorrect canonical URL to be
used following re-write. When the cloning above was corrected it caused
asserts in the server-side.
* To prevent memory leaks the urnParse() function internal to URL parsing
is adjusted to accept and update an existing request in identical API
semantics to urlParse() instead of always generating a new one.
Amos Jeffries [Sun, 29 May 2011 03:59:10 +0000 (15:59 +1200)]
Compile fixes for binutils-gold and gcc-4.6 support
These two tools are much stricter about dependency linkages.
* Drop testAuth due to major dependency loops they dislike.
* make many peviously implicit denendencies explicit
- Add tests/STUB.h with macros for simpler stub file creation
- Add tests/stub_DiskIOModule.cc for DiskIO main API
The bug appeared after commit with revno:11364 which fixes Bug 3192.
In the case of SSL-bumped connections the ConnStateData::flags.readMore flag
must be reset (set to true) when we are switching to HTTPs,
because we have to read the new unencrypted HTTP request.
This patch reset this flag in ConnStateData::switchToHttps method.
Uses hard-coded string "cachemgr.cgi/" instead of progname to avoid
complications from alternative names and when running under a browser.
May be elided in transit, however the VERSION sent here will help the
queried proxy respond appropriate to the CGI capabilities as we extend
the types and content of reports coming back from the future releases.
Portability Fix: getrlimit() / setrlimit() incompatible type 'struct rlimit'
On Linux (at least) with large file support but not full 64-bit environment.
The getrlimt / setrlimit are #define'd to getrlimite64 / setrlimit64
BUT, the struct rlimit internal fields are updated to 64-bit types individually
instead of a matching #define to struct rlimit64 as a whole.
One can only assume that GCC is casting to void* or some such major voodoo
which hides this type collision.
ICC: support 64-bit environments dirent definitions
struct dirent is not consistently defined for 32-bit and 64-bit enabled
environments. Provide a dirent_t type defined appropriate to the environment
for use instead.
This npending test bug was preventing any poll() errors from being
noticed and displayed. Possibly leading to some of the weird hanging
reports we have been unable to replicate.
Portability: Provide stdio wrappers for 64-bit in cstdio C++ builds
stdio.h in that case provides fgetpos64, fopen64 if
__USE_FILE_OFFSET64 is defined. It then checks whether a gcc-specific
__REDIRECT macro is available (defined in sys/cdefs.h, depending on
__GNUC__ begin available).
If it is not available, it does a preprocessor #define.
Which <cstdio> undefines, with this comment:
"// Get rid of those macros defined in <stdio.h> in lieu of real functions.".
When it does a namespace redirection ("namespace std { using ::fgetpos; }")
it goes blam, as fgetpos64 is available, while fgetpos is not.
To fix it, we need to supply global functions matching those
signatures (not macros).
Display ERROR in cache.log for invalid configured paths
The validator that checks system paths for files and directories in the
configuration file sends error messages to stderr. It should send them to
cache.log for the admin to see easily.
Also, this makes the error display as FATAL ERROR when using -k parse to
indicate that it is fatal to the startup. Other management signals where
it is not necessarily fatal will only display as an ERROR.
Within reason. Check that at least the port matches. That gives us some
small measure of reason to believe its the same protocol inside or the
same app being CONNECTed to.
Alex Rousskov [Sun, 3 Apr 2011 12:03:58 +0000 (06:03 -0600)]
Optimization (performance regression fix): Use bigger buffer for server reads.
Change the server read buffer limits to 16KB minimum and 256KB maximum.
Used to be: 2KB and 2GB. And before r9766: 4KB and SQUID_TCP_SO_RCVBUF.
Trunk r9766 (Remove limit on HTTP headers read) made the default HTTP
server read buffer size 2KB instead of 4KB, visibly slowing down Squid
when kernel network buffers are full and can sustain larger Squid reads.
Doing up to twice as many network reads is expensive (and probably not
just because of the extra system call overheads).
We never grow that buffer size if the _parser_ does not need a bigger
buffer: Even if the HTTP client is slower than the server, the buffer
stays small because it gives all the data to Store and Store eventually
just stalls reading via delayAwareRead() and read_ahead_gap. The
situation may be different with RESPMOD, but if the adaptation service
is fast, the buffer would still not grow.
This change does not reset the minimum buffer size to the old 4KB
default because memory is much cheaper compared to the days where that
default was set. 8KB may have worked too, but with 12KB median typical
response size a larger buffer may be a good idea for a busy Squid. More
performance work is needed to find the optimal value (which could depend
on the environment).
This change does not set the maximum buffer size to the current 2GB
limit because we have not tested how header and chunking parsers would
cope with malicious messages trying to run Squid out of RAM; and also
because no good parser should need that much lookahead space. Is 256KB
enough for all legitimate real-world response headers? We do not know.
It is tempting to use Config.tcpRcvBufsz or SQUID_TCP_SO_RCVBUF to find
the right minimum or maximum buffer size, but those parameters deal with
low-level TCP buffering aspects while this buffer deals with HTTP
parsing.
Alex Rousskov [Sun, 3 Apr 2011 11:33:34 +0000 (05:33 -0600)]
Make cache.log less noisy about usually benign events outside of admin control.
Raised reporting level from 1 to 2 for 'statusIfComplete: Request not yet
fully sent' and 'clientProcessRequest: Invalid Request' messages after
observing important information drowning in their noise on busy proxies.
Alex Rousskov [Sun, 3 Apr 2011 11:23:21 +0000 (05:23 -0600)]
Report eCAP service [re]start to cache.log by default.
Reporting eCAP services may be important, especially since we have no other
interface to detect their presence and since folks will have to deal with
rogue services eventually.
Also raised eCAP service configuration notice level to 2.
Apply uri_whitespace before logging malformed requests
This patch try to implement the first option from those described at the
squid-dev thread with subject "Request URI logging for malformed requests":
http://www.squid-cache.org/mail-archive/squid-dev/201101/0004.html
Currently the logged URI set using the setLogUri method (in client_side.cc and
client_side_reply.cc files). Also the setLogUri called at least two times for
every normal request. Moreover the setLogUri always check if the URI contains
characters which must escaped which in the case of normal requests it is not
needed because urlCanonicalClean always used before pass the URI to setLogUri.
This patch:
- add a parameter to the setLogUri to say if the URI must cleaned and the
uri_whitespace filtering must applied.
- Remove the setLogUri call from the parseHttpRequest.
- Call in all cases (HTTP request error or not) the setLogUri in
clientProcessRequest
- In the case the URL is not a valid HTTP request applies the uri_whitespace
filtering.
- In the case the URI is valid the uri_whitespace filtering is not required
because it is already applied by the urpParse function.
Alex Rousskov [Sun, 3 Apr 2011 11:05:39 +0000 (05:05 -0600)]
Give full Request-URI to eCAP adapters.
Implement libecap::RequestLine::uri() to return full Request-URI instead
of URL path.
Niether full URL nor URL path is perfect because the actual request may
have full URI or a path, but Squid does not really keep that
information. This change makes our eCAP implementation consistent with
our ICAP implementation.
Eventually, eCAP may have an API that is guaranteed to return full
Request-URI and Squid may remember what kind of URI it got in the virgin
request, allowing for a more truthful implementation.
Make DNS report failure on all packet construction errors
The attached patch alters the DNS lookup behaviour to abort with an error
in ALL cases where the rfc1035 library generates an error (negative result).
I'm not sure there is any noticable effect other than better code. The
error case *should* in old code be picked up on the initial packet
construction rather than the repeat packet. This may have been incorrect
given that the packet type is changing between A/AAAA.
Marco Beck [Fri, 1 Apr 2011 01:21:46 +0000 (19:21 -0600)]
Regression fix: Replacing reply headers
Restores the functionality to replace reply headers as found in Squid 2.
header_replace worked for both request and reply headers back then.
The creation of request_header_access and reply_header_access altered
replace_headers to only work on requests. It should have received this
name split back then.
Abort with an error when a wildcard entry is going to be
discarded because of a sub-domain entry.
Also whenever there is a mixup between a domain and its
sub-domain wildcard alternative.
Raise a non-fatal warning when a useless subdomain entry
is being discarded and its super-set wildcard kept.
Care is taken to present the singular subdomain for
possible removal and keep the wildcard.
Amos Jeffries [Wed, 30 Mar 2011 12:38:06 +0000 (06:38 -0600)]
squidclient: send cachemgr password via -w option
Preparation for internal cachemgr updates to use real proxy-auth.
The cachamgr password may now be sent in three ways:
Deprecated: mgr:info@password
Current Option: -w password mgr:info
Preferred: -u username -w password mgr:info
The old explicit @ syntax is now deprecated for visible use. The background
systems will still send it that way for cache_object: URLs. Use of this
overrides any -w option set. So it is still possible to login to a proxy
with one set of credentials and pass a separate password to the cachemgr.
The long-term plan is to drop @ completely in future.
The current option of just -w will convert the password to @ syntax in the
background but not add Proxy-Authentication headers. This may die in future.
The preferred alternative is to use -u and -w which triggers addition of real
Proxy-Authenticate headers. The username is not yet used by cachemgr but
may be required by the proxy ACL configuration.
Alex Rousskov [Wed, 30 Mar 2011 12:02:11 +0000 (06:02 -0600)]
Bug 2621: Provide request headers to RESPMOD when using cache_peer.
A short-term fix.
When FwdServer::_peer is set, HttpStateData constructor creates a new special
HttpRequest, overwriting the request pointer set in the parent
(ServerStateData) constructor to fwd->request.
To make matters worse, this special peer request has no headers at all (even
though flags and some cached/computed header values are copied). We initialize
it with the right URL, method, and protocol. We copy flags and a few other
random properties from the original request. We never copy the original
headers.
Furthermore, regardless of the peering, when we create the headers to send to
the next hop, those headers are temporary and not stored in any request
structure (see HttpStateData::buildRequestPrefix). The non-peering code
survives this because the request member points to fwd->request, which has the
headers. The peering code fails as illustrated by this bug.
I believe both cases are buggy because server-side adaptation and core code
should have access to the request headers we sent rather than the request
headers we received and adapted (or no headers at all). After all, it is the
sent headers that determine the next hop view of our Squid and adaptation
services should see a pair of _matching_ request and response headers.
I am pretty sure there are other bugs related to HttpStateData using a special
peer request structure instead of fwd->request. Please note that FwdState has
no idea that this substitution is going on.
This quick short-term fix uses the original request and its headers when
checking RESPMOD ACLs. This is what the patch in bug #2562 did for Squid v3.0.
For the reasons described above, this patch may be either insufficient or
wrong for the long-term fix.
Alex Rousskov [Wed, 30 Mar 2011 11:53:48 +0000 (05:53 -0600)]
Bug 3173: Assertion bodyPipe!=NULL on SslBump CONNECT response writing failure
Do not call ConnStateData::startClosing() when we fail to write our CONNECT
response while bumping a connection. startClosing() can only be used when we
handle response bodies. Just close the connection, in hope that the connection
close handler kicks in and cleans up.
Frank Schmirler [Mon, 28 Mar 2011 11:31:28 +0000 (05:31 -0600)]
Bug 2330: AuthUser objects are never unlocked
This is a partial port of the Buug 2305 auth fixes.
These changes involve combining the auth operation state links to
credentials data such that the shared code can lock/unlock them properly.
Amos Jeffries [Tue, 22 Mar 2011 12:04:26 +0000 (06:04 -0600)]
Bug 2976: invalid URL on intercepted requests during reconfigure
Listening ports abuse the cbdata type as a pseudo refcount. This breaks
during reconfigure when the config is erased and the active requests
handles all become invalid pointers.
Interception only works on HTTP protocol. We can hard-code the scheme
and avoid this problem until a complete fix is written.
projects / thirdparty / squid.git / log
