Dmytro Shteflyuk created THRIFT-6143:
----------------------------------------
Summary: Ruby protocol writers should report invalid argument
types consistently
Key: THRIFT-6143
URL: https://issues.apache.org/jira/browse/THRIFT-6143
Project: Thrift
Issue Type: Bug
Components: Ruby - Library
Reporter: Dmytro Shteflyuk
Assignee: Dmytro Shteflyuk
h3. Problem
Ruby Binary and Compact protocol writers report {{nil}} integer arguments with
inconsistent and non-idiomatic exceptions. Depending on the protocol and
whether the native extension is loaded, the same invalid call can raise
{{RuntimeError}}, {{StandardError}}, or {{NoMethodError}}.
The pure-Ruby implementations use either a string-only {{raise}}, which creates
a {{RuntimeError}}, or allow {{nil}} to reach arithmetic. Native writers use a
shared C macro that raises {{StandardError}}. Other non-integer values are
normally reported as {{TypeError}}, while out-of-range integers use
{{RangeError}}.
This is caller argument validation rather than malformed wire data, so
{{Thrift::ProtocolException}} would not be appropriate.
h3. Client impact
Applications that directly wrap or exercise the low-level protocol writers
cannot handle invalid arguments consistently across Binary and Compact
protocols or across native and pure-Ruby installations. Code rescuing
{{TypeError}} for non-integer inputs does not handle {{nil}}, and tests that
expect an exact exception class can change behavior when the native extension
is enabled.
Normal writes with valid generated values are unaffected.
h3. Reproduction
Save the following as {{/tmp/thrift-nil-writer.rb}} and run the commands from
{{lib/rb}}:
{code:ruby}
require "thrift"
protocols = [
["BinaryProtocol", Thrift::BinaryProtocol],
["CompactProtocol", Thrift::CompactProtocol]
]
if defined?(Thrift::BinaryProtocolAccelerated)
protocols << ["BinaryProtocolAccelerated", Thrift::BinaryProtocolAccelerated]
end
protocols.each do |name, protocol_class|
protocol = protocol_class.new(Thrift::MemoryBufferTransport.new)
protocol.write_i16(nil)
rescue Exception => error
puts "#{name} (native=#{protocol.native?}): #{error.class}: #{error.message}"
end
{code}
Run with the native extension:
{code:bash}
ruby -Ilib -Iext /tmp/thrift-nil-writer.rb
{code}
Run without the native extension:
{code:bash}
ruby -Ilib -e '$LOADED_FEATURES <<
File.expand_path("lib/thrift/thrift_native.rb"); load
"/tmp/thrift-nil-writer.rb"'
{code}
Testing on master commit {{9c08ae7ebaee0f4a1a810a338e012ec03a007898}} produces:
{code}
BinaryProtocol (native=false): RuntimeError: nil argument not allowed!
CompactProtocol (native=true): StandardError: nil argument not allowed!
BinaryProtocolAccelerated (native=true): StandardError: nil argument not
allowed!
{code}
Without the native extension:
{code}
BinaryProtocol (native=false): RuntimeError: nil argument not allowed!
CompactProtocol (native=false): NoMethodError: undefined method '<<' for nil
{code}
h3. Expected behavior
BinaryProtocol, BinaryProtocolAccelerated, and CompactProtocol should
consistently raise Ruby {{TypeError}} for {{nil}} and other non-integer writer
arguments in both native and pure-Ruby modes. Integer values outside their
declared Thrift widths should continue to raise {{RangeError}}.
The validation should cover primitive integer writers and integer protocol
metadata such as field IDs, sequence IDs, and container sizes, reject values
before emitting bytes, and assert exact native/pure exception parity.
{{Thrift::ProtocolException}} should remain reserved for malformed wire data
and invalid protocol state.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
