* Protect Squid Client classes from new requests that compete with
ongoing pinned connection use and
* resume dealing with new requests when those Client classes are done
using the pinned connection.
Replaced primary ConnStateData::pinConnection() calls with a pair of
pinBusyConnection() and notePinnedConnectionBecameIdle() calls,
depending on the pinned connection state ("busy" or "idle").
Removed pinConnection() parameters that were not longer used or could be computed from the remaining parameters.
Removed ConnStateData::httpsPeeked() code "hiding" the originating
request and connection peer details while entering the first "idle"
state. The old (trunk r11880.1.6) bump-server-first code used a pair of
NULLs because "Intercepted connections do not have requests at the
connection pinning stage", but that limitation no longer applicable
because Squid always fakes (when intercepting) or parses (a CONNECT)
request now, even during SslBump step1.
The added XXX and TODOs are not directly related to this fix. They
were added to document problems discovered while working on this fix.
In v3.5 code, the same problems manifest as Read.cc
"fd_table[conn->fd].halfClosedReader != NULL" assertions.
Alex Rousskov [Sat, 1 Jul 2017 09:59:16 +0000 (21:59 +1200)]
Fix mgr query handoff from the original recipient to Coordinator.
This bug has already been fixed once, in trunk r11164.1.61, but that fix
was accidentally undone shortly after, during significant cross-branch
merging activity combined with the Forwarder class split. The final
merge importing the associated code (trunk r11730) was buggy.
The bug (explained in r11164.1.61) leads to a race condition between
* Store notifying Server classes about the entry completion (which might
trigger a bogus error message sent to the cache manager client while
Coordinator sends its own valid response on the same connection!) and
* post-cleanup() connection closure handlers of Server classes silently
closing everything (and leaving Coordinator the only responding
process on that shared connection).
The bug probably was not noticed for so long because, evidently, the
latter actions tend to win in the current code.
Amos Jeffries [Thu, 22 Jun 2017 15:31:46 +0000 (03:31 +1200)]
Bug 4671 pt3: various GCC 7 compile errors
Also, remove limit on FTP realm strings
Convert ftpRealm() from generating char* to SBuf. This fixes issues identified
by GCC 7 where the realm string may be longer than the available buffer and
gets truncated.
The size of the buffer was making the occurance rather rare, but it is still
possible.
Alex Rousskov [Wed, 21 Jun 2017 20:12:48 +0000 (08:12 +1200)]
Replace new/delete operators using modern C++ rules.
This change was motivated by "Mismatched free()/delete/delete[]" errors
reported by valgrind and mused about in Squid source code.
I speculate that the old new/delete replacement code was the result of
slow accumulation of working hacks to accomodate various environments,
as compiler support for the feature evolved. The cumulative result does
not actually work well (see the above paragraph), and the replacement
functions had the following visible coding problems according to [1,2]:
a) Declared with non-standard profiles that included throw specifiers.
b) Declared inline. C++ says that the results of inline declarations
have unspecified effects. In Squid, they probably necessitated
complex compiler-specific "extern inline" workarounds.
c) Defined in the header file. C++ says that defining replacements "in
any source file" is enough and that multiple replacements per
program (which is what a header file definition produces) result in
"undefined behavior".
d) Declared inconsistently (only 2 out of 4 flavors). Declaring one base
flavor should be sufficient, but if we declare more, we should
declare all of them.
The replacements were not provided to clang (trunk r13219), but there
was no explanation why. This patch does not change that exclusion.
I have no idea whether any of the old hacks are still necessary in some
cases. However, I suspect that either we do not care much if the
replacements are not enabled on some poorly supported platforms OR we
can disable them (or make them work) using much simpler hacks for the
platforms we do care about.
Fixes
error: %s directive output may be truncated writing up to 8191 bytes
into a region of size 1019
note: snprintf output between 8 and 8199 bytes into a destination of
size 1024
Bug 2833 pt3: Do not respond with HTTP/304 to unconditional requests
... after internal revalidation. The original unconditional HttpRequest
was still marked (and processed) as conditional after internal
revalidation because the original (clear) Last-Modified and ETag values
were not restored (cleared) after the internal revalidation abused them.
TODO: Isolate the code converting the request into conditional one _and_
the code that undoes that conversion, to keep both actions in sync.
The security fix in v5 r14979 had a negative effect on collapsed
forwarding. All "private" entries were considered automatically
non-shareable among collapsed clients. However this is not true: there
are many situations when collapsed forwarding should work despite of
"private" entry status: 304/5xx responses are good examples of that.
This patch fixes that by means of a new StoreEntry::shareableWhenPrivate
flag.
The suggested fix is not complete: To cover all possible situations, we
need to decide whether StoreEntry::shareableWhenPrivate is true or not
for all contexts where StoreEntry::setPrivateKey() is used. This patch
fixes only few important cases inside http.cc, making CF (as well
collapsed revalidation) work for some [non-cacheable] response status
codes, including 3xx, 5xx and some others.
The original support for internal revalidation requests collapsing
was in trink r14755 and referred to Squid bugs 2833, 4311, and 4471.
Bug 4682: ignoring http_access deny when client-first bumping mode is used
Squid fails to identify HTTP requests which are tunneled inside an already
established client-first bumped tunnel, and this is results in ignoring
http_access denied for these requests.
xstrndup() does not work like strndup(3), and some callers got confused:
1. When n is the str length or less, standard strndup(str,n) copies all
n bytes but our xstrndup(str,n) drops the last one. Thus, all callers
must add one to the desired result length when calling xstrndup().
Most already do, but it is often hard to see due to low code quality
(e.g., one must remember that MAX_URL is not the maximum URL length).
2. xstrndup() also assumes that the source string is 0-terminated. This
dangerous assumption does not contradict many official strndup(3)
descriptions, but that lack of contradiction is actually a recently
fixed POSIX documentation bug (i.e., correct implementations must not
assume 0-termination): http://austingroupbugs.net/view.php?id=1019
The OutOfBoundsException bug led to truncated exception messages.
The ESI bug led to truncated 'literal strings', but I do not know what
that means in terms of user impact. That ESI fix is untested.
cachemgr.cc bug was masked by the fact that the buffer ends with \n
that is unused and stripped by the custom xstrtok() implementation.
TODO. Fix xstrndup() implementation (and rename the function so that
fixed callers do not misbehave if carelessly ported to older Squids).
Bug 4711: SubjectAlternativeNames is missing in some generated certificates
Squid may generate certificates which have a Common Name, but do not have
a subjectAltName extension. For example when squid generated certificates
do not mimic an origin certificate or when the certificate adaptation
algorithm sslproxy_cert_adapt/setCommonName is used.
This is causes problems to some browsers, which validates a certificate using
the SubjectAlternativeNames but ignore the CommonName field.
This patch fixes squid to always add a SubjectAlternativeNames extension in
generated certificates which do not mimic an origin certificate.
Squid still will not add a subjectAltName extension when mimicking an origin
server certificate, even if that origin server certificate does not include
the subjectAltName extension. Such origin server may have problems when
talking directly to browsers, and patched Squid is not trying to fix those
problems.
Bug 4682: ignoring http_access deny when client-first bumping mode is used
Squid fails to identify HTTP requests which are tunneled inside an already
established client-first bumped tunnel, and this is results in ignoring
http_access denied for these requests.
Amos Jeffries [Mon, 29 May 2017 04:38:52 +0000 (16:38 +1200)]
Add OpenSSL library details to -v output
This is partially to meet the OpenSSL copyright requirement that binaries
mention when they are using the library, and partially for admin to see
which library their Squid is using when multiple are present in the system.
Fixes squid documentation to correctly describe the squid behavior when the
"bump" action is selected on step SslBump1. In this case squid selects
the client-first bumping mode.
Most SslBump splicing happens after getting SNI. SNI goes into the
second fake CONNECT request, where it may fail the host forgery check.
A failed check triggers an HTTP error response from Squid. When
attempting to send that response to the TLS client, Squid checks whether
all previously pipelined HTTP requests on the connection have finished.
Prior to this fix, Squid left the first fake CONNECT request in the
connection pipeline despite adding the second fake CONNECT. That first
CONNECT stalled the error response described above, with Squid waiting,
in vain, for that already handled [fake] transaction to finish.
Also call quitAfterError() to force Squid to close the connection (after
writing the discussed error response) instead of just logging a
[misleading] "kick abandoning [connection]" message in cache.log.
TODO: Always pop the first CONNECT when generating a second one.
Unifying CONNECT treatment is difficult because code like tunnel.cc
wants that CONNECT to be in the pipeline. Polishing that would probably
require disassociating ConnStateData from tunnel.cc (at least).
TODO: Apply the existing "delayed error" logic (that optionally bumps
TLS connections to deliver [some] errors to [some] SSL/TLS clients) to
host forgery errors. Otherwise, the plain HTTP error message cannot be
understood by the intercepted TLS client.
Alex Rousskov [Sun, 26 Feb 2017 08:46:24 +0000 (21:46 +1300)]
Fix crash when configuring with invalid delay_parameters restore value.
... like none/none. Introduced in rev which fixed another, much
bigger delay_parameters parsing bug.
TODO: Reject all invalid input, including restore/max of "-/100".
TODO: Fix misleading/wrong associated error messages. For example:
ERROR: invalid delay rate 'none/none'. Expecting restore/max or 'none'
ERROR: restore rate in '1/none' is not a number.
Bump SSL client on [more] errors encountered before ssl_bump evaluation
... such as ERR_ACCESS_DENIED with HTTP/403 Forbidden triggered by an
http_access deny rule match.
The old code allowed ssl_bump step1 rules to be evaluated in the
presence of an error. An ssl_bump splicing decision would then trigger
the useless "send the error to the client now" processing logic instead
of going down the "to serve an error, bump the client first" path.
Furthermore, the ssl_bump evaluation result itself could be surprising
to the admin because ssl_bump (and most other) rules are not meant to be
evaluated for a transaction in an error state. This complicated triage.
Also polished an important comment to clarify that we want to bump on
error if (and only if) the SslBump feature is applicable to the failed
transaction (i.e., if the ssl_bump rules would have been evaluated if
there were no prior errors). The old comment could have been
misinterpreted that ssl_bump rules must be evaluated to allow an
"ssl_bump splice" match to hide the error.
SSLv2 records force SslBump bumping despite a matching step2 peek rule.
If Squid receives a valid TLS Hello encapsulated into ancient SSLv2
records (observed on Solaris 10), the old code ignored the step2 peek
decision and bumped the transaction instead.
Now Squid peeks (or stares) at the origin server as configured, even
after detecting (and parsing) SSLv2 records.
Mitigate DoS attacks that use client-initiated SSL/TLS renegotiation.
There is a well-known DoS attack using client-initiated SSL/TLS
renegotiation. The severety or uniqueness of this attack method
is disputed, but many believe it is serious/real.
There is even a (disputed) CVE 2011-1473:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-1473
The old Squid code tried to disable client-initiated renegotiation, but
it did not work reliably (or at all), depending on Squid version, due
to OpenSSL API changes and conflicting SslBump callbacks. That
code is now removed and client-initiated renegotiations are allowed.
With this change, Squid aborts the TLS connection, with a level-1 ERROR
message if the rate of client-initiated renegotiate requests exceeds
5 requests in 10 seconds (approximately). This protection and the rate
limit are currently hard-coded but the rate is not expected to be
exceeded under normal circumstances.
Amos Jeffries [Fri, 27 Jan 2017 15:00:05 +0000 (04:00 +1300)]
Detect HTTP header ACL issues
rep_header and req_header ACL types cannot match multiple different
headers in one test (unlike Squid-2 appears to have done). Produce
an ERROR and ignore the extra line(s) instead of silently changing
all the previous regex to match the second header name.
Also detect and ERROR when header name is missing entirely. Ignore
these lines instead of asserting.
Update External ACL helpers error handling and caching
The helper protocol for external ACLs [1] defines three possible return values:
OK - Success. ACL test matches.
ERR - Success. ACL test fails to match.
BH - Failure. The helper encountered a problem.
The external acl helpers distributed with squid currently do not follow this
definition. For example, upon connection error, ERR is returned:
$ ext_ldap_group_acl ... -d
ext_ldap_group_acl: WARNING: could not bind to binddn 'Can't contact LDAP server'
ERR
This does not allow to distinguish "no match" and "error" either and
therefore negative caches "ERR", also in the case of an error.
Moreover there are multiple problems inside squid when trying to handle BH
responses:
- Squid-5 and Squid-4 retry requests for BH responses but crashes after the
maximum retry number (currently 2) is reached.
- If an external acl helper return always BH (eg because the LDAP server is
down) squid sends infinitely new request to the helper.
Fix "Source and destination overlap in memcpy" Valgrind errors
Before this patch, source and destination arguments in
log_quoted_string() could point to the same static memory area, causing
multiple Valgrind-reported errors. Fixed by creating another buffer to
store quoted-processed output string.
Reduce crashes due to unexpected ClientHttpRequest termination.
The underlying problem has been known since r13480: If a
ClientHttpRequest job ends without Http::Stream (and ConnStateData)
knowledge, then Squid is likely to segfault or assert. This patch does
not resolve the underlying issue (a proper fix would require
architectural changes in a consensus-lacking area) but makes an
unexpected ClientHttpRequest job destruction less likely.
BodyPipe and Adaptation-related exceptions are the major causes of
unexpected ClientHttpRequest job destruction. This patch handles them by
closing the client connection. Connection closure should trigger an
orderly top-down cleanup, including Http::Stream, ConnStateData, and
ClientHttpRequest destruction.
If there is no connection to close, then the exception is essentially
ignored with a level-1 error message disclosing the problem. The side
effects of ignoring such exceptions are unknown, but without a client
connection, it is our hope that they would be relatively benign.
Amos Jeffries [Mon, 26 Dec 2016 02:22:00 +0000 (15:22 +1300)]
Bug 3940 pt2: Make 'cache deny' do what is documented
Instead of overriding whatever cacheability was previously set to
(including changing non-cacheables to be cacheable) actually
prevent both cache read and write.
Do not share private responses with collapsed client(s).
This excessive sharing problem with collapsed forwarding code has
several layers. In most cases, the core CF code does not share
uncachable or private response with collapsed clients because of the
refreshCheckHTTP() check. However, some responses might not be subject
to that (or equivalent) check. More importantly, collapsed revalidation
code does not check its responses at all and, hence, easily shares
private responses.
This short-term fix incorrectly assumes that an entry may become private
(KEY_PRIVATE) only when it cannot be shared among multiple clients
(e.g., because of a Cache-Control:private response header). However,
there are a few other cases when an entry becomes private. One of them
is a DISK_NO_SPACE_LEFT error inside storeSwapOutFileClosed() where
StoreEntry::releaseRequest() sets KEY_PRIVATE for a sharable entry [that
may still be perfectly preserved in the memory cache]. Consequently, the
short-term fix reduces CF effectiveness. The extent of this reduction is
probably environment-dependent.
Also: do not re-use SET_COOKIE headers for collapsed revalidation slaves,
i.e., adhere to the same requirement as for regular response HITs.
Garri Djavadyan [Thu, 15 Dec 2016 09:36:34 +0000 (22:36 +1300)]
Bug 3940 (partial): hostHeaderVerify failures MISS when they should be HIT
This fixes the critical condition leading to the HIT. However not all
code is correctly setting flags.noCache and flags.cacheable (see bugzilla).
So there may be other fixes needed after this.
The following sequence of events triggers this assertion:
- The server sends an 1xx control message.
- http.cc schedules ConnStateData::sendControlMsg call.
- Before sendControlMsg is fired, http.cc detects an error (e.g., I/O
error or timeout) and starts writing the reply to the user.
- The ConnStateData::sendControlMsg is fired, starts writing 1xx, and
hits the "no concurrent writes" assertion.
We could only reproduce this sequence in the lab after changing Squid
code to trigger a timeout at the right moment, but the sequence looks
plausible. Other event sequences might result in the same outcome.
To avoid concurrent writes, Squid now drops the control message if
Http::One::Server detects that a reply is already being written. Also,
ConnStateData delays reply writing until a pending control message write
has been completed.
Bug 4004 partial: Fix segfault via Ftp::Client::readControlReply
Added nil dereference checks for Ftp::Client::ctrl.conn, including:
- Ftp::Client::handlePasvReply() and handleEpsvReply() that dereference
ctrl.conn in DBG_IMPORTANT messages.
- Many functions inside FtpClient.cc and FtpGateway.cc files.
TODO: We need to find a better way to handle nil ctrl.conn. It is only
a matter of time when we forget to add another dereference check or
discover a place we missed during this change.
Also disabled forwarding of EPRT and PORT commands to origin servers.
Squid support for those commands is broken and their forwarding may
cause segfaults (bug #4004). Active FTP is still supported, of course.
Alex Rousskov [Mon, 14 Nov 2016 12:40:51 +0000 (01:40 +1300)]
Honor SBufReservationRequirements::minSize regardless of idealSize.
In a fully specified SBufReservationRequirements, idealSize would
naturally match or exceed minSize. However, the idealSize default value
(zero) may not. We should honor minSize regardless of idealSize, just as
the API documentation promises to do.
No runtime changes expected right now because the only existing user of
SBufReservationRequirements sets .idealSize to CLIENT_REQ_BUF_SZ (4096)
and .minSize to 1024.
Fix ssl::server_name ACL badly broken since inception.
The original server_name code mishandled all SNI checks and some rare
host checks:
* The SNI-derived value was pointing to an already freed memory storage.
* Missing host-derived values were not detected (host() is never nil).
* Mismatches were re-checked with an undocumented "none" value
instead of being treated as mismatches.
Same for ssl::server_name_regex.
Also set SNI for more server-first and client-first transactions.
Amos Jeffries [Sun, 30 Oct 2016 09:45:03 +0000 (22:45 +1300)]
HTTP/1.1: make Vary:* objects cacheable
Under new clauses from RFC 7231 section 7.1.4 and HTTP response
containing header Vary:* (wifcard variant) can be cached, but
requires revalidation with server before each use.
Use the new mandatory revalidation flags to allow storing of any
wildcard Vary:* response.
Note that responses with headers like Vary:A,B,C,* are equivalent
to Vary:*. The cache key string for these objects is normalized.
Squid crashed because HttpMsg::body_pipe was used without check that it
was initialized. The message lacks body pipe when it has no body or
empty body.
HTTP: MUST ignore a [revalidation] response with an older Date header.
Before this patch, Squid violated the RFC 7234 section 4 MUST
requirement: "When more than one suitable response is stored, a cache
MUST use the most recent response (as determined by the Date header
field)." This problem may be damaging in cache hierarchies where
parent caches may have different responses. Trusting the older response
may lead to excessive IMS requests, cache thrashing and other problems.
Alex Rousskov [Sun, 9 Oct 2016 14:30:11 +0000 (03:30 +1300)]
Hide OpenSSL tricks from Valgrind far-reaching initialization errors.
This change has no effect unless ./configured --with-valgrind-debug.
OpenSSL, including its Assembly code, contains many optimizations and
timing defenses that Valgrind misinterprets as uninitialized value
usage. Most of those tricks can be disabled by #defining PURIFY when
building OpenSSL, but some are not protected with PURIFY and most
OpenSSL libraries are (and should be) built without that #define.
To make matters worse, once Valgrind misdetects uninitialized memory, it
will complain about every usage of that memory. Those complaints create
a lot of noise, complicate triage, and effectively mask true bugs.
AFAICT, they cannot be suppressed by listing the source of that memory.
For example, this OpenSSL Assembly trick:
Uninitialised value was created by a stack allocation
at 0x556C2F7: aesni_cbc_encrypt (aesni-x86_64.s:2081)
Triggers many false errors like this one:
Conditional jump or move depends on uninitialised value(s)
by 0x750838: Debug::Finish()
by 0x942E68: Http::One::ResponseParser::parse(SBuf const&)
...
This change marks OpenSSL-returned decrypted bytes as initialized. This
might miss some true OpenSSL bugs, but we should focus on Squid bugs.
Bug 4471: revalidation doesn't work when expired cached object lacks Last-Modified.
The bug was caused by the fact that Squid used only Last-Modified header
value for evaluating entry's last modification time while making an
internal revalidation request. So, without Last-Modified it was not
possible to correctly fill If-Modified-Since header value. As a result,
the revalidation request was not initiated when it should be.
To fix this problem, we should generate If-Modified-Since based on other
data, showing how "old" the cached response is, such as Date and Age
header values. Both Date and Age header values are utilized while
calculating entry's timestamp. Therefore, we should use the timestamp if
Last-Modified is unavailable.
TODO: HttpRequest::imslen support looks broken/deprecated. Using this
field inside StoreEntry::modifiedSince() leads to several useless checks
and probably may cause other [hidden] problems.
... also address Bug 4311 and partially address Bug 2833 and Bug 4471.
Extend collapsed_forwarding functionality to internal revalidation
requests. This implementation does not support Vary-controlled cache
objects and is limited to SMP-unaware caching environments, where each
Squid worker knows nothing about requests and caches handled by other
workers. However, it also lays critical groundwork for future SMP-aware
collapsed revalidation support.
Prior to these changes, multiple concurrent HTTP requests for the same
stale cached object always resulted in multiple internal revalidation
requests sent by Squid to the origin server. Those internal requests
were likely to result in multiple competing Squid cache updates, causing
cache misses and/or more internal revalidation requests, negating
collapsed forwarding savings.
Internal cache revalidation requests are collapsed if and only if
collapsed_forwarding is enabled. There is no option to control just
revalidation collapsing because there is no known use case for it.
* Public revalidation keys
Each Store entry has a unique key. Keys are used to find entries in the
Store (both already cached/swapped_out entries and not). Public keys are
normally tied to the request method and target URI. Same request
properties normally lead to the same public key, making cache hits
possible. If we were to calculate a public key for an internal
revalidation request, it would have been the same as the public key of
the stale cache entry being revalidated. Adding a revalidation response
to Store would have purged that being-revalidated cached entry, even if
the revalidation response itself was not cachable.
To avoid purging being-revalidated cached entries, the old code used
private keys for internal revalidation requests. Private keys are always
unique and cannot accidentally purge a public entry. On the other hand,
for concurrent [revalidation] requests to find the store entry to
collapse on, that store entry has to have a public key!
We resolved this conflict by adding "scope" to public keys:
* Regular/old public keys have default empty scope (that does not affect
key generation). The code not dealing with collapsed revalidation
continues to work as before. All entries stored in caches continue to
have the same keys (with an empty scope).
* When collapsed forwarding is enabled, collapsable internal
revalidation requests get public keys with a "revalidation" scope
(that is fed to the MD5 hash when the key is generated). Such a
revalidation request can find a matching store entry created by
another revalidation request (and collapse on it), but cannot clash
with the entry being revalidated (because that entry key is using a
different [empty] scope).
This change not only enables collapsing of internal revalidation
requests within one worker, but opens the way for SMP-aware workers to
share information about collapsed revalidation requests, similar to how
those workers already share information about being-swapped-out cache
entries.
After receiving the revalidation response, each collapsed revalidation
request may call updateOnNotModified() to update the stale entry [with
the same revalidation response!]. Concurrent entry updates would have
wasted many resources, especially for disk-cached entries that support
header updates, and may have purged being-revalidated entries due to
locking conflicts among updating transactions. To minimize these
problems, we adjusted header and entry metadata updating logic to skip
the update if nothing have changed since the last update.
Also fixed Bug 4311: Collapsed forwarding deadlocks for SMP Squids using
SMP-unaware caches. Collapsed transactions stalled without getting a
response because they were waiting for the shared "transients" table
updates. The table was created by Store::Controller but then abandoned (not
updated) by SMP-unaware caches. Now, the transients table is not created
at all unless SMP-aware caches are present. This fix should also address
complaints about shared memory being used for Squid instances without
SMP-aware caches.
A combination of SMP-aware and SMP-unaware caches is still not supported
and is likely to stall collapsed transactions if they are enabled. Note
that, by default, the memory cache is SMP-aware in SMP environments.
Alex Rousskov [Fri, 23 Sep 2016 12:40:44 +0000 (00:40 +1200)]
Bug 3819: "fd >= 0" assertion in file_write() during reconfiguration
Other possible assertions include "NumberOfUFSDirs" in UFSSwapDir and
"fd >= 0" in file_close(). And Fs::Ufs::UFSStoreState::drainWriteQueue()
may segfault while dereferencing nil theFile, although I am not sure
that bug is caused specifically by reconfiguration.
Since trunk r9181.3.1, reconfiguration is done in at least two steps:
First, mainReconfigureStart() closes/cleans/disables all modules
supporting hot reconfiguration and then, at the end of the event loop
iteration, mainReconfigureFinish() opens/configures/enables them again.
UFS code hits assertions if it has to log entries or rebuild swap.state
between those two steps. The tiny assertion opportunity window hides
these bugs from most proxies that are not doing enough disk I/O or are
not being reconfigured frequently enough.
Asynchronous UFS cache_dirs such as diskd were the most exposed, but
even blocking UFS caching code could probably hit [rebuild] assertions.
The swap.state rebuilding (always initiated at startup) probably did not
work as intended if reconfiguration happened during the rebuild time
because reconfiguration closed the swap.state file being rebuilt. We now
protect that swap.state file and delay rebuilding progress until
reconfiguration is over.
TODO: To squash more similar bugs, we need to move hot reconfiguration
support into the modules so that each module can reconfigure instantly.
Alex Rousskov [Fri, 23 Sep 2016 11:11:48 +0000 (23:11 +1200)]
Do reset $HOME if needed after r13435. Minimize putenv() memory leaks.
While r13435 is attributed to me, the wrong condition was not mine. This
is a good example of why "cmp() op 0" pattern is usually safer because
the "==" or "!=" operator "tells" us whether the strings are equal,
unlike "!cmp()" that is often misread as "not equal".
Alex Rousskov [Wed, 21 Sep 2016 02:09:36 +0000 (14:09 +1200)]
Bug 4228: ./configure bug/typo in r14394.
The bug resulted in "test: too many arguments" error messages when
running ./configure and effectively replaced AC_MSG_ERROR() with
AC_MSG_WARN() for missing but required Heimdal and GNU GSS Kerberos
libraries.
Fix logged request size (%http::>st) and other size-related %codes.
The %[http::]>st logformat code should log the actual number of
[dechunked] bytes received from the client. However, for requests with
Content-Length, Squid was logging the sum of the request header size and
the Content-Length header field value instead. The logged value was
wrong when the client sent fewer than Content-Length body bytes.
Also:
* Fixed %http::>h and %http::<h format codes for icap_log. The old code
was focusing on preserving the "request header" (i.e. not "response
header") meaning of the %http::>h logformat code, but since ICAP
services deal with (and log) both HTTP requests and responses, that
focus on the HTTP message kind actually complicates ICAP logging
configuration and will eventually require introduction of new %http
logformat codes that would be meaningless in access.log context.
- In ICAP context, %http::>h should log to-be-adapted HTTP message
headers embedded in an ICAP request (HTTP request headers in REQMOD;
HTTP response headers in RESPMOD). Before this change, %http::>h
logged constant/"FYI" HTTP request headers in RESPMOD.
- Similarly, %http::<h should log adapted HTTP message headers
embedded in an ICAP response (HTTP request headers in regular
REQMOD; HTTP response headers in RESPMOD and during request
satisfaction in REQMOD). Before this change, %http::<h logged "-" in
REQMOD.
In other words, in ICAP logging context, the ">" and "<" characters
should indicate ICAP message kind (where the being-logged HTTP message
is embedded), not HTTP message kind, even for %http codes.
TODO: %http::>h code logs "-" in RESPMOD because AccessLogEntry does
not store virgin HTTP response headers.
* Correctly initialize ICAP ALE HTTP fields related to HTTP message
sizes for icap_log, including %http::>st, %http::<st, %http::>sh, and
%http::>sh format codes.
* Initialize HttpMsg::hdr_sz in HTTP header parsing code. Among other
uses, this field is needed to initialize HTTP fields inside ICAP ALE.
* Synced default icap_log format documentation with the current code.
projects / thirdparty / squid.git / log
