authproxy: strip request-body framing from auth sub-requests#13333
Open
moonchen wants to merge 1 commit into
Open
authproxy: strip request-body framing from auth sub-requests#13333moonchen wants to merge 1 commit into
moonchen wants to merge 1 commit into
Conversation
The head/range/redirect auth transforms copy the whole client request and then force it bodyless (method override + Content-Length: 0) to probe the auth server, but left Transfer-Encoding, Trailer, and Expect in place. A chunked or Expect: 100-continue client request therefore produced a self-contradictory sub-request: a bodyless HEAD/GET still advertising a body. ATS honors the framing and sets up a request-body tunnel for a body that never arrives, stalling the probe until the inactivity timeout; and proxy.config.http.reject_head_with_content rejects a HEAD that declares content outright. Strip Transfer-Encoding, Trailer, and Expect when normalizing the sub-request to bodyless.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the authproxy plugin’s auth sub-request generation by ensuring “bodyless” probes (HEAD / Range GET / redirected requests with Content-Length: 0) don’t retain request-body framing headers copied from the client request, which can otherwise cause stalls or outright rejection.
Changes:
- Add
HttpRemoveMimeHeader()utility to remove all instances of a named header field. - Strip
Transfer-Encoding,Trailer, andExpectwhen normalizing auth sub-requests to bodyless. - Apply the same framing cleanup across the head/range/redirect auth transforms.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plugins/authproxy/utils.h | Declares a new helper for removing all instances of a header field. |
| plugins/authproxy/utils.cc | Implements HttpRemoveMimeHeader() via repeated find/destroy. |
| plugins/authproxy/authproxy.cc | Uses the new helper to remove body-framing headers for bodyless auth sub-requests. |
This file contains hidden or 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
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.
The head/range/redirect auth transforms in the
authproxyplugin copy thewhole client request and then force it bodyless (method override +
Content-Length: 0) to probe the auth server, but leftTransfer-Encoding,Trailer, andExpectin place. A chunked orExpect: 100-continueclientrequest therefore produced a self-contradictory sub-request: a bodyless
HEAD/GET still advertising a body.
ATS honors the framing and sets up a request-body tunnel for a body that never
arrives, stalling the probe until the inactivity timeout; and
proxy.config.http.reject_head_with_contentrejects a HEAD that declarescontent outright. This strips
Transfer-Encoding,Trailer, andExpectwhen normalizing the sub-request to bodyless, in all three transforms
(head, range, redirect).
A small
HttpRemoveMimeHeader()helper is added toutils.{h,cc}thatdestroys every instance of a named field (these fields can be repeated across
multiple lines).