Alex Rousskov [Thu, 19 Nov 2015 05:51:49 +0000 (22:51 -0700)]
Store API and layout polishing. No functionality changes intended.
Fixes "any Store is a Root" API that forced us to bloat the base
Store class with methods needed only in Store::Root() Controller.
Unblocks bug #7 (cached headers update) fixes.
The Store namespace hierarchy now looks like this:
* Storage: Any storage. Similar to the old Store class, but leaner.
* Controller: Combined memory/disks caches and transients. Root API.
* Controlled: Memory cache, disk(s) cache, or transient Storage.
* Disks: All disk caches combined.
* Disk: A single cache_dir Storage.
* Memory: A memory cache.
* Transients: Entries capable of being collapsed for CF.
Alex Rousskov [Wed, 18 Nov 2015 23:56:16 +0000 (15:56 -0800)]
Bug 4368: A simpler and more robust HTTP request line parser.
The primary changes are: Removed incremental parsing and revised parsing
sequence to accept virtually any URI (by default and also configurable
as before).
Also doubled hard-coded 16-character method length limit.
No changes to parsing HTTP header fields (a.k.a. the MIME block) were
intended.
Known side effects:
* Drastically simpler code.
* Some unit test case adjustments.
* The new parser no longer treats some request lines ending with
"HTTP/1.1" as HTTP/0.9 requests for URIs that end with "HTTP/1.1".
* The new parser no longer re-allocates character sets while parsing
each request.
Intentional Changes:
* Removal of incremental request line parsing.
Squid parsed the request line incrementally. That optimization was
unnecessary:
- most request lines are short enough to fit into one network I/O,
- the long lines contain only a single long field (the URI), and
- the user code must not use incomplete parsing results anyway.
Incremental parsing made code much more complex and possibly slower than
necessary.
The only place where incremental parsing of request lines potentially
makes sense is the URI field itself, and only if we want to accept URIs
exceeding request buffer capacity. Neither the old code, nor the
simplified one do that right now.
* Accept virtually any request-target (when allowed).
1. relaxed_header_parser allows whitespace in request-target.
2. relaxed_header_parser combined with USE_HTTP_VIOLATIONS now allows
any characters except non-whitespace CTL characters (see RFC 5234
appendix B.1) in the message request-target (aka URI).
#2 being the default build and configuration situation allows virtually
any URI that Squid can isolate by stripping method (prefix) and
HTTP/version (suffix) off the request line. This approach allows Squid to
forward slightly malformed (in numerous ways) URIs instead of misplacing
on the Squid admin the burden of explaining why something does not work
going through Squid but works fine when going directly or through another
popular proxy (or through an older version of Squid!).
URIs in what Squid considers an HTTP/0.9 request obey the same rules.
Whether the rules should differ for HTTP/0 is debatable, but the current
implementation is the simplest possible one, and the code makes it easy
to add complex rules.
* Code simplification.
RequestParser::parseRequestFirstLine() is now a simple sequence of
sequential if statements. There is no longer a path dedicated for the
strict parser. The decisions about parsing individual fields and
delimiters are mostly isolated to the corresponding methods.
* Unit test cases adjustments.
Removal of incremental request line parsing means that we should not
check parsed fields when parsing fails or has not completed yet.
Some test cases made arguably weird decisions apparently to accommodate
the old parser. The expectations of those test cases are more natural now.
Also, added optional (and disabled by default) debugging, to help pin-point
failures to test sub-cases that CPPUNIT cannot see.
Changing request methods to "none" in test sub-cases with invalid input
was not technically necessary because the new code ignores the method
when parsing fails, but it may help whoever would decide to reduce test
code duplication (by replacing hand-written expected outcomes for failed
test cases with a constant assignment or function call).
Alex Rousskov [Wed, 18 Nov 2015 20:03:55 +0000 (13:03 -0700)]
Do not _require_ anchor/updateCollapsed() re-implementation.
Also do not override Controlled methods that Disk is not going to
provide because doing so will complicate changing or deleting those
methods later as we revise the APIs.
Amos Jeffries [Wed, 18 Nov 2015 13:28:57 +0000 (05:28 -0800)]
C++ convert the global C functions that operate on class CacheDigest
This is largely a symbol renaming change. But there are two relatively
small logic changes:
1) convert the class to MEMPROXY_CLASS.
Which alters the pool creation timing from general memory pool
initialization time, to whenever the CacheDigest object is first used.
A nice side effect is removal the macro conditional within the old pool
type enumeration. Macros like that in enumeration lists such as this one
have been causing some builds to have run-time errors accessing memory
arrays out-of-bounds or incorrect postions when the build-time
dependency detection issues caused build objects to link with different
./configure'd versions.
2) Constructor logic sequence alteration.
The old *Create function used to set some members then call the *Init
function which would re-set some of them, and initialize most of the
rest (but not all).
The old *UpdateCap function would call a helper that emulated
safe_free(mask) then *Init to alter the objects mask related members
whether they needed it or not.
The class constructor now initializes all members via initialization
list then calls updateCapacity(), which calls a simplified init(). This
altered sequence contains the same operational acts while the new order
avoids repeated or unnecesarily setting members on create and update.
Alex Rousskov [Wed, 18 Nov 2015 05:46:36 +0000 (22:46 -0700)]
Store API and layout polishing. No functionality changes intended.
This first step towards bug #7 fix focuses on fixing "any Store is a
Root" API that forced us to bloat the base Store class with methods
needed only in Store::Root() Controller.
We resolved about 15 XXXs and 10 TODOs (although these counts are
inflated by many duplicated/repeated problems). We added a few new
XXXs and TODOs as well, but they are just marking already problematic
code, not adding more problems or genuinely new work.
The code movement to files in parenthesis is not tracked by bzr
because bzr cannot track file splits, and most of the moved code had
to be split across multiple files to untangle various messes. When
deciding what to tell "bzr mv", we picked file pairs that would allow
us to track the most complex, most voluminous code but there is
probably no single correct way to do that.
src/disk.* files were renamed to src/fs_io.* to avoid "src/foo
conflicts with src/store/Foo" problems expected on some case-
insensitive platforms.
The Store namespace hierarchy now looks like this:
* Storage: Any storage. Similar to the old Store class, but leaner.
* Controller: Combined memory/disks caches and transients. Root API.
* Controlled: Memory cache, disk(s) cache, or transient Storage.
* Disks: All disk caches combined.
* Disk: A single cache_dir Storage.
* Memory: A memory cache.
* Transients: Entries capable of being collapsed for CF.
The last two are not moved/finalized yet, but it should not be too
difficult to do that later because there are few direct references to
them from the high-level code.
Related polishing touches:
Moved a lot of misplaced code into the right class and/or source file.
Simplified Store::search() interface to match the actual code that
does not support any search parameters. Removed the search API from
all other stores because the code did not really support store-
specific searches. Resisted the temptation to rename parameterless
search() to iterate() or similar because the actual future of this API
is murky. We may add search parameters or even remove the method
completely. This could quickly snowball into a separate project.
Removed Store::get(x,y,z) API as unused and unsupported.
Removed FreeObject() template as unused (and possibly technically
flawed).
Simplified default Store initialization/cleanup sequence. Removed
empty disk_init(). The non-default Store::Init() parameter is used by
the unit testing code only.
Simplified Store::dereference() API by moving the second parameter to
dedicated Controller::dereferenceIdle() method that is the only ones
using that parameter.
Alex Rousskov [Wed, 18 Nov 2015 05:34:33 +0000 (22:34 -0700)]
Fixed STUB_RETREF() implementation to return the right type.
Removed bogus STUB_RETREF() comment about memory leaks in _unreachable_ code.
Deprecated STUB_RETSTATREF() as essentially duplicating STUB_RETREF().
Alex Rousskov [Wed, 18 Nov 2015 05:32:24 +0000 (22:32 -0700)]
Make RefCount pointers behave more like regular pointers.
Allow default (but safe, thanks to C++11) conversion of RefCount
pointers to bool. This helps keep the code succinct, minimizes changes
during conversion of reference counting pointers to/from other pointer
types, and avoids nullptr/NULL differences.
Amos Jeffries [Wed, 18 Nov 2015 03:23:59 +0000 (19:23 -0800)]
Combine the https_port list internal state with http_port state.
These two lists have been near identical for some time now and we can
easily reduce code by simply merging the two and using either the
secure.encryptTransport flag or the transport.protocol type to select
the remaining non-identical code paths.
Alex Rousskov [Sun, 15 Nov 2015 17:54:58 +0000 (10:54 -0700)]
Stop using dangling pointers for eCAP-set custom HTTP reason phrases.
Squid still does not support [external] custom reason phrases and,
hence, cannot reliably support eCAP API that sets the reason phrase to
the one supplied by the adapter. This and r14398 changes fix [known]
regression bugs introduced by r12728 ("SourceLayout").
Alex Rousskov [Sun, 15 Nov 2015 16:59:12 +0000 (09:59 -0700)]
Fixed status code-based HTTP reason phrase for eCAP-generated messages.
Calling .reason() on a not-yet-set theMessage.sline object resulted in
"Init" status reason phrase for all from-scratch (i.e., not cloned)
eCAP-made HTTP responses. This fix lets Squid compute the reason phrase
based on the status code, just like Squid does for forwarded responses
(IIRC).
The ERR_SECURE_ACCEPT_FAIL and ERR_REQUEST_START_TIMEOUT errors apears that
have missing templates on squid startup.
Actually these errors does not produce any error page. Move them under the
TCP_RESET error in err_type.h to mark them as optional.
- Squid receives TLS Hello from the client (TCP connection A).
- Squid successfully negotiates an TLS connection with the origin server
(TCP connection B).
- Squid successfully negotiates an TLS connection with the client
(TCP connection A).
- Squid marks connection B as "idle" and waits an HTTP request from
connection A.
- The origin server continues talking to Squid (TCP connection B).
Squid detects a network read on an idle connection and closes TCP
connection B (and then the associated TCP connection A as well).
This patch:
- When squid detects a network read on server idle connection do an
SSL_read to:
a) see if application data received from server and abort in this case
b) detect possible TLS error, or TLS shutdown message from server
c) or ignore if only TLS protocol related packets received.
Amos Jeffries [Sun, 8 Nov 2015 15:09:16 +0000 (07:09 -0800)]
Fix compile erorr on clang undefined reference to '__atomic_load_8'
Later versions of GCC on some architectures push atomic functions
out into a separate atomic library. Older versions of clang do not
handle that automatically and require the library to be linked
explicitly.
Add a check for when this is required and set ATOMICLIB if needed.
Amos Jeffries [Sat, 7 Nov 2015 12:08:33 +0000 (04:08 -0800)]
Split core Server operations from ConnStateData
This improves the servers/libserver.la class hierarchy in
preparation for HTTP/2 and other non-HTTP/1.1 protocol support.
The basic I/O functionality of ConnStateData is moved to Server
class and a set of virtual methods designed to allow for child
class implementation of data processing operations.
No logic is changed in this patch, just symbol renaming and
moving of method logics as-is into libservers.la
The autoconf check for SQUID_SSLGETCERTIFICATE_BUGGY fails on ssl library
builds which don't include SSLv3; as a result of the autoconf decision
this can end up triggering the assert(0) in Ssl::verifySslCertificate()
in ssl/support.cc (line 1712 in 3.5.11).
Allow unlimited LDAP search filter for ext_ldap_group_acl helper.
The LDAP search filter in ext_ldap_group_acl is limited to 256 characters.
In some environments the user DN or group filter can be larger than this
limitation.
This patch uses dynamic allocated buffers for LDAP search filters.
Amos Jeffries [Sun, 1 Nov 2015 10:07:41 +0000 (02:07 -0800)]
Fix shutdown aborts after rev.14374
Changes to signal processing introduced by rev.14374 causse Squid to
ignore repeated signals.
However, repeated shutdown signals actually has meaning and need to abort
the shutdown delay timeout. So we need to allow those through to the
shutdown signal handler.
Alex Rousskov [Fri, 30 Oct 2015 20:38:57 +0000 (14:38 -0600)]
Bug 3574: To avoid crashes, prohibit reconfiguration during shutdown.
Also consolidated and polished signal action handling code:
1. For any executed action X, clear do_X at the beginning of action X
code because once we start X, we should accept/queue more X
requests (or inform the admin if we reject them).
2. Delay any action X requested during startup or reconfiguration
because the latter two actions modify global state that X depends
on. Inform the admin that the requested action is being delayed.
3. Cancel any action X requested during shutdown. We cannot run X
during shutdown because shutdown modifies global state that X
depends on, and we never come back from shutdown so there is no
point in delaying X. Inform the admin that the requested action is
canceled.
The child signal handling action is exempt from rules #2 and #3
because its code does not depend on Squid state.
Repeated failed attempts to fix crashes related to various overlapping
actions confirm that this code is a lot trickier than it looks. This
change introduces a more systematic/comprehensive approach to
resolving associated conflicts compared to previous ad hoc attempts.
These changes were not inspired by bug 3574 but they provide a
more comprehensive version of the earlier bug 3574 fix (r14354).
Amos Jeffries [Fri, 30 Oct 2015 12:59:17 +0000 (05:59 -0700)]
Add Locker friend class to SBuf for protection against memory issues
When appending or otherwise modifying an SBuf based on a SBuf& or char*
the parameter used may be pointing at the MemBlob memory buffer
indirectly without holding a separate ref-count lock to it.
If 'this' SBuf then requires reallocation for any reason the char* or
buffer pointer taken from the SBuf&, which is being manipulated may in
fact be left pointing at invalid memory.
Utilize a private Locker class to create relatively cheap ref-count locks
on the store_ MemBlob when this problem MAY occur. This Locker needs to
be used on all non-const SBuf methods accepting char* or SBuf& argument.
Amos Jeffries [Thu, 29 Oct 2015 18:53:48 +0000 (11:53 -0700)]
Add Locker friend class to SBuf for protection against memory issues
When appending or otherwise modifying an SBuf based on a SBuf& or char*
the parameter used may be pointing at the MemBlob memory buffer
indirectly without holding a separate ref-count lock to it.
If 'this' SBuf then requires reallocation for any reason the char* or
buffer pointer taken from the SBuf&, which is being manipulated may in
fact be left pointing at invalid memory.
Utilize a private Locker class to create relatively cheap ref-count locks
on the store_ MemBlob when this problem MAY occur. This Locker needs to
be used on all non-const SBuf methods accepting char* or SBuf& argument.
Alex Rousskov [Tue, 27 Oct 2015 03:45:40 +0000 (21:45 -0600)]
Connection stats, including %<lp, missing for persistent connections.
The code reusing a pconn was missing a hier.note() call, resulting in 0
values logged for %<lp (local port number of the last server or peer
connection) and probably other missing stats.
Also refactored poorly copied statistics collection code to remove
duplication and always update to-server connection stats when the actual
connection becomes available.
Positive side effect: Upon setsockopt(2) failures, the tos and nfmark
fields of a pinned connection were set to the desired (but not actually
applied) values, while persistent connection fields were left intact
(and, hence, stale). Both fields are now reset to zero on failures, for
both types of connections.
Aymeric Vincent [Mon, 26 Oct 2015 02:53:30 +0000 (19:53 -0700)]
Fix incorrect authentication headers on cache digest requests
login=NEGOTIATE can have an additional parameter specified,
like login=NEGOTIATE:xxx
One test added in rev.12714 does not take this case into account and it
will send a garbage "login:password" (== "NEGOTIATE:xxx") to its peer
when requesting a digest.
This is a workaround patch to remove the broken Authentication headers
entirely. Support for Negotiate to the peer on these digest requests is
still needed.
Amos Jeffries [Fri, 23 Oct 2015 05:36:51 +0000 (22:36 -0700)]
Avoid errors when parsing manager ACL in old squid.conf
ACL manager is now a built-in definition and has a different type. That
has been causing FATAL errors when parsing old squid.conf. We can be
nicer and just ignore the obsolete config lines.
Alex Rousskov [Wed, 21 Oct 2015 11:59:13 +0000 (04:59 -0700)]
Fixed chunked parsing by mimicking psChunkEnd state removed in trunk r14108.
... or, more precisely, in r13994.1.4 (parser-ng-chunked: re-write parse
sequence using ParseState stages instead of Step method pointers). Before
parser-ng-chunked, reaching zero theLeftBodySize would switch the chunk
parser to the psChunkEnd state. It was possible to pause parsing in that
state and resume it when more data becomes available, including the CRLF that
follows the chunk data. After parser-ng-chunked, the state remains
HTTP_PARSE_CHUNK which implies positive theLeftBodySize.
Amos Jeffries [Fri, 16 Oct 2015 14:28:52 +0000 (07:28 -0700)]
Bug 4351: compile errors when authentication modules disabled
Authentication modules can be selectively disabled. This means the module
header files need to be wrapped with disable macros, and also code that
depends on module internal definitions.
Alex Rousskov [Thu, 15 Oct 2015 02:52:58 +0000 (19:52 -0700)]
1xx response terminates Squid-to-server connection, breaking many PUTs.
Since trunk revision 13688.1.6 (Use Http1::ResponseParser to process
HTTP server responses), HttpStateData::processReplyHeader() sets
flags.headers_parsed after successfully parsing a 1xx control message.
The rest of the code interprets that flag as "parsed the final response"
and throws a !flags.headers_parsed exception because we have not parsed
the final (non-1xx) response yet. The exception kills virtually any PUT
or similar transaction that triggers an HTTP 100 (Continue) response
from the origin server.
This fix restores the original position of the flags.headers_parsed
update.
Amos Jeffries [Mon, 12 Oct 2015 01:38:02 +0000 (18:38 -0700)]
Bug 3574: crashes on reconfigure and startup
When Squid receives a reconfigure signal before its signal handler
has been registered on startup it will crash with unhandled signal
exceptions. This can be triggered on system boot when a resolv.conf
alteration signal wins a race with the daemon service initialization.
Register the reconfigure signal handler early and ignoring signals
until initial squid.conf load has completed.
When Squid receives a reconfigure signal while it is already in the
process of reconfiguring, the two async sequences can interfere and
result in confusing fatal error or crashes.
Only allowing one reconfigure sequence to be initiated at a time.
Also, if shutdown signal has been received while waiting for a
reconfigure to finish, let shutdown take precedence over any pending
reconfigure repeats.
Based on work by Clint Byrum and D J Gardner, Ubuntu
Amos Jeffries [Sun, 11 Oct 2015 14:08:47 +0000 (07:08 -0700)]
Support logformat %macros in external_acl_type format
Update the external_acl_type helper interface to use libformat and thus
make any logformat token valid in its format parameter field.
As a result much of the logic surrounding format code parsing, display
and helper query generation has been completely dropped. What remains is
a basic parse loop handling backward compatibility for the unusual
%CERT_* token syntax, space delimiter and field default encodings.
Extensions to logformat resulting from the merger:
* adds \-escape encoding of output fields
* allows {arg} field to be placed before or after the format code.
* extended to accept the old external_acl_type %macros. But not
documented, these are deprecated and only for backward compatibility.
* extended to support outputting formats without a format-name prefix
as was required by the original logformat config lines.
The major side effect of this change is that these ACLs now require
AccessLogEntry to be filled out with state data, rather than just the
ACLChecklist object members.
The requires*() mechanism of ACLChecklist has been extended to catch
some cases resulting from missing the ALE entirely. But it cannot catch
the more subtle problem of data members inside the ALE being unset.
To try and catch those a syncAle() mechanism has been added that fills
out missing ALE members and prints out debug warnings about the action.
Amos Jeffries [Sun, 11 Oct 2015 13:56:33 +0000 (06:56 -0700)]
TLS: shuffle EECDH configuration to libsecurity
* add class ServerOptions to libsecurity to manage server specific
configuration options. Based on class PeerOptions.
* shuffle the DH config parse and dump logics to ServerOptions
* shuffle the DH params pre-loading logic to ServerOptions
* add configuration warning when tls-dh= is used and overrides
dhparams= logacy configuration. Also, auto-upgrade the config
settings when dhparams= is dumped in mgr:config report.