From 8985ffedd798e1f624c5a1136e0713b52bcca6c5 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 8 Mar 2026 16:00:00 +0800 Subject: [PATCH 1/2] fix: align HTTP/2 malformed-header handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../impl/engine/http2/RequestParsing.scala | 28 +++++- .../http2/hpack/Http2HeaderParsing.scala | 6 +- .../engine/http2/RequestParsingSpec.scala | 94 +++++++++++++++++-- 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala index 8b3f148b7..d692fee82 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/RequestParsing.scala @@ -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 @@ -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`) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala index 7b2066065..df8761fd0 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/Http2HeaderParsing.scala @@ -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 } @@ -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") + } else try { Uri.parseHttp2PathPseudoHeader(value, mode = parserSettings.uriParsingMode) } catch { case IllegalUriException(info) => throw new ParsingException(info) diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala index 9c692d577..17c9a1d55 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala @@ -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") } } @@ -152,12 +154,14 @@ 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 { @@ -165,11 +169,14 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer with Inside with Insp // 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") } @@ -457,11 +464,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") } } @@ -477,7 +486,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 + + // 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") From bad9ff08aabf5fa27247c8eed947022491e1285c Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 8 Mar 2026 17:52:19 +0800 Subject: [PATCH 2/2] fix: improve handling of asterisk in HTTP/2 ':path' pseudo-header --- .../impl/engine/http2/hpack/HeaderDecompression.scala | 9 +++------ .../apache/pekko/http/impl/model/parser/UriParser.scala | 3 ++- .../http/impl/engine/http2/RequestParsingSpec.scala | 8 +++++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala index 6d992e36b..4dfab40bb 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderDecompression.scala @@ -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)) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala index 98d914f83..2d09bf017 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/model/parser/UriParser.scala @@ -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) + ) /** * @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 diff --git a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala index 17c9a1d55..79cbec174 100644 --- a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala +++ b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/RequestParsingSpec.scala @@ -449,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"