Fix SOFA PBRPC parser not limiting metadata size - #3449
Merged
Conversation
ParseSofaMessage only checked body_size against max_body_size, while meta_size and the total frame size were left unbounded. A frame with a large meta_size and zero body_size passed the body_size check and made the connection keep buffering far beyond the configured limit before the invalid metadata was rejected. Bound meta_size by max_body_size as well, consistent with other protocols such as baidu_std and hulu_pbrpc. Add unit tests covering oversized body and oversized metadata.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the SOFA PBRPC protocol parser (src/brpc/policy/sofa_pbrpc_protocol.cpp) to better enforce FLAGS_max_body_size so that a peer can’t cause excessive buffering by declaring a large metadata block.
Changes:
- Add an early rejection path in
ParseSofaMessagewhen declared sizes exceed the configured limit. - Add unit tests that exercise oversized SOFA body and metadata headers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/brpc/policy/sofa_pbrpc_protocol.cpp | Adds additional size validation during SOFA frame parsing to fail early on oversized frames. |
| test/brpc_sofa_pbrpc_protocol_unittest.cpp | Adds helper to craft SOFA headers and new test cases for oversized body/meta. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
What problem does this PR solve?
Issue Number: null
Problem Summary:
The SOFA PBRPC parser (
ParseSofaMessage) enforcesFLAGS_max_body_sizeonly on the frame's
body_sizefield. Themeta_sizefield, andtherefore the whole frame size, is not bounded. A peer can send a frame
with a large
meta_sizeandbody_size = 0, which passes the currentcheck, so the connection keeps reading and buffering the declared frame
until the metadata is finally parsed (and rejected). This makes the
configured per-message limit ineffective and can consume much more memory
than expected.
What is changed and the side effects?
Changed:
ParseSofaMessage, reject frames whosemeta_sizeexceedsFLAGS_max_body_size, in addition to the existingbody_sizecheck.test/brpc_sofa_pbrpc_protocol_unittest.cppcoveringan oversized body and an oversized metadata block.
Side effects:
max_body_size(previously buffered and then rejected at protobufparsing time) are now rejected earlier with a "too big data" error and
the connection is closed.
Check List:
Tests run:
test/brpc_sofa_pbrpc_protocol_unittest(all cases passed)