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 64752d44a fix: reject uppercase HTTP/2 field names and values with
surrounding whitespace (#1343)
64752d44a is described below
commit 64752d44a45cb5d07fa33ff857f37854bab2a45d
Author: PJ Fanning <[email protected]>
AuthorDate: Sun Oct 11 17:23:25 2026 +0100
fix: reject uppercase HTTP/2 field names and values with surrounding
whitespace (#1343)
* fix: reject uppercase HTTP/2 field names and values with surrounding
whitespace
Motivation:
RFC 9113 8.2.1 makes an HTTP/2 message malformed when a field name contains
an
uppercase character or a field value starts or ends with SP or HTAB. Pekko
HTTP accepted
both: regular fields are handed to the HTTP/1.1 header-line parser, which
matches names
case-insensitively and trims the value, so the message was silently
normalised instead of
rejected. Other HTTP/2 implementations (nghttp2, netty for names) reject
them, so two hops
can disagree about the same field.
On the client, a field rejected in HeaderDecompression left the response
with no headers
and was reported as a missing ':status' rather than as what was wrong.
Modification:
- HeaderDecompression rejects an uppercase field name and a value with
leading or
trailing SP/HTAB, next to the existing NUL/CR/LF check. The messages from
this stage
drop the "Malformed request:" prefix, since it decodes responses on the
client too.
- ResponseParsing reports a header error from decompression as
"Malformed response: <reason>".
- Lowercase the incidental uppercase names ("Foo", "Connection", "TE") in
RequestParsingSpec
tests that check other rules, and let the ':authority'/':path' tests that
pass
whitespace-only or whitespace-terminated values accept the earlier
field-check message.
- Add a ResponseParsingSpec for the client side.
Result:
Requests with such fields get a 400 for the stream; responses with them
fail with a
protocol error naming the field problem. Pekko HTTP's own client and server
always render
lowercase names, so traffic between them is unaffected.
Tests:
- Without the main change the new RequestParsingSpec uppercase/whitespace
tests and the
ResponseParsingSpec rejection tests fail
- sbt http2-tests/test: 393 passed
- sbt http-core/mimaReportBinaryIssues and scalafmt checks: pass
References:
None - follow-up to #1341
* scalafmt
* reject every character RFC 9113 8.2.1 excludes from field names, not just
uppercase
---
.../impl/engine/http2/client/ResponseParsing.scala | 7 +-
.../engine/http2/hpack/HeaderCompression.scala | 14 ++++
.../engine/http2/hpack/HeaderDecompression.scala | 15 +++-
.../impl/engine/http2/RequestParsingSpec.scala | 48 ++++++++++--
.../engine/http2/client/ResponseParsingSpec.scala | 91 ++++++++++++++++++++++
5 files changed, 162 insertions(+), 13 deletions(-)
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala
index 78868b2c6..9e49e182e 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsing.scala
@@ -118,7 +118,12 @@ private[http2] object ResponseParsing {
headers += httpHeader)
}
- rec(subStream.initialHeaders.keyValuePairs)
+ subStream.initialHeaders.headerParseErrorDetails match {
+ // a field HeaderDecompression rejected leaves no headers behind, which
would otherwise be reported as a
+ // missing ':status' rather than as what was wrong
+ case Some(info) => malformedResponse(info.summary)
+ case None => rec(subStream.initialHeaders.keyValuePairs)
+ }
}
private def malformedResponse(msg: String): Nothing =
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
index c86ef6367..8e39f21c0 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
@@ -38,6 +38,20 @@ private[http2] object HeaderCompression extends
GraphStage[FlowShape[FrameEvent,
private[http2] def hasIllegalChar(s: String): Boolean =
s.indexOf('\r') >= 0 || s.indexOf('\n') >= 0 || s.indexOf('\u0000') >= 0
+ /**
+ * RFC 9113 8.2.1: an HTTP/2 field name must not contain characters in the
ranges 0x00-0x20, 0x41-0x5a or
+ * 0x7f-0xff, i.e. no control characters, SP, uppercase characters or
non-ASCII characters. Anything above 0xff
+ * is rejected too, since such a character can only come from a non-ASCII
octet sequence.
+ */
+ private[http2] def hasInvalidFieldNameChar(s: String): Boolean =
+ s.exists(c => c <= 0x20 || (c >= 'A' && c <= 'Z') || c >= 0x7F)
+
+ /** RFC 9113 8.2.1: an HTTP/2 field value must not start or end with SP or
HTAB. */
+ private[http2] def hasSurroundingWhitespace(s: String): Boolean =
+ s.nonEmpty && (isWhitespace(s.charAt(0)) || isWhitespace(s.charAt(s.length
- 1)))
+
+ private def isWhitespace(c: Char): Boolean = c == ' ' || c == '\t'
+
def createLogic(inheritedAttributes: Attributes): GraphStageLogic =
new GraphStageLogic(shape) with StageLogging with InHandler with
OutHandler { logic =>
setHandlers(eventsIn, eventsOut, this)
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 4cd22a15a..408ae35a8 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
@@ -85,12 +85,19 @@ private[http2] final class
HeaderDecompression(masterHeaderParser: HttpHeaderPar
// here, before the field is dispatched on its name: a regular
field goes through the HTTP/1.1 line
// parser, which reads up to the first CRLF it finds and would
silently accept the value truncated
// there. Neither the name nor the value is echoed, since either
may be what is malformed.
+ // The same section makes a name with control, SP, uppercase or
non-ASCII characters and a value with
+ // leading or trailing whitespace malformed; the HTTP/1.1 parser
would otherwise match the name
+ // case-insensitively and trim the value.
+ // This stage decodes responses on the client too, so the messages
do not say "request".
if (HeaderCompression.hasIllegalChar(name))
- throw new ParsingException(
- ErrorInfo("Malformed request: header field name must not
contain CR, LF or NUL"))
+ throw new ParsingException(ErrorInfo("header field name must not
contain CR, LF or NUL"))
if (HeaderCompression.hasIllegalChar(value))
- throw new ParsingException(
- ErrorInfo("Malformed request: header field value must not
contain CR, LF or NUL"))
+ throw new ParsingException(ErrorInfo("header field value must
not contain CR, LF or NUL"))
+ if (HeaderCompression.hasInvalidFieldNameChar(name))
+ throw new ParsingException(ErrorInfo(
+ "header field name must not contain control, whitespace,
uppercase or non-ASCII characters"))
+ if (HeaderCompression.hasSurroundingWhitespace(value))
+ throw new ParsingException(ErrorInfo("header field value must
not start or end with whitespace"))
if (parsed ne null) {
headers += name -> parsed
parsed
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 65067d8c6..1af04d359 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
@@ -109,6 +109,12 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
futureValueEx.getCause.asInstanceOf[Http2ProtocolException]
}
+ "accept a header value with whitespace inside it, or an empty value" in {
+ val request = parseExpectOk(Vector(":method" -> "GET", ":scheme" ->
"https", ":path" -> "/", "x-a" -> "a b",
+ "x-b" -> ""))
+ (request.headers.map(h => h.lowercaseName -> h.value) should
contain).allOf("x-a" -> "a b", "x-b" -> "")
+ }
+
"reject a malformed header field with a bad request rather than accepting
or failing the connection" should {
// RFC 9113 8.2.1: a field name or value carrying NUL, CR or LF makes
the message malformed
def request(extra: (String, String)*): Vector[(String, String)] =
@@ -132,6 +138,26 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
val info = parseExpectError(request("x-a\r\nx-b" -> "v"))
info.summary should include("header field name must not contain CR, LF
or NUL")
}
+ "a header name containing an uppercase character" in {
+ // the HTTP/1.1 parser this is handed to matches names
case-insensitively, so without the check the
+ // request was accepted as if the name had been lowercase
+ for (name <- Seq("X-A", "x-A", "Content-Type"))
+ parseExpectError(request(name -> "v")).summary should ===(
+ "header field name must not contain control, whitespace, uppercase
or non-ASCII characters")
+ }
+ "a header name containing a control, whitespace or non-ASCII character"
in {
+ // RFC 9113 8.2.1 also excludes 0x00-0x20 and 0x7f-0xff from field
names
+ for (name <- Seq("x a", "x\ta", "x\u0001a", "x-a ", "x\u007fa",
"x\u0080a", "x\u00ffa", "x\u00e9a"))
+ parseExpectError(request(name -> "v")).summary should ===(
+ "header field name must not contain control, whitespace, uppercase
or non-ASCII characters")
+ }
+ "a header value with leading or trailing whitespace" in {
+ // the HTTP/1.1 parser this is handed to trims it, so without the
check the request was accepted with the
+ // value silently changed
+ for (value <- Seq(" v", "v ", "\tv", "v\t"))
+ parseExpectError(request("x-a" -> value)).summary should ===(
+ "header field value must not start or end with whitespace")
+ }
"a header value longer than max-header-value-length" in {
// the HTTP/1.1 parser reports this with its own, internal exception
type, which used to escape the
// decompression stage and fail the whole connection instead of
answering the one stream
@@ -182,9 +208,9 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
":scheme" -> "https",
":path" -> "/")
forAll(pseudoHeaders.indices: Seq[Int]) { (insertPoint: Int) =>
- // Insert the Foo header so it occurs before at least one
pseudo-header
+ // 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
+ val modified = before ++ Vector("foo" -> "bar") ++ after
parseExpectProtocolError(modified)
}
}
@@ -200,8 +226,8 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
":method" -> "GET",
":scheme" -> "https",
":path" -> "/",
- "Connection" -> "foo",
- "Foo" -> "bar"))
+ "connection" -> "foo",
+ "foo" -> "bar"))
}
"not accept TE with other values than 'trailers'" in {
@@ -213,7 +239,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
":method" -> "GET",
":scheme" -> "https",
":path" -> "/",
- "TE" -> "chunked"))
+ "te" -> "chunked"))
}
@@ -222,7 +248,7 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
":method" -> "GET",
":scheme" -> "https",
":path" -> "/",
- "TE" -> "trailers"))
+ "te" -> "trailers"))
}
// 8.1.2.3. Request Pseudo-Header Fields
@@ -301,7 +327,10 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
":scheme" -> "https",
":authority" -> authority,
":path" -> "/"))
- info.summary should include("http2-authority-pseudo-header")
+ // a value that is all whitespace is caught by the RFC 9113 8.2.1
field check before the URI parser
+ info.summary should
+ (include("http2-authority-pseudo-header").or(
+ include("header field value must not start or end with
whitespace")))
}
}
}
@@ -412,7 +441,10 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
"http://localhost/foo";)
forAll(invalidAbsolutePaths) { (absPath: String) =>
val info = parsePathExpectError(absPath)
- info.summary should include("http2-path-pseudo-header")
+ // a value ending in whitespace is caught by the RFC 9113 8.2.1
field check before the URI parser
+ info.summary should
+ (include("http2-path-pseudo-header").or(
+ include("header field value must not start or end with
whitespace")))
}
}
diff --git
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsingSpec.scala
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsingSpec.scala
new file mode 100644
index 000000000..87173e9be
--- /dev/null
+++
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/client/ResponseParsingSpec.scala
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.http.impl.engine.http2.client
+
+import scala.util.Try
+
+import org.apache.pekko
+import pekko.http.impl.engine.http2.{ HPackEncodingSupport, Http2SubStream }
+import pekko.http.impl.engine.http2.FrameEvent._
+import pekko.http.impl.engine.http2.Http2Compliance.Http2ProtocolException
+import pekko.http.impl.engine.http2.hpack.HeaderDecompression
+import pekko.http.impl.engine.parsing.HttpHeaderParser
+import pekko.http.impl.util.PekkoSpecWithMaterializer
+import pekko.http.scaladsl.model._
+import pekko.http.scaladsl.settings.ClientConnectionSettings
+import pekko.stream.Attributes
+import pekko.stream.scaladsl.{ Sink, Source }
+import pekko.util.{ ByteString, OptionVal }
+
+class ResponseParsingSpec extends PekkoSpecWithMaterializer {
+ "ResponseParsing" should {
+
+ def parse(keyValuePairs: Seq[(String, String)]): Try[HttpResponse] = {
+ val settings = ClientConnectionSettings(system)
+ val headerParser = HttpHeaderParser(settings.parserSettings, log)
+ val encoder = new HPackEncodingSupport {}
+ val frame = HeadersFrame(1, endStream = true, endHeaders = true,
encoder.encodeHeaderPairs(keyValuePairs), None)
+
+ Source.single(frame)
+ .via(new HeaderDecompression(headerParser, settings.parserSettings,
settings.http2Settings.maxHeaderListSize))
+ .map { // emulate demux
+ case headers: ParsedHeadersFrame =>
+ Http2SubStream(
+ initialHeaders = headers,
+ trailingHeaders = OptionVal.None,
+ data = Right(Source.empty[ByteString]),
+ correlationAttributes = Map.empty)
+ }
+ .map(subStream =>
+ Try(ResponseParsing.parseResponse(headerParser,
settings.parserSettings, Attributes())(subStream)))
+ .runWith(Sink.head)
+ .futureValue
+ }
+
+ def malformedResponseMessage(keyValuePairs: Seq[(String, String)]): String
=
+ parse(keyValuePairs).failed.get match {
+ case ex: Http2ProtocolException => ex.getMessage
+ case other => fail(s"Expected an
Http2ProtocolException but got $other")
+ }
+
+ "parse a response with lowercase field names" in {
+ val response = parse(Seq(":status" -> "200", "x-a" -> "v")).get
+ response.status should ===(StatusCodes.OK)
+ response.headers.map(h => h.lowercaseName -> h.value) should
contain("x-a" -> "v")
+ }
+
+ // RFC 9113 8.2.1: a field name must not contain control, SP, uppercase or
non-ASCII characters and a value must not start or end with
+ // whitespace. HeaderDecompression rejects either, and the failure has to
name what was wrong rather than surface as
+ // the ':status' missing from the headers it dropped.
+ "reject a field name containing an uppercase character" in {
+ malformedResponseMessage(Seq(":status" -> "200", "X-A" -> "v")) should
===(
+ "Malformed response: header field name must not contain control,
whitespace, uppercase or non-ASCII characters")
+ }
+
+ "reject a field value with leading or trailing whitespace" in {
+ for (value <- Seq(" v", "v ", "\tv", "v\t"))
+ malformedResponseMessage(Seq(":status" -> "200", "x-a" -> value))
should ===(
+ "Malformed response: header field value must not start or end with
whitespace")
+ }
+
+ "reject a field value containing CR, LF or NUL" in {
+ malformedResponseMessage(Seq(":status" -> "200", "x-a" -> "a\r\nb"))
should ===(
+ "Malformed response: header field value must not contain CR, LF or
NUL")
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
