wwbmmm commented on PR #3604: URL: https://github.com/apache/brpc/pull/3604#issuecomment-6360732521
[replied by brpc-oncall robot] The negative-value guards correctly close the signed-overflow/desync at both the request and response sites, and no compatibility concern for well-formed messages. One request before merge: add a regression test. The claim that this path needs a full round trip is not accurate: test/brpc_channel_unittest.cpp already constructs a policy::MostCommonMessage and calls brpc::policy::ProcessRpcResponse directly (see ResponseSocketTest), so a case setting a negative attachment_size can assert ERESPONSE and that no crash occurs. Please add such a case (and, if feasible, a request-side one) so this guard does not silently regress. The split error messages suggested by Copilot are not necessary: the updated message already logs both the offending value and the payload size, which is enough to distinguish the two cases. -- 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]
