6.7

Extra v6.7 ChangeLog addition (#1657)

Bug 5337: workaround for crash on startup if -a option is used (#1653)

Interpreting command-line arguments requires AnyP::UriScheme to be fully
initialized. Initialize AnyP::UriScheme earlier to ensure that happens.

Fix memory leak in ssl/gadgets/mimicAuthorityKeyId() (#1651)

An unnecessary std::unique_ptr::release() call prevented temporary
extOct string from being automatically deallocated. The leak usually
happened when SslBump mimicked certificates with an Authority Key
Identifier extension. The leak was added in 2016 commit 5f1318b.

Fix a possible integer overflow in Ftp::Gateway (#1647)

A static analysis tool has discovered that const int csize,
might have overflowed before being passed to writeReplyBody().

ChangeLog for v6.7 (#1646)

NTLM/Negotiate: Fix crash on bad helper TT responses (#1645)

Helper lookup may be made without a client HTTP Request,
(stored in lm_request->request). But in Helper::TT cases the
lm_request->request was dereferenced without any checks.

Extend cache_log_message to Bug 5187 and job invalidation BUGs (#1639)

    70 ERROR: Squid BUG: Job invalidated during ... that threw...
    71 ERROR: Squid BUG: Job invalidated during ...
    72 WARNING: Squid bug 5187 workaround triggered

Bug 5274: Successful tunnels logged as TCP_TUNNEL/500 (#1608)

Stop calling retryOrBail() when the tunneled Squid-server connection
(that we have committed to use) closes. Our retryOrBail() is dedicated
to handling errors. Most[^1] serverClosed() calls are _not_ related to
errors because our tunneling code abuses asynchronous connection closure
callbacks for TunnelStateData work termination. Depending on the
transaction details (e.g., TLS interception vs. true CONNECT), calling
retryOrBail() on these no-error code paths may result in retryOrBail()
"catch all other errors" code creating bogus ERR_CANNOT_FORWARD errors.

Most tunneling errors are already detailed, and retryOrBail() does not
have enough information to correctly detail the remaining ones anyway.
Removing this retryOrBail() call selects the arguably lesser evil.

The client-Squid connection closure callback, clientClosed(), already
uses the same logic.

This change does not resurrect Bug 5132 fixed by commit 752fa20 that
added the now-replaced retryOrBail() call to serverClosed(). That commit
fixed the leak by calling deleteThis() (via retryOrBail()). Our
finishWritingAndDelete() call preserves that logic. That commit also
claimed to allow more retries, but that claim was a mistake: To-server
closure callback registration (e.g. commitToServer()) bans retries.

[^1]: The fact that severClosed() is called for both successful and
problematic outcomes prevents TunnelStateData from properly handling
certain (rare) errors. We tried to fix that as well, but the changes
quickly snowballed, so we left a few XXXs instead.

basic_sspi_auth: Fix build on Windows with libnettle (#1640)

Our detection logic relies on nettle's base64 decoding, if available.
Add libnettle to the basic/SSPI helper when libnettle is available.

Avoid file name conflict with Windows WinSvc.h (#1637)

There is a conflict between our WinSvc.h header file and
Windows system include <winsvc.h>, which is one of the
factors preventing our ability to build native windows helpers.

Remove beta version warning

Improve and broadly use asHex()

Docs: Describe more ACL effects on (re)authentication (#1611)

Existing documentation was

* silent about %ul, max_user_ip, ident, and ident_regex side effects;
* silent about adapted_http_access context triggering authentication;
* vague about (re)authentication triggers.

6.6 (#1615)

Limit the number of allowed X-Forwarded-For hops (#1589)

Squid will ignore all X-Forwarded-For elements listed after the first 64
addresses allowed by the follow_x_forwarded_for directive. A different
limit can be specified by defining a C++ SQUID_X_FORWARDED_FOR_HOP_MAX
macro, but that macro is not a supported Squid configuration interface
and may change or disappear at any time.

Squid will log a cache.log ERROR if the hop limit has been reached.

This change works around problematic ACLChecklist and/or slow ACLs
implementation that results in immediate nonBlockingCheck() callbacks.
Such callbacks have caused many bugs and development complications. In
clientFollowXForwardedForCheck() context, they lead to indirect
recursion that was bound only by the number of allowed XFF entries,
which could reach thousands and exhaust Squid process call stack.

This recursion bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/xff-stackoverflow.html
where it was filed as "X-Forwarded-For Stack Overflow".

FTP: Ignore credentials with a NUL-prefixed username (#1557)

    FATAL: FTP login parsing destroyed username info

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/ftp-fatal.html
where it was filed as "FTP Authentication Crash".

Bug 5154: Do not open IPv6 sockets when IPv6 is disabled (#1567)

... but allow basic IPv6 manipulations like getSockAddr().

    Address.cc:663 getAddrInfo() assertion failed: false

Squids receives IPv6 addresses from traffic, configuration, or
hard-coded constants even when ./configured with --disable-ipv6 or when
IPv6 support was automatically disabled at startup after failing IPv6
tests. To handle IPv6 correctly, such Squids must support basic IPv6
operations like recognizing an IPv6 address in a request-target or
reporting an unsolicited IPv6 DNS record. At least for now, such Squids
must also correctly parse configuration-related IPv6 addresses.

All those activities rely on various low-level operations like filling
addrinfo structure with IP address information. Since 2012 commit
c5fbbc7, Ip::Address::getAddrInfo() was failing for IPv6 addresses when
Ip::EnableIpv6 was falsy. That change correctly recognized[^1] the need
for such Squids to handle IPv6, but to support basic operations, we need
to reject IPv6 addresses at a higher level and without asserting.

That high-level rejection work is ongoing, but initial attempts have
exposed difficult problems that will take time to address. For now, we
just avoid the assertion while protecting IPv6-disabled Squid from
listening on or opening connections to IPv6 addresses. Since Squid
already expects (and usually correctly handles) socket opening failures,
disabling those operations is better than failing in low-level IP
manipulation code.

The overall IPv6 posture of IPv6-disabled Squids that lack http_access
or other rules to deny IPv6 requests will change: This fix exposes more
of IPv6-disabled Squid code to IPv6 addresses. It is possible that such
exposure will make some IPv6 resources inside Squid (e.g., a previously
cached HTTP response) accessible to external requests. Squids will not
open or accept IPv6 connections but may forward requests with raw IPv6
targets to IPv4 cache_peers. Whether these and similar behavior changes
are going to be permanent is open for debate, but even if they are
temporary, they are arguably better than the corresponding assertions.

These changes do not effect IPv6-enabled Squids.

The assertion in IPv6-disabled Squid was reported by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/ipv6-assert.html where
it was filed as "Assertion on IPv6 Host Requests with --disable-ipv6".

[^1]: https://bugs.squid-cache.org/show_bug.cgi?id=3593#c1

log_db_daemon: Fix DSN construction (#1570)

Bug 5317: FATAL attempt to read data from memory (#1579)

    FATAL: Squid has attempted to read data ... that is not present.

Recent commit 122a6e3 attempted to deliver in-memory response body bytes
to a Store-reading client that requested (at least) response headers.
That optimization relied on the old canReadFromMemory() logic, but that
logic results in false positives when the checked read offset falls into
a gap between stored headers and the first body byte of a Content-Range.
In that case, a false positive leads to a readFromMemory() call and a
FATAL mem_hdr::copy() error.

This workaround disables the above optimization without fixing
canReadFromMemory(). We believe that a readFromMemory() call that comes
right after response headers are delivered to the Store-reading client
will not suffer from the same problem because the client will supply the
read offset of the first body byte, eliminating the false positive.

Do not update StoreEntry expiration after errorAppendEntry() (#1580)

errorAppendEntry() is responsible for setting entry expiration times,
which it does by calling StoreEntry::storeErrorResponse() that calls
StoreEntry::negativeCache().

This change was triggered by a vulnerability report by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/cache-uaf.html where
it was filed as "Use-After-Free in Cache Manager Errors". The reported
"use after free" vulnerability was unknowingly addressed by 2022 commit
1fa761a that removed excessively long "reentrant" store_client calls
responsible for the disappearance of the properly locked StoreEntry in
this (and probably other) contexts.

Just close after a write(2) response sending error (#1582)

    FATAL: assertion failed: Http1Server.cc:322: "rep"

2015 commit 21cd322 started to continue ClientStream processing after
socket write(2) failures. In most cases, the code still "worked". For
example, initiateClose() would close the client-Squid connection, and
connStateClosed() would be called before Store has a chance to deliver
response body data requested by pullData() in writeComplete().

However, that response body data could sometimes reach Server, and
handleReply() would assert because startOfOutput() says that we have not
written the headers, but ClientStream state (i.e. a nil `rep` parameter)
says that we have. These assertion can be triggered by disabling
initiateClose(), and they can probably be triggered by traffic as well.

Now, after a Comm::Write() error, we terminateAll() client transactions
on the failed connection[^1] and do not call afterClientWrite() that is
not equipped to handle I/O errors and would continue ClientStream
processing if called.

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/stream-assert.html
where it was filed as "Implicit Assertion in Stream Handling".

[^1]: We terminateAll() instead of potentially postponing closure with
initiateClose() because the failed client-Squid connection most likely
cannot be salvaged for, say, reading the remainder of the request body.

Bug 5318: peer_digest.cc:399: "fetch->pd && receivedData.data" (#1584)

Recent commit 122a6e3 removed HTTP response headers from store_client
responses. That removal created the possibility of an empty
StoreIOBuffer at the beginning of the feeding sequence. Pending Bug 5317
fix will make such buffers even more frequent. Existing store_client
recipients have varying requirements with regard to empty response
buffers, as documented in store_client::finishCallback(). We missed this
requirement conflict in Cache Digest code. This fix adjusts Cache
Digests code to be compatible with empty StoreIOBuffer representation in
current store_client code.

Bug 5319: QOS Netfilter MARK preservation is always disabled (#1585)

Default ./configure options and explicit --enable-zph-qos enabled ZPH
QOS support (USE_QOS_TOS) as expected but did not enable QOS Netfilter
MARK preservation support (USE_LIBNETFILTERCONNTRACK). For example,
qos_flows directive became available, but clientside_mark and
client_connection_mark ACL types were not recognized.

The missing opening bracket before AS_IF() condition injected a trailing
closing bracket into that condition, resulting in an always-false
condition for setting USE_LIBNETFILTERCONNTRACK.

Broken since 2022 commit a1c2236.

Bug 5328: Fix ESI build with libxml2 v2.12.0 (#1600)

    Libxml2Parser.cc:147:40: error: invalid conversion from
    'const xmlError*' to 'xmlErrorPtr' {aka 'xmlError*'} [-fpermissive]

libxml2 recently made xmlGetLastError() return a constant object.

6.5

Remove mem_hdr::freeDataUpto() assertion (#1562)

    stmem.cc:98: "lowestOffset () <= target_offset"

The assertion is conceptually wrong: The given target_offset parameter
may have any value; that value does not have to correlate with mem_hdr
state in any way. It is freeDataUpto() job to preserve nodes at or above
the given offset and (arguably optionally) remove nodes below it, but
the assertion does not actually validate that freeDataUpdo() did that.

The old mem_hdr::freeDataUpto() assertion incorrectly assumed that,
after zero or more unneeded memory nodes were freed, the remaining
memory area never starts after the given target_offset parameter. That
assumption fails in at least two use cases, both using target_offset
values that do not belong to any existing or future mem_hdr node:

1. target_offset is points to the left of the first node. freeDataUpto()
   correctly keeps all memory nodes in such calls, but then asserts. For
   example, calling freeDataUpto(0) when mem_hdr has bytes [100,199)
   triggers this incorrect assertion.

2. target_offset is in the gap between two nodes. For example, calling
   freeDataUpto(2000) when mem_hdr contains two nodes: [0,1000) and
   [3000,3003) will trigger this assertion (as happened in Bug 5309).
   Such gaps are very common for HTTP 206 responses with a Content-Range
   header because such responses often specify a range that does not
   start with zero and create a gap after the node(s) with HTTP headers.

Bugs notwithstanding, it is unlikely that relevant calls exist today,
but they certainly could be added, especially when freeDataUpto() stops
preserving the last unused node. The current "avoid change to [some
unidentified] part of code" hoarding excuse should not last forever.

Prior to commit 122a6e3, Squid did not (frequently) assert in gap cases:
Callers first give target_offset 0 (which results in freeDataUpto()
doing nothing, keeping the header node(s)) and then they give
target_offset matching the beginning of the first body node (which
results in freeDataUpto() freeing the header nodes(s) and increasing
lowerOffset() from zero to target_offset). A bug in commit 122a6e3
lowered target_offset a bit, placing target_offset in the gap and
triggering frequent (and incorrect) assertions (Bug 5309).

Bug 5309: frequent "lowestOffset () <= target_offset" assertion (#1561)

Recent commit 122a6e3 left store_client::readOffset() unchanged but
should have adjusted it to match changed copyInto.offset semantics:
Starting with that commit, storeClientCopy() callers supply HTTP
response _body_ offset rather than HTTP response offset.

This bug decreased readOffset() values (by the size of stored HTTP
response headers), effectively telling Store that we are not yet done
with some of the MemObject/mem_hdr bytes. This bug could cause slightly
higher transaction memory usage because the same response bytes are
trimmed later. This bug should not have caused any assertions.

However, the old mem_hdr::freeDataUpto() code that uses readOffset() is
also broken -- the assertion in that method only "works" when
readOffset() returns values matching a memory node boundary. The smaller
values returned by buggy readOffset() triggered buggy assertions.

This minimal fix removes the recent store_client::readOffset() bug
described above. We will address old mem_hdr problems separately.

Exit without asserting when helper process startup fails (#1543)

... to dup() after fork() and before execvp().

Assertions are for handling program logic errors. Helper initialization
code already handled system call errors correctly (i.e. by exiting the
newly created helper process with an error), except for a couple of
assert()s that could be triggered by dup(2) failures.

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/ipc-assert.html
where it was filed as 'Assertion in Squid "Helper" Process Creator'.

RFC 1123: Fix date parsing (#1538)

The bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/datetime-overflow.html
where it was filed as "1-Byte Buffer OverRead in RFC 1123 date/time
Handling".

Improve handling of expanding HTTP header values (#1536)

Squid manipulations often increase HTTP header value length compared to
the corresponding raw value received by Squid. Raw header length is
checked against request_header_max_size and reply_header_max_size that
default to 64KB, making the raw value safe to store in a String object
(by default). However, when the increased length of a manipulated value
exceeds String class limits, Squid leaks memory, asserts, or possibly
stalls affected transactions. The long-term fix for this problem is a
complete String elimination from Squid sources, but that takes time.

Known manipulations may effectively concatenate headers and/or increase
header value length by 50%. This workaround makes such known increases
safe by essentially tripling String class limits:

    (64KB + 64KB) * 150% = 3 * 64KB

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/response-memleaks.html
where it was filed as "Memory Leak in HTTP Response Parsing".

Maintenance: reduce output of CONTRIBUTORS update script (#1527)

Reduce output to brief summary of actual changes (if any).

Add --quiet to completely silence all non-error output.

Add --verbose for extra information. May be repeated.

6.4

Fix validation of certificates with CN=* (#1523)

The bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/
where it was filed as "Buffer UnderRead in SSL CN Parsing".

Fix stack buffer overflow when parsing Digest Authorization (#1517)

The bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/digest-overflow.html
where it was filed as "Stack Buffer Overflow in Digest Authentication".

RFC 9112: Improve HTTP chunked encoding compliance (#1498)

Fix userinfo percent-encoding (#1367)

%X expects an unsigned int, and that is what we were giving it. However,
to get to the correct unsigned int value from a (signed) char, one has
to cast to an unsigned char (or equivalent) first.

Broken since inception in commit 7b75100.

Also adjusted similar (commented out) ext_edirectory_userip_acl code.

Fix store_client caller memory leak on certain errors (#1347)

When a storeUnregister() code path destroys store_client before the
latter has a chance to deliver the answer, the cbdataReferenceDone()
call in store_client::finishCallback() is not reached, keeping the
callback data (e.g., clientReplyContext) alive forever. These
storeClientCopy() "cancellations" may happen, for example, when the
client-to-Squid connection is closed while store_client waits for Store.

Use CallbackData to guarantee cbdataReferenceDone() when store_client is
destructed before it can finishCallback(). These synchronous callbacks
will be replaced with AsyncCalls. For now, we use the "discouraged"
CallbackData API to accommodate the existing legacy callbacks.

Probably broken since 2002 commit fa80a8e.

CI: More HTTP caching and revalidation tests (#1440)

Use Daft cache-response test to monitor for bugs in basic caching code,
including bugs like the one fixed by commit c203754. Unlike the old
proxy-collapsed-forwarding test that uses concurrent requests, this test
varies basic response properties.

Use Daft accumulate-headers-after-304 test to monitor for bugs like the
one fixed by commit 55e1c6e. Unlike old proxy-update-headers-after-304,
this test focuses on certain _problematic_ HTTP 304 header updates.

CI: Remove unnecessary test-functionality test wrappers (#1393)

These workarounds are not needed for the current and future code in this
branch. Other branches get their own test-functionality.sh files that
can be used to maintain a branch-specific collection of test wrappers.

The (now unused) has_commit_by_message() function was left in the script
because that function is likely to be used by future workarounds. Unlike
specific test workarounds that only apply to a subset of old code, this
and similar functions can be viewed as a reusable code "library".

CI: Remove pass-through test-functionality test wrappers (#1383)

Instead of requiring a custom test wrapper for each test and, hence,
creating an ever-increasing number of pass-through wrappers that do
nothing useful, use a custom test wrapper if and only if it exists. By
default (i.e. when there is no custom wrapper), just run the named test.

As a positive side effect, this change also simplifies running tests
that are not on the $default_tests list hard-coded in main():

    test-suite/test-functionality.sh some-new-test-i-am-working-on

Miss if a 304 update would exceed reply_header_max_size (#1420)

Fetch the resource unconditionally when a 304 (Not Modified) response to
an internal cache revalidation request grows cached HTTP response
headers beyond the reply_header_max_size limit.

Remove serialized HTTP headers from storeClientCopy() (#1335)

Do not send serialized HTTP response header bytes in storeClientCopy()
answers. Ignore serialized header size when calling storeClientCopy().

This complex change adjusts storeClientCopy() API to addresses several
related problems with storeClientCopy() and its callers. The sections
below summarize storeClientCopy() changes and then move on to callers.

Squid incorrectly assumed that serialized HTTP response headers are read
from disk in a single storeRead() request. In reality, many situations
lead to store_client::readBody() receiving partial HTTP headers,
resulting in parseCharBuf() failure and a level-0 cache.log message:

    Could not parse headers from on disk object

Inadequate handling of this failure resulted in a variety of problems.
Squid now accumulates storeRead() results to parse larger headers and
also handles parsing failures better, but we could not just stop there.

With the storeRead() accumulation in place, it is no longer possible to
send parsed serialized HTTP headers to storeClientCopy() callers because
those callers do not provide enough buffer space to fit larger headers.
Increasing caller buffer capacity does not work well because the actual
size of the serialized header is unknown in advance and may be quite
large. Always allocating large buffers "just in case" is bad for
performance. Finally, larger buffers may jeopardize hard-to-find code
that uses hard-coded 4KB buffers without using HTTP_REQBUF_SZ macro.

Fortunately, storeClientCopy() callers either do not care about
serialized HTTP response headers or should not care about them! The API
forced callers to deal with serialized headers, but callers could (and
some did) just use the parsed headers available in the corresponding
MemObject. With this API change, storeClientCopy() callers no longer
receive serialized headers and do not need to parse or skip them.
Consequently, callers also do not need to account for response headers
size when computing offsets for subsequent storeClientCopy() requests.

Restricting storeClientCopy() API to HTTP _body_ bytes removed a lot of
problematic caller code. Caller changes are summarized further below.

A similar HTTP response header parsing problem existed in shared memory
cache code. That code was actually aware that headers may span multiple
cache slices but incorrectly assumed that httpMsgParseStep() accumulates
input as needed (to make another parsing "step"). It does not. Large
response headers cached in shared memory triggered a level-1 message:

    Corrupted mem-cached headers: e:...

Fixed MemStore code now accumulates serialized HTTP response headers as
needed to parse them, sharing high-level parsing code with store_client.

Old clientReplyContext methods worked hard to skip received serialized
HTTP headers. The code contained dangerous and often complex/unreadable
manipulation of various raw offsets and buffer pointers, aggravated by
the perceived need to save/restore those offsets across asynchronous
checks (see below). That header skipping code is gone now. Several stale
and misleading comments related to Store buffers management were also
removed or updated.

We replaced reqofs/reqsize with simpler/safer lastStreamBufferedBytes,
while becoming more consistent with that "cached" info invalidation. We
still need this info to resume HTTP body processing after asynchronous
http_reply_access checks and cache hit validations, but we no longer
save/restore this info for hit validation: No need to save/restore
information about the buffer that hit validation does not use and must
never touch!

The API change also moved from-Store StoreIOBuffer usage closer to
StoreIOBuffers manipulated by Clients Streams code. Buffers in both
categories now contain just the body bytes, and both now treat zero
length as EOF only _after_ processing the response headers.

These changes improve overall code quality, but this code path and these
changes still suffer from utterly unsafe legacy interfaces like
StoreIOBuffer and clientStreamNode. We cannot rely on the compiler to
check our work. The risk of these changes exposing/causing bugs is high.

asHandleReply() expected WHOIS response body bytes where serialized HTTP
headers were! The code also had multiple problems typical for manually
written C parsers dealing with raw input buffers. Now replaced with a
Tokenizer-based code.

To skip received HTTP response headers, peerDigestHandleReply() helper
functions called headersEnd() on the received buffer. Twice. We have now
merged those two parsing helper functions into one (that just checks the
already parsed headers). This merger preserved "304s must come with
fetch->pd->cd" logic that was hidden/spread across those two functions.

urnHandleReply() re-parsed received HTTP response headers. We left its
HTTP body parsing code unchanged except for polishing NUL-termination.

netdbExchangeHandleReply() re-parsed received HTTP response headers to
find where they end (via headersEnd()). We improved handing of corner
cases and replaced some "tricky bits" code, reusing the new
Store::ParsingBuffer class. The net_db record parsing code is unchanged.

Mgr::StoreToCommWriter::noteStoreCopied() is a very special case. It
actually worked OK because, unlike all other storeClientCopy() callers,
this code does not get serialized HTTP headers from Store: The code
adding bytes to the corresponding StoreEntry does not write serialized
HTTP headers at all. StoreToCommWriter is used to deliver kid-specific
pieces of an HTTP body of an SMP cache manager response. The HTTP
headers of that response are handled elsewhere. We left this code
unchanged, but the existence of the special no-headers case does
complicate storeClientCopy() API documentation, implementation, and
understanding.

Co-authored-by: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>

Replaced clientReplyContext::tempBuffer with old_reqofs (#1304)

The tempBuffer data member was not actually used as a buffer. We only
used its offset field, and only for saving reqofs (which has a different
type than tempBuffer.offset!). Replaced the buffer with old_reqofs,
consistent with the rest of the "saved stale entry state" code.

Also fixed old_reqsize type to match reqsize and grouped that member
with the other private "saved stale entry state" fields.

Bad old types probably did not trigger runtime failures because the
associated saved numbers are saved at the very beginning of fetching the
entry, when all these accumulation-related counters are still small.

The remaining reqofs and reqsize types are wrong for platforms where
size_t is not uint64_t, but fixing that deserves a dedicated change. For
now, we just made the types of "old_" and "current" members consistent.

Y2038: improve printing of time settings (#1493)

Avoid truncation errors when printing time_t-based squid.conf directives
on platforms with 32-bit int and 64-bit time_t. Also avoid similar
errors when printing time_msec-based directives on platforms with 32-bit
int.

Detected by Coverity. CID 1529622: Use of 32-bit time_t (Y2K38_SAFETY).

Bug 5300: cachemgr.cgi assertion (#1478)

Bug 5301: cachemgr.cgi not showing new manager interface URLs (#1479)

Also fix several related UI issues uncovered during testing:

* Prune the list of servers accessible via the CGI tool login.
 Their responses would be badly mangled if accessed via
 the old tools parse logic.
Also, hide the old login form if all servers use the new
manager interface.

* Ensure the 'menu' report is always used by default after
the CGI tool login. This prevents errors about MGR_INDEX
not being available on recent Squid releases. Restoring the
expected CGI tool behavior.

Extend cache_log_message to problematic from-helper annotations (#1481)

    WARNING: Unsupported or unexpected from-helper annotation
        with a name reserved for Squid use

The above message is emitted for every helper response containing
problematic annotations. Let admins control this reporting using
cache_log_message directive and message id 69.

Improve handling of empty lines received prior to request-line (#1470)

Restore support for legacy cache_object cache manager requests (#1475)

Squid v6.3 commit 6695897 (i.e. a backport of master/v7 commit 3c383cc)
accidentally removed support of legacy cache_object URLs (that master/v7
does not support) from Squid v6. This fix restores that support in v6.

Do not use static initialization to register modules (#1422)

    ERROR: ... Unknown authentication scheme 'ntlm'.

When a translation unit does not contain main() and its code is not used
by the rest of the executable, the linker may exclude that translation
unit from the executable. This exclusion by LTO may happen even if that
code _is_ used to initialize static variables in that translation unit:
"If no variable or function is odr-used from a given translation unit,
the non-local variables defined in that translation unit may never be
initialized"[^1].

[^1]: https://en.cppreference.com/w/cpp/language/initialization

For example, src/auth/ntlm/Scheme.o translation unit contains nothing
but NtlmAuthRr class definition and static initialization code. The
linker knows that the rest of Squid does not use NtlmAuthRr and excludes
that translation unit from the squid executable, effectively disabling
NTLM module registration required to parse "auth_param ntlm" directives.

The problem does affect existing NTLM module, and may affect any future
module code as we reduce module's external footprint. This change
converts all RegisteredRunner registrations from using side effects of
static initialization to explicit registration calls from SquidMain().
Relying on "life before main()" is a design bug. This PR fixes that bug
with respect to RegisteredRunner registrations.

Due to indeterminate C++ static initialization order, no module can
require registration before main() starts. Thus, moving registration
timing to the beginning of SquidMain() should have no negative effects.

The new registration API still does not expose main.cc to any module
details (other than the name of the registration function itself). We
still do not need to #include any module headers into main.cc. Compiler
or linker does catch most typos in RegisteredRunner names.

Unfortunately, explicit registration still cannot prevent bugs where we
forget to register a module or fail to register a module due to wrong
registration code guards. Eventually, CI will expose such bugs.

6.3 (#1467)

Reference point for automated CONTRIBUTORS updates

basic_smb_lm_auth: fix 'no previous declaration' warnings (#1462)

... that breaks build with --enable-strict-error-checking

CacheManager: require /squid-internal-mgr/ URL path prefix (#1426)

    ERROR: Squid BUG: assurance failed: tok.skip(internalMagicPrefix)
    exception location: cache_manager.cc(173) ParseUrl

Due to poor code duplication, commit 92a5adb accidentally classified
URLs without a trailing slash in the magical prefix as valid cache
manager URLs, triggering the above ERRORs. We were denying such
"slashless" cache manager URLs (as invalid internal URLs) prior to that
commit. Since that commit, the ERRORs triggered by that commit
effectively denied them as well. Denying them properly results in
simpler/smaller code (than allowing them would), so we should avoid a UI
change and continue to deny them, at least for now.

This change also reduces duplication of magic prefix definitions. Other
pending work will completely eliminate that duplication in src/ code.

Docs: Update all Squid Bugzilla URLs (#1447)

The Squid Bugzilla now uses https:// instead of http://

Bug 5294: ERR_CANNOT_FORWARD returned instead of ERR_DNS_FAIL (#1453)

Since 2017 commit fd9c47d, peer selection code stopped reporting
ERR_DNS_FAIL cases because PeerSelector::noteIps() treated DNS answers
without IP addresses as if at least one IP address was received. Without
seeing a DNS resolution error, the ultimate recipient of the DNS
resolution results (e.g., CONNECT tunneling or regular forwarding code)
used ERR_CANNOT_FORWARD to indicate a failure to find a forwarding path.

PeerSelector::noteIps() code mimicked legacy IPH code with regard to
handling of the addresses parameter. However, IPH caller had a special
emptyIsNil adjustment that was missing from the noteIps() call! We now
apply that adjustment to both noteIps() and IPH code paths.

Long-term, we should probably remove nil address container pointers.
Having two different ways to signal lack of IPs is dangerous. Currently,
there is only one known supplier of nil address container:
IpcacheStats.invalid code that validates ipcache_nbgethostbyname() name
parameter. Either the corresponding nil/empty name check should be
converted into an assertion (blaming the ipcache_nbgethostbyname()
caller for not validating the name) OR that checking code should supply
an empty address container to finalCallback().

Bug 4981: Work around in-call job invalidation bugs (#1428)

Bug 4981 is one known case of such invalidation, but this workaround is
much broader than that bug context. We can speculate that architectural
problems described in commit e3b6f15 are behind (some of) these bugs.

Start building ChangeLog for v6.3

ESI: Fix build [-Wsingle-bit-bitfield-constant-conversion] (#1432)

clang 16, the default on current fedora rawhide and centos stream 9,
complains about several ESI places:

    error: implicit truncation from 'int' to a one-bit wide bit-field
    changes value from 1 to -1
    [-Werror,-Wsingle-bit-bitfield-constant-conversion]

Turn 1-bit flags in ESI data structures from ints to unsigned ints to
avoid the problem.

Docs: Update all Squid wiki URLs (#1448)

The Squid Wiki now uses https:// instead of http://

Also, conversion to github altered the URL fragment
label syntax to all lowercase with hyphen instead of
underscore.

Maintenance: stop using doxygen bug markers (#1445)

6.2

Bug 5290: pure virtual call in Ftp::Client constructor (#1429)

    FATAL: Dying from an exception handling failure;
    exception: [no active exception]

Converting `this` to CbcPointer in a constructor of an abstract class
like Ftp::Client does not work because our virtual toCbdata() method
remains pure until the final/child class constructor runs.

Conceptually, the bug was probably introduced in 2013 commit 434a79b,
when FTP class hierarchy grew, making Ftp::Client an abstract class, but
the trigger was recent commit 337b9aa that removed CBDATA_CLASS() from
Ftp::Client class declaration. We discovered, described, and addressed
several such bugs in that commit, but we missed this case.

Maintenance: replace most NULL with nullptr

Bug 5187: Work around REQMOD satisfaction regression (#1400)

Commit ba3fe8d broke ICAP REQMOD satisfaction transactions. In some
cases, this workaround may resurrect Squid Bug 5187. Triage available at
https://bugs.squid-cache.org/show_bug.cgi?id=5187#c6

Maintenance: Remove unused xstrtoul() from the global scope (#1407)

xstrtoul() is only used by xstrtoui() in the same module, no need to
export it to the global namespace

Fix build on GNU/Hurd (#1417)

Definitions of htonl() and htons() require these headers, in this order.

Fix memory leak when reconfiguring multiline all-of ACLs (#1425)

Normally, Acl::InnerNode::add() automatically registers stored ACL nodes
for future cleanup, but when we find the second all-of rule/line (with
the same ACL name), we do not add() the newly created OrNode and have to
explicitly register it to avoid memory leaks on reconfiguration.

Leaking since all-of ACL support was added in 2013 commit 6f58d7d.

6.1 (#1410)

Do not cache (and do not serve cached) cache manager responses (#1185)

The fixed bug affected cache manager transactions that were using
/squid-internal-mgr URL path prefix with http(s) URL scheme. It did not
affect transactions that were using legacy cache_object URL scheme.

Stale cache manager responses had their Age response header set to the
number of seconds since Unix epoch. If disk and memory caches were
disabled, then cache manager requests just triggered "found KEY_PRIVATE"
WARNINGs in cache.log (for reasons that remain unclear).

Probably broken since 2011 commit e37bd29 that did not expand
HttpRequest::maybeCacheable() (called cacheable() back then)
PROTO_CACHE_OBJECT check to include /squid-internal-mgr requests.

Also added missing Access-Control-* response headers to cache manager
responses in SMP mode and reduced code duplication related to sending
those headers (which led to them missing in SMP Squids).

Fix key equality comparison in LookupTable map (#1379)

An std::unordered_map with case-insensitive keys must use a
case-insensitive key equality comparison. lookupTable_t used (the
default) std::equal_to<SBuf> comparison which is case-sensitive.

The full extent of this bug effects is unknown, but Squid was
mishandling Cache-Control and Surrogate-Control directives with
non-canonical (i.e. other than all-lower-case) spelling. Similar
problems affected WWW-Authenticate Digest parameter names, but the
related code remains inconsistent, with both case-sensitive and
case-insensitive checks applied to some of the key names in
Auth::Digest::Config::decode().

Also removed a similarly buggy (and, technically, unused) "typical use"
example from the CaseInsensitiveSBufHash class description. C++
documentation and Squid code are better sources of usage examples when
it comes to STL-used concepts like hash function objects.

This minimal fix excludes LookupTable class polishing.

The bug was introduced in 2015 commit 81ab22b.

Honor DNS RR TTLs larger than negative_dns_ttl (#1380)

Since 2017 commit fd9c47d, Squid was effectively ignoring DNS RR TTLs
that exceeded negative_dns_ttl (i.e. 60 seconds by default) because the
"find the smallest TTL across the DNS records seen so far" code in
ipcache_entry::updateTtl() mistook the "default" ipcache_entry::expires
value as the one based on an earlier seen DNS record.

In most cases, this bug decreased IP cache hit ratio.

Existing fqdncache code does not suffer from the same bug because
fqdncacheParse() always resets fqdncache_entry::expires instead of
updating it incrementally. ipcacheParse() has to update incrementally
because it is called twice per entry, once with an A answer and once
with an AAAA answer.

Ideally, ipcache_entry::expires should be made optional to eliminate
awkward "first updateTtl() call" detection, but doing so well requires
significant code changes, so that entries without a known expiration
value are not cached forever _unless_ they were loaded from /etc/hosts.
And those changes should probably be propagated to fqdncache.cc.

Bug 5278: Log %err_code for "early" request handling errors (#1382)

Certain "early" request handling errors (e.g., ERR_TOO_BIG,
ERR_UNSUP_REQ, ERR_UNSUP_HTTPVERSION, ERR_PROTOCOL_UNKNOWN, and
ERR_INVALID_URL) are handled with nil ClientHttpRequest::request object.
ErrorState relied on that object to record error type and details, so
%err_code expanded to "-" in the corresponding access.log records.

Now, ErrorState also uses ALE (where available). ALE already handles the
combination of an early error and late HttpRequest (e.g., the fake one
clientReplyContext::createStoreEntry() creates when handling the error).

This minimal fix does not address known problems with ErrorState::detail
and ErrorState::BuildHttpReply(). It also duplicates related xerrno
detailing code. We will address some of these problems shortly,
including the increased code duplication problem.

Documentation: Update stale SMP cache_dir caveats (#1394)

The requirement to specify "workers" before "cache_dir" was added in
2010 commit acf69d7. It became obsolete since 2011 commit 095ec2b.

The "dedicated cache directory" hack for UFS-based stores has always led
to HTTP violations, but the increased complexity of worker-to-worker
synchronization code (required to improve HTTP support) also increased
the probability of crashes or worse outcomes when SMP conditionals are
used. Those hacks violate the "all processes see the same configuration"
and similar basic code assumptions. We do not test (and usually do not
even consider the needs of) such unsupported configurations.

6.0.3 (#1376)

---------

Co-authored-by: Francesco Chemolli <5175948+kinkie@users.noreply.github.com>

Maintenance: Remove a stale sponsor URL (#1351)

SpinUp has ceased operations. Their URL is used by an unrelated entity.

Maintenance: update release process script (#1340)

* release notes no longer mention specific version since
  c0789db1f479206e43f96add370363127834757a. No more need to
  validate it is correct before release.

* web server has not been the direct release destination for
  many years. Remove mentions of $dst/changesets/

* web server references github repository for basic
  information. Do not push CONTRIBUTORS, COPYING, README,
  CREDITS and SPONSORS files to staging area.

* release notes no longer link to local ChangeLog file.
  No need to adjust the hyperlink.

Remove obsolete caddr_t (#1362)

Fix time_t type conflicts in libdebug (#1357)

    error: cannot convert 'const long int*' to 'const time_t*'

On Windows, time_t is 64-bit, and long is not.

Bug 5148: Log %Ss of failed tunnels as TCP_TUNNEL (#1354)

    ERR_CONNECT_FAIL/errno=111 NONE_NONE/503 CONNECT

    TAG_NONE/503 0 CONNECT example.com:443 - HIER_NONE/- -

When Squid failed to open a TCP connection to an origin server (or
through a cache peer) while handling a CONNECT request, Squid logged %Ss
as NONE_NONE because TunnelStateData waited for a successful TCP
connection to update the log tag. That unnecessary wait (modeled after
the necessary HTTP status code wait) complicated code. Squid now logs
TCP_TUNNEL to indicate the request handling path chosen by Squid.

We already use the same "handling path" approach for most other request
status codes. For example, TCP_MISS does not mean the miss transaction
was successful. It only means that Squid satisfied the request using the
cache miss forwarding logic. Even the LOG_TCP_REFRESH_FAIL_ERR status
code does not imply that the error response was successfully forwarded
to the client; only that the request was satisfied on a particular
(albeit very detailed in this case!) handling path.

Apply the same logic to tunneling attempts blocked by miss_access. They
were also logged as NONE_NONE and are now logged as TCP_TUNNEL.

The actual outcome of a tunneling attempt can usually be determined
using %err_code/%err_detail and %>Hs logformat codes.

Fix fatalf() undefined for environments without syslog (#1356)

src/debug/debug.cc:1087
error: 'fatalf' was not declared in this scope

Do not erase aborted StoreMap entries that are still being read (#1344)

The old `!s.lock.readers` precondition for calling freeChain() in
StoreMap::abortWriting() was not broad enough: It covered "old" readers
that have incremented ReadWriteLock::readers already while missing those
lockShared() callers that have fully qualified for becoming a reader but
have not incremented the `readers` counter yet. A correct test is now
implemented by ReadWriteLock::stopAppendingAndRestoreExclusive().

This code was broken since appending mode was introduced in 2013 commit
ce49546. Evidently, the mishandled race condition is short-lived enough
and abortWriting() calls are rare enough, that there are no (known)
relevant bug reports. This bug was accidentally discovered while reading
the code to explain another SMP bug.

Controller::markedForDeletionAndAbandoned() similarly misjudges
readers(), but that bug does not lead to serious problems, and we would
like to find a fix that identifies such entries without false positives
(that looking at ReadWriteLock::readLevel instead of readers creates).

Also polished Ipc::StoreMap::abortWriting() debugging a little.

Do not leak Security::CertErrors created in X509_verify_cert() (#1346)

ACLFilledChecklist was using a raw C pointer for handling cbdata-managed
Security::CertErrors. Some sslErrors users knew about hidden cbdata
requirements, some did not, resulting in inconsistent locking/unlocking
and associated memory leaks. Upgrading ACLFilledChecklist::sslErrors to
smart CbcPointer fixes those leaks (and simplifies code).

6.0.2

* Prepare ChangeLog for v6.0.2 (#1326)

* 6.0.2

---------

Co-authored-by: Francesco Chemolli <5175948+kinkie@users.noreply.github.com>

Fix default build with libgnutls but absent GnuTLS headers (#1332)

Ensure that when libgnutls is found but any of the required GnuTLS
headers cannot be used, GnuTLS support is not enabled by default and an
explicit request for GnuTLS support is fatally rejected during
./configure. Otherwise, the build fails later anyway, during "make".

The problem was exposed by a mingw-cross build.

Docs: Update 6.0.1 bug reference (#1328)

Prepare ChangeLog for v5.9 (#1327)

Improve reply_body_max_size matching accuracy (#1324)

    HttpReply.cc(553) receivedBodyTooLarge: -4096 >? -1

ClientHttpRequest::out.offset is documented to only count body bytes,
and that is what receivedBodyTooLarge() needs. We should not subtract
the guessed header size, occasionally violating reply_body_max_size
configuration and often leading to a confusing negative body size in
receivedBodyTooLarge() debugging.

Possibly broken since 2003 commit b51aec6: out.offset was not documented
back then, but, AFAICT, it was already used just for the body bytes.

Fix GCC v13 build [-Warray-bounds] (#1318)

    src/ipc/mem/FlexibleArray.h:34:52: error: array subscript -1 is
    below array bounds of 'int [1]' [-Werror=array-bounds]

We suspect this warning is a GCC v13 regression bug because the callers
marked as problematic by GCC (e.g., Rock::LoadingEntry::LoadingEntry) do
not use "array subscript -1", and the Ipc::StoreMapItems::at() operator
they use even asserts that the subscript is not negative. It might be
GCC bug 107699: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107699

This change replaces the fake one-item array hack with a properly
aligned byte (still used as the "start of the real array" marker).

Also removed some unused and problematic code (instead of polishing it).

Improve detection of lonely StoreEntry clients (#1312)

The first storeClientType() call still sees zero nclients because
storeClientListAdd() creates a store_client object (which triggers
storeClientType()) first and only then adds it to the list of clients
(which increases nclients). This off-by-one bug was introduced in commit
528b2c6 that reordered nclients increase and the storeClientType() call.

The bug could lead to excessive disk I/O in some environments because
Squid could see more STORE_DISK_CLIENT store_client objects, and those
may trigger startSwapin() in store_client::doCopy(). However, the exact
effects of this bug/fix are difficult to predict because startSwapin()
may be separated by async calls and is guarded by additional conditions.

A full fix requires refactoring the caller, but there are so many
problems associated with storeClientType() and startSwapin() calls that
such refactoring deserves a dedicated effort.

Technically, this "first" client may be 10th client this StoreEntry has
seen, but it is the first one among the _current_ entry clients.

negotiate_kerberos_auth: Fix build [-Wunused-but-set-variable] (#1294)

Unused variable discovered by clang v15.

ESI: Fix build [-Wdeprecated-declarations] (#1295)

Replaced deprecated htmlDefaultSAXHandlerInit() libxml2 initialization
call with xmlInitParser().

Fix build with clang v15 [-Wunused-but-set-variable] (#1296)

Remove unused variable in neigbours.cc:neighborsUdpPing

Fix build with clang v15.0.6 [-Warray-parameter] (#1297)

Adapt a mismatching function signature in stub_libip.cc to the
corresponding declaration.

Fix several broken or missing copyright statements (#1300)

bootstrap.sh: Polish output (#1301)

We use bootstrap.sh a lot in maintenance scripts but do not
need to see its output on success.

 * Fix some incorrect uses of stderr and stdout such that
   all (and only) errors appear on stderr

 * Polish the message display when an error does occur
    detecting one of the tools.

CI: Remove irrelevant branch from v6 GitHub Actions config (#1311)

The original GitHub Actions configuration (see commit 2ed8cc4)
incorrectly assumed that GitHub will use that configuration (stored in
the master branch) for testing _all_ repository branches. In reality,
GitHub uses configuration from the being-tested branch. Thus, we do not
need to (and, hence, should not) list any branches in the X branch
configuration, with the exception of the X branch itself and the "auto"
branch where all PRs (including all X-targeting PRs) are staged for the
final "as if merged" testing.

----

Cherry-picked master/v7 commit 58ecf6052ef258aaafd6d4c810a0757299113e9b
and then replaced "master" with "v6" because "this branch" is v6.

6.0.1

Maintenance: Updated CONTRIBUTORS (#1289)

This change is a reference point for automated CONTRIBUTORS updates.

Translation: Update Georgian translation (#1279)