You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "moonchen (via GitHub)" <gi...@apache.org> on 2026/06/25 15:48:54 UTC
[PR] authproxy: strip request-body framing from auth sub-requests [trafficserver]
moonchen opened a new pull request, #13333:
URL: https://github.com/apache/trafficserver/pull/13333
The head/range/redirect auth transforms in the `authproxy` plugin 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. This strips `Transfer-Encoding`, `Trailer`, and `Expect`
when normalizing the sub-request to bodyless, in all three transforms
(head, range, redirect).
A small `HttpRemoveMimeHeader()` helper is added to `utils.{h,cc}` that
destroys every instance of a named field (these fields can be repeated across
multiple lines).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] authproxy: strip request-body framing from auth sub-requests [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13333:
URL: https://github.com/apache/trafficserver/pull/13333#discussion_r3475757568
##########
plugins/authproxy/authproxy.cc:
##########
@@ -296,6 +296,16 @@ AuthWriteHeadRequest(AuthRequestContext *auth)
// Next, we need to rewrite the client request URL to be a HEAD request.
TSReleaseAssert(TSHttpHdrMethodSet(rq.buffer, rq.header, TS_HTTP_METHOD_HEAD, -1) == TS_SUCCESS);
+ // This sub-request is bodyless (HEAD + Content-Length: 0), but the copied
+ // client request may carry request-body framing (e.g. a chunked POST or
+ // Expect: 100-continue). Left in place it is self-contradictory: ATS sets up
+ // a request-body tunnel for a body that never arrives (stalling the probe
+ // until timeout), and proxy.config.http.reject_head_with_content rejects a
+ // HEAD that declares content. Strip the framing when forcing Content-Length: 0.
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_TRANSFER_ENCODING);
+ HttpRemoveMimeHeader(rq.buffer, rq.header, "Trailer");
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_EXPECT);
+
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_CONTENT_LENGTH, 0u);
Review Comment:
This sub-request is being normalized to bodyless, but only Transfer-Encoding/Trailer/Expect are removed. If the copied client request had multiple Content-Length fields, HttpSetMimeHeader() only updates one instance, leaving any other Content-Length headers intact and potentially reintroducing a non-zero length/body framing contradiction. Remove all existing Content-Length fields before setting it to 0 to ensure the request is unambiguously bodyless.
##########
plugins/authproxy/authproxy.cc:
##########
@@ -333,6 +343,13 @@ AuthWriteRangeRequest(AuthRequestContext *auth)
TSReleaseAssert(TSHttpHdrMethodSet(rq.buffer, rq.header, TS_HTTP_METHOD_GET, -1) == TS_SUCCESS);
}
+ // The body is dropped (Content-Length: 0), so strip any request-body framing
+ // (Transfer-Encoding/Trailer/Expect) copied from the client request to keep
+ // the sub-request well-formed.
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_TRANSFER_ENCODING);
+ HttpRemoveMimeHeader(rq.buffer, rq.header, "Trailer");
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_EXPECT);
+
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_CONTENT_LENGTH, 0u);
Review Comment:
This sub-request is being forced bodyless, but only Transfer-Encoding/Trailer/Expect are stripped. If the client request included multiple Content-Length headers, HttpSetMimeHeader() will only rewrite one of them and any remaining Content-Length fields can conflict with the intended "Content-Length: 0". Remove all existing Content-Length fields before setting it to 0.
##########
plugins/authproxy/authproxy.cc:
##########
@@ -386,6 +403,14 @@ AuthWriteRedirectedRequest(AuthRequestContext *auth)
TSHandleMLocRelease(rq.buffer, rq.header, murl);
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_HOST, hostbuf);
+
+ // The body is dropped (Content-Length: 0), so strip any request-body framing
+ // (Transfer-Encoding/Trailer/Expect) copied from the client request to keep
+ // the sub-request well-formed.
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_TRANSFER_ENCODING);
+ HttpRemoveMimeHeader(rq.buffer, rq.header, "Trailer");
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_EXPECT);
+
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_CONTENT_LENGTH, 0u);
Review Comment:
This sub-request is being normalized to bodyless, but existing Content-Length duplicates (if present in the copied client request) are not removed. Since HttpSetMimeHeader() only updates a single field instance, any additional Content-Length headers can remain and conflict with the intended "Content-Length: 0". Remove all Content-Length fields before setting the header.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] authproxy: strip request-body framing from auth sub-requests [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on code in PR #13333:
URL: https://github.com/apache/trafficserver/pull/13333#discussion_r3475851076
##########
plugins/authproxy/authproxy.cc:
##########
@@ -296,6 +296,16 @@ AuthWriteHeadRequest(AuthRequestContext *auth)
// Next, we need to rewrite the client request URL to be a HEAD request.
TSReleaseAssert(TSHttpHdrMethodSet(rq.buffer, rq.header, TS_HTTP_METHOD_HEAD, -1) == TS_SUCCESS);
+ // This sub-request is bodyless (HEAD + Content-Length: 0), but the copied
+ // client request may carry request-body framing (e.g. a chunked POST or
+ // Expect: 100-continue). Left in place it is self-contradictory: ATS sets up
+ // a request-body tunnel for a body that never arrives (stalling the probe
+ // until timeout), and proxy.config.http.reject_head_with_content rejects a
+ // HEAD that declares content. Strip the framing when forcing Content-Length: 0.
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_TRANSFER_ENCODING);
+ HttpRemoveMimeHeader(rq.buffer, rq.header, "Trailer");
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_EXPECT);
+
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_CONTENT_LENGTH, 0u);
Review Comment:
ATS core already guarantees at most one `Content-Length` field here. During request parsing, `validate_hdr_content_length()` (`src/proxy/hdrs/HTTP.cc`) enforces RFC 7230 §3.3.3: a request with differing duplicate `Content-Length` values is rejected with 400 (so it never reaches the plugin), same-value duplicates are collapsed to a single field, and all `Content-Length` is dropped when `Transfer-Encoding` is present. authproxy copies the request at `POST_REMAP`, after parsing — so there are no duplicate `Content-Length` fields to leave behind, and the single `Set` to 0 is sufficient.
`Transfer-Encoding`/`Trailer`/`Expect` get the looping remove instead because they can survive parsing (and TE is not deduplicated), so they need explicit stripping.
##########
plugins/authproxy/authproxy.cc:
##########
@@ -333,6 +343,13 @@ AuthWriteRangeRequest(AuthRequestContext *auth)
TSReleaseAssert(TSHttpHdrMethodSet(rq.buffer, rq.header, TS_HTTP_METHOD_GET, -1) == TS_SUCCESS);
}
+ // The body is dropped (Content-Length: 0), so strip any request-body framing
+ // (Transfer-Encoding/Trailer/Expect) copied from the client request to keep
+ // the sub-request well-formed.
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_TRANSFER_ENCODING);
+ HttpRemoveMimeHeader(rq.buffer, rq.header, "Trailer");
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_EXPECT);
+
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_CONTENT_LENGTH, 0u);
Review Comment:
Same as the comment on the HEAD transform: duplicate `Content-Length` can't reach this point. `validate_hdr_content_length()` (RFC 7230 §3.3.3) rejects or collapses duplicates during request parsing, before authproxy's `POST_REMAP` hook, so the request carries at most one `Content-Length` and the single `Set` to 0 is sufficient.
##########
plugins/authproxy/authproxy.cc:
##########
@@ -386,6 +403,14 @@ AuthWriteRedirectedRequest(AuthRequestContext *auth)
TSHandleMLocRelease(rq.buffer, rq.header, murl);
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_HOST, hostbuf);
+
+ // The body is dropped (Content-Length: 0), so strip any request-body framing
+ // (Transfer-Encoding/Trailer/Expect) copied from the client request to keep
+ // the sub-request well-formed.
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_TRANSFER_ENCODING);
+ HttpRemoveMimeHeader(rq.buffer, rq.header, "Trailer");
+ HttpRemoveMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_EXPECT);
+
HttpSetMimeHeader(rq.buffer, rq.header, TS_MIME_FIELD_CONTENT_LENGTH, 0u);
Review Comment:
Same as the comment on the HEAD transform: duplicate `Content-Length` can't reach this point. `validate_hdr_content_length()` (RFC 7230 §3.3.3) rejects or collapses duplicates during request parsing, before authproxy's `POST_REMAP` hook, so the request carries at most one `Content-Length` and the single `Set` to 0 is sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] authproxy: strip request-body framing from auth sub-requests [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen merged PR #13333:
URL: https://github.com/apache/trafficserver/pull/13333
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] authproxy: strip request-body framing from auth sub-requests [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13333:
URL: https://github.com/apache/trafficserver/pull/13333#issuecomment-4959243654
Cherry-picked to 10.2.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org