fix: count repeated Connection headers towards max-header-count (#1255) - #1268
Open
pjfanning wants to merge 1 commit into
Open
fix: count repeated Connection headers towards max-header-count (#1255)#1268pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
…he#1255) 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cherry pick 4fab7d6 #1255