]> git.ipfire.org Git - thirdparty/squid.git/commit
Bug 5274: Successful tunnels logged as TCP_TUNNEL/500 (#1608)
authorAlex Rousskov <rousskov@measurement-factory.com>
Sat, 9 Dec 2023 04:46:55 +0000 (04:46 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Sun, 10 Dec 2023 21:40:49 +0000 (21:40 +0000)
commite6a3ff699fecb7d6fd83c037119101be676d06f6
treecca9321bbd3b491950a42ad10f9979c4abd955b7
parent5baf2c78554e319b6798bbf9b6fbb98ad0a7fd42
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.
src/tunnel.cc