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 40b07a21d http/2: reject a header field carrying CR, LF or NUL, and
answer a malformed field with a 400 (#1297)
40b07a21d is described below
commit 40b07a21d4f805e0662826adff21daba1b469900
Author: PJ Fanning <[email protected]>
AuthorDate: Fri Sep 11 14:55:01 2026 +0100
http/2: reject a header field carrying CR, LF or NUL, and answer a
malformed field with a 400 (#1297)
Motivation:
Two things go wrong when an HTTP/2 peer sends a malformed header field.
A regular field whose value contains CR LF is silently accepted with the
value truncated. `RequestParsing.parseHeaderPair` reuses the HTTP/1.1
line parser by building `name + ": " + value + "\r\nx"`, so the parser
stops at the first CRLF it meets, which is now the peer's, and the header
that comes back is whatever preceded it. RFC 9113 section 8.2.1 says a
field name or value carrying NUL, CR or LF must be treated as malformed.
A field the HTTP/1.1 parser does reject -- a NUL in the value, an illegal
character in the name, a value over `max-header-value-length` -- is
reported with that parser's own, internal `ParsingException`, which is
not the model `ParsingException` that `HeaderDecompression` catches. It
escapes the decompression stage and fails it, and with it the whole
connection and every stream on it, where the HTTP/2 engine answers other
malformed fields with a 400 on the one stream (#59). On a connection a
proxy multiplexes for many users, one bad header from one of them takes
the connection down for all of them.
Modification:
Check the name and the value for CR, LF and NUL at the top of the HPACK
listener, before the field is dispatched on its name, reusing the
predicate the rendering side uses. Neither is echoed in the error, since
either may be what is malformed. Widen the internal `ParsingException`
from `private[parsing]` to `private[http]` and rethrow it from
`parseHeaderPair` as the model exception, so that every failure the
HTTP/1.1 parser reports for an HTTP/2 field takes the 400 path.
Result:
A header field carrying CR, LF or NUL is answered with a 400 instead of
being accepted truncated, and a field the HTTP/1.1 parser rejects is
answered with a 400 on its own stream instead of failing the connection.
Tests:
- `RequestParsingSpec`: CR LF, bare LF and NUL in a value, CR LF in a
name, and a value over `max-header-value-length` each produce a
`BadRequest` with the expected summary.
- `Http2ServerSpec`: the CR LF, NUL and over-long cases each get a 400 on
their stream and the next stream on the same connection is served.
- With both source changes reverted all eight fail; with only the CR/LF/
NUL check in place the two over-long cases still fail, so the exception
translation is covered on its own.
- sbt "http2-tests/test" - 374 pass.
- sbt "http-core/mimaReportBinaryIssues" - pass.
- native scalafmt clean.
References:
Refs #59
---
.../http/impl/engine/http2/RequestParsing.scala | 11 ++++--
.../engine/http2/hpack/HeaderDecompression.scala | 10 ++++++
.../pekko/http/impl/engine/parsing/package.scala | 2 +-
.../http/impl/engine/http2/Http2ServerSpec.scala | 39 ++++++++++++++++++++++
.../impl/engine/http2/RequestParsingSpec.scala | 33 ++++++++++++++++++
5 files changed, 92 insertions(+), 3 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 386873947..af019fbfd 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
@@ -218,8 +218,15 @@ private[http2] object RequestParsing {
// The odd-looking 'x' below is a by-product of how current parser and
HTTP/1.1 work.
// Without '\r\n\x' (x being any additional byte) parsing will fail. See
HttpHeaderParserSpec for examples.
val concHeaderLine = name + ": " + value + "\r\nx"
- httpHeaderParser.parseHeaderLine(ByteString(concHeaderLine))()
- httpHeaderParser.resultHeader
+ try {
+ httpHeaderParser.parseHeaderLine(ByteString(concHeaderLine))()
+ httpHeaderParser.resultHeader
+ } catch {
+ // the HTTP/1.1 parser reports a malformed field with its own, internal
exception type, which nothing on the
+ // HTTP/2 side catches: left alone it fails the decompression stage and
with it the whole connection. Rethrow
+ // it as the model exception `HeaderDecompression` turns into a 400 for
the one stream.
+ case e: pekko.http.impl.engine.parsing.ParsingException => throw new
ParsingException(e.info)
+ }
}
private[http2] def checkRequiredPseudoHeader(name: String, value: AnyRef):
Unit =
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 d51dd85e8..4cd22a15a 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
@@ -81,6 +81,16 @@ private[http2] final class
HeaderDecompression(masterHeaderParser: HttpHeaderPar
var parsingError: Option[ErrorInfo] = None
object Receiver extends HeaderListener {
def addHeader(name: String, value: String, parsed: AnyRef,
sensitive: Boolean): AnyRef = try {
+ // RFC 9113 8.2.1: a field name or value carrying a NUL, CR or LF
makes the message malformed. Check it
+ // 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.
+ if (HeaderCompression.hasIllegalChar(name))
+ throw new ParsingException(
+ ErrorInfo("Malformed request: 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"))
if (parsed ne null) {
headers += name -> parsed
parsed
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
index 92b2036fb..9ac295cf2 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
@@ -70,7 +70,7 @@ package parsing {
* INTERNAL API
*/
@InternalApi
- private[parsing] class ParsingException(
+ private[http] class ParsingException(
val status: StatusCode,
val info: ErrorInfo) extends RuntimeException(info.formatPretty) {
def this(status: StatusCode, summary: String) =
diff --git
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
index 662078d44..9ced005fb 100644
---
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
+++
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
@@ -57,6 +57,45 @@ class Http2ServerSpec extends Http2SpecWithMaterializer("""
override def failOnSevereMessages: Boolean = true
"The Http/2 server implementation" should {
+ "answer a malformed header field with a 400 on its stream and keep the
connection" should {
+ abstract class MalformedHeaderSetup extends TestSetup with
RequestResponseProbes {
+ def badRequestThenStillUsable(streamId: Int, headerPairs: Seq[(String,
String)]): Unit = {
+ // the 400 is produced where the parsed request would otherwise be
handed to the handler, so the handler
+ // has to be asking for one
+ user.requestIn.request(1)
+ network.sendHEADERS(streamId, endStream = true, endHeaders = true,
network.encodeHeaderPairs(headerPairs))
+ network.expectDecodedResponseHEADERSPairs(streamId, endStream =
false).toMap should contain(
+ ":status" -> "400")
+ network.expectDATAFrame(streamId)
+
+ // the connection is still open and serving: the next stream gets
through to the handler
+ val nextStreamId = streamId + 2
+ network.sendRequest(nextStreamId,
+ HttpRequest(HttpMethods.GET, "https://www.example.com/";, protocol
= HttpProtocols.`HTTP/2.0`))
+ user.expectRequest()
+ user.emitResponse(nextStreamId, HttpResponse())
+ network.expectDecodedResponseHEADERSPairs(nextStreamId).toMap should
contain(":status" -> "200")
+ }
+ def request(extra: (String, String)*): Seq[(String, String)] =
+ Seq(":method" -> "GET", ":scheme" -> "https", ":path" -> "/",
":authority" -> "www.example.com") ++ extra
+ }
+
+ "for a value containing CR LF".inAssertAllStagesStopped(new
MalformedHeaderSetup {
+ badRequestThenStillUsable(1, request("x-a" -> "foo\r\nx-b: bar"))
+ })
+ "for a value containing NUL".inAssertAllStagesStopped(new
MalformedHeaderSetup {
+ // before the fix this failed the decompression stage and took the
whole connection down
+ badRequestThenStillUsable(1, request("x-a" -> "foo\u0000bar"))
+ })
+ "for a value longer than
max-header-value-length".inAssertAllStagesStopped(new MalformedHeaderSetup {
+ override def settings: ServerSettings = {
+ val s = super.settings
+ s.withParserSettings(s.parserSettings.withMaxHeaderValueLength(16))
+ }
+ badRequestThenStillUsable(1, request("x-a" -> ("v" * 17)))
+ })
+ }
+
"support simple round-trips" should {
abstract class SimpleRequestResponseRoundtripSetup extends TestSetup
with RequestResponseProbes {
def requestResponseRoundtrip(
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 e972a87e1..65067d8c6 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,39 @@ class RequestParsingSpec extends PekkoSpecWithMaterializer
with Inside with Insp
futureValueEx.getCause.asInstanceOf[Http2ProtocolException]
}
+ "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)] =
+ Vector(":method" -> "GET", ":scheme" -> "https", ":path" -> "/") ++
extra
+
+ "a header value containing CR LF" in {
+ // the HTTP/1.1 line parser this is handed to stops at the first CRLF
it finds, so without the check the
+ // request was accepted with the value silently truncated to `foo`
+ val info = parseExpectError(request("x-a" -> "foo\r\nx-b: bar"))
+ info.summary should include("header field value must not contain CR,
LF or NUL")
+ }
+ "a header value containing a bare LF" in {
+ val info = parseExpectError(request("x-a" -> "foo\nbar"))
+ info.summary should include("header field value must not contain CR,
LF or NUL")
+ }
+ "a header value containing NUL" in {
+ val info = parseExpectError(request("x-a" -> "foo\u0000bar"))
+ info.summary should include("header field value must not contain CR,
LF or NUL")
+ }
+ "a header name containing CR LF" in {
+ 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 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
+ val settings = ServerSettings(system)
+ val small =
settings.withParserSettings(settings.parserSettings.withMaxHeaderValueLength(16))
+ val info = parseExpectError(request("x-a" -> ("v" * 17)), settings =
small)
+ info.summary should include("HTTP header value exceeds the configured
limit of 16 characters")
+ }
+ }
+
"follow RFC7540" should {
// 8.1.2.1. Pseudo-Header Fields
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
