Skip to content

THRIFT-5310: Harden Ruby binary protocol integer bounds#3502

Open
kpumuk wants to merge 1 commit into
apache:masterfrom
kpumuk:rb-binary-bounds
Open

THRIFT-5310: Harden Ruby binary protocol integer bounds#3502
kpumuk wants to merge 1 commit into
apache:masterfrom
kpumuk:rb-binary-bounds

Conversation

@kpumuk
Copy link
Copy Markdown
Member

@kpumuk kpumuk commented May 20, 2026

Fix Ruby binary protocol integer serialization so signed Thrift integer types are validated consistently before writing.

Previously, Ruby BinaryProtocol could silently wrap or clip out-of-range values, for example:

  • write_byte(255) wrote 0xff, which read back as -1
  • write_i16(40000) succeeded
  • write_i64(2**63) succeeded

This now rejects out-of-range values for byte, i16, i32, and i64 in both the pure Ruby and native accelerated binary protocol implementations. Container sizes and binary/string lengths are also checked before writing as non-negative signed i32 lengths.

Implementation Notes

  • Added explicit signed bounds checks to pure Ruby BinaryProtocol.
  • Added matching checks to binary_protocol_accelerated.c.
  • Kept nil behavior unchanged: nil still raises the existing "nil argument not allowed!" error.
  • Added shared regression specs so the same boundary checks cover both pure Ruby and native accelerated implementations.
  • Avoided Integer#between? in the write hot path. A direct micro-benchmark showed:
  between?     1.948s
  < || >       0.892s

for 20M checks, so the final Ruby implementation uses direct comparisons.

Benchmark Results

Benchmarked serialization with test/rb/benchmarks/protocol_benchmark.rb.

Command:

ruby ../../test/rb/benchmarks/protocol_benchmark.rb \
  --json \
  --large-runs 120 \
  --small-runs 2500000 \
  --scenarios rb-bin-write-large,c-bin-write-large,rb-bin-write-small,c-bin-write-small

Results:

scenario             baseline median  patched median  delta
rb-bin-write-large   21.480146s       20.685707s      -3.70%
c-bin-write-large    11.176904s       10.834723s      -3.06%
rb-bin-write-small   21.915494s       21.641400s      -1.25%
c-bin-write-small    11.784214s       11.387855s      -3.36%

No serialization regression was observed.

  • Did you create an Apache Jira ticket? THRIFT-5310
  • 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.

@mergeable mergeable Bot added the ruby Pull requests that update Ruby code label May 20, 2026
@kpumuk kpumuk force-pushed the rb-binary-bounds branch from 90c0650 to 4b35da5 Compare May 20, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ruby Pull requests that update Ruby code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant