pgwire: make the frame length ceiling a per-call-site argument - #38809
Draft
jubrad wants to merge 1 commit into
Draft
pgwire: make the frame length ceiling a per-call-site argument#38809jubrad wants to merge 1 commit into
jubrad wants to merge 1 commit into
Conversation
`parse_frame_len` applied one ceiling, `MAX_FRAME_SIZE` (64 MiB), to three call sites whose requirements differ by orders of magnitude. It is sized for bulk `COPY` traffic, which is the only path that needs it. The ceiling is now an argument, so each site states the bound it needs and a caller added later cannot inherit 64 MiB by default: * balancerd's startup path takes `MAX_STARTUP_FRAME_SIZE`, 10,000 bytes, matching PostgreSQL's `MAX_STARTUP_PACKET_LENGTH`. * balancerd's `Codec` takes `MAX_PREAUTH_FRAME_SIZE`, 16 KiB. It decodes at most one frame per connection, the credential, before `copy_bidirectional` splices the connection and the rest is proxied unframed. * environmentd's two call sites pass `MAX_FRAME_SIZE`, the value `parse_frame_len` used before, so their behavior is unchanged. Part of: CLO-270 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jubrad
force-pushed
the
justin/clo-270-right-size-pgwire-frame-bounds
branch
from
September 13, 2026 03:24
6e912e4 to
8bdd7ba
Compare
jubrad
changed the title
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.
Issue
parse_frame_lenapplies one ceiling,MAX_FRAME_SIZE(64 MiB), to three call sites whose requirements differ by orders of magnitude. It is sized for bulkCOPYtraffic, which is the only path that needs it. A startup frame is well under 10 KB, and balancerd'sCodeconly ever decodes a credential, so both are bounded far above anything they legitimately carry.Linear: CLO-270
Solution
The ceiling becomes an argument to
parse_frame_len, so each call site states the bound it needs and a caller added later cannot inherit 64 MiB by default.MAX_STARTUP_FRAME_SIZE, 10,000 bytesCodecMAX_PREAUTH_FRAME_SIZE, 16 KiBMAX_FRAME_SIZECodecMAX_FRAME_SIZEScope. This PR tightens balancerd only. Both environmentd call sites pass
MAX_FRAME_SIZE, the exact valueparse_frame_lenused before, so their behavior is unchanged and the diff there is mechanical. Tightening environmentd needs a phase-dependent bound onCodec, since one instance serves a connection both before and after authentication and no single constant suits both, plus an allowance for the parameters balancerd appends when forwarding startup. That is a follow-up PR.10,000 bytes matches PostgreSQL's
MAX_STARTUP_PACKET_LENGTH, so any client that can complete a PostgreSQL handshake can complete this one.16 KiB rather than ~1 KB for balancerd's
Codecbecause an authenticator may accept a bearer token rather than an app password, and those run to several KB. That codec decodes at most one frame per connection, the credential, beforecopy_bidirectionalsplices the connection and the rest is proxied unframed.Testing
New unit tests in
mz-pgwire-commoncover a frame over the budget being rejected before any buffer is sized, a frame at the budget still decoding, and the buffer a declared frame produces being bounded by the budget.A new integration test in
mz-balancerddrives a real listener and asserts an oversized declaration is refused while one at the budget is still served, so the rejection is the budget doing its job rather than the listener refusing everything.Release notes
This release will reject pgwire startup messages larger than 10,000 bytes at the balancer, matching PostgreSQL's limit.
🤖 Generated with Claude Code