Conversation
Motivation: `Raw-Request-URI` is the escape hatch for sending a request target verbatim, and the HTTP/1.1 renderer honours that literally: it writes the value into the request line with `r ~~ rawUri`, with no check on what it contains. Two things go wrong with an unchecked value. A space, CR or LF ends the target early and lets whatever follows be read as the protocol, a header or a second request, so an application that builds the header from input it did not validate has a request-line injection. And the renderer writes a String one char at a time truncated to a byte, so a character outside ASCII does not arrive as itself: U+010D lands on the wire as 0x0D, a CR, without the value ever containing one. Over HTTP/2 (apache#1280) the same value becomes the `:path` pseudo-header, where the HPACK guard drops a field carrying CR, LF or NUL -- a malformed request rather than an injection, but a silent one. Modification: Validate on construction, following `Referer`: the value must be non-empty and consist of visible ASCII only (0x21-0x7E), which is what a request target may contain on the wire (RFC 9112 section 3.2) and the only thing the renderer can send faithfully. Anything else fails with an `IllegalArgumentException` naming the character and its index. `copy` and the Java `RawRequestURI.create` go through the same constructor. The server-side creation under `raw-request-uri-header = on` is unaffected: it builds the header from a target the URI parser has already accepted, which is visible ASCII by construction. Document the constraint in the scaladoc and in the `Raw-Request-URI` section of the model docs, together with the warning not to build the header from unvalidated request input. Result: A request target that would corrupt the request line, or that the renderer would corrupt, is rejected where the application creates it, instead of being sent. Tests: - `HeaderSpec` gains two cases: the full visible-ASCII range and the existing `%80%fe%ff` value are accepted; space, CR+LF, LF, tab, NUL, DEL, two non-ASCII characters, the empty string and a `copy` are each rejected, and the message names the offending character. The rejecting case fails with the validation reverted. - sbt "http-core/testOnly ...HeaderSpec ...RequestRendererSpec ...RequestParserCRLFSpec ...NewConnectionPoolSpec" - 133 pass. - sbt "http2-tests/testOnly ...Http2ClientSpec" (Raw-Request-URI cases), "docs/testOnly ...ModelSpec ...ModelDocTest" - pass. - sbt "http-core/mimaReportBinaryIssues" - pass. - native scalafmt clean. References: Refs apache#1280
pjfanning
force-pushed
the
raw-request-uri-validation
branch
from
September 11, 2026 10:59
69643ae to
c381d7c
Compare
Motivation: The request line has two application-supplied parts, and the previous commit validated one of them. `HttpMethod.custom` checked only that the name was non-empty, and the HTTP/1.1 renderer writes it into the request line as given (`r ~~ method ~~ ' '`), so a name carrying a space, CR or LF ended the method early and let the rest be read as the target, the protocol or a header. Over HTTP/2 the same name becomes `:method`, where the HPACK guard drops a field carrying CR, LF or NUL and sends a malformed request. Modification: Require the name to be a token (RFC 9110 section 5.6.2) in both `custom` overloads that construct a method, using the parser's own `tchar` class; the single-argument overload and the Java `HttpMethods.custom` delegate to them. The server never constructs a method from wire bytes -- it looks the parsed token up among the registered custom methods -- so only application code is affected. Document the constraint at the custom method section of the model docs, and correct the HTTP/2 renderer's comment on `Raw-Request-URI`, which still said the value was taken on trust. Result: Neither part of the request line an application can supply can carry a character that would corrupt it, and neither can reach the HPACK guard's silent drop of a pseudo-header. Tests: - `HttpMethodsSpec` gains two cases: every tchar and two real-world custom methods are accepted; a request-line injection, CR+LF, tab, NUL, DEL, `/`, `:`, a non-ASCII character, the empty string and the five-argument overload are each rejected, and the message names the offending character. The rejecting case fails with the validation reverted. - sbt "http-core/testOnly ...HttpMethodsSpec ...HeaderSpec ...RequestParserCRLFSpec ...HttpHeaderSpec ...RequestRendererSpec" - 191 pass (the last three register custom methods). - sbt "http-core/mimaReportBinaryIssues" - pass. - native scalafmt clean. References: Refs apache#1280
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.
Motivation
The HTTP/1.1 request line has two parts the application supplies, and the renderer writes both as given (
HttpRequestRendererFactory.scala:48-53):No check on what either contains. Two things go wrong with an unchecked value:
Raw-Request-URI— or a custom method name — from input it did not validate has a request-line injection.Rendering.~~(String)writes a string one char at a time truncated to a byte (array(oldSize) = char.toByte), so a character outside ASCII does not arrive as itself.U+010D(č) lands on the wire as0x0D— a CR — without the value ever containing one.Over HTTP/2 the same two values become
:methodand (since #1280):path, where #1258's HPACK guard drops a field carrying CR, LF or NUL. That's a malformed request rather than an injection, but a silent one.Modification
The fix: validate on construction, matching the existing
Refereridiom.Raw-Request-URImust be non-empty and visible ASCII only (0x21–0x7E). That's the set a request target may contain on the wire (RFC 9112 §3.2), and coincidentally the only set the renderer can send faithfully. Constructor validation covers every path in one place:copy, the JavaRawRequestURI.create, and the HTTP/2:pathroute from feat: honour Raw-Request-URI as the HTTP/2 :path #1280 — so the HPACK guard's silent drop of:pathcan no longer be reached from this header either.HttpMethod.custommust be a token (RFC 9110 §5.6.2), checked with the parser's owntcharclass in both overloads that construct a method; the single-argument overload and the JavaHttpMethods.customdelegate to them. Same reasoning, same coverage of:method.Both fail with an
IllegalArgumentExceptionnaming the character and its index:Neither check touches the server side. The header created under
raw-request-uri-header = onis built from a target the URI parser has already accepted, which is visible ASCII by construction; and the server never constructs a method from wire bytes — it looks the parsed token up among the registered custom methods.Documented in the scaladoc, in the
Raw-Request-URIand custom-method sections of the model docs (with the warning not to build either from unvalidated request input), and the HTTP/2 renderer's comment onRaw-Request-URI— which still said the value was taken on trust — is corrected.Result
Neither part of the request line an application can supply can carry a character that would corrupt it, or that the renderer would corrupt, and neither can reach the HPACK guard's silent drop of a pseudo-header. Either is rejected where the application creates it, instead of being sent.
Tests
HeaderSpec: the full visible-ASCII range and the existing%80%fe%ffvalue are accepted; space, CR+LF, LF, tab, NUL, DEL, two non-ASCII characters, the empty string and acopyare each rejected, and the message names the offending character.HttpMethodsSpec: every tchar and two real-world custom methods are accepted; a request-line injection, CR+LF, tab, NUL, DEL,/,:, a non-ASCII character, the empty string and the five-argument overload are each rejected.sbt "http-core/testOnly ...HeaderSpec ...HttpMethodsSpec ...RequestRendererSpec ...RequestParserCRLFSpec ...HttpHeaderSpec ...NewConnectionPoolSpec"— pass (the last three register custom methods or the header).sbt "http2-tests/testOnly ...Http2ClientSpec"(the twoRaw-Request-URIcases from feat: honour Raw-Request-URI as the HTTP/2 :path #1280) andsbt "docs/testOnly ...ModelSpec ...ModelDocTest"— pass.sbt "http-core/mimaReportBinaryIssues"— pass.scalafmt --list --mode diff-ref=upstream/main— clean.References
Refs #1280