Jens-G opened a new pull request, #3764: URL: https://github.com/apache/thrift/pull/3764
[THRIFT-6166](https://issues.apache.org/jira/browse/THRIFT-6166) `thrift_framed_transport_read_frame()` has the exact size of the frame in hand and never tells the budget about it, so a protocol reading an eight-byte frame is still measured against `maxMessageSize` and may be talked into sizing a string body or a container element count from a number the frame comes nowhere near carrying. Neither `updateKnownMessageSize` nor `resetConsumedMessageSize` is called anywhere in the c_glib library outside `thrift_transport.c` itself: the mechanism is there and entirely unused. So the check the protocol makes before allocating — nine call sites across `thrift_binary_protocol.c` and `thrift_compact_protocol.c` — passes. ### Change Two parts. - **`read_frame()` resets the budget and then binds it to what the frame delivered.** The full reset first is not optional: `resetConsumedMessageSize()` refuses to grow a budget, so the frame after a smaller one would be rejected outright. The bind uses `resetConsumedMessageSize()` rather than `updateKnownMessageSize()` for two reasons — nothing has been consumed against the budget that was just reset, so the two are equivalent here, and `updateKnownMessageSize()` would reject a zero-length frame, which `flush()` with an empty write buffer produces and which `testframedtransport` already sends. - **`thrift_framed_transport_read()` checks the budget against the bytes the buffer can deliver** rather than the bytes asked for. One frame is as far as a single read goes and a short read is the documented answer, so once the budget is one frame an ordinary "give me up to N bytes" would otherwise be refused from the second frame onwards. This does not loosen the bound the protocol is held to: every allocation-gating check calls `checkReadBytesAvailable()` directly with the size the wire declared and does not come through `read()`. ### Why this shape and not the C++ one The endpoint decides. `thrift_socket_read()` checks the budget but never decrements it, so a framed transport that answered short from its own buffer instead of binding would hand the shortfall to an endpoint still holding the whole of `maxMessageSize`. Only `thrift_zlib_transport` charges reads, and it is not on this path. Nothing else in the binding needs the same treatment: c_glib ships only `thrift_simple_server`, so unlike C++ there is no server that takes the frame apart itself and hands a memory buffer straight to the protocol. ### Tests Five in a new `testframedreadbudget`, written before the change, driving the framed transport over a memory buffer holding pre-built frames — no sockets, no fork and no timing. Failing against the unmodified library: | | | |---|---| | `BudgetIsBoundToTheFrame` | eight-byte frame, and a request for nine bytes and for a megabyte are both allowed | | `ALargerFrameMayFollowASmallerOne` | the bound must track each frame, not stay at the first one seen | | `ManyFramesDoNotExhaustTheBudget` | sixty-four frames in a row, each bound to its own size | Passing unmodified, as regression guards: | | | |---|---| | `AReadLargerThanTheFrameIsShortNotRefused` | one frame is as far as a single read goes, and asking for more must stay a short read rather than becoming an error | | `AnOversizedFrameIsStillRefused` | the `max_frame_size` check keeps its meaning | Verified against a baseline of the same tree with the fix stashed: 21 of 21 ctest cases before, 22 of 22 after, and the c_glib cross-language client and server complete five loops with zero failures over framed transport on both binary and compact. ### Same defect elsewhere C++ is [THRIFT-5371](https://issues.apache.org/jira/browse/THRIFT-5371), Java is [THRIFT-6165](https://issues.apache.org/jira/browse/THRIFT-6165). Separate PRs, one per binding. 🤖 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]
