Copilot commented on code in PR #3669:
URL: https://github.com/apache/brpc/pull/3669#discussion_r4386253624
##########
src/brpc/policy/http_rpc_protocol.cpp:
##########
@@ -420,8 +421,15 @@ void ProcessHttpResponse(InputMessageBase* msg) {
}
const std::string* grpc_status =
res_header->GetHeader(common->GRPC_STATUS);
if (grpc_status) {
- // TODO: More strict parsing
- GrpcStatus status = (GrpcStatus)strtol(grpc_status->data(),
nullptr, 10);
+ int code = 0;
+ if (!butil::StringToInt(*grpc_status, &code)) {
+ cntl->SetFailed(ERESPONSE, "Invalid grpc-status header");
+ break;
+ }
+ const GrpcStatus status =
+ code >= GRPC_OK && code < GRPC_MAX
+ ? static_cast<GrpcStatus>(code)
+ : GRPC_UNKNOWN;
Review Comment:
The PR description promises `ERESPONSE` for values outside 0–16, but this
fallback maps values such as `"17"` and `"256"` to `GRPC_UNKNOWN`, which
becomes `EINTERNAL`. The expectations in
`test/brpc_http_rpc_protocol_unittest.cpp:665–667` also enforce that different
result. Reject out-of-range values with `ERESPONSE` and update those tests, or
correct the description if UNKNOWN mapping is intentional.
##########
src/brpc/policy/http_rpc_protocol.cpp:
##########
@@ -420,8 +421,15 @@ void ProcessHttpResponse(InputMessageBase* msg) {
}
const std::string* grpc_status =
res_header->GetHeader(common->GRPC_STATUS);
if (grpc_status) {
- // TODO: More strict parsing
- GrpcStatus status = (GrpcStatus)strtol(grpc_status->data(),
nullptr, 10);
+ int code = 0;
+ if (!butil::StringToInt(*grpc_status, &code)) {
Review Comment:
`butil::StringToInt` accepts leading signs, so both `"+0"` and `"-0"` parse
as zero and still report RPC success despite the digits-only requirement.
Reject non-ASCII-digit characters before conversion. Update the `"+0"`
expectation in `test/brpc_http_rpc_protocol_unittest.cpp:663` to `ERESPONSE`
and cover `"-0"` as well.
--
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]
