From c381d7cd033ee83c7c418caed2c767aff82ac65d Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 11 Sep 2026 11:56:58 +0100 Subject: [PATCH 1/2] Reject a Raw-Request-URI that cannot be sent as a request target 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 (#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 #1280 --- docs/src/main/paradox/common/http-model.md | 8 +++++ .../http/scaladsl/model/headers/headers.scala | 29 ++++++++++++++++++- .../scaladsl/model/headers/HeaderSpec.scala | 27 +++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/src/main/paradox/common/http-model.md b/docs/src/main/paradox/common/http-model.md index cd399bcf1..3ec749327 100644 --- a/docs/src/main/paradox/common/http-model.md +++ b/docs/src/main/paradox/common/http-model.md @@ -107,6 +107,14 @@ The `Raw-Request-URI` header is honoured by both the HTTP/1.1 and the HTTP/2 cli the `:path` pseudo-header. It is consumed by the request engine and never rendered as a header of its own, and its value is used exactly as given — it is the caller's responsibility to supply a valid request target. +Because the value goes to the wire as it is, it has to be something that *can* be sent as a request target: non-empty, +and made of visible ASCII characters only (`0x21`–`0x7E`). The header rejects anything else on construction with an +`IllegalArgumentException` — a space, CR or LF would end the request line early and let whatever follows be read as +the protocol, a header or a second request, and a character outside ASCII cannot be rendered faithfully at all. +Percent-encode what the target needs to carry; the encoded form is sent untouched. Never build this header from +request input you have not validated: it is the one place where the client sends bytes you hand it without parsing +them first. + This is the supported way to send a request target that @apidoc[Uri] cannot reproduce on its own. `Uri` percent-decodes path segments when parsing and re-encodes them with a keep-set that leaves sub-delims raw, so an encoded *pchar* does not survive the round trip — `%2B` is rendered back as `+`, for instance. Callers that must reproduce the target diff --git a/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/headers/headers.scala b/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/headers/headers.scala index 2a7919ec8..ca193fa8f 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/headers/headers.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/headers/headers.scala @@ -826,8 +826,35 @@ object RawHeader { Some(customHeader.name -> customHeader.value) } -object `Raw-Request-URI` extends ModeledCompanion[`Raw-Request-URI`] +object `Raw-Request-URI` extends ModeledCompanion[`Raw-Request-URI`] { + // A request target goes to the wire exactly as given: into the HTTP/1.1 request line, where a space, CR or LF ends + // the target early and lets whatever follows be read as the protocol, a header or a second request; and character + // by character truncated to a byte, so that a character outside ASCII lands as its low byte -- '\u010d' is sent + // as CR. Only visible ASCII (RFC 9112 section 3.2) can be rendered faithfully, so only visible ASCII is accepted. + private[http] def firstIllegalCharIndex(uri: String): Int = { + var ix = 0 + while (ix < uri.length && { val c = uri.charAt(ix); c > ' ' && c < '\u007f' }) ix += 1 + if (ix < uri.length) ix else -1 + } +} + +/** + * Carries a request target to send verbatim, in place of the one rendered from the request's `Uri`. + * + * The value is written to the wire as given, so it must be a request target that can be sent as it is: non-empty, + * and consisting of visible ASCII characters only (0x21-0x7E). Anything else -- a space, CR, LF, a control + * character, a character outside ASCII -- is rejected on construction rather than rendered into the request line. + * Percent-encode what the target has to carry; the encoded form is sent untouched. + */ final case class `Raw-Request-URI`(uri: String) extends jm.headers.RawRequestURI with SyntheticHeader { + require(uri.nonEmpty, "Raw-Request-URI must not be empty") + require( + `Raw-Request-URI`.firstIllegalCharIndex(uri) < 0, { + val ix = `Raw-Request-URI`.firstIllegalCharIndex(uri) + "Raw-Request-URI may only contain visible ASCII characters (0x21-0x7E), the request target is sent to the " + + s"wire as given: found U+${"%04X".format(uri.charAt(ix).toInt)} at index $ix" + }) + def renderValue[R <: Rendering](r: R): r.type = r ~~ uri protected def companion = `Raw-Request-URI` } diff --git a/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/headers/HeaderSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/headers/HeaderSpec.scala index f5af11aff..d834c2d19 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/headers/HeaderSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/headers/HeaderSpec.scala @@ -256,4 +256,31 @@ class HeaderSpec extends AnyFreeSpec with Matchers { } } } + "Raw-Request-URI should" - { + "accept any request target made of visible ASCII" in { + `Raw-Request-URI`("/def%80%fe%ff").uri shouldEqual "/def%80%fe%ff" + `Raw-Request-URI`("/a+b=c?d=e&f=%2B#g").uri shouldEqual "/a+b=c?d=e&f=%2B#g" + // every visible ASCII character, 0x21 to 0x7E + val allVisible = (0x21 to 0x7E).map(_.toChar).mkString + `Raw-Request-URI`(allVisible).uri shouldEqual allVisible + } + "reject anything that cannot be sent as a request target" in { + // the value is written into the request line as given: a space, CR or LF would end the target early and let + // what follows be read as the protocol, a header or a second request + val e = the[IllegalArgumentException] thrownBy `Raw-Request-URI`("/a HTTP/1.0") + e.getMessage should include("found U+0020 at index 2") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a\u000d\u000aHost: evil") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a\u000aX: y") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a\u0009b") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a\u0000b") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a\u007fb") + // rendered a character at a time truncated to a byte, so a character outside ASCII is not sent as itself: + // U+010D would land on the wire as 0x0D, a CR + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a\u010d") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/caf\u00e9") + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("") + // `copy` goes through the constructor too + an[IllegalArgumentException] should be thrownBy `Raw-Request-URI`("/a").copy(uri = "/a\u000d\u000a") + } + } } From 9c0908813cc50b45b2bd4fd47250648c5b39d134 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 11 Sep 2026 12:05:06 +0100 Subject: [PATCH 2/2] Require a custom HTTP method name to be a token 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 #1280 --- docs/src/main/paradox/common/http-model.md | 6 ++++ .../engine/http2/HttpMessageRendering.scala | 5 ++-- .../http/scaladsl/model/HttpMethod.scala | 18 ++++++++++-- .../http/scaladsl/model/HttpMethodsSpec.scala | 28 +++++++++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/docs/src/main/paradox/common/http-model.md b/docs/src/main/paradox/common/http-model.md index 3ec749327..96878220b 100644 --- a/docs/src/main/paradox/common/http-model.md +++ b/docs/src/main/paradox/common/http-model.md @@ -482,3 +482,9 @@ Scala Java : @@snip [CustomHttpMethodsExampleTest.java](/docs/src/test/java/docs/http/javadsl/server/directives/CustomHttpMethodExamplesTest.java) { #customHttpMethod } + +The name of a custom method must be a *token* (RFC 9110 §5.6.2): letters, digits and the characters +`` !#$%&'*+-.^_`|~ ``. Anything else — a space, a control character, a delimiter such as `/` or `:`, a character +outside ASCII — is rejected with an `IllegalArgumentException` when the method is created. The name is written into the +request line as given, ahead of the request target, so a space, CR or LF in it would end the method early and let the +rest be read as the target, the protocol or a header. diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala index c70f1d0db..c157277ef 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/HttpMessageRendering.scala @@ -84,8 +84,9 @@ private[http2] class RequestRendering( } // `Raw-Request-URI` is a SyntheticHeader, so it is already excluded from the rendered header block by the - // `renderInRequests` filter and is only consumed here. As in HTTP/1.1, the value is taken as given: it is the - // caller's responsibility that it is a valid origin-form target. + // `renderInRequests` filter and is only consumed here. As in HTTP/1.1, the value is taken as given; the header + // rejects anything but visible ASCII on construction, so it cannot carry a character the HPACK guard would drop + // the `:path` field for. Whether it is a well-formed origin-form target is still the caller's responsibility. private def rawRequestTarget(request: HttpRequest): Option[String] = request.headers.collectFirst { case `Raw-Request-URI`(rawUri) => rawUri } diff --git a/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala b/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala index 128fbdd31..c54f9216d 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/scaladsl/model/HttpMethod.scala @@ -16,6 +16,7 @@ package org.apache.pekko.http.scaladsl.model import java.util.Locale import org.apache.pekko +import pekko.http.impl.model.parser.CharacterClasses import pekko.http.impl.util._ import pekko.http.javadsl.{ model => jm } import pekko.http.scaladsl.model.RequestEntityAcceptance._ @@ -65,7 +66,7 @@ object HttpMethod { @deprecated("Use the overload with contentLengthAllowed parameter", since = "1.4.0") def custom(name: String, safe: Boolean, idempotent: Boolean, requestEntityAcceptance: RequestEntityAcceptance) : HttpMethod = { - require(name.nonEmpty, "value must be non-empty") + requireToken(name) require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent") apply(name, safe, idempotent, requestEntityAcceptance, oldContentLengthCondition) } @@ -75,7 +76,7 @@ object HttpMethod { */ def custom(name: String, safe: Boolean, idempotent: Boolean, requestEntityAcceptance: RequestEntityAcceptance, contentLengthAllowed: Boolean): HttpMethod = { - require(name.nonEmpty, "value must be non-empty") + requireToken(name) require(!safe || idempotent, "An HTTP method cannot be safe without being idempotent") apply(name, safe, idempotent, requestEntityAcceptance, if (contentLengthAllowed) anyToTrue else anyToFalse) } @@ -86,6 +87,19 @@ object HttpMethod { */ def custom(name: String): HttpMethod = custom(name, safe = false, idempotent = false, requestEntityAcceptance = Expected, contentLengthAllowed = true) + + // A method name is written into the HTTP/1.1 request line as given, ahead of the request target, so a space, CR + // or LF in it would end the method early and let the rest be read as the target, the protocol or a header. Only a + // token (RFC 9110 section 5.6.2) is a method on the wire, and only a token is accepted. + private def requireToken(name: String): Unit = { + require(name.nonEmpty, "value must be non-empty") + var ix = 0 + while (ix < name.length && CharacterClasses.tchar(name.charAt(ix))) ix += 1 + require( + ix == name.length, + "an HTTP method name must be a token (RFC 9110 section 5.6.2), it is written into the request line as given: " + + s"found U+${"%04X".format(name.charAt(ix).toInt)} at index $ix") + } } object HttpMethods extends ObjectRegistry[String, HttpMethod] { diff --git a/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/HttpMethodsSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/HttpMethodsSpec.scala index f60dd55cb..3e92cd86e 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/HttpMethodsSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/HttpMethodsSpec.scala @@ -37,6 +37,34 @@ class HttpMethodsSpec extends AnyWordSpec { } } + "HttpMethod.custom" must { + "accept any token" in { + assert(HttpMethod.custom("PROPFIND").value == "PROPFIND") + assert(HttpMethod.custom("M-SEARCH").value == "M-SEARCH") + // every tchar: ALPHA, DIGIT and the sixteen special characters RFC 9110 allows in a token + val allTchars = ('A' to 'Z').mkString + ('a' to 'z').mkString + ('0' to '9').mkString + "!#$%&'*+-.^_`|~" + assert(HttpMethod.custom(allTchars).value == allTchars) + } + "reject a name that is not a token" in { + // the name is written into the request line as given, ahead of the request target + val e = intercept[IllegalArgumentException]( + HttpMethod.custom("GET /admin HTTP/1.1\u000d\u000aX-Injected: 1\u000d\u000aFOO")) + assert(e.getMessage.contains("found U+0020 at index 3")) + intercept[IllegalArgumentException](HttpMethod.custom("GET\u000d\u000a")) + intercept[IllegalArgumentException](HttpMethod.custom("GET\u0009")) + intercept[IllegalArgumentException](HttpMethod.custom("GET\u0000")) + intercept[IllegalArgumentException](HttpMethod.custom("GET\u007f")) + // delimiters are visible ASCII but not tchar + intercept[IllegalArgumentException](HttpMethod.custom("GET/")) + intercept[IllegalArgumentException](HttpMethod.custom("GET:")) + intercept[IllegalArgumentException](HttpMethod.custom("G\u010dT")) + intercept[IllegalArgumentException](HttpMethod.custom("")) + // every overload validates + intercept[IllegalArgumentException](HttpMethod.custom("GET ", safe = false, idempotent = false, + requestEntityAcceptance = RequestEntityAcceptance.Expected, contentLengthAllowed = true)) + } + } + "HttpMethods.QUERY" must { "be safe per RFC 10008" in { assert(HttpMethods.QUERY.isSafe)