Alex Rousskov [Mon, 4 Oct 2010 15:44:32 +0000 (09:44 -0600)]
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
Alex Rousskov [Mon, 4 Oct 2010 15:40:04 +0000 (09:40 -0600)]
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
Alex Rousskov [Mon, 4 Oct 2010 15:32:20 +0000 (09:32 -0600)]
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
Alex Rousskov [Mon, 4 Oct 2010 15:16:18 +0000 (09:16 -0600)]
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
Alex Rousskov [Mon, 4 Oct 2010 14:52:38 +0000 (08:52 -0600)]
Use InstanceId for async job and calls identification.
Side-effect: removes inconsistent prefixes for job debugging: ecapxN, icapxN,
asyncN are now all jobN, simplifying searching and processing debugging logs.
Alex Rousskov [Mon, 4 Oct 2010 14:37:32 +0000 (08:37 -0600)]
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.
Alex Rousskov [Tue, 28 Sep 2010 15:20:36 +0000 (09:20 -0600)]
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
Alex Rousskov [Mon, 27 Sep 2010 18:42:05 +0000 (12:42 -0600)]
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.
will have an effect for kids 1 through 4 only and will place them on even
cores starting with core #1.
If there are conflicts for a given process, the latest option wins and a
warning is printed. If the number of specified processes do not match the
number of specified cores, Squid quits with an error. Multiple
cpu_affinity_map options are merged.
Squid builds on systems without Linux CPU affinity calls, but setting affinity
only works if there are sched_getaffinity(2) and sched_setaffinity(2)
available. If there is no OS support but cpu_affinity options are configured,
Squid quits with an error. If there is OS support but calls fail, Squid prints
an error but does not quit.
Alex Rousskov [Wed, 22 Sep 2010 23:09:58 +0000 (17:09 -0600)]
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
Henrik Nordstrom [Mon, 20 Sep 2010 19:27:24 +0000 (21:27 +0200)]
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.
Alex Rousskov [Mon, 20 Sep 2010 19:00:02 +0000 (13:00 -0600)]
Author: Stefan Fritsch <sf@sfritsch.de>
Bug #3058 fix: 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().
Alex Rousskov [Mon, 20 Sep 2010 17:47:46 +0000 (11:47 -0600)]
Bug #3048 fix: "commio_has_callback(fd, IOCB_READ, ccb)" assertion;
may also be applicable to a similar IOCB_WITE assertion.
When we start closing a descriptor, we call commio_finish_callback() to remove
I/O callbacks. If this is not done from commHandleRead or commHandleWrite,
then select(2) structures may still have our descriptor registration and will
call Comm back to read or write before the descriptor is closed for good. This
will trigger a commio_has_callback() assertion.
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.
Alex Rousskov [Tue, 14 Sep 2010 07:45:30 +0000 (01:45 -0600)]
Do not send chunked requests without a "Transfer-Encoding: chunked" header
or with a "Content-Length: 0" header.
Whether we are sending a chunked request depends not just on whether the
received request was chunked (condition A) but also on whether we still do not
know the request body size (condition B). The old code added the
"Transfer-Encoding: chunked" header if (A && B) but chunked the request body
if (A). This resulted in malformed requests with chunked request bodies but
without the "Transfer-Encoding: chunked" header.
When adding the Transfer-Encoding field, the old code also considered zero
Content-Length as "unknown", which was, apparently wrong. This resulted in the
"Content-Length: 0" header sent with a chunked encoded [empty] body, violating
HTTP rules. I am not 100% sure we never use zero request->content_length value
to mark "unknown" length though, so this may need more work.
Alex Rousskov [Tue, 14 Sep 2010 05:40:33 +0000 (23:40 -0600)]
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?
Alex Rousskov [Mon, 13 Sep 2010 00:34:44 +0000 (18:34 -0600)]
Partial bug #2964 fix: writer stuck in a "waiting for preview answer" state.
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.
Alex Rousskov [Sun, 12 Sep 2010 22:35:35 +0000 (16:35 -0600)]
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).
Alex Rousskov [Sun, 12 Sep 2010 21:58:38 +0000 (15:58 -0600)]
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.
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.
Alex Rousskov [Sun, 12 Sep 2010 00:04:20 +0000 (18:04 -0600)]
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.
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.
TODO: Convert other HttpMsg pointer members to use safe HttpMsg::Pointer.
Alex Rousskov [Fri, 10 Sep 2010 21:12:54 +0000 (15:12 -0600)]
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
Alex Rousskov [Fri, 10 Sep 2010 20:56:24 +0000 (14:56 -0600)]
Compliance: Forward 1xx control messages to clients that support them.
Forward 1xx control messages to all HTTP/1.1 clients and to HTTP/1.0
clients that sent an Expect: 100-continue header unless the 1xx message
fails the optional http_reply_access check described below.
RFC 2616 requires clients to accept 1xx control messages, even if they
did not send Expect headers. However, 1xx control messages prohibited by
http_reply_access are ignored and not forwarded. This can be used to
protect broken HTTP/1.1 clients or naive HTTP/1.0 clients that
unknowingly forward 100-continue headers, for example. Only fast checks
are supported at this time.
The patch removes ignore_expect_100 squid.conf option and the
corresponding code because
- the reasons to treat 100-continue specially have changed since we
can now forward 1xx responses;
- rejection of 100-continue request can still be done using a
combination of the existing http_access and deny_info options;
- hiding of 100-continue header from next hops can still be done using
an existing request_header_access option;
- 100 (Continue) responses can be hidden from clients using
http_reply_access check described above.
We still respond with 417 Expectation Failed to requests with
expectations other than 100-continue.
Implementation notes:
We forward control messages one-at-a-time and stop processing the server
response while the 1xx message is being written to client, to avoid
server-driven DoS attacks with large number of 1xx messages.
1xx forwarding is done via async calls from HttpStateData to
ConnStateData/ClientSocketContext. The latter then calls back to notify
HttpStateData that the message was written out to client. If any one of
the two async messages is not fired, HttpStateData will get stuck unless
it is destroyed due to an external event/error. The code assumes such
event/error will always happen because when
ConnStateData/ClientSocketContext is gone, HttpStateData job should be
terminated. This requires more testing/thought, but should still be
better than not forwarding 1xx messages at all.
Alex Rousskov [Fri, 10 Sep 2010 20:06:41 +0000 (14:06 -0600)]
Dechunk incoming requests as they come in and chunk outgoing requests.
The server side always chunks the request if and only if the original request
was chunked. No next hop version checks are performed.
* Client-side changes:
Removed clientIsRequestBodyValid() as unused. It was called with a
content-length>0 precondition that made the function always return true.
Removed old dechunking hack that was trying to buffering the entire request
body, pretending that we are still reading the headers. Adjusted related
code. More work may be needed to identify client-side code that assumes
the request size is always known.
Removed ConnStateData::bodySizeLeft() because we do not always know how much
body is left to read -- chunked requests do not have known sizes until we read
the last-chunk. Moreover, it was possibly used wrong because sometimes we want
to know whether we want to comm_read more body bytes and sometimes we want to
know whether we want to "produce" more body bytes (i.e., copy already read
bytes into the BodyPipe buffer, which can get full).
Added ConnStateData::mayNeedToReadMoreBody() to replace conn->bodySizeLeft()
with something more usable and precise.
XXX: If there is a chunks parsing error, the new code just resets the
connection. I tried to find a way to send an error page to the client, but
failed to do so. It is easy to do when the headers and the body prefix is
parsed, but if the error is send later, the server-side may start sending us
its response, and the two responses would clash, causing assertions. I do not
know how to fully avoid that. Perhaps we need to unregister from Store before
responding with an error? Search for WE_KNOW_HOW_TO_SEND_ERRORS.
Tried to break deep recursion/iteration around clientParseRequest. When
chunked parser fails during the message prefix parsing, the rest of the code
may decide that the connection is no longer used (and that there is no pending
transaction, even though the currentobject member is not NULL!) and start
parsing the second request. If that second parse fails (for example), there
will be two concurrent errors to be sent to the client and the client-side
code cannot handle that. However, due to the XXX above, we never send an error
when chunking parser fails, making most of the related code polishing useless,
at least for now.
Removed my wrong XXX related to closing after initiateClose.
Removed my(?) XXX related to endless chunked requests. There is nothing special
about them, I guess, as a non-chunked request can be virtually endless as
well if it has a huge Content-Length value.
Use commIsHalfClosed() instead of fd_table[fd].flags.socket_eof for
consistency with other client-side code and to improve readability. I think
these should return the same value in our context but I am not sure.
Correctly handle identity encoding. TODO: HTTPbis dropped it. Should we?
Polished request_header_max_size handling code, converting old
connKeepReadingIncompleteRequest and connCancelIncompleteRequests functions
into a new ConnStateData::checkHeaderLimits() method.
* Server-side changes:
Separated "received whole request body" state from "sent whole request
body". When we chunk requests, we need to add last-chunk. Thus, we may
receive (and written) the whole raw body but still need to write
last-chunk. This is not trivial because we should not write last-chunk
if the body producer aborted. XXX: check all pipe->exhausted() callers
to make sure all code has been adjusted.
Added getMoreRequestBody() virtual method that Server uses to get
encoded body bytes from its kids. FTP does not encode and uses default
implementation.
Fixed HTTP/FTP doneSendingRequestBody() to call its parent. I am not sure
it helps with correctly processing transactions, but the parent method was
designed to be called, and calling it make the transaction state more clear.
Moved "broken POSTS" handling code into its own method and polished it
(HttpStateData::finishingBrokenPost). We now skip the "broken POSTS" fix
if the request is chunked.
Resolved old XXX: HttpStateData::handleRequestBodyProducerAborted() was
indeed doing nothing useful despite all the pretense. Now it aborts the
transaction.
Author: Christian Wittmer <chris@computersalat.de>
Add --with-swapdir=PATH to override default /var/cache/squid
For use on systems where /var/cache is not the required placement.
Adds the 'squid' part to the default path in accordance to the UNIX
standard filesystem layout definition. In squid-3 it is no longer used
for creating an implicit active cache. Only documentation and potential
core dumps use the variable set by this now.
Alex Rousskov [Mon, 6 Sep 2010 00:05:09 +0000 (18:05 -0600)]
Added producedSize() method to report total data size put by the producer.
It is often very convenient to know this size, but one must be careful not to
use its value for body size, available body size, or exhaustion-calculating
expressions.
Alex Rousskov [Sun, 5 Sep 2010 23:34:08 +0000 (17:34 -0600)]
Bug #2311 fix: 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.
An upgrade/fix to handling HTTP request-lines as specific by
section 5.1 of the RFCs. Specifically to handle a sequence of
unknown bytes up to a terminating LF (\n) octet.
* The semantics as previously documented are taken on. No changes
there, but documentation clarified a bit. Some things previously not
erroring are now doing so. External code impact is in the nature of
reduced special cases to be handled. Specifically raw-CR weirdness in
the request line fields. This occuring in URL was a vulnerability at
least once before.
* Prior updates to HttpParser object for other parse stages opens the
possibility of this parse action returning HTTP status code directly.
Additions are done to make use of this (with the existing status codes
only).
* Input permutations where the unit-tests showed the old parser was
violating its own documentation have been fixed to produce expected
outputs.
* Old parser operated three distinct potentially long parse loops.
Added several local variables to remember various octets seen while
searching for the terminal LF. This removed the need for two of the
parse re-scans (length of method, length of URI).
* relaxed_header_parser will enable it to also skip prefix whitespace
(space character only) and multiple-\r sequences at the end of line.
* --enable-http-violations is still required before it will accept
non-HTTP version types 'downgraded' to HTTP/0.9
Alex Rousskov [Wed, 1 Sep 2010 03:44:05 +0000 (21:44 -0600)]
Check for NULL and empty strings before calling str*cmp().
These checks are necessary to ensure consistent comparison results (important
for sorting and searching) and to avoid segfaults on NULL buffers (because
termedBuf() may return NULL instead of the expected "0-terminated buffer").
projects / thirdparty / squid.git / log
