pjfanning opened a new pull request, #1255:
URL: https://github.com/apache/pekko-http/pull/1255
### Motivation
`HttpMessageParser.parseHeaderLines` merges repeated `Connection` headers
into one accumulated header (`Some(x) => ...Some(x.append(h.tokens))...`), but
that merge branch re-entered the loop with `headerCount` **unchanged**:
```scala
case h: Connection => ch match {
case None =>
parseHeaderLines(input, lineEnd, headers += h, headerCount + 1,
Some(h), ...)
case Some(x) => parseHeaderLines(input, lineEnd, headers, headerCount,
Some(x.append(h.tokens)), ...) // headerCount not incremented
}
```
So the `headerCount < settings.maxHeaderCount` guard at the top of the loop
never trips on additional `Connection` headers, and a single message can carry
an unbounded number of them. `Connection.append` is `Connection(this.tokens ++
tokens)` — an O(n) copy of the accumulated token list per header — and there is
no total-header-block byte limit, so N repeated `Connection` headers force
~N²/2 token copies plus an N-element list from one request. Every other
non-deduplicated header increments the count via the generic branch; only this
accumulating path did not, making it a `max-header-count` bypass with quadratic
amplification.
The `Content-Length`/`Content-Type` duplicate branches also skip the
increment, but they require a structurally identical value and do not
accumulate, so they cannot be grown — `Connection` is the only unbounded merge.
### Modification
Increment `headerCount` in the `Connection` merge branch so each repeated
`Connection` header counts towards `max-header-count`, matching the generic
header path. 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` (default 64).
### Result
A flood of `Connection` headers is rejected at `max-header-count` instead of
accumulating unboundedly; the parser's work on this path is bounded by the
configured limit like every other header. Legitimate use (one or a few
`Connection` headers) is unaffected — they were already counted the same way
once the merge is charged.
### 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, no
error is emitted).
- `sbt http-core/mimaReportBinaryIssues` — pass (internal
`impl.engine.parsing` change, no public API).
- Native `scalafmt` on the two changed files — clean.
### References
None - bounds repeated Connection header accumulation to max-header-count
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
