Add byte-count limit to TCompactProtocol varint reader

[Jens-G]

Summary

The varint reader in TCompactProtocol loops until it encounters a non-continuation byte (bit 7 clear), without bounding the number of bytes it will consume. The protobuf wire-format specification, which the compact protocol wire format is based on, defines a maximum of 10 bytes for a 64-bit varint (5 bytes for 32-bit). C++ and Node.js already enforce this limit and raise a protocol exception when it is exceeded.

This change extends that limit to all remaining runtimes:

• Python (lib/py): readVarint() uses a bounded for loop; raises TProtocolException(INVALID_DATA) on overflow.
• Go (lib/go): readVarint64() uses a bounded loop; returns TProtocolException(INVALID_DATA) on overflow.
• PHP (lib/php): readVarint() uses a bounded while loop; throws TProtocolException(INVALID_DATA) on overflow.
• Java (lib/java): slow-path readVarint32() (5-byte limit) and readVarint64() (10-byte limit) both throw TProtocolException(INVALID_DATA) on overflow.
• netstd (lib/netstd): ReadVarInt32Async() and ReadVarInt64Async() use bounded loops; throw TProtocolException(INVALID_DATA) on overflow.
• Delphi (lib/delphi): ReadVarInt32() and ReadVarInt64() use bounded loops; raise TProtocolException(INVALID_DATA) on overflow.
• Haxe (lib/haxe): readVarint32() and readVarint64() use bounded loops; throw TProtocolException(INVALID_DATA) on overflow.
• Lua (lib/lua): readVarint() uses a bounded loop; returns an error on overflow.
• Swift (lib/swift): readVarInt() uses a bounded loop; throws TProtocolError(.invalidData) on overflow.
• Ruby (lib/rb): both the pure-Ruby implementation and the C extension are fixed. read_varint32() gains a separate 5-byte limit; read_varint64() a 10-byte limit. Both raise ProtocolException(INVALID_DATA) on overflow.
• Dart (lib/dart): _readVarInt32() and _readVarInt64() use bounded for loops; throw TProtocolError(INVALID_DATA) on overflow.
• Erlang (lib/erl): read_varint/3 gains a when Count < 10 guard; call sites propagate the error tuple.
• Rust (lib/rs): VarIntReader trait replaced with local bounded loops for both 32-bit (5 bytes) and 64-bit (10 bytes); zigzag decode moved to free functions independent of the integer_encoding crate.

Named constants replace bare integer literals in all runtimes, with a comment explaining the derivation (ceil(64/7) = 10, ceil(32/7) = 5). Java, Ruby, and Rust enforce both the 5-byte (32-bit) and 10-byte (64-bit) limits separately; other runtimes enforce the 10-byte ceiling uniformly for both 32-bit and 64-bit reads.

Test plan

• Python: two new varint limit tests pass
• Go: two new varint limit tests pass
• PHP: existing test suite passes
• Java: existing test suite passes
• netstd: dotnet build clean
• Ruby: bundle exec rspec spec/compact_protocol_spec.rb — 4 new varint limit tests pass (23/23 total); pure-Ruby path exercised (C extension not compiled in Docker)
• Swift: 2 new varint limit tests pass (13/13 total)
• Dart: 2 new varint limit tests added and verified
• Rust: cargo test compact — 75/75 pass including 3 new varint limit tests (rust:latest image, toolchain override required due to root rust-toolchain pinning 1.83.0)

🤖 Generated with Claude Code

[mhlakhani]

nit: instead of 10 everywhere, should we use a named constant or comment in the code? as it stands this is kind of like a magic number and hard to reason about

[Jens-G]

nit: instead of 10 everywhere, should we use a named constant or comment in the code? as it stands this is kind of like a magic number and hard to reason about

Should be covered now.

This only covers pure Ruby implementation, we also have C extension here https://github.com/apache/thrift/blob/master/lib/rb/ext/compact_protocol.c#L442

The description also mentions 5 bytes for 32-bit, which is not covered in the change.

Issue 1 — Ruby C extension missing: lib/rb/ext/compact_protocol.c:read_varint64 had an unbounded while (true) loop, and read_varint32 just delegated to it. Both are now fixed with bounded for loops using rb_exc_raise(get_protocol_exception(PROTOERR_INVALID_DATA, ...)).

Issue 2 — 5-byte limit for 32-bit not enforced: The description claimed ceil(32/7) = 5 but neither Ruby path enforced it. Fixed in both the C extension and pure Ruby — read_varint32 now has its own 5-byte loop instead of delegating.

What changed:

• lib/rb/ext/compact_protocol.c — added MAX_VARINT32_BYTES 5 / MAX_VARINT64_BYTES 10 defines; replaced both reader functions
• lib/rb/lib/thrift/protocol/compact_protocol.rb — added MAX_VARINT32_BYTES = 5; read_varint32 now has its own 5-byte loop
• lib/rb/spec/compact_protocol_spec.rb — 2 new tests for 32-bit overlong rejection and valid 5-byte acceptance (23/23 pass)
• Commit amended and force-pushed; PR description updated to explain that Java, Ruby, and Rust enforce the 5/10 split, while other runtimes use 10 uniformly