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 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/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/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 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") + } + } }