]> git.ipfire.org Git - thirdparty/squid.git/commit
Bug 5360: FwdState::noteDestinationsEnd() assertion "err" (#1767)
authorAlex Rousskov <rousskov@measurement-factory.com>
Mon, 1 Apr 2024 02:53:39 +0000 (02:53 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Mon, 1 Apr 2024 17:07:02 +0000 (17:07 +0000)
commitdb8b598f02b6adbdf0acc26770b3ab34282dcdfa
treeb42298cc931702b8d5087f6fca8b0708d18742d3
parent984577acdcd448a8a6893e04070712cf27d58a3b
Bug 5360: FwdState::noteDestinationsEnd() assertion "err" (#1767)

    FATAL: assertion failed: FwdState.cc:660: "err"

When FwdState decides to re-forward a request, it forgets the original
response[^1] but does not create an error object. Since commit e2bbd3b,
FwdState::noteDestinationsEnd() correctly assumed that we only idly wait
for additional destinations after encountering a problem, but
incorrectly asserted that past problems imply error object existence.

Now Squid generates an HTTP 502 (Bad Gateway) response while setting
%err_code/%err_detail to ERR_CANNOT_FORWARD/REFORWARD_TO_NONE.

TunnelStateData::noteDestinationsEnd() code is similar, but it probably
does not suffer from the same bug because an error object is created
before every retryOrBail() call, and there is no re-forwarding code that
forgets an HTTP error response without creating an error. Those
invariants are not well tracked, and this change mimics FwdState changes
in TunnelStateData just in case and to keep the two methods in sync. In
those hypothetical cases, ERR_CANNOT_FORWARD/RETRY_TO_NONE is logged.

[^1]: Long-term we probably want to preserve that original response so
that we do not have to replace it with a generated error, but doing so
requires significant refactoring and is outside this minimal fix scope.
src/FwdState.cc
src/tunnel.cc