THRIFT-6142: Enforce Ruby unframed HeaderTransport limits

[kpumuk]

Ruby HeaderTransport enforced max_frame_size for Header and framed clients but passed unframed Binary and Compact reads directly to the underlying transport without applying the configured limit.

This change counts bytes consumed by each unframed protocol message, including the initial protocol signature. Reads are capped at the remaining budget, so partial reads and exact-limit messages remain supported; a later read after the budget is exhausted raises TransportException::SIZE_LIMIT.

Binary and Compact protocols query BaseTransport#message_boundaries? once during construction and cache the result. The base transport returns false, while HeaderTransport returns true, so pure-Ruby and native readers notify HeaderTransport when sequential messages need independent budgets. The normal read path performs one cached boolean check without repeated method lookup, runtime class changes, or per-instance extensions; native readers resolve the transport only inside the enabled branch.

Benchmarks

I compared the commit before this PR (a9663bc) with the current commit (1fff4ca) on Ruby 4.0.6 for aarch64 Linux. The benchmark reads 100,000 minimal message envelopes; each result is the median of 21 warmed trials, with payload construction outside the timed section.

Mode	Reader	Before	After	Change
Native extension loaded	Binary (Ruby reader)	984 ns/message	986 ns/message	+0.3%
Native extension loaded	Compact	645 ns/message	675 ns/message	+4.7%
Native extension loaded	Accelerated Binary	531 ns/message	548 ns/message	+3.2%
Native extension loaded	Header / Binary	6,395 ns/message	6,512 ns/message	+1.8%
Native extension loaded	Header / Compact	5,954 ns/message	6,113 ns/message	+2.7%
Pure Ruby	Binary	2,574 ns/message	2,561 ns/message	-0.5%
Pure Ruby	Compact	1,983 ns/message	1,978 ns/message	-0.2%
Pure Ruby	Header / Binary	6,729 ns/message	6,765 ns/message	+0.5%
Pure Ruby	Header / Compact	6,959 ns/message	7,062 ns/message	+1.5%

On this deliberately small workload, the measurable direct-protocol cost is about 17 ns per message for accelerated Binary and 30 ns per message for native Compact. The pure-Ruby direct readers were within run-to-run noise. Normal RPCs also perform field decoding, transport I/O, and application work, so this benchmark emphasizes the fixed per-message cost.

• Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit? (not required, but preferred)
• Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes a gap in Ruby HeaderTransport where unframed Binary/Compact reads bypassed max_frame_size, adding byte accounting and size-limit enforcement for unframed messages (including protocol signatures) and resetting budgets at message boundaries.

Changes:

• Track and enforce unframed message byte budgets in HeaderTransport, raising TransportException::SIZE_LIMIT before exceeding max_frame_size.
• Reset unframed size budgets at message boundaries by notifying transports from pure-Ruby and native protocol read_message_begin.
• Add specs covering exact-limit acceptance, over-limit rejection, signature handling, partial reads, and sequential message budgeting (incl. accelerated binary).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
lib/rb/spec/header_transport_spec.rb	Adds tests validating unframed size-limit enforcement and per-message budgeting.
lib/rb/lib/thrift/transport/header_transport.rb	Implements unframed byte accounting, limit checks, and message-boundary resets.
lib/rb/lib/thrift/protocol/compact_protocol.rb	Notifies transports of new-message boundaries to reset unframed budgets.
lib/rb/lib/thrift/protocol/binary_protocol.rb	Notifies transports of new-message boundaries to reset unframed budgets.
lib/rb/ext/thrift_native.c	Interns reset_message_size method ID for native protocol boundary notifications.
lib/rb/ext/constants.h	Exposes reset_message_size method ID for native extension usage.
lib/rb/ext/compact_protocol.c	Calls reset_message_size at message start in native compact reader.
lib/rb/ext/binary_protocol_accelerated.c	Calls reset_message_size at message start in accelerated binary reader.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lib/rb/lib/thrift/transport/header_transport.rb:358

• In handle_unframed, when req_sz > 4 and the configured @max_frame_size is exactly the 4-byte protocol signature, read_unframed(bytes_left) will raise because remaining == 0, causing this read call to raise after consuming first_word from the underlying transport (dropping bytes). Since HeaderTransport#read now supports returning partial unframed data up to the limit, this path should also allow returning just first_word when no budget remains, and defer raising SIZE_LIMIT until a subsequent read attempts to exceed the limit.

      bytes_left = req_sz - 4
      if bytes_left > 0
        rest = read_unframed(bytes_left)
        @read_buffer = StringIO.new(first_word + rest)

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lib/rb/lib/thrift/protocol/binary_protocol.rb:41

• BinaryProtocol#initialize now calls message_boundaries? unconditionally. This is a behavioral API requirement change for transports: any duck-typed transport (or test double) that previously only needed read/write/read_all will now raise NoMethodError unless it also implements message_boundaries?. Since the check is cached once per protocol instance, you can preserve backward compatibility by guarding the call with respond_to? and defaulting to false when absent.

    def initialize(trans, strict_read = true, strict_write = true)
      super(trans)
      @reset_message_size = trans.message_boundaries?
      @strict_read = strict_read

lib/rb/lib/thrift/protocol/compact_protocol.rb:115

• CompactProtocol#initialize now calls message_boundaries? unconditionally. This makes message_boundaries? a required method for any duck-typed transport used with CompactProtocol, which can break existing custom transports and lightweight test doubles. Since you only need this once per instance, guarding with respond_to? retains compatibility while still caching the result.

    def initialize(transport)
      super(transport)
      @reset_message_size = transport.message_boundaries?