Skip to content

fix(protocol): fix attachment size limit issue when size exceeds uint32 #3169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
InJunee wants to merge 7 commits into
base: master
Choose a base branch
from
Open

fix(protocol): fix attachment size limit issue when size exceeds uint32 #3169

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
76f0954
fix(protocol): fix attachment size limit issue when size exceeds uint32
InJune Dec 9, 2025
94ef1ae
fix(protocol): fix attachment size limit issue with backward compatib…
InJune Dec 9, 2025
7ec37a0
fix: correct format specifiers for size_t values
InJune Dec 10, 2025
5b5b623
fix: remove unreachable else branch in rpc_replay
InJune Dec 10, 2025
e27ed16
refactor: remove redundant GetAttachmentSize calls
InJune Dec 11, 2025
a4df271
fix: improve attachment_size validation and fix compilation warnings
InJune Dec 11, 2025
e1fdd37
test: cover baidu extended header and fix unused macro
InJune Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/brpc/nshead_meta.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ message NsheadMeta {

optional int64 correlation_id = 2;
optional int64 log_id = 3;
optional int32 attachment_size = 4;
optional int64 attachment_size = 4;
Copy link
Copy Markdown
Contributor

@wwbmmm wwbmmm Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change nshead protocol.

Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This protobuf field change from int32 to int64 is a breaking change that breaks backward compatibility. Unlike baidu_rpc_meta.proto and rpc_dump.proto which preserve the old int32 field and add a new int64 field for compatibility, this directly changes the field type. This will cause issues when old clients/servers communicate with new ones, as they expect different wire formats for field 4.

Consider adding a new field attachment_size_long (similar to the pattern in other proto files) and keeping the original attachment_size as int32 for backward compatibility.

Suggested change
optional int64 attachment_size = 4;
// For backward compatibility, keep attachment_size as int32.
optional int32 attachment_size = 4;
// Use attachment_size_long for large attachments.
optional int64 attachment_size_long = 10;

Copilot uses AI. Check for mistakes.
optional CompressType compress_type = 5;

optional int64 trace_id = 6;
Expand Down
2 changes: 1 addition & 1 deletion src/brpc/policy/baidu_rpc_meta.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ message RpcMeta {
optional RpcResponseMeta response = 2;
optional int32 compress_type = 3;
optional int64 correlation_id = 4;
optional int32 attachment_size = 5;
optional int64 attachment_size = 5;
Copy link
Copy Markdown
Contributor

@wwbmmm wwbmmm Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot directly change the type of an existing protobuf field. This will cause client and server of different version not compatible.
You could add a new field, like optional int64 attachment_size_long = 13.
If the attachment size not exceed int32, you can use the original attachment_size field for compatibility. If the attachment size exceed int32, you can use the attachment_size_long field.

optional ChunkInfo chunk_info = 6;
optional bytes authentication_data = 7;
optional StreamSettings stream_settings = 8;
Expand Down
53 changes: 36 additions & 17 deletions src/brpc/policy/baidu_rpc_protocol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
// under the License.


#include <cinttypes> // PRId64, PRIu64
#include <cstdint> // UINT32_MAX
#include <google/protobuf/descriptor.h> // MethodDescriptor
#include <google/protobuf/message.h> // Message
#include <google/protobuf/io/zero_copy_stream_impl_lite.h>
Expand Down Expand Up @@ -69,16 +71,29 @@ DECLARE_bool(pb_enum_as_number);
// 5. Not supported: chunk_info

// Pack header into `buf'
inline void PackRpcHeader(char* rpc_header, uint32_t meta_size, int payload_size) {
inline void PackRpcHeader(char* rpc_header, uint32_t meta_size, size_t payload_size) {
uint32_t* dummy = (uint32_t*)rpc_header; // suppress strict-alias warning
*dummy = *(uint32_t*)"PRPC";
butil::RawPacker(rpc_header + 4)
.pack32(meta_size + payload_size)
.pack32(meta_size);
// Check for overflow: meta_size + payload_size must fit in uint32_t
const uint64_t total_size = static_cast<uint64_t>(meta_size) + payload_size;
if (total_size > UINT32_MAX) {
LOG(ERROR) << "Total size (meta_size=" << meta_size
<< " + payload_size=" << payload_size
<< " = " << total_size
<< ") exceeds uint32_t maximum (" << UINT32_MAX << ")";
// Truncate to maximum, but this will cause protocol error
Copy link
Copy Markdown
Contributor

@wwbmmm wwbmmm Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know this will cause protocol error, so what's the meaning of this code?
Maybe you can use pack32(UINT32_MAX) as a flag, then pack64(total_size) after it?
Then you can update the corresponding parsing code in ParseRpcMessage.

butil::RawPacker(rpc_header + 4)
.pack32(UINT32_MAX)
.pack32(meta_size);
} else {
butil::RawPacker(rpc_header + 4)
.pack32(static_cast<uint32_t>(total_size))
.pack32(meta_size);
}
}

static void SerializeRpcHeaderAndMeta(
butil::IOBuf* out, const RpcMeta& meta, int payload_size) {
butil::IOBuf* out, const RpcMeta& meta, size_t payload_size) {
const uint32_t meta_size = GetProtobufByteSize(meta);
if (meta_size <= 244) { // most common cases
char header_and_meta[12 + meta_size];
Expand Down Expand Up @@ -676,12 +691,13 @@ void ProcessRpcRequest(InputMessageBase* msg_base) {
break;
}

const int req_size = static_cast<int>(msg->payload.size());
const size_t req_size = msg->payload.size();
if (meta.has_attachment_size()) {
if (req_size < meta.attachment_size()) {
const int64_t attachment_size = meta.attachment_size();
if (attachment_size < 0 || static_cast<size_t>(attachment_size) > req_size) {
cntl->SetFailed(EREQUEST,
"attachment_size=%d is larger than request_size=%d",
meta.attachment_size(), req_size);
"attachment_size=%" PRId64 " is larger than request_size=%zu",
attachment_size, req_size);
break;
}
}
Expand Down Expand Up @@ -723,9 +739,10 @@ void ProcessRpcRequest(InputMessageBase* msg_base) {
}

messages = BaiduProxyPBMessages::Get();
const int64_t attachment_size = meta.has_attachment_size() ? meta.attachment_size() : 0;
msg->payload.cutn(
&((SerializedRequest*)messages->Request())->serialized_data(),
req_size - meta.attachment_size());
req_size - static_cast<size_t>(attachment_size));
if (!msg->payload.empty()) {
cntl->request_attachment().swap(msg->payload);
}
Expand Down Expand Up @@ -792,9 +809,10 @@ void ProcessRpcRequest(InputMessageBase* msg_base) {
}

butil::IOBuf req_buf;
int body_without_attachment_size = req_size - meta.attachment_size();
const int64_t attachment_size = meta.has_attachment_size() ? meta.attachment_size() : 0;
const size_t body_without_attachment_size = req_size - static_cast<size_t>(attachment_size);
msg->payload.cutn(&req_buf, body_without_attachment_size);
if (meta.attachment_size() > 0) {
if (attachment_size > 0) {
cntl->request_attachment().swap(msg->payload);
}

Expand Down Expand Up @@ -963,16 +981,17 @@ void ProcessRpcResponse(InputMessageBase* msg_base) {
}
// Parse response message iff error code from meta is 0
butil::IOBuf res_buf;
const int res_size = msg->payload.length();
const size_t res_size = msg->payload.length();
butil::IOBuf* res_buf_ptr = &msg->payload;
if (meta.has_attachment_size()) {
if (meta.attachment_size() > res_size) {
const int64_t attachment_size = meta.attachment_size();
if (attachment_size < 0 || static_cast<size_t>(attachment_size) > res_size) {
cntl->SetFailed(
ERESPONSE, "attachment_size=%d is larger than response_size=%d",
meta.attachment_size(), res_size);
ERESPONSE, "attachment_size=%" PRId64 " is larger than response_size=%zu",
attachment_size, res_size);
break;
}
int body_without_attachment_size = res_size - meta.attachment_size();
const size_t body_without_attachment_size = res_size - static_cast<size_t>(attachment_size);
msg->payload.cutn(&res_buf, body_without_attachment_size);
res_buf_ptr = &res_buf;
cntl->response_attachment().swap(msg->payload);
Expand Down
2 changes: 1 addition & 1 deletion src/brpc/rpc_dump.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ message RpcDumpMeta {
optional ProtocolType protocol_type = 5;

// baidu_std, hulu_pbrpc
optional int32 attachment_size = 6;
optional int64 attachment_size = 6;

// baidu_std
optional bytes authentication_data = 7;
Expand Down
Loading