sahvx655-wq commented on code in PR #3591:
URL: https://github.com/apache/brpc/pull/3591#discussion_r4227182951
##########
src/brpc/grpc.cpp:
##########
@@ -169,6 +169,11 @@ void PercentDecode(const std::string& str, std::string*
str_out) {
}
}
+// The gRPC wire format defines TimeoutValue as "a positive integer as ASCII
+// string of at most 8 digits", so this is the largest value a conforming peer
+// can send. See doc/PROTOCOL-HTTP2.md in grpc/grpc.
+static const int64_t MAX_GRPC_TIMEOUT_VALUE = 99999999;
Review Comment:
Kept `static const` to match the rest of the tree - `acceptor.cpp:32`,
`compress.cpp:47`, `controller.cpp:121`, `rpc_dump.cpp:66`, `ts.cpp:134` and
about twenty other file-scope constants under `src/brpc` all use `static const`
with this naming. `static` at namespace scope already gives internal linkage,
so there is no ODR-use hazard here; making this one site `constexpr` would just
make it the odd one out.
##########
test/brpc_grpc_protocol_unittest.cpp:
##########
@@ -233,7 +233,15 @@ TEST_F(GrpcTest, GrpcTimeOut) {
"HHHH", "-1",
"112", "-1",
"H999m", "-1",
- "", "-1"
+ "", "-1",
+ // beyond the 8 digits the gRPC wire format allows; the large ones
+ // overflow the conversion to microseconds
+ "100000000S", "-1",
+ "18446744073710S", "-1",
+ "2562047789H", "-1",
+ "9223372036854775807u", "-1",
+ "9223372036854775807n", "-1",
+ "-1S", "-1"
Review Comment:
Fair, the comment did not cover `-1S`. Reworded to "out of range: more
digits than the gRPC wire format allows, or negative. The large ones overflowed
the conversion to microseconds."
--
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]
