Use ERR_ACCESS_DENIED for HTTP 403 (Forbidden) errors #1899
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
... when request authentication fails. Do not use ERR_CACHE_ACCESS_DENIED for those "permanent" errors. Default ERR_CACHE_ACCESS_DENIED is meant for cases where the user is likely to eventually gain access (e.g., by supplying credentials). Its default text says "not currently allowed... until you have authenticated yourself". When the error page was added in 1998 commit cb69b4c it was only used for HTTP 407 errors. The same logic was preserved when that code was refactored in 1999 commit 1cfdbcf, but exceptions started to creep in, perhaps accidentally, since 2011 when HTTP 403 sub-case was added in commit 2f1431e that introduced USE_AUTH macro. 2011 commit 2151291 added a similar "no possible to authenticate" SslBump sub-case. Other HTTP 403 (Forbidden) cases already use ERR_ACCESS_DENIED or a similar "permanent" error (e.g., ERR_FORWARDING_DENIED or ERR_TOO_BIG). It is still possible to customize the returned error page via deny_info.
rousskov
added
M-cleared-for-merge
squid-anubis
pushed a commit
that referenced
this pull request
Sep 20, 2024
... when request authentication fails. Do not use ERR_CACHE_ACCESS_DENIED for those "permanent" errors. Default ERR_CACHE_ACCESS_DENIED is meant for cases where the user is likely to eventually gain access (e.g., by supplying credentials). Its default text says "not currently allowed... until you have authenticated yourself". When the error page was added in 1998 commit cb69b4c it was only used for HTTP 407 errors. The same logic was preserved when that code was refactored in 1999 commit 1cfdbcf, but exceptions started to creep in, perhaps accidentally, since 2011 when HTTP 403 case was added in commit 2f1431e that introduced USE_AUTH macro. 2011 commit 2151291 added a similar "not possible to authenticate" SslBump case. Other HTTP 403 (Forbidden) cases already use ERR_ACCESS_DENIED or a similar "permanent" error (e.g., ERR_FORWARDING_DENIED or ERR_TOO_BIG). It is still possible to customize the returned error page via deny_info.
squid-anubis
added
the
M-waiting-staging-checks
rousskov
removed
the
S-could-use-an-approval
squid-anubis
added
M-merged
kinkie
pushed a commit
to kinkie/squid
that referenced
this pull request
Oct 12, 2024
... when request authentication fails. Do not use ERR_CACHE_ACCESS_DENIED for those "permanent" errors. Default ERR_CACHE_ACCESS_DENIED is meant for cases where the user is likely to eventually gain access (e.g., by supplying credentials). Its default text says "not currently allowed... until you have authenticated yourself". When the error page was added in 1998 commit cb69b4c it was only used for HTTP 407 errors. The same logic was preserved when that code was refactored in 1999 commit 1cfdbcf, but exceptions started to creep in, perhaps accidentally, since 2011 when HTTP 403 case was added in commit 2f1431e that introduced USE_AUTH macro. 2011 commit 2151291 added a similar "not possible to authenticate" SslBump case. Other HTTP 403 (Forbidden) cases already use ERR_ACCESS_DENIED or a similar "permanent" error (e.g., ERR_FORWARDING_DENIED or ERR_TOO_BIG). It is still possible to customize the returned error page via deny_info.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
... when request authentication fails. Do not use
ERR_CACHE_ACCESS_DENIED for those "permanent" errors.
Default ERR_CACHE_ACCESS_DENIED is meant for cases where the user is
likely to eventually gain access (e.g., by supplying credentials). Its
default text says "not currently allowed... until you have authenticated
yourself". When the error page was added in 1998 commit cb69b4c it was
only used for HTTP 407 errors. The same logic was preserved when that
code was refactored in 1999 commit 1cfdbcf, but exceptions started to
creep in, perhaps accidentally, since 2011 when HTTP 403 case was added
in commit 2f1431e that introduced USE_AUTH macro. 2011 commit 2151291
added a similar "not possible to authenticate" SslBump case.
Other HTTP 403 (Forbidden) cases already use ERR_ACCESS_DENIED or a
similar "permanent" error (e.g., ERR_FORWARDING_DENIED or ERR_TOO_BIG).
It is still possible to customize the returned error page via deny_info.