THRIFT-5371: Bind the C++ read budget to the frame that carries the message

[Jens-G]

THRIFT-5371

TFramedTransport::readFrame() knows the exact size of the frame it hands to the protocol, but the budget the protocol is checked against stays at TConfiguration::maxMessageSize for the life of the connection. updateKnownMessageSize() had no caller anywhere in the C++ library: the mechanism was there and entirely unused.

That has two consequences, and this closes both.

The one on the ticket. The budget is a per-connection allowance that only consume() draws down and only readEnd() or flush() restores, so a long run of frames read without either exhausts it and starts refusing frames well within the limit. Sixty-four frames over a transport with room for any single one of them, but not for their sum, ends in TTransportException: MaxMessageSize reached — the symptom reported in 2021, still reproducible on master today.

The other direction. A 68-byte frame could declare a 64 MB field, and readStringBody() would resize the caller's string to 64 MB before the following read discovered there was nothing behind it. A 5-byte frame could likewise declare an 8-million-element list.

Change

Three parts.

• readFrame() resets the budget and then binds it to the frame, after the maxFrameSize check and before the buffer is allocated. The full reset first is not optional: resetConsumedMessageSize() refuses to grow a budget, so a frame larger than its predecessor would be rejected outright, and updateKnownMessageSize() on its own carries the previous frame's consumption forward. The reset belongs here rather than in readEnd() as the reporter suggested, because a oneway call never reaches readEnd().

• TBufferBase::read() checks the budget against the bytes it can deliver rather than the bytes asked for, for a transport that sets the new budgetBoundToBuffer_. read() may always return less than requested, so once the budget is one frame, an ordinary "give me up to N bytes" would otherwise be refused instead of short-read; TransportTest does exactly that with random chunk sizes. This does not loosen the bound the protocol is held to — every allocation-gating check, readStringBody() and the container element counts in all three protocols, calls checkReadBytesAvailable() directly with the size the wire declared and does not come through read().

• TNonblockingServer takes the frame apart itself and hands the payload to a TMemoryBuffer, with an identity transport factory by default, so the protocol reads straight from that buffer and none of the above reaches it. New public TMemoryBuffer::bindMessageSizeToBuffer() binds the budget to what the buffer holds, called at both resetBuffer sites.

Scope

THeaderTransport overrides readFrame() and so is unchanged; binding there is a separate question, since in unframed mode it hands out four bytes at a time.

THRIFT-5464 is related but not closed by this. TBufferBase::read() still checks rather than decrements; what changes is that for a framed transport the budget it checks against is now one frame rather than the whole connection.

Tests

Twelve, written before the change. Five describe traffic that is legitimate today and pass unmodified; three fail unmodified — the field case asserts on the size of the string the protocol was asked to fill, because both outcomes raise TTransportException(END_OF_FILE) and only the allocation tells them apart. The remaining four cover the new TMemoryBuffer entry point.

Against unmodified master the three failures are:

test_declared_field_larger_than_frame_is_rejected      check str.size() < 1024u failed [67108864 >= 1024]
test_declared_container_larger_than_frame_is_rejected  exception TTransportException expected but not raised
test_frame_budget_does_not_accumulate_across_frames    TTransportException: MaxMessageSize reached

With the change, 12 of 12 pass and the full UnitTests suite is 101 cases with the one pre-existing TServerSocketTest/test_bind_to_address failure, which reproduces on unmodified master in the same container.

Same defect elsewhere

Java is THRIFT-6165, c_glib is THRIFT-6166. Separate PRs, one per binding.

🤖 Generated with Claude Code