]> git.ipfire.org Git - thirdparty/squid.git/log
thirdparty/squid.git
11 months agoMinGW: fix winsock dependency issues (#1375)
Amos Jeffries [Sun, 2 Jul 2023 09:33:16 +0000 (09:33 +0000)] 
MinGW: fix winsock dependency issues (#1375)

  error: warning: Please include winsock2.h before windows.h

Cleanup these header includes across all Squid code built for
Windows.

Also, detect these headers early to fix autoconf tests relying
on them.

11 months agobasic_sspi_auth: MinGW build fixes (#1399)
Amos Jeffries [Thu, 29 Jun 2023 18:20:28 +0000 (18:20 +0000)] 
basic_sspi_auth: MinGW build fixes (#1399)

Also, remove #error check preventing cross-compile

11 months agoDrop cache_object protocol support (#1250)
Eduard Bagdasaryan [Thu, 29 Jun 2023 10:42:29 +0000 (10:42 +0000)] 
Drop cache_object protocol support (#1250)

Removing this non-standard protocol (already mentioned as deprecated
in Squid sources) helps eliminate duplication and simplifies the
existing error-prone forwarding logic (causing CVEs).

Separate commits replace cache_object URLs sent by squidclient and
cachemgr.cgi tools with URLs using an http scheme.

11 months agoUpdate libsspwin32 (#1348)
Amos Jeffries [Wed, 28 Jun 2023 21:14:28 +0000 (21:14 +0000)] 
Update libsspwin32 (#1348)

Move library files to lib/sspi/ for better modularity.

Add SQUID_CHECK_WIN32_SSPI autoconf test for dependency
checking by helpers.

Add missing HAVE_FOO_H wrappers around includes.
Fixes Squid coding style compliance.

Remove unnecessary __cplusplus protections.

Remove many redundant includes from helpers code.

Remove CygWin hack which is unreachable with precompiler
conditions.

Fix several structure initialization compiler errors.

11 months agoMaintenance: Remove dead Multicast Miss Stream feature (#1320)
Alex Rousskov [Wed, 28 Jun 2023 16:15:49 +0000 (16:15 +0000)] 
Maintenance: Remove dead Multicast Miss Stream feature (#1320)

This feature was added in 1998 commit e66d792 for special "you are
absolutely certain you understand what you are doing" experiments and
considered "largely undocumented and unsupported" by its primary
author[^1]. The corresponding MULTICAST_MISS_STREAM code in
access_log.cc has not built since 2007 commit cc192b5 (at least):

* 'class Ip::Address' has no member named 's_addr'
* 'comm_open' was not declared in this scope
* 'comm_udp_sendto' was not declared in this scope
* 'mcastSetTtl' was not declared in this scope
* 'METHOD_GET' was not declared in this scope
* 'no_addr' was not declared in this scope
* no match for 'operator!=' (operand types are LogTags and LogTags_ot)

[^1]: Duane Wessels. 2004. Squid: The Definitive Guide. O'Reilly &
Associates, Inc., USA. See mcast_miss_addr documentation on page 390.

11 months agoReject more CONNECT requests with malformed targets (#1253)
Alex Rousskov [Wed, 28 Jun 2023 09:28:59 +0000 (09:28 +0000)] 
Reject more CONNECT requests with malformed targets (#1253)

Squid silently ignored many syntax violations, interpreting a malformed
target as a valid host:port address of some origin server and
establishing a CONNECT tunnel with that origin server. While being
"tolerant" probably did not compromise the Squid instance itself, some
known attacks abuse the _difference_ in treatment of malformed requests.
Rejecting malformed requests and closing the connection prevents many
such attacks.

Among other syntax violations, this change rejects bracketed pure IPv4
addresses and bracketed domain names in CONNECT targets. Bracketed IPv6
addresses that include an IPv4address suffix are still accepted (e.g.,
look for ABNF containing ls32 in RFC 3986, Section 3.2.2).

CONNECT target hosts are no longer covered by uri_whitespace: CONNECT
targets containing whitespace are now rejected with in ERR_INVALID_URL
regardless of uri_whitespace and check_hostnames settings. With the
exception of whitespaces in host names when uri_whitespace is set to
"strip" (default), the uri_whitespace directive was already ignored for
all request targets. For example, whitespaces in the port component were
always handled without checking uri_whitespace. In CONNECT context,
stripping whitespace is not only risky but probably is not needed in
practice because user typos should not lead to spaces in CONNECT
targets, and even if they do, virtually all legitimate use cases ought
to fail during certificate validation and similar post-CONNECT checks.

11 months agoFix store_client caller memory leak on certain errors (#1347)
Alex Rousskov [Tue, 27 Jun 2023 22:47:36 +0000 (22:47 +0000)] 
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.

11 months agoMinGW-w64: update libcompat structure (#1341)
Amos Jeffries [Tue, 27 Jun 2023 16:50:16 +0000 (16:50 +0000)] 
MinGW-w64: update libcompat structure (#1341)

Separate the Squid for Windows portability fixes for MinGW and
MSVC builds. Beginning a new MinGW-w64 specific port with
a clean starting base.

With these changes the libcompat portability library builds.
The rest of Squid remains mostly non-building.

11 months agoDo not cache (and do not serve cached) cache manager responses (#1185)
Alex Rousskov [Tue, 27 Jun 2023 11:58:16 +0000 (11:58 +0000)] 
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).

11 months agoRemove serialized HTTP headers from storeClientCopy() (#1335)
Alex Rousskov [Sat, 24 Jun 2023 08:18:55 +0000 (08:18 +0000)] 
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.

### storeClientCopy() changes

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.

### Primary code path (clientReplyContext::SendMoreData, CacheHit, etc.)

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.

### AS number WHOIS lookup

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.

### Cache Digests

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.

### URN resolver

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

### NetDB exchange

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.

### SMP Cache Manager

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>
11 months agoDocumentation: Update stale SMP cache_dir caveats (#1394)
Alex Rousskov [Fri, 23 Jun 2023 23:05:27 +0000 (23:05 +0000)] 
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.

11 months agoImprove PSAPI.dll detection (#1391)
Amos Jeffries [Thu, 22 Jun 2023 16:33:05 +0000 (16:33 +0000)] 
Improve PSAPI.dll detection (#1391)

Detect Windows PSAPI library like any other library dependency. Allowing
build --with(out) options to disable or to set paths.

Detect the psapi.h properly and set HAVE_* macro for proper include
wrapping.

Wrap PSAPI logic in check for PSAPI_VERSION which is defined whenever
the API support is actually available.

11 months agoDrop PRIuSIZE macro (#1388)
Amos Jeffries [Tue, 20 Jun 2023 15:13:41 +0000 (15:13 +0000)] 
Drop PRIuSIZE macro (#1388)

C++11 requires compiler support for %zu so
we no longer need to check for it.

11 months agoWindows: Drop obsolete WinSock v1 library (#1384)
Amos Jeffries [Tue, 20 Jun 2023 08:52:03 +0000 (08:52 +0000)] 
Windows: Drop obsolete WinSock v1 library (#1384)

wsock32 library and its winsock.h are for Windows 95
and older version socket support. We no longer need
Squid to run on such old Windows machines.

11 months agoHonor DNS RR TTLs larger than negative_dns_ttl (#1380)
Alex Rousskov [Mon, 19 Jun 2023 01:48:38 +0000 (01:48 +0000)] 
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.

11 months agoCI: Remove pass-through test-functionality test wrappers (#1383)
Alex Rousskov [Sun, 18 Jun 2023 00:30:35 +0000 (00:30 +0000)] 
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

12 months agoBug 5278: Log %err_code for "early" request handling errors (#1382)
Alex Rousskov [Sat, 17 Jun 2023 16:34:11 +0000 (16:34 +0000)] 
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.

12 months agoForget non-peer access details (#1378)
Eduard Bagdasaryan [Fri, 16 Jun 2023 15:10:59 +0000 (15:10 +0000)] 
Forget non-peer access details (#1378)

We were using the CachePeer class to record the address of each non-peer
sending us certain unwanted ICP responses, along with the number of such
responses and a histogram of associated ICP opcodes. Since 1997 commit
e102ebd, these historical records were accumulated without a limit and
linearly searched, endangering a Squid instance that used ICP. Using
CachePeer to store this non-cache_peer information also complicated
cache_peer code.

This change removes these records and the corresponding mgr:non_peers
report, leaving just the cache.log warning about unexpected messages.
The warning is useful because these ICP messages from non-peers indicate
a cache hierarchy misconfiguration or a fairly sophisticated attack.

This is the simplest fix that minimizes Squid and developer resources
spent on handling these errors.

12 months agoFix key equality comparison in LookupTable map (#1379)
Alex Rousskov [Wed, 14 Jun 2023 14:09:38 +0000 (14:09 +0000)] 
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.

12 months agoStop leaking PeerDigests on reconfiguration (#1371)
Eduard Bagdasaryan [Mon, 12 Jun 2023 13:51:16 +0000 (13:51 +0000)] 
Stop leaking PeerDigests on reconfiguration (#1371)

peerDigestCreate() cbdata-locked the digest twice. The second lock was a
"self lock" -- a lock without storing the locked pointer somewhere. That
self lock was introduced (with an XXX) in 2002 commit fa80a8e. It did
not have the corresponding unlock, and Squid leaked the digest object.
That leak became conditional in 2018 commit b56b37c: Since then, Squid
was missing one digest unlock only when the CachePeer object was gone
before a peerDigestDestroy() call, as happens during reconfiguration.

Also removed a pair of excessive cbdata locks/unlocks (that triggered
this leak): Long-term users should lock when they start storing a
pointer to the cbdata-protected object and unlock when they stop.

Also converted PeerDigest::peer to CbcPointer, addressing a TODO.

Also fixed subtle problems with detecting gone CachePeer and gone
PeerDigest objects in peerDigestFetchedEnough().

Also removed stale peerNoteDigestGone() declaration.

12 months agoDo not report DNS answers without A/AAAA records by default (#1369)
Alex Rousskov [Sun, 4 Jun 2023 19:41:19 +0000 (19:41 +0000)] 
Do not report DNS answers without A/AAAA records by default (#1369)

Default (i.e. level-0/1) cache.log error reports should focus on
problems that may affect Squid components or transaction security rather
than on individual transaction failures due to external agents (that
most Squid admins cannot prevent or fix). The latter should be detailed
in access.log, where individual transactions are reported.

We were already using this principle for ipcache_entry::latestError()
calls except for one call devoted to DNS responses that have no A/AAAA
records in the answer section _but_ have other records in that section.
Removing that exceptional treatment simplifies Squid code and addresses
admin complaints about cache.log pollution with minor error messages
that they often cannot prevent or stop.

12 months agoPrepare changelog for v6.0.3 (#1374)
Francesco Chemolli [Sun, 4 Jun 2023 15:25:09 +0000 (15:25 +0000)] 
Prepare changelog for v6.0.3 (#1374)

12 months agoDestroy an idle PeerDigest after its CachePeer disappears (#1372)
Eduard Bagdasaryan [Sat, 3 Jun 2023 16:46:17 +0000 (16:46 +0000)] 
Destroy an idle PeerDigest after its CachePeer disappears (#1372)

Delaying this destruction until the next peerDigestCheck() event results
in accumulation of PeerDigests (and related objects) during frequent
reconfigurations.

12 months agoFix missing portability warning for Solaris (#1360)
Amos Jeffries [Wed, 31 May 2023 23:46:51 +0000 (23:46 +0000)] 
Fix missing portability warning for Solaris (#1360)

These headers contain implementations of POSIX API
and cannot be touched without the required OS-specific
knowledge.

The addition or update of Solaris portability logic
wrongly omitted this warning.

12 months agoMaintenance: Remove a stale sponsor URL (#1351)
Francesco Chemolli [Wed, 31 May 2023 17:13:51 +0000 (17:13 +0000)] 
Maintenance: Remove a stale sponsor URL (#1351)

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

12 months agoFix userinfo percent-encoding (#1367)
Alex Rousskov [Thu, 25 May 2023 02:10:28 +0000 (02:10 +0000)] 
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.

12 months agoMaintenance: update release process script (#1340)
Amos Jeffries [Wed, 24 May 2023 03:57:07 +0000 (03:57 +0000)] 
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.

12 months agoRemove obsolete caddr_t (#1362)
Amos Jeffries [Mon, 22 May 2023 14:32:31 +0000 (14:32 +0000)] 
Remove obsolete caddr_t (#1362)

12 months agoFix time_t type conflicts in libdebug (#1357)
Amos Jeffries [Mon, 22 May 2023 04:30:00 +0000 (04:30 +0000)] 
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.

12 months agoBug 5148: Log %Ss of failed tunnels as TCP_TUNNEL (#1354)
Alex Rousskov [Sat, 20 May 2023 21:38:38 +0000 (21:38 +0000)] 
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.

12 months agoFix fatalf() undefined for environments without syslog (#1356)
Amos Jeffries [Sat, 20 May 2023 05:23:23 +0000 (05:23 +0000)] 
Fix fatalf() undefined for environments without syslog (#1356)

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

12 months agoDo not apply custom debugs() format to Debug::Extra lines (#1345)
Alex Rousskov [Thu, 18 May 2023 16:28:35 +0000 (16:28 +0000)] 
Do not apply custom debugs() format to Debug::Extra lines (#1345)

Some debugs() callers set and leave custom std::ostream formats like
std::hex. Such behavior should be considered natural/correct because,
from those callers point of view, they see and control the entire
debugging message. Stream formats obviously must not cross debugs()
boundaries, and, from those callers point of view, those boundaries are
the message they explicitly call with. Here is a hypothetical example:

    debugs(33, DBG_CRITICAL, "Invalid QoS flag: " << std::hex << flag);

When we automatically add extra information to debugging messages (e.g.,
to detail which transaction has failed, what listening port is
misconfigured, or where the exception was thrown from), the "extra"
lines are added in a completely different code context. That detailing
code also sees a seemingly isolated printing context and should not be
expected to remember to reset debugging stream flags "just in case":

    os << Debug::Extra << "listening port: " << s.port();

The combination of natural code expectations, may result in very
misleading messages because, for example, hexadecimal numbers may look
just like decimal ones. We have seen that happening to transaction IDs
and here is an example based on the above debugs() that complains about
an invalid QoS flag on http_port 80 (but logs port 50):

    2023/05/09 16:44:13| Invalid QoS flag: deadbeef
        listening port: 50

This change would break any _deliberate_ format "bleeds" from primary
debugs() messages to Debug::Extra lines, but no such bleeding cases are
known, and they are too confusing/fragile to be encouraged/supported.

13 months agoDo not erase aborted StoreMap entries that are still being read (#1344)
Alex Rousskov [Wed, 17 May 2023 19:33:51 +0000 (19:33 +0000)] 
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.

13 months agoDo not populate StoreEntry basics from Transients (#1343)
Alex Rousskov [Sat, 13 May 2023 00:30:52 +0000 (00:30 +0000)] 
Do not populate StoreEntry basics from Transients (#1343)

    client_side_reply.cc:1078: "http->storeEntry()->objectLen() >= 0"
    inside clientReplyContext::storeOKTransferDone()

Since commit 24c9378, Transients are not supposed to maintain "basic"
cache entry information (i.e. StoreMapAnchor::Basics a.k.a. StoreEntry
STORE_META_STD fields). Using often-at-their-defaults Transient basics
may unexpectedly erase valuable StoreEntry information obtained from one
of the caches (e.g., swap_file_sz) and lead to assertions.

Consequently, do not populate unused Transients entry basics either.

Eventually, we may find a good way to parameterize StoreMapAnchor to
avoid keeping those unused fields in Transients StoreMap.

13 months agoDo not leak Security::CertErrors created in X509_verify_cert() (#1346)
Alex Rousskov [Wed, 10 May 2023 20:45:27 +0000 (20:45 +0000)] 
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).

13 months agoDo not check store_status when checking ENTRY_BAD_LENGTH (#1342)
Alex Rousskov [Wed, 3 May 2023 22:20:27 +0000 (22:20 +0000)] 
Do not check store_status when checking ENTRY_BAD_LENGTH (#1342)

Before 1997 commit b34ed72 (that introduced the current ENTRY_BAD_LENGTH
"cache"), checking store_status before calling storeEntryValidLength()
kind of made sense because storeEntryValidLength() did not support
STORE_PENDING entries with their unset object_len. Since that commit,
checking store_status does not cause any visible problems (i.e. the code
"works") because ENTRY_BAD_LENGTH implies STORE_OK. However, that
implication is not backed by some fundamental invariant and might
suddenly disappear. Checking two flags instead of one is also wasteful.

No Squid functionality changes are expected.

13 months agoPrepare ChangeLog for v6.0.2 (#1326)
Francesco Chemolli [Thu, 27 Apr 2023 23:25:38 +0000 (23:25 +0000)] 
Prepare ChangeLog for v6.0.2 (#1326)

13 months agoFix default build with libgnutls but absent GnuTLS headers (#1332)
Francesco Chemolli [Thu, 27 Apr 2023 16:42:02 +0000 (16:42 +0000)] 
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.

13 months agoDocs: Update 6.0.1 bug reference (#1328)
Amos Jeffries [Tue, 25 Apr 2023 20:58:51 +0000 (20:58 +0000)] 
Docs: Update 6.0.1 bug reference (#1328)

13 months agoDrop pointless testCharacterSet method (#1329)
Amos Jeffries [Tue, 25 Apr 2023 16:15:54 +0000 (16:15 +0000)] 
Drop pointless testCharacterSet method (#1329)

It does nothing and CharacterSet is tested elsewhere.

13 months agoPrepare ChangeLog for v5.9 (#1327)
Francesco Chemolli [Tue, 25 Apr 2023 07:59:45 +0000 (07:59 +0000)] 
Prepare ChangeLog for v5.9 (#1327)

13 months agoMaintenance: remove some unit test headers (#1293)
Francesco Chemolli [Mon, 24 Apr 2023 20:23:37 +0000 (20:23 +0000)] 
Maintenance: remove some unit test headers (#1293)

When a unit test class is not reused across translation units, merge the
class declaration with its implementation and remove the header file. In
many cases, these classes are not meant/designed to be reusable that
way, and we should not imply otherwise by publishing a header file.

Excuse src/tests/testAuth.h until we make that test work again.

13 months agoImprove reply_body_max_size matching accuracy (#1324)
Alex Rousskov [Sat, 22 Apr 2023 16:45:07 +0000 (16:45 +0000)] 
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.

13 months agoMaintenance: Use C++ stdlib headers (#1314)
Francesco Chemolli [Fri, 21 Apr 2023 16:25:02 +0000 (16:25 +0000)] 
Maintenance: Use C++ stdlib headers (#1314)

In C++ files, use C++ standard library headers instead of their
(guarded) C equivalents (e.g., use `<cstdlib>` instead of `<stdlib.h>`).

13 months agoWCCP: fix inverted range check (#1323)
Amos Jeffries [Wed, 19 Apr 2023 14:55:13 +0000 (14:55 +0000)] 
WCCP: fix inverted range check (#1323)

Correct a typo in 464223c147a568169e940fb466947bdb556d87af

14 months agoAdd buffered_logs OFF support to UDP logger (#1315)
Rybakov Andrey [Mon, 17 Apr 2023 09:46:32 +0000 (09:46 +0000)] 
Add buffered_logs OFF support to UDP logger (#1315)

UDP log module always buffers log entries with a limit of
1400 bytes. Enable the "buffered_logs off" squid.conf
setting to disable this behaviour.

14 months agoRemove dead "binary HTTP header logging" code (-DHEADERS_LOG) (#1319)
Alex Rousskov [Mon, 3 Apr 2023 22:14:26 +0000 (22:14 +0000)] 
Remove dead "binary HTTP header logging" code (-DHEADERS_LOG) (#1319)

The secret HEADERS_LOG hack was added in 2000 commit c360932 to collect
"better stats" than mgr:http_headers. The corresponding access_log.cc
code does not build since 2006 commit 985c86b (at least):

* 'm' was not declared in this scope
* cannot convert 'int*' to 'const char*'
* cannot convert 'short unsigned int*' to 'const char*'
* invalid conversion from 'unsigned char*' to 'const char*'
* invalid conversion from 'void*' to 'HttpReply*'
* invalid conversion from 'void*' to 'HttpRequest*'
* invalid use of incomplete type 'class HttpHdrCc'
* too few arguments to function 'logfileOpen(const char*, size_t, int)'

The removed hack is not to be confused with log_mime_hdrs directive.

14 months agoFix GCC v13 build [-Warray-bounds] (#1318)
Alex Rousskov [Sun, 2 Apr 2023 07:42:10 +0000 (07:42 +0000)] 
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).

14 months agoSimplify use of DIST_SUBDIRS (#1309)
Amos Jeffries [Mon, 27 Mar 2023 10:12:46 +0000 (10:12 +0000)] 
Simplify use of DIST_SUBDIRS (#1309)

automake only needs the DIST_SUBDIRS list to be set 'manually' when
the SUBDIRS list is built using AC_SUBST variables.

For Makefile.am which exclusively use AC_CONDITIONAL we can leave
the DIST_SUBDIRS construction to automake.

14 months agoImprove detection of lonely StoreEntry clients (#1312)
Alex Rousskov [Sat, 25 Mar 2023 03:40:27 +0000 (03:40 +0000)] 
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.

14 months agoFix rock/RockSwapDir.cc "slot->sameKey()" assertion (#1310)
Alex Rousskov [Fri, 17 Mar 2023 14:50:15 +0000 (14:50 +0000)] 
Fix rock/RockSwapDir.cc "slot->sameKey()" assertion (#1310)

Introduced in 2014 commit fa2301d, the assertion was too strict: Due to
store_client idiosyncrasies, openStoreIO() may be called long after the
initial cache hit is detected and locked for reading. It is possible
that the StoreEntry object has gone private since that detection and
locking. Private StoreEntry objects lose their public keys. The existing
anchor lock assures that we still point to the right disk entry, but we
cannot check that invariant using the now-private StoreEntry key.

There are no known sightings of this assertion in the wild, but we can
now reproduce it using high-load Web Polygraph tests. It is not clear
whether some other Squid changes made the triggering sequence of events
a bit more likely, or we were just lucky to discover the right workload.

15 months agoRemove some duplicate typedef and forward declarations (#1305)
Amos Jeffries [Sun, 12 Mar 2023 20:42:20 +0000 (20:42 +0000)] 
Remove some duplicate typedef and forward declarations (#1305)

15 months agoReplaced clientReplyContext::tempBuffer with old_reqofs (#1304)
Alex Rousskov [Sat, 11 Mar 2023 05:48:14 +0000 (05:48 +0000)] 
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.

15 months agobootstrap.sh: Polish output (#1301)
Amos Jeffries [Wed, 8 Mar 2023 12:52:53 +0000 (12:52 +0000)] 
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.

15 months agoReduce stale errno usage (#1302)
Alex Rousskov [Mon, 6 Mar 2023 18:44:38 +0000 (18:44 +0000)] 
Reduce stale errno usage (#1302)

This covers a few easy cases, one of which resulted in wasted triage
time. More changes are needed to cover all potentially stale errno uses.

15 months agoFix several broken or missing copyright statements (#1300)
Amos Jeffries [Mon, 6 Mar 2023 00:05:19 +0000 (00:05 +0000)] 
Fix several broken or missing copyright statements (#1300)

15 months agoRemoved unused mem_hdr methods (#1298)
Eduard Bagdasaryan [Sat, 4 Mar 2023 18:09:05 +0000 (18:09 +0000)] 
Removed unused mem_hdr methods (#1298)

Unused probably since 2003 commit 528b2c6.

15 months agoFix build with clang v15.0.6 [-Warray-parameter] (#1297)
gkinkie@gmail.com [Sat, 4 Mar 2023 16:08:26 +0000 (16:08 +0000)] 
Fix build with clang v15.0.6 [-Warray-parameter] (#1297)

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

15 months agoMaintenance: Better inet_ntop6() local variable declarations (#1080)
gkinkie@gmail.com [Sat, 4 Mar 2023 11:08:04 +0000 (11:08 +0000)] 
Maintenance: Better inet_ntop6() local variable declarations (#1080)

15 months agoFix build with clang v15 [-Wunused-but-set-variable] (#1296)
gkinkie@gmail.com [Sat, 4 Mar 2023 04:53:55 +0000 (04:53 +0000)] 
Fix build with clang v15 [-Wunused-but-set-variable] (#1296)

Remove unused variable in neigbours.cc:neighborsUdpPing

15 months agoESI: Fix build [-Wdeprecated-declarations] (#1295)
gkinkie@gmail.com [Sat, 4 Mar 2023 01:46:20 +0000 (01:46 +0000)] 
ESI: Fix build [-Wdeprecated-declarations] (#1295)

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

15 months agonegotiate_kerberos_auth: Fix build [-Wunused-but-set-variable] (#1294)
gkinkie@gmail.com [Thu, 2 Mar 2023 23:50:10 +0000 (23:50 +0000)] 
negotiate_kerberos_auth: Fix build [-Wunused-but-set-variable] (#1294)

Unused variable discovered by clang v15.

15 months agoCI: Remove irrelevant branch from master GitHub Actions config (#1292)
Alex Rousskov [Tue, 28 Feb 2023 21:40:14 +0000 (21:40 +0000)] 
CI: Remove irrelevant branch from master GitHub Actions config (#1292)

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.

15 months agoMaintenance: Updated CONTRIBUTORS (#1289)
Alex Rousskov [Mon, 27 Feb 2023 16:54:10 +0000 (16:54 +0000)] 
Maintenance: Updated CONTRIBUTORS (#1289)

This change is a reference point for automated CONTRIBUTORS updates.

15 months agoImprove AnyP::Uri::port_ and related port storage types (#1255)
Alex Rousskov [Mon, 27 Feb 2023 10:56:57 +0000 (10:56 +0000)] 
Improve AnyP::Uri::port_ and related port storage types (#1255)

The old "unsigned short" type is used for various things all over the
code and cannot distinguish a parsed port zero from an absent port. In
theory, it could also hold values exceeding 65535, getting out of sync
with various (poorly duplicated) maximum port value checks.

No significant runtime changes are expected, but some transactions with
unknown request URI ports may now be reported correctly, showing "-"
instead of "0" for the missing port number. In some very special/rare
on_unsupported_protocol cases, Squid might no longer attempt to tunnel
non-HTTP requests to a zero port. Such attempts were failing anyway.

This change leaves many "unsigned short" port use cases untouched. Most
of them should be converted, but most (if not all) of those conversions
should be done in dedicated projects. Here are a few TODO highlights:

* ACLIntRange: Stores integers, is usable for integers, but assumes it
  is being configured with port numbers. We probably should enhance that
  class to handle "all" integers instead of expanding/deepening that
  rather limiting "input is just port numbers" assumption.

* CachePeer and internalRemoteUri(): Likely runtime changes related to
  squid.conf validity.

* Ftp::Server::listenForDataConnection() and friends: Triggers a few
  changes with runtime effects related to comm_local_port() failures.

* Ip::Address, Snmp::Session, fde: Too many low-level changes, some of
  which would be difficult to test.

* Config.Port.icp: Possible runtime changes related to squid.conf and
  command-line arguments validity.

Also used "%hu" instead of "%u" or "%d" to printf() ports. These changes
arguably improve code and might make some pedantic compiler happier, but
they do not fix any "real" bugs because variadic printf() arguments are
automatically promoted from short to int and, hence, printf()
implementations never get and never expect shorts.

15 months agoPrep for 5.8 and 6.0.1 (#1287)
Amos Jeffries [Mon, 27 Feb 2023 00:33:45 +0000 (00:33 +0000)] 
Prep for 5.8 and 6.0.1 (#1287)

15 months agopinger: improve timer accuracy and resolution (#1277)
Amos Jeffries [Sat, 25 Feb 2023 16:28:14 +0000 (16:28 +0000)] 
pinger: improve timer accuracy and resolution (#1277)

For consistency across platforms, we also increase the ICMP request
timeout on Windows systems from 5 to 10 seconds.

15 months agoTranslation: Update Georgian translation (#1279)
NorwayFun [Sat, 25 Feb 2023 14:51:58 +0000 (14:51 +0000)] 
Translation: Update Georgian translation (#1279)

15 months agoMaintenance: Change unit test class names to CamelCase (#1288)
gkinkie@gmail.com [Sat, 25 Feb 2023 11:53:17 +0000 (11:53 +0000)] 
Maintenance: Change unit test class names to CamelCase (#1288)

Besides applying Project CamelCase class naming rules to all CppUnit
test class names, these script-driven changes also adjust affected class
name-dependent code, improve consistency across unit test declarations a
little, and rename a few test classes (e.g. testURL is not TestUri). The
corresponding source code filenames are left unchanged (for now).

15 months agoAdd unit tests for ClpMap (#1262)
gkinkie@gmail.com [Fri, 24 Feb 2023 20:45:27 +0000 (20:45 +0000)] 
Add unit tests for ClpMap (#1262)

15 months agoRemoved unused StoreSearchUFS (#1285)
Eduard Bagdasaryan [Thu, 23 Feb 2023 23:35:48 +0000 (23:35 +0000)] 
Removed unused StoreSearchUFS (#1285)

It is unused since 2015 commit 2745fea.

15 months ago7.0.0 (#1284)
squidadm [Tue, 21 Feb 2023 20:52:49 +0000 (20:52 +0000)] 
7.0.0 (#1284)

15 months agoMaintenance: improve release notes automation (#1264)
Amos Jeffries [Mon, 20 Feb 2023 07:05:13 +0000 (07:05 +0000)] 
Maintenance: improve release notes automation (#1264)

* auto-update the text version numbers
* add template for automatic series creation

15 months agoRemoved unused StoreIOState::STFNCB callbacks (#1283)
Alex Rousskov [Mon, 20 Feb 2023 03:07:45 +0000 (03:07 +0000)] 
Removed unused StoreIOState::STFNCB callbacks (#1283)

Unused since before 2016 commit abf396e that discovered the problem.

15 months agoRemoved unused storeClientCopyPending() (#1282)
Alex Rousskov [Mon, 20 Feb 2023 00:06:21 +0000 (00:06 +0000)] 
Removed unused storeClientCopyPending() (#1282)

Unused since 2002 commit edce4d9.

15 months agoEnsure StoreEntry presence during store_client lifetime (#1281)
Alex Rousskov [Sun, 19 Feb 2023 00:23:31 +0000 (00:23 +0000)] 
Ensure StoreEntry presence during store_client lifetime (#1281)

Most likely, all store_client users already lock their StoreEntry
objects, so this change is unlikely to affect any current use cases.
However, objects like store_client that store StoreEntry objects for
long-term/asynchronous reuse should lock their entries rather than rely
on others to do that for them. Being able to rely on StoreEntry locking
also clarifies existing logic in several store_client methods.

15 months agoSource Format Enforcement (#1280)
Alex Rousskov [Sat, 18 Feb 2023 18:12:17 +0000 (18:12 +0000)] 
Source Format Enforcement (#1280)

    NOTICE: File tools/squidclient/squidclient.cc changed: by astyle

15 months agoCI: Disable optimizations in "minimal" build tests (#1278)
Alex Rousskov [Tue, 14 Feb 2023 21:31:11 +0000 (21:31 +0000)] 
CI: Disable optimizations in "minimal" build tests (#1278)

Our "minimal" layer tests had to stop disabling optimizations in 2019
commit 4f3c41b due to problems specific to GCC v9 (v9.1 or earlier). GCC
v9.4 (and later) appear to work OK (at least on Ubuntu 22.04), so let's
try to resume testing --disable-optimizations ./configure option.

16 months agoEnable compiler checks for printf-like SBuf methods (#1270)
Eduard Bagdasaryan [Tue, 14 Feb 2023 16:25:32 +0000 (16:25 +0000)] 
Enable compiler checks for printf-like SBuf methods (#1270)

After commit 0f17e25, these two are the only known printf()-like
functions not already covered by these compiler checks. Some helper
debugging hacks do not have PRINTF_FORMAT_ARG2 or similar attributes,
but they are covered because their `__GNUC__` variants use standard
fprintf().

16 months agoFix --disable-optimizations build: Undefined RetryGapUsec (#1276)
Alex Rousskov [Tue, 14 Feb 2023 14:23:33 +0000 (14:23 +0000)] 
Fix --disable-optimizations build: Undefined RetryGapUsec (#1276)

Since inception (2017 commit e99fa72), FileOpeningConfig::RetryGapUsec
was declared but not defined in any translation unit (a bug). The build
probably "worked" because compilers simply inlined the value of the
constant data member when calling xusleep(). Commit 5eee0e4 removed code
that passed RetryGapUsec to that external "C" function. Now, the same
member is passed to a heavily-inlined and convoluted STL code, and at
least some compilers (e.g., GCC v10-v12 on Ubuntu 22.04) stopped
inlining RetryGapUsec unless optimization is enabled. Our CI tests
passed because we do not test --disable-optimizations builds (yet).

We do not need FileOpeningConfig::RetryGapUsec to be static, breaking
builds, requiring extra code, and triggering questions. We do not even
need it to be constant, but removing "const" is an arguably out-of-scope
change, as is improving its type.

File::InvalidHandle is missing a definition as well, except on Windows,
but we are leaving that code "as is" for now, until we can test whether
Windows is OK with removing that "static" keyword.

16 months agoCI: Extend GitHub Actions build-tests to doc/release-notes/ (#1275)
Alex Rousskov [Tue, 14 Feb 2023 10:03:34 +0000 (10:03 +0000)] 
CI: Extend GitHub Actions build-tests to doc/release-notes/ (#1275)

"make dist[check]" does nothing in doc/release-notes unless a linuxdoc
tool has been detected during ./configure, missing errors like this:

    make[6]: Entering directory /squid/doc/release-notes
    linuxdoc -B html -T 2 --split=0 release-6.sgml
    Processing file release-6.sgml
    onsgmls: ...E: end tag for element "E" which is not open

16 months agoPlace more msgs under cache_log_message control, downgrade some (#1273)
Alex Rousskov [Tue, 14 Feb 2023 00:00:20 +0000 (00:00 +0000)] 
Place more msgs under cache_log_message control, downgrade some (#1273)

Also support cache_log_message-controlled messages in ::Config-unaware
libraries (e.g., libip) used by tools (e.g., cachemgr.cgi).

Restricting the very first (i.e. "depth 0") "Processing Configuration
File" message does not work during startup (before that file is parsed),
and does not work during reconfiguration (because the old configuration
is reset shortly before logging that line). Future reconfiguration
support improvements may fix the reset problem. Restrictions do work as
expected for included files (i.e. positive "depth" levels).

Restricting "BCP 177 violation" WARNINGs does not work because the
warnings are printed _before_ Squid configuration is parsed. Future
initialization improvements may fix this.

Also downgraded the importance of a few (re)configuration
progress-reporting messages from level 1 to level 2.

16 months agoRemove unused clientReplyContext::holdingBuffer (#1272)
Alex Rousskov [Mon, 13 Feb 2023 20:38:38 +0000 (20:38 +0000)] 
Remove unused clientReplyContext::holdingBuffer (#1272)

The data member has been unused since inception in 2003 commit 4993f57.
The underlying dumb StoreIOBuffer class does not lock/copy anything.

16 months agoFix typo in release notes (#1274)
Amos Jeffries [Mon, 13 Feb 2023 14:31:03 +0000 (14:31 +0000)] 
Fix typo in release notes (#1274)

16 months agoRemove unused Ftp::Gateway::printfReplyBody() (#1271)
Eduard Bagdasaryan [Mon, 13 Feb 2023 02:57:14 +0000 (02:57 +0000)] 
Remove unused Ftp::Gateway::printfReplyBody() (#1271)

Ftp::Gateway::printfReplyBody() has been unused since 0477a07.

16 months agoRun cache_peer probes without transaction context (#1256)
Eduard Bagdasaryan [Fri, 10 Feb 2023 15:44:18 +0000 (15:44 +0000)] 
Run cache_peer probes without transaction context (#1256)

Even when triggered by a master transaction, these background probes
should not be associated with it. That transaction just wants to know
whether the neighbor is "up" right now. The transaction does not wait
(and is not aware of) the background probe its question may or may not
have triggered.

16 months agoCleanup: remove xusleep() hacks (#613)
Amos Jeffries [Fri, 10 Feb 2023 12:25:07 +0000 (12:25 +0000)] 
Cleanup: remove xusleep() hacks (#613)

usleep() has been obsolete for a long time. C++11 provides better and
portable waiting mechanism we can use easily instead of these hacks
trying to replicate and extend usleep().

16 months agoMaintenance: Add ACL key change tests (#1269)
Eduard Bagdasaryan [Tue, 7 Feb 2023 20:22:54 +0000 (20:22 +0000)] 
Maintenance: Add ACL key change tests (#1269)

Squid bans key changes in req_header/rep_header and note ACLs
since 4a3b853.

16 months agoPrep for 6.0.1 (#1263)
Amos Jeffries [Mon, 6 Feb 2023 23:06:48 +0000 (23:06 +0000)] 
Prep for 6.0.1 (#1263)

16 months agoDocumentation: Polish "refresh_pattern percent" description (#1260)
Amos Jeffries [Sun, 5 Feb 2023 21:00:38 +0000 (21:00 +0000)] 
Documentation: Polish "refresh_pattern percent" description (#1260)

The original text contained a "last modification age" typo and was
misinterpreted by some admins as if Squid applied the configured percent
to the current object age.

The text also did not make it clear that the percent-based heuristic is
effectively only applied to responses with a Last-Modified header (in
addition to the usual "without an explicit expiry time" precondition).

16 months agoAdd memFreeRigid to mem/libminimal.la (#1258)
gkinkie@gmail.com [Sun, 5 Feb 2023 10:08:41 +0000 (10:08 +0000)] 
Add memFreeRigid to mem/libminimal.la (#1258)

Complement the already-included memAllocRigid,
to better support unit tests

16 months agoUpdate ChangeLog for latest merges (#1257)
Amos Jeffries [Sat, 4 Feb 2023 08:43:30 +0000 (08:43 +0000)] 
Update ChangeLog for latest merges (#1257)

16 months agoFix stub for ip/libip.la (#1254)
Amos Jeffries [Fri, 3 Feb 2023 14:07:53 +0000 (14:07 +0000)] 
Fix stub for ip/libip.la (#1254)

Use to build disk I/O tests which do not need IP address
operations. This also ensures bitrot does not reappear.

16 months agocachemgr.cgi: Drop cache_object protocol support (#1252)
Eduard Bagdasaryan [Thu, 2 Feb 2023 22:42:48 +0000 (22:42 +0000)] 
cachemgr.cgi: Drop cache_object protocol support (#1252)

16 months agoV6 release prep (#1245)
Amos Jeffries [Thu, 2 Feb 2023 19:47:05 +0000 (19:47 +0000)] 
V6 release prep (#1245)

16 months agosquidclient: Drop cache_object protocol support (#1251)
Eduard Bagdasaryan [Thu, 2 Feb 2023 12:25:10 +0000 (12:25 +0000)] 
squidclient: Drop cache_object protocol support (#1251)

Use `http` URL scheme for `mgr:command` cache manager requests instead.

Also fixed a bug: When `mgr:command` was used together with a `-w X`
command-line option, squidclient sent that proxy authentication password
`X` in the `cache_object` URL instead of sending the `Y` password from
the `-W Y` option (or no password at all if no `-W` option was given).
If no Authorization header had been sent, Squid's cachemgr_passwd
honored the `-w X` password sent in the cache_object URL.  If
Authorization header had been sent, Squid's cachemgr_passwd honored the
corresponding `-W Y` password, ignoring any password sent in the
cache_object URL.

Now, squidclient uses Authorization HTTP header for sending cache
manager authentication credentials with `mgr:command` requests. Those
credentials are taken either from the `-U` and `-W` command-line options
(if `mgr:command` parameter lacks an embedded password) or from the `-U`
command line option and from the `mgr:command@password` parameter
(otherwise).

squidclient now sends Proxy authentication credentials if and only if
`-u` and `-w` command line options are given. These credentials may be
required to authenticate with the proxy, but they do not affect cache
manager authentication driven by the cachemgr_passwd directive.

Also prohibit specifying a command-line option with a password (`-w` or
`-W`) without specifying the corresponding user name (`-u` or `-U`).
Prior to this change, squidclient did not send the Proxy-Authorization
or Authorization header if the corresponding user name was missing but
did not complain about the problem.

16 months agoRemove broken -sha1 option from server_cert_fingerprint (#1249)
Eduard Bagdasaryan [Wed, 1 Feb 2023 19:25:10 +0000 (19:25 +0000)] 
Remove broken -sha1 option from server_cert_fingerprint (#1249)

server_cert_fingerprint support for the sha1 parameter has been broken
for years, probably since its inception (2012 commit 42d3334). The bug
was known since at least 2018 when it was mentioned in Bug 4847
discussion.  The single-dash syntax violates the double-dash pattern
used for other --long ACL options. If fixed, using the option would not
change Squid behavior because SHA1 is the default (and the only
supported) fingerprinting algorithm.

The option was meant to allow admins to be explicit about that default
in case it changes in the future, but implementation bugs derailed that
plan. The fix is not straightforward, and we should be focusing on more
important things.

16 months agoImprove error message storage in Dns::LookupDetails (#1241)
Francesco Chemolli [Sun, 29 Jan 2023 21:59:41 +0000 (21:59 +0000)] 
Improve error message storage in Dns::LookupDetails (#1241)

Removed one more deprecated String usage and optimized handling of a
common "no error, no lookup" case as well as portions of the "no-error
lookup" code paths. Further optimizations need similar ipcache_entry and
fqdncache_entry error storage changes (at least).

Co-authored-by: Alex Rousskov <rousskov@measurement-factory.com>
16 months agoFix logging of responses truncated by premature EOF (#1244)
Eduard Bagdasaryan [Sat, 28 Jan 2023 09:36:56 +0000 (09:36 +0000)] 
Fix logging of responses truncated by premature EOF (#1244)

When a transaction failed due to a premature EOF while receiving an HTTP
response, %Ss logformat code was missing the ABORTED suffix (e.g.,
logging TCP_MISS instead of TCP_MISS_ABORTED). When premature EOF
truncated the header, the default "squid" access log format contained a
502 sent status code (%>Hs), hinting at the problem. When the body was
truncated, even that weak hint was usually absent because the actual
status code returned by the server (usually 200) was usually logged.

Similarly, %err_code/%err_detail logformat codes for HTTP responses with
truncated by a premature EOF bodies carried no information. Now they
expand to ERR_READ_ERROR/SRV_PREMATURE_EOF in those cases.

No changes to requests truncated by Squid-server read timeouts. They
were and are still logged with TIMEDOUT %Ss suffix and
ERR_READ_TIMEOUT/WITH_SERVER %err_code/%err_detail.

Also adjusted WHOIS to mark zero-length responses with
ERR_ZERO_SIZE_OBJECT instead of default ERR_READ_ERROR. This affects
forwarded WHOIS transactions and AS number queries generated during
(re)configuration.

16 months agoReduce code duplication in ACLCertificateData::parse() (#1242)
Eduard Bagdasaryan [Sat, 28 Jan 2023 05:34:27 +0000 (05:34 +0000)] 
Reduce code duplication in ACLCertificateData::parse() (#1242)

ACLs ca_cert and user_cert already prohibit key changes. And
server_cert_fingerprint ACL only supports one option name spelling, so,
while "-sha1" is not, technically, a "key", it still cannot be
"changed". Adjust SetAclKey() (added in recent commit 4a3b853) and reuse
it to implement those existing "key change" checks.