Amos Jeffries [Fri, 3 Dec 2010 13:40:18 +0000 (02:40 +1300)]
Reduce implicit Comm::Connection closures
* permits the ConnOpener parent Jobs to abort the opening by canceling
their callbacks early.
* makes FTP, forwarding and ConnStateData components cancel pending comm
opens from their destructors (swanSong in ConnStateData).
This seems to reduce the cycles consumed by ConnOpener and also the Async
call delays somewhat in the cases where client connections are aborted.
Alex Rousskov [Thu, 2 Dec 2010 23:33:27 +0000 (16:33 -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.
Alex Rousskov [Sun, 28 Nov 2010 15:29:51 +0000 (08:29 -0700)]
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 [Sat, 27 Nov 2010 01:46:22 +0000 (18:46 -0700)]
SourceLayout: Comm Write cleanups
* creates namespace Comm.
* The comm_write() functions are moved into that scope as Comm::Write()
and only accept AsyncCall now. Old wrapper functions are removed.
* commio_* functions are all moved to methods of a new Comm::IoCallback
object. Which represents either a read or a write callback event
waiting to happen. Old wrapper functions have been removed.
* The fdc_table of pending read and write callbacks has been moved into
the Comm scope with (the name iocb_table) and should be considered private.
For now the COMMIO_*_CB() macros are retained to produce a pointer to
a callback object in this table.
* libcomm-listener.la has been renamed to libcomm.la
Alex Rousskov [Fri, 19 Nov 2010 02:10:21 +0000 (19:10 -0700)]
HTTP Compliance: 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
Author: Alex Rousskov, Andrew Balabohin, Christos Tsantilas
Dynamic SSL certificate generartion
This patch implements dynamic SSL certificate generartion in Squid.When
used with SSL Bump, the feature allows Squid to dynamically
generate (using a configurable CA certificate) and cache SSL
certificates for the proxied hosts.
A description for this feature can be found at:
http://wiki.squid-cache.org/Features/DynamicSslCert
A first version of the patch posted by Alex, some months before:
http://www.squid-cache.org/mail-archive/squid-dev/201003/0201.html
Some words about the patch:
* ssl related source files moved under the src/ssl directory
* Introduce the TidyPointer class similar to std::auto_ptr, which implements
a pointer that deletes the object it points to when the pointer's owner
or context is gone. It is designed to avoid memory leaks in the presence
of exceptions and processing short cuts.
* Implements ssl context cache to use with generated ssl contexts. The
Ssl::LocalContextStorage class stores the hostname/ssl context pairs for
a local listening address/port. The Ssl::GlobalContextStorage class used
to store Ssl::LocalContextStorages per local listening address and handles
squid shutdown/configure/reconfigure
* Ssl::Helper class implements the squid part of the ssl_crtd helpers.
* The ssl_crtd helper implemented in ssl_crtd.cc and certificate_db.* files
* The Ssl::CertificateDb class (certificate_db.* files) implements a
database of certificates on disk files. It is used by ssl_crtd helper to
manipulate generated certificates.
* The ssl related files included in the libraries libsslutil.a which
contains common classes and functions and the libsquidssl.a which has
squid related ssl objects and functions
Amos Jeffries [Tue, 16 Nov 2010 05:43:47 +0000 (18:43 +1300)]
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 [Wed, 10 Nov 2010 09:50:38 +0000 (22:50 +1300)]
Policy: detect config.h and squid.h include problems
We currently have a policy that config.h MUST be included first so as to
pull in the portability definitions early. It MAY be included via the
legacy squid.h at present.
This alteration to the source maintenance script validates that each .c
and .cc file in the sources includes config.h or squid.h first in its
include order. Also that each .h and .cci do not include config.h which
is a double include with enforced .c/.cc requirement.
* an ERROR: line is produces for each violating file
* as yet the maintenance run is not blocked so as to catch as many
errors as possible in one run
* detection only, as yet no code alterations are performed by this script
* FORMAT: informative lines are silenced to make ERROR: more visible
Alex Rousskov [Sun, 7 Nov 2010 03:07:29 +0000 (21:07 -0600)]
Bug 3091 fix: 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).
Fixed DNS query leaks and increased defense against DNS cache poisoning.
We were leaking (i.e. forgetting about) DNS queries under several conditions.
The most realistic leak case would go like this:
- We send UDP query1.
No response.
- We send UDP query2.
The response for query1 comes, with TC bit.
- We try to connect over TCP, sending TCP query3.
The response for query2 comes, with TC bit, matching TCP query3 ID.
Since we are waiting a response over TCP, we drop the UDP response,
and delete the query from the queue. We leak.
This change avoids forgetting the query under the above scenario.
Moreover, the above steps are hiding another problem: we are accepting responses
to timed out queries, making DNS cache poisoning easier. This change avoids
that by using unique query ID for each sent query. We have also added an
instance ID so that we still can track/identify a single "transaction" from
Squid point of view, even when that transaction involves many DNS query
messages.
When we forget about a DNS query, the caller may get stuck, holding a cbdata
lock. This is typical for ACLs that require domain name resolution, for example.
On a busy server with a long ACL list, the lock counter keeps growing due to
forgotten requests and may overflow, causing a "c->locks < 65535" assertion.
This change fixes the assertion unless there are more DNS leaks or different
lock leaks present.
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 [Mon, 1 Nov 2010 05:44:28 +0000 (23:44 -0600)]
Bug 3038: Detatch libmisc from libcompat
* Migrates many of the remaining libmisc portability wrappers into
libcompat.
* Splits libmisc into:
libprofiler - Squid internal profiler (developer-only)
libmiscencoding - Various binary encoding / crypto algorithms
libmisccontainers - Various data container algorithms
* Makes all binaries which need to link the libmisc* pieces directly instead
of via $(COMPAT_LIB) which now only links the libcompat and internal
profiler due to profiling being used on some libcompat functions.
* Adds a stub_debug for binaries needing the Debug.h API without squid
timers and globals.
Some effort has been made to identify binaries whose dependencies can be
reduced. More of this dependency removal can be done in future.
Amos Jeffries [Mon, 1 Nov 2010 00:21:57 +0000 (18:21 -0600)]
Harden quoted-string parser to RFC requirements
Fix RFC 2616 section 2.2 quote-string handling.
* Restrict the parser to the known length of the value string to prevent
buffer over-reads from specially crafted inputs.
* Drop quoted-string values containing CTL octets.
Alex Rousskov [Thu, 28 Oct 2010 18:52:59 +0000 (12:52 -0600)]
SMP Cache Manager, Phase2 implementation.
Cache Manager actions are forwarded to Coordinator. Coordinator iterates over
Kids, aggregating their stats if possible and/or allowing each kid to dump
non-aggregatable output directly into response if needed. Non-aggregated
output is wrapped in "by kidN { ... } by kidN" markup to ease auto-processing.
Regressions and small output formatting changes are probably unavoidable
because stats are aggregated and passed around as doubles instead of integers
(no more overflows though!) and because many stats collection and formatting
lines had to be touched. These are steps in the right direction though, IMO.
Old code both computed and dumped stats to Store at the same time. To avoid
computing code duplication, we now collect stats in primitive Stats objects
and then either dump those to Store or send them to Coordinator for
aggregation and, eventual Store dump. What stats to collect, when to
aggregate, and when to dump is decided by action-specific Mgr::Action classes.
The Cache Manager menu now consists of ActionProfile objects. ActionProfile
maintains hard-coded information about specific actions. It uses ActionCreator
member to create Action objects when a cache manager request is received.
Added Mgr::ActionParams class to maintain action parameters, including HTTP
request details necessary for Store entry creation (in another strand) and
action-specific parameters (currently just credentials). In Phase3, this class
can be extended to supply more parameters such as kid IDs to which the action
should apply.
Added Mgr::Command that combines hard-coded ActionProfile details with
user-specified ActionParams. This simplifies many interfaces because we no
longer need to supply a long list of parameters, covering various parts of
action config.
Moved Cache Manager registration to Mgr::RegisterAction() globals to reduce
dependency on the CacheManager class, which is a singleton anyway, and which
is unused by most of the registration callers. On the other hand, without
this change, no legacy (function-based actions) code would have been changed!
Enhanced TypedMsgHdr class to simplify storing and loading non-POD classes.
The caller can now easily handle a non-POD class as a series of put/get calls,
one for each POD member. This was necessary to send Mgr::ActionParams to
Coordinator and back. Will probably be useful for sending other complex
structures as well.
Reconfigure, shutdown, and other "basic" actions have been moved to
src/mgr/BasicActions.cc. Mgr::RegisterBasics() registers them.
Most of the Cache Manager code is now in src/mgr/.
Many more polishing touches.
More polishing left for future projects: Move CacheManager to Mgr namespace
and src/mgr/ directory. Use SBuf instead of String for ActionParams and
TypedMsgHdr. Rename Ipc::TypedMsgHdr to Ipc::Msg, Ipc::SocketMsg, or similar
because it maintains more than just msghdr struct. More stats aggregation,
and Phase3 changes.
Alex Rousskov [Wed, 27 Oct 2010 23:29:55 +0000 (17:29 -0600)]
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