Skip to content

fix: handle a ':path' with an asterisk as the RFC required #981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
He-Pin wants to merge 2 commits into
base: main
Choose a base branch
from
+123 −25
Open

fix: handle a ':path' with an asterisk as the RFC required #981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8985ffe
fix: align HTTP/2 malformed-header handling
He-Pin Mar 8, 2026
bad9ff0
fix: improve handling of asterisk in HTTP/2 ':path' pseudo-header
He-Pin Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,19 @@ private[http2] object RequestParsing {
cookies: StringBuilder,
headers: VectorBuilder[HttpHeader]): HttpRequest = {
// https://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2.3: these pseudo header fields are mandatory for a request
checkRequiredPseudoHeader(":scheme", scheme)
checkRequiredPseudoHeader(":method", method)
checkRequiredPseudoHeader(":path", pathAndRawQuery)
// except for CONNECT requests which MUST NOT include :path or :scheme
if (method == HttpMethods.CONNECT) {
// CONNECT requests must not include :path or :scheme pseudo-headers
if (pathAndRawQuery ne null)
protocolError("Pseudo-header ':path' must not be included in CONNECT request")
if (scheme ne null)
protocolError("Pseudo-header ':scheme' must not be included in CONNECT request")
} else {
// Non-CONNECT requests must include :method, :scheme, and :path pseudo-headers
checkRequiredPseudoHeader(":scheme", scheme)
checkRequiredPseudoHeader(":method", method)
checkRequiredPseudoHeader(":path", pathAndRawQuery)
}

if (cookies != null) {
// Compress 'cookie' headers if present
Expand All @@ -97,9 +107,17 @@ private[http2] object RequestParsing {

val entity = subStream.createEntity(contentLength, contentType)

val (path, rawQueryString) = pathAndRawQuery
val authorityOrDefault: Uri.Authority = if (authority == null) Uri.Authority.Empty else authority
val uri = Uri(scheme, authorityOrDefault, path, rawQueryString)
// For CONNECT requests, the URI is constructed differently since :scheme and :path are not included
// The authority contains the host:port to connect to
val uri = if (method == HttpMethods.CONNECT) {
// CONNECT requests use the authority as the request target (e.g., "example.com:443")
// We construct a URI with just the authority and an empty path
Uri("", authorityOrDefault, Uri.Path.Empty, None)
} else {
val (path, rawQueryString) = pathAndRawQuery
Uri(scheme, authorityOrDefault, path, rawQueryString)
}
val attributes = baseAttributes.updated(Http2.streamId, subStream.streamId)

new HttpRequest(method, uri, headers.result(), attributes, entity, HttpProtocols.`HTTP/2.0`)
Expand Down
9 changes: 3 additions & 6 deletions ...re/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,9 @@ private[http2] final class HeaderDecompression(masterHeaderParser: HttpHeaderPar
}

name match {
case "content-type" => handle(ContentType.parse(name, value, parserSettings))
case ":authority" => handle(Authority.parse(name, value, parserSettings))
case ":path" =>
if (value.isEmpty)
throw new Http2ProtocolException("Malformed request: ':path' must not be empty")
handle(PathAndQuery.parse(name, value, parserSettings))
case "content-type" => handle(ContentType.parse(name, value, parserSettings))
case ":authority" => handle(Authority.parse(name, value, parserSettings))
case ":path" => handle(PathAndQuery.parse(name, value, parserSettings))
case ":method" => handle(Method.parse(name, value, parserSettings))
case ":scheme" => handle(Scheme.parse(name, value, parserSettings))
case "content-length" => handle(ContentLength.parse(name, value, parserSettings))
Expand Down
6 changes: 4 additions & 2 deletions ...ore/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ package org.apache.pekko.http.impl.engine.http2.hpack
import org.apache.pekko
import pekko.annotation.InternalApi
import pekko.http.impl.engine.http2.RequestParsing
import pekko.http.impl.engine.http2.RequestParsing.parseError
import pekko.http.impl.engine.http2.RequestParsing.{ parseError, protocolError }
import pekko.http.scaladsl.model
import pekko.http.scaladsl.model.HttpHeader.ParsingResult
import model.{ HttpHeader, HttpMethod, HttpMethods, IllegalUriException, ParsingException, StatusCode, Uri }
Expand All @@ -41,7 +41,9 @@ private[pekko] object Http2HeaderParsing {
}
object PathAndQuery extends HeaderParser[(Uri.Path, Option[String])](":path") {
override def parse(name: String, value: String, parserSettings: ParserSettings): (Uri.Path, Option[String]) =
try {
if (value.isEmpty) {
protocolError("Pseudo-header ':path' must not be empty")
Copy link
Member Author

@He-Pin He-Pin Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2

Copy link
Member Author

@He-Pin He-Pin Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ":path" pseudo-header field includes the path and query parts
of the target URI (the "path-absolute" production and optionally a
'?' character followed by the "query" production (see Sections 3.3
and 3.4 of [RFC3986]). A request in asterisk form includes the
value '*' for the ":path" pseudo-header field.

  This pseudo-header field MUST NOT be empty for "http" or "https"
  URIs; "http" or "https" URIs that do not contain a path component
  MUST include a value of '/'.  The exception to this rule is an
  OPTIONS request for an "http" or "https" URI that does not include
  a path component; these MUST include a ":path" pseudo-header field
  with a value of '*' (see [[RFC7230], Section 5.3.4](https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.4)).

Belshe, et al. Standards Track [Page 55]

RFC 7540 HTTP/2 May 2015

All HTTP/2 requests MUST include exactly one valid value for the
":method", ":scheme", and ":path" pseudo-header fields, unless it is
a CONNECT request (Section 8.3). An HTTP request that omits
mandatory pseudo-header fields is malformed (Section 8.1.2.6).

HTTP/2 does not define a way to carry the version identifier that is
included in the HTTP/1.1 request line.

} else try {
Uri.parseHttp2PathPseudoHeader(value, mode = parserSettings.uriParsingMode)
} catch {
case IllegalUriException(info) => throw new ParsingException(info)
Expand Down
3 changes: 2 additions & 1 deletion http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@ private[http] final class UriParser(
// https://tools.ietf.org/html/rfc7540#section-8.1.2.3
def `http2-path-pseudo-header` = rule(
`absolute-path` ~ optional('?' ~ rawQueryString) // origin-form
) // TODO: asterisk-form
| '*' ~ run(setPath(Path.Empty)) ~ run { _rawQueryString = None } // asterisk-form (for OPTIONS requests)
)
Copy link
Member Author

@He-Pin He-Pin Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pseudo-header field MUST NOT be empty for "http" or "https"
URIs; "http" or "https" URIs that do not contain a path component
MUST include a value of '/'. The exception to this rule is an
OPTIONS request for an "http" or "https" URI that does not include
a path component; these MUST include a ":path" pseudo-header field
with a value of '*' (see [RFC7230], Section 5.3.4).

Copy link
Contributor

@jrudolph jrudolph Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test for this behavior? We need to demonstrate how users can deal with these requests. Path.Empty is likely not what user code wants to see for these requests.


/**
* @return path and percent-encoded query string. When in in 'relaxed' mode, characters not permitted by https://tools.ietf.org/html/rfc3986#section-3.4
Expand Down
102 changes: 91 additions & 11 deletions http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
// Insert the Foo header so it occurs before at least one pseudo-header
val (before, after) = pseudoHeaders.splitAt(insertPoint)
val modified = before ++ Vector("Foo" -> "bar") ++ after
parseExpectProtocolError(modified)
val ex = parseExpectProtocolError(modified)
ex.getMessage should
===(s"Malformed request: Pseudo-header field '${after.head._1}' must not appear after a regular header")
}
}

Expand All @@ -152,24 +154,29 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp

"not accept connection-specific headers" in {
// Add Connection header to indicate that Foo is a connection-specific header
parseExpectProtocolError(Vector(
val ex = parseExpectProtocolError(Vector(
":method" -> "GET",
":scheme" -> "https",
":path" -> "/",
"Connection" -> "foo",
"Foo" -> "bar"))
"Foo" -> "bar"
))
ex.getMessage should ===("Malformed request: Header 'Connection' must not be used with HTTP/2")
}

"not accept TE with other values than 'trailers'" in {

// The only exception to this is the TE header field, which MAY be
// present in an HTTP/2 request; when it is, it MUST NOT contain any
// value other than "trailers".
parseExpectProtocolError(Vector(
val ex = parseExpectProtocolError(Vector(
":method" -> "GET",
":scheme" -> "https",
":path" -> "/",
"TE" -> "chunked"))
"TE" -> "chunked"
))
ex.getMessage should
===("Malformed request: Header 'TE' must not contain value other than 'trailers', value was 'chunked")

}

Expand Down Expand Up @@ -442,13 +449,15 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
// ... A request in asterisk form includes the
// value '*' for the ":path" pseudo-header field.

"handle a ':path' with an asterisk" in pendingUntilFixed {
"handle a ':path' with an asterisk" in {
val request: HttpRequest = parseExpectOk(
keyValuePairs = Vector(
":method" -> "OPTIONS",
":scheme" -> "http",
":path" -> "*"))
request.uri.toString should ===("*") // FIXME: Compare in a better way
":path" -> "*"
))
request.uri.path.toString should ===("")
request.uri.rawQueryString should ===(None)
}

// [The ":path"] pseudo-header field MUST NOT be empty for "http" or "https"
Expand All @@ -457,11 +466,13 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
"reject empty ':path' pseudo-headers for http and https" in {
val schemes = Seq("http", "https")
forAll(schemes) { (scheme: String) =>
parseExpectProtocolError(
val ex = parseExpectProtocolError(
keyValuePairs = Vector(
":method" -> "POST",
":scheme" -> scheme,
":path" -> ""))
":path" -> ""
))
ex.getMessage should ===("Malformed request: Pseudo-header ':path' must not be empty")
}
}

Expand All @@ -477,7 +488,76 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp
// a CONNECT request (Section 8.3). An HTTP request that omits
// mandatory pseudo-header fields is malformed

// [assume CONNECT not supported]
// 8.3. HTTP/2 CONNECT Method
Copy link
Contributor

@jrudolph jrudolph Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we support CONNECT at all? Is that even something that user code could implement?

Since we make the logic more strict, it could still be ok to fix this here but we should make sure we don't mask an error that reports that CONNECT is not supported with a different parsing error now.


// The CONNECT method can be used to convert the HTTP/2 connection into
// a tunnel for a non-HTTP/2 protocol. The CONNECT method is identified
// by the ":method" pseudo-header field having a value of "CONNECT".
//
// A CONNECT request that does not conform to the following restrictions
// is malformed (Section 8.1.2.6):
//
// * The ":scheme" and ":path" pseudo-header fields MUST NOT be included
//
// * The ":authority" pseudo-header field contains the host and port to
// connect to (equivalent to the authority-form of the request-target
// of CONNECT requests, see [RFC7230], Section 5.3.3).

"handle CONNECT requests correctly" should {

"accept valid CONNECT requests without :scheme and :path" in {
val request: HttpRequest = parseExpectOk(
keyValuePairs = Vector(
":method" -> "CONNECT",
":authority" -> "example.com:443"
))
request.method should ===(HttpMethods.CONNECT)
request.uri.authority.host.toString should ===("example.com")
request.uri.authority.port should ===(443)
}

"reject CONNECT requests with :path pseudo-header" in {
val ex = parseExpectProtocolError(
keyValuePairs = Vector(
":method" -> "CONNECT",
":authority" -> "example.com:443",
":path" -> "/"
))
ex.getMessage should ===("Malformed request: Pseudo-header ':path' must not be included in CONNECT request")
}

"reject CONNECT requests with :scheme pseudo-header" in {
val ex = parseExpectProtocolError(
keyValuePairs = Vector(
":method" -> "CONNECT",
":authority" -> "example.com:443",
":scheme" -> "https"
))
ex.getMessage should ===("Malformed request: Pseudo-header ':scheme' must not be included in CONNECT request")
}

"reject CONNECT requests with both :scheme and :path pseudo-headers" in {
val ex = parseExpectProtocolError(
keyValuePairs = Vector(
":method" -> "CONNECT",
":authority" -> "example.com:443",
":scheme" -> "https",
":path" -> "/"
))
// Should fail on :path first since it's checked before :scheme in the header parsing order
ex.getMessage should ===("Malformed request: Pseudo-header ':path' must not be included in CONNECT request")
}

"reject CONNECT requests with empty :path" in {
val ex = parseExpectProtocolError(
keyValuePairs = Vector(
":method" -> "CONNECT",
":authority" -> "example.com:443",
":path" -> ""
))
ex.getMessage should ===("Malformed request: Pseudo-header ':path' must not be empty")
}
}

"reject requests without a mandatory pseudo-headers" in {
val mandatoryPseudoHeaders = Seq(":method", ":scheme", ":path")
Expand Down