Prevent idnsVCClosed segfaults during shutdown or reconfiguration (at least).
idnsShutdown() schedules comm_close and then frees nameservers[] by
calling idnsFreeNameservers. The closing handler tried to access freed
nameservers[]. The patch prevents access to the freed nameservers[]
array in idnsVCClosed and other functions.
TODO: Nameservers[] array management should be rewritten. The array
should not be freed while there are nameservers using it. It should be
freed when the last entry is gone.
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP/1.1: handle HTTP OPTIONS and TRACE requests with asterisk URIs.
Handle '*' URIs in urlParse(). This allows Squid properly respond to
OPTIONS and TRACE requests with '*' URIs and Max-Forwards value of
zero. Forwarding such requests is out of this change scope and still
does not work because the upstream host and port are not set.
Co-Advisor test cases:
test_case/rfc2616/options-bodyless-asterisk
test_case/rfc2616/maxForwardsZero-OPTIONS-asterisk
test_case/rfc2616/maxForwardsZero-TRACE-asterisk
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
Author: Alex Rousskov <rousskov@measurement-factory.com>
Added safe HttpMsg pointer wrapper that locks and unlocks the message.
This class will not be needed if we switch to refcounting HttpMsg. Meanwhile,
it allows to pass message pointers in AsyncCalls and avoid auto_ptr<> in
exception-sensitive code.
Author: Alex Rousskov <rousskov@measurement-factory.com>
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").
Amos Jeffries [Tue, 31 Aug 2010 01:43:31 +0000 (19:43 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Made eCAP compile again after CbcPointer<> changes.
Old eCAP code tried to call stillProducing(this) and stillConsuming(this)
methods from a const status() method. Doing so produces compiler errors
because stillProducing() and stillConsuming() do not accept pointers to
constant jobs.
CBDATA_CLASSes and, hence, CbcPointer<>, do not support const-correctness
well: In order to create/destroy a cbdata-based smart pointer, one has to
lock/unlock cbdata, which requires modifying the object. Thus, the smart
pointer cannot point to a truly constant object. The core of the problem is
that CBDATA_CLASSes keep cbdata and object data together. When all raw/dumb
CBDATA_CLASS pointers are gone, we can separate the two "datas" and solve the
const-correctness problem for good. The "separate-datas" design would even be
consistent with the original cbdata design which we often violate, IMO.
There are other workarounds. We could declare toCbdata() constant, for
example. However, that would essentially disable checks where a
cbdata-protected object is being destroyed despite the caller's intent to keep
the object constant. This change is not as general but is also much less
intrusive.
Amos Jeffries [Sun, 29 Aug 2010 00:52:30 +0000 (18:52 -0600)]
Author: Stefan Fritsch <sf@sfritsch.de>
Bug 2872: leaking file descriptors
As I understand it, the leak happens this way: A client request kicks off an
asynchronous file open request. If the client request is aborted and disappears
before the file open has completed, the file is never closed again. This
explains why this leak can only happen with aufs and not with ufs.
Amos Jeffries [Sun, 29 Aug 2010 00:49:56 +0000 (18:49 -0600)]
Author: Henrik Nordstrom <henrik@henriknordstrom.net>
Harden DNS client against packet queue attacks.
Strengthen the internal DNS client somewhat by making sure to keep
the receive queue drained. Also avoid parsing messages unless we
have a pending query.
Amos Jeffries [Fri, 27 Aug 2010 16:46:02 +0000 (10:46 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP Compliance: remove Content-Length header if Transfer-Encoding is present.
If after HTTP header parsing we have both "Transfer-Encoding: chunked"
and Content-Length headers, remove the Content-Length entry. The
adjusted behavior follows httpbis recommendations (ticket #95, part 2).
The old client-side code forwarded the original Content-Length header
which did not match the [dechunked] response, resulting in a malformed
response.
HttpHeader::chunked() method added to check if HTTP headers contain
chunked Transfer-Encoding header. Use this method in code that checks
for chunked encoding.
Co-Advisor test cases: test_case/rfc2616/chunked-1p0-badClen-toClt
test_case/rfc2616/chunked-1p1-badClen-toClt
Amos Jeffries [Fri, 27 Aug 2010 16:45:08 +0000 (10:45 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP/1.1: respond to OPTIONS requests with a zero Max-Forwards value.
RFC 2616 section 9.2 says that a proxy MUST NOT forward requests with a
zero Max-Forwards value. RFC 2616 does not define any proper OPTIONS
responses, so we consider successful responses optional and reply with
501 Not Implemented.
No change in handling OPTIONS requests with positive Max-Forwards values.
While TRACE and OPTIONS are similar with regard to Max-Forwards, we
handle them in different places because OPTIONS responses do not need to
echo the request via Store.
Co-Advisor test case: test_case/rfc2616/maxForwardsZero-OPTIONS-absolute
Amos Jeffries [Fri, 27 Aug 2010 16:41:32 +0000 (10:41 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP/1.1: Send chunked responses if body size is unknown.
Apply HTTP chunked transfer encoding to the response body sent to client
if all of the following conditions are met:
* client claims HTTP version 1.1 or later support
* response does not have a Content-Length header already
* response does not use multipart/byteranges encoding
* connection is persistent
If we decide to send chunked reply, chunked_reply flag is set. Chunked
encoding is done in ClientSocketContext::packChunk(). The last-chunk
is sent only when clientReplyContext complete flag is set.
This change helps keep client-side connections persistent.
Amos Jeffries [Fri, 27 Aug 2010 16:16:29 +0000 (10:16 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 2583: pure virtual method called
When a cbdata-protected class holds its own cbdata and has virtual
toCbdata(), there is a catch22 problem: we need cbdata to know whether
the pointer to the class object is valid, and we need to dereference
that pointer to get cbdata.
Added CbcPointer class to hold both a pointer to a potentially freed
class object and the cbdata pointer protecting that object. Keeping the
cbdata pointer allows us to test whether the object is still there
without dereferencing the object pointer.
Use the CbcPointer class to hold safe pointers to AsyncJobs. This
prevents "pure virtual method called" failures because we no longer
dereference freed job pointers.
Removed Initiator parameter from many initiatee constructors. The
Adaptation::Initiator::initiateAdaptation method now sets the initiator
of the job. This makes the constructor profile simpler and removes the
need to propagate Initiator changes through all the [nested]
constructors.
Renamed AsyncJob::AsyncStart() to AsyncJob::Start(). I had to change the
callers code anyway and it was a good opportunity to remove the
redundant "Async".
Special thanks to Stefan Fritsch for updating and testing an earlier
version of this patch.
With the change to correct HTTP/1.1 keep-alive behaviour it has become
apparent that the major browsers fail to handle NTLM re-challenge cleanly
over persistent connections.
This alters the default behaviour for NTLM and Negotiate auth when not
explicitly configured. It causes connections to break during initial
protocol negotiation, which while not desirable was the original HTTP/1.0
behaviour of Squid 3.1 on which the browsers appear to depend.
Installs which have explicitly set their keep_alive parameters to ON, may
need to re-asses.
Amos Jeffries [Thu, 19 Aug 2010 02:27:34 +0000 (20:27 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 3016: HTTP/1.1 compliance: default keep-alive for 1.0/1.1 clients.
aka. NTLM Authentication with Java UA + SSL Problem
Moved httpMsgIsPersistent(version, headers) to HttpMsg::persistent(void).
This move makes it clear that the logic applies only to the message being
examined and not some irrelevant information such as HTTP version supported
by Squid.
Side-effects:
* Squid starts using persistent connections with HTTP/1.1 clients
that do not send "Connection: close".
Amos Jeffries [Wed, 18 Aug 2010 01:46:52 +0000 (19:46 -0600)]
Fix 32-bit wrap in refresh_pattern min/max values.
Attached patch limits the values to 1 year (arbitrary based on rumours
about good caching times). Checking for 32-bit wrap and setting the max
1 year limit instead of cutting them to zero.
The expected outcome of this is correct cache storage time extension
according to refresh_pattern documentation when people desperately set
min/max to > a million minutes. Instead of a silent always-stale verdict.
Amos Jeffries [Wed, 18 Aug 2010 01:45:20 +0000 (19:45 -0600)]
HTTP/1.1 compliance: Stop using Proxy-Connection header
The Proxy-Connection header is not part of any HTTP standard. It was added
by Netscape to differentiate persistent connections to intermediary proxies
but that duty has been formally superceded by the Connection: header.
This compliance update makes Squid stop sending Proxy-Connection on outbound
requests. Starts consistently using Connection: header instead.
The Proxy-Connection header is also ignored on HTTP-strict builds.
For compatibility we must do a small violation and drop it as a hop-by-hop
header despite strict-mode technically being required to pass it through.
For origin server connections the non-strict builds will retain the
status-quo: interpret it, but treat it as an HTTP/0.9 thing to be
upgraded to HTTP/1.1 Connection: header.
Amos Jeffries [Wed, 18 Aug 2010 01:38:05 +0000 (19:38 -0600)]
Author: Christos Tsantilas <chtsanti@users.sourceforge.net>
Fix: In the case of an error while accessing a gopher server, squid will crash
The GopherStateData::req used to retrieve the releated HttpRequest object in
gopherSendComplete function when a server while accessing the server occurs.
The GopherStateData::req is never assigned and it is always NULL (should be
removed?). The gopherState->fwd->request must be used instead.
Solaris 9 is not fully RFC 3493 compliant. It does not provide the
IPV6_V6ONLY even as a null-op option.
There are potentially other systems in the same situation. Fix is to
detect the absence of the option and fall back to split-stack on
IPv6-enabled systems without it.
httpHdrCcParseInit() ignored all unknown Cache-Control directives
except for the first one because the (type != CC_OTHER) check
applied to the debugging statement only.
Co-Advisor test case: test_case/rfc2616/endHdr-fwd-set-Cache-Control-toSrv
Amos Jeffries [Tue, 10 Aug 2010 07:53:05 +0000 (01:53 -0600)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
HTTP/1.1 Compliance: Improved HTTP Range header field validation.
1) Improved HttpHdrRangeSpec::parseInit() to parse syntactically valid
range specs:
* Suffix ranges with 0 length (i.e. -0) are syntactically valid.
* Check that last-byte-pos is greater than or equal to first-byte-pos.
After the change, HttpHdrRangeSpec::parseInit() successfully parses suffix
ranges with 0 length. They were rejected before. RFC 2616 section 14.35.1 says
such range specs are syntactically valid but unsatisfiable. Thus, we should
ignore the range spec itself, but not the whole range header. These range
specs will be rejected later, during canonization.
2) In HttpHdrRangeSpec::parseInit(), ignore the whole range header if one of
range specs is syntactically invalid (i.e. range spec parsing fails).
Co-Advisor test case: test_clause/rfc2616/invalidRange
Amos Jeffries [Sun, 1 Aug 2010 13:34:08 +0000 (07:34 -0600)]
Hack: define top_build_prefix for old autotools
This hack was removed during the libtool 2.2 upgrade.
The issue shows up as bundle builds failing to link libltdl/libltdlc.la
when they should in fact be linking ../libltdl/libltdlc.la in src/Makefile
It is caused by the macros of libtool 2.2 assuming the presence of
top_build_prefix but since autoconf 2.62 that was replaced with
ac_top_build_prefix and is no longer automatically defined in Makefile's.
Some distros also seem to have back-ported the removal of top_build_prefix
into their old autoconf causing macro version tests to fail.
Amos Jeffries [Sun, 1 Aug 2010 13:29:09 +0000 (07:29 -0600)]
Bug 2994: Basic split-stack functionality
Enable split-stack detection by default.
There is now enough split-stack support to enable accepting IPv6 client
connections on systems with separate IPv4/IPv6 stacks. Also some limited
server connection capabilities (see tcp_outgoing_addr config hacks).
SNMP, ICP, HTCP listeners and outbound connections currently default to
IPv4-only on these systems to retain backward-compatibility.
But may be explicity configured to IPv6-only. There is no support as yet
for dual-protocol behaviour in the sockets of these three protocols.
Author: Christos Tsantilas <chtsanti@users.sourceforge.net>
Fix: 32bit integer used in HttpStateData to store the bytes received from next hop
A simple integer used to store the bytes received from the next hop
(http server of proxy), which my cause problems when receives huge
http objects on 32bit systems.
Author: Christos Tsantilas <chtsanti@users.sourceforge.net>
Fix: The bytes to/from the ICAP server may not logged correctly on 32bit systems
A simple long int (doint/outint) used to log bytes sent/received to/from
the ICAP server, ICAP requests with very large http objects will not
logged correctly. Use int64_t (dooff/outoff) instead
Replace most USE_IPV6 with run-time support probing
This unifies the code built for IPv4-only, dual-stack and split-stack.
* --disable-ipv6 option remains, however it now prevents the run-time probe
* Probing previously done in ./configure at build time is now merged and
performed run-time on every startup. IPv6 is enabled or disabled based on
the underlying OS support for sockets and setsockopt operations required.
* Parsing and other operations which can be performed without specific IPv6
connectivity are enabled.
* Some DNS logic alterations have had to be made to merge the split-stack
DNS and leverage it for IPv4-only mode. Otherwise the logics are unchanged
from previous dual-stack builds which have been well tested.
This breaks the DNS sockets into two when split-stack mode is used.
DnsSocketA becomes IPv4-only and DnsSocketB becomes used as IPv6-only.
Without Split-stack mode DnsSocketA is the only one actually used.
NP: comm outgoing socket support is still a blocker on split-stack support.
However this update is required for the upcoming run-time support.
Author: Alex Rousskov <rousskov@measurement-factory.com>
Fixed comm.cc:377: "fd_table[fd].halfClosedReader != NULL" assertion
Client side must stop reading when switching to a tunnel mode. The old code
called low-level commSetSelect to stop reading, but that left Comm tables in
an inconsistent state, with the client side reader callback still scheduled.
Squid would assert when the tunnel called comm_read with its own callback.
The bug is unrelated to half-closed connections despite halfClosedReader
mentioned in the assertion text. The assertion means "no more than one active
reader per FD".
Author: Alex Rousskov <rousskov@measurement-factory.com>
Prevent memory leaks when cloning Range requests.
HttpRequest::range field was set to a new HttpHdrRange object twice:
once in HttpRequest::clone() and once in HttpRequest::hdrCacheInit()
called from clone().
Polished HttpReply::clone() to make sure HttpReply::hdrCacheInit()
does not use uninitialized HttpReply::sline field and to prevent
benign double-initialization of HttpReply::keep_alive.
Bundle a slightly hacked version of libltdl/ltdl.h which does not use <>
for inclusion of its dependency files. This makes it portable to OS with
older libtool versions.
Amos Jeffries [Sun, 27 Jun 2010 09:35:30 +0000 (21:35 +1200)]
Author: Robert Collins <robertc@robertcollins.net>
Bug 2950: HTTP responses with no Date, L-M or Expires can now be cached
HTTP responses with no Date, Last-modified or Expires headers can
now be cached (given an appropriate refresh_pattern). Previously we
were not caching them in case of an infinite loop in cache farms:
however Squid adds Date: headers now which remove that concern.
projects / thirdparty / squid.git / log
