rajvarun77 commented on issue #3177:
URL: https://github.com/apache/brpc/issues/3177#issuecomment-4687878377

   I dug into this. Confirming the report: `-max_body_size` is documented as 
"Maximum size of a single message body in **all protocols**," but the HTTP/2 
path never enforces it.
   
   The length-prefixed protocols (`baidu_std`, `hulu`, `sofa`, `nshead`, 
`thrift`, `mongo`, `esp`, `streaming`) each reject early in their parser via 
`body_size > FLAGS_max_body_size` → `PARSE_ERROR_TOO_BIG_DATA`, because the 
wire format carries the total body length up front. `H2StreamContext::OnData` 
has no equivalent check — body bytes accumulate across DATA frames with only 
HTTP/2 flow control as a bound, and flow control is replenished mid-message 
(`WINDOW_UPDATE`), so it acts as backpressure, not a size cap. Note that 
protobuf's own limit is deliberately raised (`SetTotalBytesLimit(INT_MAX)` in 
`protocol.cpp`) on the assumption that `max_body_size` is the real gate — which 
the h2 path skips.
   
   Before opening a PR, I'd like to confirm the intended direction. Proposal: 
maintain a cumulative received-body byte counter in `H2StreamContext::OnData`, 
compare it against `FLAGS_max_body_size`, and fail the stream when exceeded — 
i.e. make h2 honor the existing flag rather than introducing a new one. The one 
nuance: the progressive-read path (`read_body_progressively()`) drains the body 
incrementally and is intended for large/unbounded streaming, so it should be 
exempt (or the cap applied per-drain rather than cumulatively).
   
   Two questions for maintainers:
   1. Is honoring `max_body_size` on the h2 path the desired behavior, or 
should the doc string be narrowed instead?
   2. Is exempting the progressive-read path acceptable?
   
   Happy to send the patch + unittest once the direction is confirmed.
   


-- 
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]

Reply via email to