Conversation
`validate_rpc_limits` compared `src.len()` with `max_transmit_size`, but `FramedRead` hands the decoder everything it has read so far, which can already include the start of the next frame. Two RPCs that are both under the limit could therefore be rejected, which closes the inbound stream. Read the varint length prefix and compare that instead, so an oversized frame is still rejected before it is fully buffered.
Contributor
|
This pull request has merge conflicts. Could you please resolve them @kriss39? 🙏 |
This branch has not been deployed
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.
Description
validate_rpc_limits(added in #6491) comparesbuf.len()withmax_transmit_size. The decoder gets everythingFramedReadhas read so far, andFramedReadreads in 8 KiB chunks, so the buffer can already contain the beginning of the next frame. If a frame close to the limit is followed by another one, the check fails even though both are within the limit. The inbound stream then goes toClosing, the buffered frames are lost, and after a few of these the handler disables the protocol for the connection.This reads the varint length prefix of the current frame and checks that against the limit. An oversized frame is still rejected as soon as its prefix arrives, so the goal of #6491 is kept.
Two tests are added: three RPCs of 60 000 / 64 000 / 60 000 bytes decoded through
FramedReadwith the default 64 KiB limit (fails on master withmessage with 71054b exceeds maximum of 65536b), and a partially received oversized frame that must still be rejected.Notes & open questions
A frame whose payload is exactly
max_transmit_sizewas also rejected before because the length prefix was counted; it is accepted now.Change checklist