Amos Jeffries [Sun, 30 Jan 2011 05:24:12 +0000 (22:24 -0700)]
Compat: static functions cannot be passed externally with some compilers
Solaris StudioCC, HPUX and old GCC complain about statics being passed
by pointers outside the current file.
Elected to drop 'static' instead of adding a bunch of specific wrapper hacks.
Amos Jeffries [Sun, 30 Jan 2011 05:20:41 +0000 (22:20 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Handle early eCAP transaction failures better.
Do not throw an exception if eCAP transaction had to deal with a virgin body
but was not consuming it at swangSong() time. This may happen if the eCAP
adapter throws an exception before the adapter requests the virgin body
transmission or after it stops the transmission. In other words, the
transaction wrapper consumes only if proxyingVb is on.
Amos Jeffries [Fri, 14 Jan 2011 06:30:28 +0000 (23:30 -0700)]
HTTP/1.1 support: Send 307 status on deny_info redirection
This makes Squid send an HTTP/1.1 307 status response to 1.1+ clients if
the deny_info directive is used to redirect non-GET/HEAD requests.
Current behaviour is to use a 302, which browsers will prevent
displaying for security protection against injection attacks. Using 307
will give browsers a better chance to identify the redirects and handle
them safely.
Amos Jeffries [Fri, 14 Jan 2011 05:02:13 +0000 (22:02 -0700)]
Author: Henrik Nordstrom <hno@squid-cache.org>
Support RFC 5861 Cache-Control: stale-if-error option
The default behaviour for Squid is to present the stale object when
revalidation fails with a 5xx error.
stale-if-error places a maximum limit on how long this stale object may
be sent. After the limit has passed Squid is required to present the 5xx
message to the client.
Original code for Squid-2 was sponsored by Yahoo!.
Amos Jeffries [Tue, 21 Dec 2010 00:53:56 +0000 (17:53 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Avoid comm_read "!fd_table[fd].closing()" assertion after adaptation ACL check
The assertion was hit if Server fd was closed while we were checking
adaptation ACLs, and we have not been notified of the closure yet (because the
Adaptation::AccessCheck callback is not async while closure notification is).
Amos Jeffries [Tue, 21 Dec 2010 00:52:56 +0000 (17:52 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Polished HttpStateData::persistentConnStatus() code. No functionality changes.
Do not check for flags.headers_parsed. The removed check was:
- misplaced: connection-related conditions such as eof must be checked first;
- wasteful: we never call persistentConnStatus() unless we parsed headers.
Moreover, calling persistentConnStatus() before we parse headers would trigger
and assertion because the method uses virginReply() which does not exist until
the headers are parsed.
Moved virginReply() call closer to the first virgin reply use. This will help
re-adding "did we parse the header yet" check if we ever need it again. It
also saves a couple of CPU cycles for some transactions.
Amos Jeffries [Tue, 21 Dec 2010 00:42:37 +0000 (17:42 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Prevent memory leaks when Adaptation::AccessCheck callback ends the job.
The AccessCheckCallbackWrapper is used in nonBlockingCheck() and is called
from the ACL code, using legacy function-based API. If the job ends during
the callback processing, there are no AsyncCall wrappers to destroy the job
object. We now convert legacy to async call to enable proper wrapping and job
destruction.
These kind of job leaks are invisible to valgrind, but that is another bug.
Amos Jeffries [Mon, 20 Dec 2010 23:44:21 +0000 (16:44 -0700)]
Reduce debug level on bodypipe re-write change
- the original reason for adding is unknown
- it is an annoyance for some
- there have been no big problems tracked down to this bodypipe change
over the last few years. It appears relatively harmless.
Amos Jeffries [Mon, 20 Dec 2010 13:37:07 +0000 (06:37 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Author: Dmitry Kurochkin <dmitry.kurochkin@measurement-factory.com>
Bug 427: HTTP Compliance: Support If-Match and If-None-Match requests.
Add support for If-Match and If-None-Match headers as described in RFC 2616
(sections 14.24 and 14.26 in particular).
Moved IMS handling from clientReplyContext::cacheHit() to
clientReplyContext::processConditional() while preserving the original IMS
logic, except for the case when a request has both IMS and If-None-Match.
Co-Advisors test cases:
test_clause/rfc2616/ifMatch-mismatch-strong
test_clause/rfc2616/ifMatch-mismatch-weak
test_clause/rfc2616/ifNoneMatch-match-imsNone
and many more
Amos Jeffries [Mon, 20 Dec 2010 06:12:57 +0000 (23:12 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP/1.1: do not forward TRACE with Max-Forwards: 0 after REQMOD
Before the change, Max-Forwards request value was cached in
HttpRequest::max_forwards member. It was set once in
clientProcessRequest() function. This works fine as long as no request
adaptation is performed. Otherwise original HTTP request may be
replaced with adopted one in ClientHttpRequest::noteAdaptationAnswer()
method and max_forwards value is lost.
This change removes HttpRequest::max_forwards member and gets the value
directly from HttpHeader when needed. This adds another string-to-int
conversion for TRACE and OPTIONS requests, but those are rare, and we
save a little in the other, far more common cases by removing the
HttpRequest::max_forwards member.
Removed assertion from clientReplyContext::traceReply() since it is
called from a single place and the condition is checked right before
the call.
Co-Advisors test cases:
test_case/rfc2616/maxForwardsZero-TRACE-asterisk
test_case/rfc2616/maxForwardsZero-TRACE-absolute
and response_is_fresh is always false if freshness_lifetime is zero.
The check code was introduced in r5998 with a "Import of fix-ranges
branch" message. The code was commented out at the time of that
commit, for reasons unknown.
Test case:
test_case/rfc2616/noSrv-hit-stale-max-age-req
Amos Jeffries [Sat, 18 Dec 2010 12:46:13 +0000 (05:46 -0700)]
Author: Henrik Nordstrom <henrik@henriknordstrom.net>
Make bootstrap.sh use system default autotools versions instead of searching
- can be overridden on commandline if needed. See bootstrap.sh for details.
- Update autoconf to 2.68
- Rename configure.ac to match current autotools standards
Amos Jeffries [Fri, 17 Dec 2010 19:46:13 +0000 (12:46 -0700)]
Upgrade process for obsolete options
One problem we currently have with upgrades is leaving the parser able
to avoid its bungled/unknown option message for directives which have
been fully removed or massively syntax altered.
We are able to handle this for flags and option syntax easily but the
parser has been particularly dense and strict on the directives (first
word of each line).
This patch updates the cf_* and cfgman code to allow a special directive
type "obsolete" which causes these directives to be handled specially
without causing the directives to remain in the publicly visible
squid.conf documentation.
It allows DOC_START / DOC_END comments to be written in cf.data.pre
describing the upgrade actions that need to be taken. This text is
dumped to cache.log verbatim when the configuration option is sighted.
If "-k parse" is used the text is displayed at debug level 0, otherwise
displayed at debug level 1. One line indicating a generic "directive X
is obsolete" is always displayed at level 0 for backwards compatibility
with admin expectations of a high level "bungled" message.
After all this text display, parse_obsolete(char*) is called with the
directive name. This function exists in cache_cf.cc and can be coded to
selectivey do more complex handling of the directive. ie for upgrade
actions deeper than removal.
* cf.data.pre has entries added for all the 2.6-3.1 directives I could
find that were removed.
Amos Jeffries [Fri, 17 Dec 2010 18:56:56 +0000 (11:56 -0700)]
Author: Graham Keeling <graham@equiinet.com>
Bug 3113: Squid can eat far too much memory when uploading files
Problem description:
Uploading a large file to a web site on the internet, squid's client
input buffer will increase far faster than it can be emptied to
the target website, and the machine will swiftly run out of memory.
This patch adds the client_request_buffer_max_size configuration
parameter which specifies the maximum buffer size of a client request.
Amos Jeffries [Mon, 6 Dec 2010 02:14:40 +0000 (19:14 -0700)]
Author: Stefan Fritsch <sf@sfritsch.de>
Bug 3096: Squid destroys CbDataList<DeferredRead> objects too late
When server download speed exceeds client download speed, Squid creates a
CbDataList<DeferredRead> object and associates a comm_close handler with it.
When the server kicks the deferred read, the comm_close handler is canceled.
This create/cancel sequence happens every time the server-side code wants to
read but has to wait for the client, which may happen hundreds of times per
second.
Before this change, those canceled comm_close handlers were not removed from
Comm until the end of the entire server transaction, possibly accumulating
thousands of CbDataList<DeferredRead> objects tied to the socket descriptor
via the canceled but still stored close handler.
comm_remove_close_handler now immediately removes canceled close handlers to
avoid their accumulation.
Amos Jeffries [Sun, 7 Nov 2010 10:04:54 +0000 (03:04 -0700)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 3091: Bypassed ICAP errors are not counted as service failures.
Notify ICAP service about the failure even if we can bypass it. Otherwise,
a failing service may continue to stay "up", preventing Squid from using a
healthy backup alternative in a service_set (or bypassing ICAP completeley).
Amos Jeffries [Mon, 1 Nov 2010 05:41:38 +0000 (23:41 -0600)]
Bug 3090: Polish FTP login error handing
Reverts a regression added recently that blocked the challenge events.
Fixes another potential nul-pointer dereference bug.
* 421/426 server overload equate to HTTP overload. But do special such that
the credentials are asked of the browser on retries.
* 43x and 53x FTP status are all credentials failures of various types.
Other failures are not credential related.
This leaves the other non-credential errors as general failures.
Amos Jeffries [Sat, 23 Oct 2010 13:54:37 +0000 (07:54 -0600)]
Author: Christophe Saout <christophe@saout.de>
Bug 3084: IPv6 without Host: header in request causes connection to hang
accel and intercept mode URL re-generation used NtoA instead of ToHostname.
This results in the URL incorrectly wrapping the raw-IPv6 and problems
connecting to non-existent addresses in some cases.
Amos Jeffries [Sat, 23 Oct 2010 13:43:17 +0000 (07:43 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: do not cache replies to requests with CC/no-store.
Per RFC 2616, do not store any part of response to requests with a no-store
Cache-Control directive.
We may still _serve_ no-store requests from the cache because RFC 2616 does not
seem to prohibit that. This may change if HTTPbis or developers decide to
prohibit no-store hits.
Co-Advisor test case:
test_case/rfc2616/ccReqDirMsg-no-store-basic
Amos Jeffries [Sat, 23 Oct 2010 13:42:20 +0000 (07:42 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: reply with 504 (Gateway Timeout) if required validation fails.
RFC 2616 says that we MUST reply with 504 (Gateway Timeout) if validation
fails and cached reply has proxy-revalidate, must-revalidate or s-maxage
Cache-Control directive.
FwdState::makeConnectingError() method is added to set error status depending
on whether the request was a validation request.
Co-Advisor test cases:
test_case/rfc2616/noSrv-hit-must-reval-s-maxage-resp
test_case/rfc2616/noSrv-hit-must-reval-proxy-revalidate-resp
test_case/rfc2616/noSrv-hit-must-reval-must-revalidate-resp
Amos Jeffries [Sat, 23 Oct 2010 13:41:01 +0000 (07:41 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: support requests with Cache-Control: min-fresh.
Added min-fresh directive support for Cache-Control header. The directive is
handled in refreshCheck() by incrementing age and check_time by min-fresh
value.
Co-Advisor test case:
test_case/rfc2616/ccReqDirMsg-min-fresh-obey
Amos Jeffries [Sat, 23 Oct 2010 13:39:03 +0000 (07:39 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: add appropriate Warnings if serving a stale hit.
Per RFC 2616, we MUST add "110 Response is stale" Warning if serving a
stale reply for any reason, including configured overrides. We MUST add
"111 Revalidation failed" Warning if serving a stale reply because an
attempt to revalidate the response failed, due to an inability to reach
the server.
The patch adds a new stale_if_hit request flag, which is set in
refreshCheckHTTP() when entry freshness is calculated. refreshCheckHTTP()
is now called in offline mode, to set stale_if_hit properly. We check for
the offline mode before returning from refreshCheckHTTP() to preserve the
original logic.
refreshCheckHTTP() is no longer called for internal requests, to avoid
setting of stale_if_hit flag. It did not do anything important for
internal requests anyway.
Co-Advisor test cases:
test_case/rfc2616/noSrv-hit-stale-max-age-req
test_case/rfc2616/ccReqDirMsg-max-stale-warning
Amos Jeffries [Sat, 23 Oct 2010 13:36:27 +0000 (07:36 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Accept ICAP OPTIONS responses with unknown body types.
Warn about the unknown OPTIONS body type but ignore it instead of rejecting
the entire OPTIONS response. Note that ICAP does not standardize OPTIONS
body types, and Squid does not recognize any body type.
ICAP servers are supposed to negotiate the use of OPTIONS bodies but
negotiation mechanism is not standardized and some do not negotiate at all.
Amos Jeffries [Sat, 23 Oct 2010 13:29:15 +0000 (07:29 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: Reply with an error if required validation fails.
RFC 2616 says that proxy MUST not use stale entries that have s-maxage,
proxy-revalidate, or must-revalidate cache-directive.
Add new fail_on_validation_err request flag to store result from
refreshCheck(). It is needed to avoid refreshLimits() recalculation in
clientReplyContext::handleIMSReply().
Split LOG_TCP_REFRESH_FAIL into LOG_TCP_REFRESH_FAIL_OLD (stale reply sent)
and LOG_TCP_REFRESH_FAIL_ERR (error forwarded). However, both are still logged
as TCP_REFRESH_FAIL for backward-compatibility with external scripts and such.
We may decide to start logging more detailed codes later.
Co-Advisor test cases:
test_case/rfc2616/noSrv-hit-must-reval-s-maxage-resp
test_case/rfc2616/noSrv-hit-must-reval-proxy-revalidate-resp
test_case/rfc2616/noSrv-hit-must-reval-must-revalidate-resp
Amos Jeffries [Wed, 20 Oct 2010 06:09:27 +0000 (00:09 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Cleanup: Remove old_rep2 from clientReplyContext::handleIMSReply().
StoreEntry::getReply() returns pointer to HttpReply that belongs to MemObject.
It does not create a new object. Hence, outer old_rep, inner old old_rep, and
inner new old_rep2 in handleIMSReply() are equal.
Amos Jeffries [Wed, 20 Oct 2010 06:06:14 +0000 (00:06 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: Make date parser stricter to better handle malformed Expires.
Check that there is no zone or zone is "GMT" in parse_date_elements().
Make sure there is no junk at the end of date in parse_date().
This will affect Date, IMS, and other date-carrying header fields recognized
by Squid but should not cause any messages to be rejected. Squid would just
ignore the malformed headers as if they are not there.
Co-Advisor test case:
test_case/rfc2616/invalidExpiresMakesStale-rfc1123x
Amos Jeffries [Wed, 20 Oct 2010 05:25:13 +0000 (23:25 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Prohibit fruitless modification of httpBuildRequestHeader flags parameter.
HttpStateData::httpBuildRequestHeader is a static method, but it has a
parameter called flags, just like HttpStateData objects have a data member
called flags. Modifying that parameter "worked" but had no effect on the
caller's flags. Wasted a few good hours.
The parameter is "const" now, to prevent fruitless modification.
Also removed http_state_flags parameter from HttpStateData::buildRequestPrefix
which is not a static method and has access to the "real" flags member.
No runtime effect expected.
TODO: Rename HttpStateData::httpBuildRequestHeader to mark its static nature.
Does it belong to HttpStateData at all?
Amos Jeffries [Wed, 20 Oct 2010 05:21:34 +0000 (23:21 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Handle ICAP persistent connection races better.
When an ICAP transaction loses a persistent connection race with an ICAP
server (i.e., Squid sends the ICAP request on a persistent connection just
closed by the ICAP server), the transaction throws and the exception is
treated as a regular error. Even though the transaction may be retried, the
negative side-effects may include ICAP service suspension due to transaction
failures.
This patch logs ICAP transactions that fail due to pconn races with
ERR_ICAP_RACE status and does _not_ blame the ICAP service for the failure.
The following problem was exposed by the pconn races but its fix is
useful in other scopes as well:
When the ICAP connection times out, we now close the connection before
throwing because an exception may be bypassed, and we will throw again (during
peaceful bypass) if Comm tells us that the connection is ready after we timed
out (yes, that can happen because Comm timeouts do not auto-close the
connection).
Amos Jeffries [Wed, 20 Oct 2010 05:13:44 +0000 (23:13 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Keep ICAP connection persistent after an OPTIONS response if possible.
Old OptXact code used Xaction::doneReading() which was only true at EOF.
Thus, closeConnection() was thinking we are still reading (i.e., not
doneWithIo) at the end of the transaction and force-closed the connection.
New OptXact implements its own doneReading() which returns true at EOF and
when we read the entire response. Since we cannot parse OPTIONS body and
do not know where it ends, OPTIONS responses with bodies still result in
connection closure. We should not be getting any, but there are broken ICAP
servers that do send them. Hopefully, they all use Encapsulated: opt-body.
Amos Jeffries [Wed, 20 Oct 2010 05:03:14 +0000 (23:03 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: delete Warnings that have warning-date different from Date.
Added HttpReply::removeStaleWarnings() method that iterates over all Warning
headers and removes stale warning-values. If a reply has no valid Date header,
all warning-values with warning-date are removed. Also, we remove
warning-value if we failed to parse warning-date.
removeStaleWarnings() is called from processReplyHeader(), after reply headers
are parsed.
Co-Advisor test cases:
test_case/rfc2616/date-accept-fmt-warn-asctime-rfc1123
test_case/rfc2616/date-accept-fmt-warn-asctime-rfc850
test_case/rfc2616/date-accept-fmt-warn-rfc1123-asctime
test_case/rfc2616/date-accept-fmt-warn-rfc1123-rfc850
test_case/rfc2616/date-accept-fmt-warn-rfc850-asctime
test_case/rfc2616/date-accept-fmt-warn-rfc850-rfc1123
Amos Jeffries [Fri, 8 Oct 2010 03:25:56 +0000 (21:25 -0600)]
Author: Henrik Nordstrom <henrik@henriknordstrom.net>
Bug 3056: comm.cc "!fd_table[fd].closing()" assertion from helperServerFree when a helper crashes while processing requests
reshuffle helperServerFree so it first unregisters the failed helper
and starts new ones if needed before it calls the callbacks on any
pending requests. If not those ends up resheduling the request on
this same crashed and partially shut down helper.
Author: Stefan Fritsch <sf@sfritsch.de>
Bug 3058: errorSend and ICY leak MemBuf object.
HttpReply::pack() returns a MemBuf pointer that must be deleted. Fixed leaking
errorSend() function in errorpage.cc and ICY parsing hack in
HttpStateData::processReplyHeader().
Bug 3053: cache version 1 LFS support detection broken
Alter the upgrade detection to only be used if needed, and to load the
correct on-disk sizes for the fields being converted.
Lock the sfileno type down to 32-bits.
This also adds two new upgrade parsers:
Fix 64-bit builds previously using sfileno as full 64-bit int
Migrates swap.state from 32-bit to 64-bit time_t (old squid 2.4/2.5)
64-bit->32-bit system time_t migration remains broken.
Per-file meta data is not altered by these changes, so problems may
remain there.
Author: Alex Rousskov <rousskov@measurement-factory.com>
Added expectNoConsumption() BodyPipe method to allow the consumer side to
tell the pipe that no more consumers are expected.
Calling expectNoConsumption() is needed when all consumers are gone but none
of them consumed anything. This is typical for HTTP server-side transactions
that abort before they receive the adapted ICAP request body. If FwdState
quits without calling expectNoConsumption(), the ICAP transaction supplying
the request body may get stuck waiting for buffer space. The body pipe may be
the ICAP only link with the master transaction once the header is handled.
This change is related to Squid bug #2964.
Based on lp 3p1-rock branch, r9609.
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 2964: Prevent memory leaks when ICAP transactions fail.
We now make sure that heap-allocated objects are deleted if an exception
is thrown before the object pointers are saved/registered in a safe location
like a data member.
Assigning state.serviceWaiting=true after calling callWhenReady() in ModXact
prevents ModXact leak when callWhenReady() throws. This may need more work
to mark ModXact state appropriately for the adaptation log.
Based on lp 3p1-rock branch, r9610.
Added doneWithRetries() and used it to inform the request body sender that
there will be no more body consumers. This allows the sender (e.g., an ICAP
REQMOD transaction) to quit instead of waiting for body buffer space forever.
Moved !self check into checkRetry() because we need to call doneWithRetries()
even if self is nil. The move should not affect the old code.
Based on lp 3p1-rock branch, r9613.
At the end of preview, do not go into the writingPaused state if we already
received the final response from the ICAP server. Instead, stop writing so
that we do not get stuck waiting for the response that has already come.
May also handle header-only (zero-size) ieof Preview better.
TODO: Convert other HttpMsg pointer members to use safe HttpMsg::Pointer.
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 2311: crashes with ICAP RESPMOD for HTTP body size greater than 100kb
BodyPipe::undoCheckOut() must not assert that undo is possible because it
is not (currently) possible if the pipe buffer was modified.
BodyPipe::undoCheckOut() must not throw if undo is not possible because it is
often called when there is already an exception thrown and because it is
called from the BodyPipeCheckout destructor and destructors should not throw
(this case is an illustration for one reason why they should not).
Currently, we only use an implicit undo, and only when an exception
is being thrown while the buffer is checked out.
Currently, the code that does checkout is probably safe because it should
terminate the transaction if a parser throws. However, this is not 100%
guaranteed, and the situation may change without us noticing.
TODO: consider implementing the long-term solution discussed at
http://www.mail-archive.com/squid-dev@squid-cache.org/msg07910.html
COW-buffers may help here as well
projects / thirdparty / squid.git / log
