You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2026/08/30 12:20:49 UTC
[PR] fix: keep the HPACK decoder usable after a header fails to parse [pekko-http]
pjfanning opened a new pull request, #1252:
URL: https://github.com/apache/pekko-http/pull/1252
Noticed while working on #1251. A single malformed header currently makes every later HEADERS frame on that connection undecodable.
## The bug
`HeaderDecompression`'s `HeaderListener` threw `ParsingException` straight through the HPACK decoder. Three things follow from that, and they compound:
1. `Decoder.insertHeader` calls the listener **before** `dynamicTable.add(...)`, so the entry for the offending header never reached the table.
2. `decode()` unwound at that point, so every representation *after* it in the block was never read, and never added to the table either.
3. `endHeaderBlock()` sat inside the `try`, so it was skipped — leaving the decoder holding the `state` and `headerSize` of the abandoned block.
(1) and (2) desynchronise the decoder's dynamic table from the peer's encoder's, which is not something HPACK can recover from: from then on the peer's indexed references resolve to the wrong entries. (3) resumes the next block part way through a representation.
This is reachable, not theoretical. `HeaderDecompression` deliberately answers a parse failure with a bad request and keeps the connection open, so an unknown method — already covered by *"return bad request response when header parsing fails"* — is enough to trigger it.
## The fix
Catch `ParsingException` in the listener, remember the first `ErrorInfo`, and return `null` so decoding runs to the end of the block and the dynamic table keeps tracking the peer's. Report the remembered failure once the block is fully decoded, which produces the same bad request response as before.
`null` rather than the raw value on purpose: the decoder caches what the listener returns against the table entry, and caching an unparsed `String` where a `ContentType` is expected would hand a wrong type to a later indexed reference. `null` means it is parsed again — and fails again, consistently.
`endHeaderBlock()` also moves into a `finally`, so anything else that unwinds still resets the decoder — a malformed `:path` raises `Http2ProtocolException`, which is not a `ParsingException`. Running it twice on the success path is a no-op (`headerSize` is already 0). The outer `ParsingException` handler stays as a fallback for anything thrown outside the listener.
## Tests
New `"keep the connection usable after a header parsing failure"` in `Http2ClientServerSpec`: send a request with an unknown method, expect the bad request, then send a valid request on the same connection.
Verified it actually catches the bug — with the fix stashed so `HeaderDecompression` matched `main` exactly:
```
- should keep the connection usable after a header parsing failure *** FAILED ***
java.lang.AssertionError: assertion failed: timeout (3 seconds) during expectMsgClass
waiting for class ...Http2ClientServerSpec$ServerRequest
```
The follow-up request does not merely come back wrong — it never reaches the handler at all, which is the desync showing up end to end.
- `http2-tests/testOnly ...Http2ClientServerSpec` — 8 passed
- `http-core/testOnly org.apache.pekko.http.impl.engine.http2.*` — 13 passed
- `http2-tests/test` — 353 passed, 25 ignored, 26 pending
- `scalafmtCheckAll`, `headerCheck`, `http-core/mimaReportBinaryIssues` — clean
## Note on #1251
This is based on `main` so it can be reviewed and backported on its own. It touches the same `try` block that #1251 rewrites, so the two conflict textually; I will rebase whichever lands second.
🤖 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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: keep the HPACK decoder usable after a header fails to parse [pekko-http]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #1252:
URL: https://github.com/apache/pekko-http/pull/1252#issuecomment-5636353236
Closing: this change is already on `main`.
#1251 was stacked on this branch, and when it merged it carried this PR's commit with it — `4e07cbc98` `fix: keep the HPACK decoder usable after a header fails to parse` is on `main`, test included (the `Http2ClientServerSpec` change there is byte-identical to this PR's). Rebasing this branch onto `main` leaves an empty diff: the only conflict was against #1297's later CR/LF/NUL check in the same `addHeader` block, and once that resolves to `main`'s side there is nothing left to apply.
No follow-up needed; the fix is in and covered by its test.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: keep the HPACK decoder usable after a header fails to parse [pekko-http]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning closed pull request #1252: fix: keep the HPACK decoder usable after a header fails to parse
URL: https://github.com/apache/pekko-http/pull/1252
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org