authproxy: strip request-body framing from auth sub-requests - #13333
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.
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. |
There was a problem hiding this comment.
Approving. The stripping order is correct, since Transfer-Encoding, Trailer, and Expect are removed before Content-Length is zeroed across all three sub-request transforms. The new HttpRemoveMimeHeader() re-fetches a fresh field location each iteration, so there is no iterator invalidation when a header appears more than once.
I have one non-blocking note. The fix ships without a test for the two failure modes the description calls out, which are a chunked or Expect: 100-continue request stalling the body tunnel, and reject_head_with_content rejecting a HEAD with content. An autest driving those cases and asserting that the auth sub-request arrives bodyless would guard against a future regression, but I do not consider it merge-gating.
|
Cherry-picked to 10.2.x |
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. (cherry picked from commit 364668f)
…13333) 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.
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).