From d8b88a33f55b54d7cfce4c5910095b7883864c18 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Tue, 1 Sep 2026 11:50:37 +0100 Subject: [PATCH] fix: count repeated Connection headers towards max-header-count (#1255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: `parseHeaderLines` merges repeated `Connection` headers into a single accumulated header via `x.append(h.tokens)`, but the merge branch re-entered the loop with `headerCount` unchanged. The `headerCount < settings.maxHeaderCount` guard therefore never tripped on `Connection` headers, so a single message could carry an unbounded number of them. Because `Connection.append` is `Connection(this.tokens ++ tokens)` — an O(n) copy of the accumulated token list on each header — and there is no total-header-block byte limit, N repeated `Connection` headers drive ~N²/2 token copies and an N-element list from one request. Every other non-deduplicated header increments the count; only this accumulating path did not. Modification: Increment `headerCount` in the `Connection` merge branch so each repeated `Connection` header counts towards `max-header-count`, exactly as the generic header path does. Once the limit is reached the existing guard rejects the message with the standard "more than the configured limit" error, bounding the number of merges to `max-header-count`. Result: A flood of `Connection` headers is rejected at `max-header-count` (default 64) instead of accumulating unboundedly, so the parser's work is bounded by the configured limit like every other header. Tests: - sbt "http-core/testOnly org.apache.pekko.http.impl.engine.parsing.RequestParserCRLFSpec org.apache.pekko.http.impl.engine.parsing.RequestParserLFSpec" - pass (116 tests); two new tests assert that exceeding max-header-count is rejected both for distinct headers and for repeated Connection headers. Verified the Connection test fails with the fix stashed (the flood is never caught). - sbt http-core/mimaReportBinaryIssues - pass References: None - bounds repeated Connection header accumulation to max-header-count --- .../engine/parsing/HttpMessageParser.scala | 7 ++++-- .../engine/parsing/RequestParserSpec.scala | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala index 40803632d8..0bfd35dfd6 100644 --- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala +++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala @@ -215,8 +215,11 @@ private[http] trait HttpMessageParser[Output >: MessageOutput <: ParserOutput] { case h: Connection => ch match { case None => parseHeaderLines(input, lineEnd, headers += h, headerCount + 1, Some(h), clh, cth, isChunked, e100c, hh) - case Some(x) => parseHeaderLines(input, lineEnd, headers, headerCount, Some(x.append(h.tokens)), clh, cth, - isChunked, e100c, hh) + // count each merged Connection header towards the limit: the tokens are accumulated into `x` (an O(n) copy + // per header), so without incrementing headerCount the `headerCount < maxHeaderCount` guard never trips and + // a flood of Connection headers drives unbounded quadratic work from a single message + case Some(x) => parseHeaderLines(input, lineEnd, headers, headerCount + 1, Some(x.append(h.tokens)), clh, + cth, isChunked, e100c, hh) } case h: Host => if (!hh || isResponseParser) diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala index a0fb3dc5bf..fd9a160e30 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala @@ -640,6 +640,28 @@ abstract class RequestParserSpec(mode: String, newLine: String) extends AnyFreeS ErrorInfo("HTTP header value exceeds the configured limit of 32 characters")) } + "with more headers than the configured limit" in new Test { + override def parserSettings: ParserSettings = super.parserSettings.withMaxHeaderCount(2) + """GET / HTTP/1.1 + |A: 1 + |B: 2 + |C: 3""" should parseToError( + BadRequest, + ErrorInfo("HTTP message contains more than the configured limit of 2 headers")) + } + + "with more repeated Connection headers than the configured limit" in new Test { + // repeated Connection headers are merged into one, but each still counts towards maxHeaderCount so that a + // flood cannot bypass the limit and force unbounded quadratic token accumulation + override def parserSettings: ParserSettings = super.parserSettings.withMaxHeaderCount(2) + """GET / HTTP/1.1 + |Connection: a + |Connection: b + |Connection: c""" should parseToError( + BadRequest, + ErrorInfo("HTTP message contains more than the configured limit of 2 headers")) + } + "with an invalid Content-Length header value" in new Test { """GET / HTTP/1.0 |Content-Length: 1.5