Jens-G opened a new pull request, #3803: URL: https://github.com/apache/thrift/pull/3803
[THRIFT-6193](https://issues.apache.org/jira/browse/THRIFT-6193) **Stacked on #3802** (THRIFT-6192) — same file, same README section. Review that one first; this PR's own diff is the last commit. `THttpTransport::readContent()` accumulates the body into `readBuffer_` in a read loop, and nothing measures it against anything. The declared `Content-Length`, and in a chunked body the number of chunks, are numbers the peer chooses. It is wider than one missing check. `countConsumedMessageBytes()` is **never called anywhere in this transport** — a grep over `THttp*.{h,cpp}` finds three `resetConsumedMessageSize()` and one `checkReadBytesAvailable(len)` in `read()`. So `remainingMessageSize_` is reset and never decremented, and that check is a ceiling on a single caller request against an allowance that never depletes. The body does not pass through the line buffer whose growth was bounded by `421283a43`, so that bound does not cover this. ### Measured With `maxMessageSize` at its 100 MB default, against the unmodified library: | scenario | peer served | outcome | VmPeak | VmHWM | |---|---|---|---|---| | `Content-length: -1`, nothing sent after the headers | 39 B | `Could not refill buffer` | **+0 kB** | +92 kB | | `Content-length: -1`, then 200 MB sent | 209715239 B | same | +262228 kB | **+205820 kB** | Nothing is sized from the declared length up front, so a large declaration on its own costs nothing. What is not held is the accumulation: 200 MB arrived in one message against a 100 MB configured maximum, and a chunked body has no declared total at all — it simply runs until the peer stops sending. ### The change `readContent()` charges what it is about to read against `maxMessageSize` and refuses the message once it would exceed it. The running total is reset by `readHeaders()`, so each message on a keep-alive connection gets its own allowance. ### Tests Four cases added to `lib/cpp/test/THttpBufferBoundTest.cpp`. **Two fail against the unmodified library**, and both count the bytes the peer was *asked for* rather than asking whether something threw — an over-declared length runs the stream out and throws either way. 262188 bytes served against a 65536 maximum for the declared case, 262708 for the chunked one. The other two are regression guards: a body inside the maximum still arrives, and a second request on the same connection still gets a full allowance. One thing worth knowing for future tests in this binary: the chunked test builds its chunk sizes through an `ostringstream`, and `ToStringTest.cpp:59` calls `std::locale::global(std::locale("de_DE.UTF-8"))` without restoring it, so a later stream groups digits and 4096 came out as `1.000`. The test imbues `std::locale::classic()`, which is what `TToString.h` does and for the same reason. It only shows up in a full run, since `ToStringTest` is disabled by default. ### Behaviour change A client or server exchanging bodies larger than the configured maximum has to raise it, the same way it would for any other transport. `lib/cpp/README.md` gains a third paragraph under Breaking Changes 0.25.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
