pjfanning opened a new pull request, #1264: URL: https://github.com/apache/pekko-http/pull/1264
### Motivation `Http2FrameParsing` read a frame's 24-bit length field and then took that many bytes with no upper bound: ```scala val length = reader.readShortBE() << 8 | reader.readByte() ... val payload = reader.take(length) ``` So a peer could declare a length up to the field's maximum of 16 MiB - 1 and make the frame parser buffer that much for a single frame. This happens *before* the HPACK and entity limits apply, so `max-header-list-size`, `incoming-stream-level-buffer-size` and `incoming-connection-level-buffer-size` do not bound it — and it is multiplied by the number of connections. ### Modification Add a `max-frame-size` setting to the HTTP/2 server and client settings, defaulting to `512kB`, and reject a larger frame with a `FRAME_SIZE_ERROR` on its frame header, before the payload is buffered. The value is validated against the bounds RFC 9113 §4.2 sets for `SETTINGS_MAX_FRAME_SIZE` (16 KiB … 16 MiB - 1). **The setting is a limit on what is accepted, not an advertisement**, and that is deliberate. An earlier revision of this branch *did* advertise it as `SETTINGS_MAX_FRAME_SIZE`, and h2spec's "4.2 Frame Size" case showed that to be actively harmful: DATA frames are flow controlled and the initial window is 64 KiB, so a peer that sizes its first frame to a larger advertised value trips a `FLOW_CONTROL_ERROR` before the window has grown. h2spec did exactly that and the conformance test went red. Advertising nothing keeps a well-behaved peer at the 16 KiB default it already assumes, and the configured value is then pure leniency for peers that exceed it — we always accept at least as much as any peer is told it may send. That also preserves pekko's existing behaviour of accepting frames well above 16 KiB, which its own tests rely on. Why 512 kB and not the RFC ceiling: defaulting to 16 MiB would leave the default posture exactly as it is today and fix nothing unless an operator tuned it. 512 kB matches `incoming-stream-level-buffer-size`, so the parser's transient buffering stays the same order as the stream buffer instead of 32× larger, while still allowing 32× the protocol default. ### Result A single frame can no longer make the parser hold up to 16 MiB; the bound is 512 kB by default and configurable. Frames from a spec-compliant peer are unaffected. ### Tests - `sbt "http2-tests/testOnly ...Http2ServerSpec ...Http2ClientSpec ...H2SpecIntegrationSpec"` — pass (236 tests), including the h2spec conformance suite. A new test sends a frame one byte over a configured 16 KiB limit and expects `GOAWAY(FRAME_SIZE_ERROR)`; verified it fails with the size check disabled. - The existing "fail if more data is received than stream-level window allows" test deliberately sends a single 512001-byte frame to exceed the 512 kB stream buffer, so it now raises `max-frame-size` to 1 MiB in order to reach the flow-control check it is actually about. It was the only test in the suite that needed adjusting. - `sbt http-core/mimaReportBinaryIssues` — pass. - `sbt "+http-core/compile"` — pass on 2.13.18 and 3.3.8. - Native `scalafmt` on the changed files — clean. ### References None - bounds how much a single HTTP/2 frame can buffer 🤖 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]
