Copilot commented on code in PR #3604:
URL: https://github.com/apache/brpc/pull/3604#discussion_r4332672028
##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -709,9 +709,10 @@ void ProcessRpcRequest(InputMessageBase* msg_base) {
const int req_size = static_cast<int>(msg->payload.size());
if (meta.has_attachment_size()) {
- if (req_size < meta.attachment_size()) {
+ if (meta.attachment_size() < 0 ||
+ req_size < meta.attachment_size()) {
cntl->SetFailed(EREQUEST,
- "attachment_size=%d is larger than request_size=%d",
+ "Invalid attachment_size=%d (request_size=%d)",
Review Comment:
The new error string collapses two distinct failure modes (negative vs
larger-than-payload) into a generic “Invalid”, which makes diagnosing
peer/protocol issues harder. Consider using separate messages (or include the
reason) so logs clearly indicate whether the value was negative or exceeded the
payload size.
##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -1018,9 +1019,10 @@ void ProcessRpcResponse(InputMessageBase* msg_base) {
const int res_size = msg->payload.length();
butil::IOBuf* res_buf_ptr = &msg->payload;
if (meta.has_attachment_size()) {
- if (meta.attachment_size() > res_size) {
+ if (meta.attachment_size() < 0 ||
+ meta.attachment_size() > res_size) {
cntl->SetFailed(
- ERESPONSE, "attachment_size=%d is larger than
response_size=%d",
+ ERESPONSE, "Invalid attachment_size=%d (response_size=%d)",
meta.attachment_size(), res_size);
Review Comment:
`meta.attachment_size()` is evaluated multiple times in the condition and
again in the error message. Capturing it once into a local (e.g., `const
int32_t attachment_size = meta.attachment_size();`) improves readability and
ensures the same value is used consistently across checks and logging.
--
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]
