Alex Rousskov [Sun, 19 Dec 2010 03:51:48 +0000 (20:51 -0700)]
Fixed BodyPipe.cc:144 "!theConsumer" assertion.
BodySink must hold a pointer to the body pipe it is consuming. Otherwise, the
pipe may be deleted before BodySink received the final notification and had a
chance to stop consumption, causing the assertion in the pipe destructor.
These extra parameters are passed as (name, opaque value) pairs to the
adapter, using the newly added libecap::Config API. Adapters should throw if
they cannot understand the parameters to protect users from typos in optional
Squid-recognized parameters.
Squid-recognized service parameters can also be passed to adapters. Adapters
can distinguish them from custom ones or typos because their names have host
IDs set. We currently only pass one Squid recognized service parameter called
"bypassable", with a boolean values of "1" or "0". This tells Adapter whether
Squid can try to bypass the adapter in case of problems. In our experience,
many real adapters benefit from such knowledge because they can be less strict
and more forgiving if Squid might ignore their decisions anyway.
To support optional adapter parameters for eCAP without bothering ICAP,
we now allow ICAP and eCAP to create protocol-specific configuration objects.
ICAP code uses old defaults. eCAP implements parsing of optional adapter
parameters and sharing them with adapters
As a side effect, service configuration objects are now refcounted and each
service (once created) is responsible for its config. The global collection of
configs is emptied once the services are created.
(B) eCAP transaction wrapper code (Ecap::XactionRep) failed to pass a few test
cases when dealing with virgin bodies. The code used complex state and
mishandled several proxyingVb, nil body_pipe, and stillConsuming value
combinations. proxyingVb was especially troubling because it was not clear
whether it refers to us receiving vb from Squid core or sending vb to the
adapter. The two states are related but different because we could be
receiving vb from core but not sending it to the adapter and vice versa.
I have removed proxyingVb completely as the body pipe state alone is
sufficient to understand our dealings with Squid core. I added makingVb to
track adapter vb needs.
Alex Rousskov [Thu, 16 Dec 2010 06:12:06 +0000 (23:12 -0700)]
Allow uri=value parameter when specifying adaptation service URIs.
When adaptation service URI contains a "=" character, Squid thinks you are
specifying a name=value option rather than a service URI. This leads to a
fatal configuration error.
This change lets you specify the service URI using the uri=value
syntax where the value may contain "=" character(s). For example, the
following works after and only after this change:
TODO: Should the adaptation service parser be changed to treat the last word
on the icap_service line as a service URI, regardless of whether it contains
the "=" character?
Alex Rousskov [Wed, 15 Dec 2010 17:52:35 +0000 (10:52 -0700)]
Support libecap::host::xaction::blockVirgin() API, serving ERR_ACCESS_DENIED.
deny_info logic is supported for these blocked responses, with the ACL name
replaced by the adaptation service name. This allows eCAP adapters to focus on
adaptation and blocking logic while letting Squid to serve a configurable
block message, with language negotiation and such.
Merged noteAdaptationAnswer(msg) and noteAdaptationQueryAbort(bool) into
noteAdaptationAnswer(answer). The Adaptation::Answer class manages all
currently supported adaptation decisions: forward the adapted message, block
user access to the virgin response, and bypassable or fatal error.
This "single answer hook" design allows us to add more information to adaption
answers without rewriting all the code that forwards those answers to the
adaptation initiator. We still often use multiple methods to handle multiple
answer categories, but that "forking" is optional and the decision to fork is
made locally, inside each answer recepient, reducing the overall code
complexity.
Also fixed a few virgin body handling corner cases that led to unnecessary
exceptions in Adaptation::Ecap::XactionRep despite correct adapter behavior.
Alex Rousskov [Mon, 13 Dec 2010 00:16:00 +0000 (17:16 -0700)]
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.
Alex Rousskov [Tue, 7 Dec 2010 19:32:43 +0000 (12:32 -0700)]
Tolerate adapted body delivery failures in REQMOD request satisfaction mode.
Without these changes, Squid may assert if an ICAP or eCAP service fails
while delivering response body in REQMOD:
assertion failed: client_side.cc:1438: "rep"
Other assertions may be possible as well, because we were trying to
serve a freshly built, HttpReply-free error response while already
writing the REQMOD "request satisfaction" response. The assertion
probably depends on that writing stage (wrote nothing yet, wrote
headers, wrote some body, etc).
Polished ERR_DETAIL constants to distinguish the special "request
satisfaction" case and to remove ICAP-specific labels from general
adaptation code.
Alex Rousskov [Tue, 7 Dec 2010 19:10:11 +0000 (12:10 -0700)]
Support upcoming "fresh message creation" eCAP API.
Adapters need "fresh" (i.e., empty and not cloned) messages to support
"request satisfaction" mode and other cases where cloning a virgin message is
not possible or is awkward because the adapted message is not an adjusted
version of the virgin one.
fix compile error on OpenSolaris using SunStudio CC
The way the TidyPointer used inside ssl/gadgets.h which included
through ssl/support.h in squid.h file requires the ssl libraries
linked with every binary, even if not realy needed.
To solve this probelm remove the 'include "ssl/support.h"' line
from the squid.h and add it only where required.
The *_free SSL API functions can not be used with TidyPointer in some OSes/compilers
The *_free SSL functions are declared as extern "C" and can not be used as
DeAllocator argument of the TidyPointer class with some compilers and OSes.
To solve this problem:
- Define the CtoCpp1 macro to allow easy implementation of the C++ equivalent
function of an extern C function.
Currently defined in gadgets.h file, if required in the future can be
moved to TidyPointer.h file
- Use the CtoCpp macro to define the X509_free_cpp
- Remove the Ssl::BIO_free_wrapper function does not needed any more
Amos Jeffries [Mon, 6 Dec 2010 14:06:06 +0000 (03:06 +1300)]
Fix 'can't create ./src/***: No such file' errors on linking
Side effect of the AIX port fixes using .o instead of duplicating and
re-building the .cc individually. "It seemed a good idea at the time"(tm).
It is a bit strange that it should only show up now, those changes were
made long ago.
Anyways, I've come to the conclusion from other places that the rebuild is
slower to compile but a lot safer to deal with linkage and dependencies.
Amos Jeffries [Mon, 6 Dec 2010 08:24:12 +0000 (01:24 -0700)]
Fix host OS detection in configure.ac
squid_host_os was being left with the version appended. Which screwed all
the switch and if cases which depended on exact matching the name only.
This corrects the squid_host_os to not contain numeric versions and also
corrects several of the test cases using host_os instead of squid_host_os
or using squid_host_os with version digits.
Amos Jeffries [Sun, 5 Dec 2010 14:29:27 +0000 (03:29 +1300)]
Build libprofiler only when it will be operational
The internal CPU profiler requires specific CPU support. It is not useful
to build and link the library unless it is going to work.
Uses AC_PREPROC_IFELSE to allow building on cross-compilers.
Alex Rousskov [Fri, 3 Dec 2010 23:04:01 +0000 (16:04 -0700)]
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).
Alex Rousskov [Fri, 3 Dec 2010 22:59:37 +0000 (15:59 -0700)]
Polished HttpStateData::persistentConnStatus() code. No functionality changes.
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.
Alex Rousskov [Fri, 3 Dec 2010 22:56:17 +0000 (15:56 -0700)]
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.
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).
projects / thirdparty / squid.git / log
