wwbmmm commented on code in PR #3325:
URL: https://github.com/apache/brpc/pull/3325#discussion_r3359958859
##########
src/brpc/policy/http2_rpc_protocol.cpp:
##########
@@ -698,9 +698,14 @@ H2ParseResult H2StreamContext::OnContinuation(
H2ParseResult H2Context::OnData(
butil::IOBufBytesIterator& it, const H2FrameHead& frame_head) {
+ const bool has_padding = (frame_head.flags & H2_FLAGS_PADDED);
+ if (frame_head.payload_size < (size_t)has_padding) {
Review Comment:
The logic is not clear to converter a bool to a size_t here
##########
src/brpc/policy/http2_rpc_protocol.cpp:
##########
@@ -698,9 +698,14 @@ H2ParseResult H2StreamContext::OnContinuation(
H2ParseResult H2Context::OnData(
butil::IOBufBytesIterator& it, const H2FrameHead& frame_head) {
+ const bool has_padding = (frame_head.flags & H2_FLAGS_PADDED);
+ if (frame_head.payload_size < (size_t)has_padding) {
+ LOG(ERROR) << "Invalid payload_size=" << frame_head.payload_size;
+ return MakeH2Error(H2_FRAME_SIZE_ERROR);
+ }
uint32_t frag_size = frame_head.payload_size;
uint8_t pad_length = 0;
- if (frame_head.flags & H2_FLAGS_PADDED) {
+ if (has_padding) {
Review Comment:
I think a better way is to add the check code here:
```
if (frag_size <= 0) {
LOG(ERROR) << ...
return ...;
}
```
--
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]
