This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/main by this push:
new e697e5416 fix: reject a request-line part that cannot be sent as given
(#1296)
e697e5416 is described below
commit e697e5416bc52e072628d97288e9694911d265f9
Author: PJ Fanning <[email protected]>
AuthorDate: Fri Sep 18 09:37:07 2026 +0100
fix: reject a request-line part that cannot be sent as given (#1296)
* 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
* 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 | 14 +++++++++++
.../impl/engine/http2/HttpMessageRendering.scala | 5 ++--
.../pekko/http/scaladsl/model/HttpMethod.scala | 18 ++++++++++++--
.../http/scaladsl/model/headers/headers.scala | 29 +++++++++++++++++++++-
.../http/scaladsl/model/HttpMethodsSpec.scala | 28 +++++++++++++++++++++
.../http/scaladsl/model/headers/HeaderSpec.scala | 27 ++++++++++++++++++++
6 files changed, 116 insertions(+), 5 deletions(-)
diff --git a/docs/src/main/paradox/common/http-model.md
b/docs/src/main/paradox/common/http-model.md
index cd399bcf1..96878220b 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
@@ -474,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 b9242a2af..5d999ebd5 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/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 17adc8486..97f2211f5 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/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)
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 9527fb9d2..4050705d5 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
@@ -254,4 +254,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")
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
