wwbmmm commented on code in PR #3169:
URL: https://github.com/apache/brpc/pull/3169#discussion_r2602557553
##########
src/brpc/policy/baidu_rpc_meta.proto:
##########
@@ -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;
Review Comment:
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.
##########
src/brpc/nshead_meta.proto:
##########
@@ -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;
Review Comment:
No need to change nshead protocol.
##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -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
Review Comment:
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`.
--
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]
