thrift
thrift copied to clipboard
THRIFT-5909: Raise an error when nil is sent to binary protocol write operations
Currently, when accelerated binary protocol is used, and nil is passed to write operations like write_byte, write_i16, write_i32, write_i64, write_double, write_string, or write_binary, the code raises a StandardError with 'nil argument not allowed!' message. Ruby version (non-accelerated) instead raises random exceptions like NoMethodError or TypeError. This behavior is inconsistent with the expectation that a StandardError with the message 'nil argument not allowed!' should be raised.
Specs passed before because any exception was considered ok, even if it was not graceful.
1) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil byte
Failure/Error: expect { @prot.write_byte(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<NoMethodError: undefined method '<' for nil> with backtrace:
# ./lib/thrift/protocol/binary_protocol.rb:85:in 'Thrift::BinaryProtocol#write_byte'
# ./spec/binary_protocol_spec_shared.rb:125:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:125:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:125:in 'block (2 levels) in <top (required)>'
2) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil i16
Failure/Error: expect { @prot.write_i16(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<TypeError: no implicit conversion of nil into Integer> with backtrace:
# ./lib/thrift/protocol/binary_protocol.rb:90:in 'Thrift::BinaryProtocol#write_i16'
# ./spec/binary_protocol_spec_shared.rb:144:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:144:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:144:in 'block (2 levels) in <top (required)>'
3) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil i32
Failure/Error: expect { @prot.write_i32(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<NoMethodError: undefined method '<' for nil> with backtrace:
# ./lib/thrift/protocol/binary_protocol.rb:94:in 'Thrift::BinaryProtocol#write_i32'
# ./spec/binary_protocol_spec_shared.rb:161:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:161:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:161:in 'block (2 levels) in <top (required)>'
4) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil i64
Failure/Error: expect { @prot.write_i64(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<NoMethodError: undefined method '<' for nil> with backtrace:
# ./lib/thrift/protocol/binary_protocol.rb:99:in 'Thrift::BinaryProtocol#write_i64'
# ./spec/binary_protocol_spec_shared.rb:184:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:184:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:184:in 'block (2 levels) in <top (required)>'
5) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil double
Failure/Error: expect { @prot.write_double(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<TypeError: can't convert nil into Float> with backtrace:
# ./lib/thrift/protocol/binary_protocol.rb:106:in 'Thrift::BinaryProtocol#write_double'
# ./spec/binary_protocol_spec_shared.rb:197:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:197:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:197:in 'block (2 levels) in <top (required)>'
6) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil string
Failure/Error: expect { @prot.write_string(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<NoMethodError: undefined method 'encoding' for nil> with backtrace:
# ./lib/thrift/bytes.rb:79:in 'Thrift::Bytes.convert_to_utf8_byte_buffer'
# ./lib/thrift/protocol/binary_protocol.rb:110:in 'Thrift::BinaryProtocol#write_string'
# ./spec/binary_protocol_spec_shared.rb:250:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:250:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:250:in 'block (2 levels) in <top (required)>'
7) BinaryProtocol it should behave like a binary protocol should error gracefully when trying to write a nil binary
Failure/Error: expect { @prot.write_binary(nil) }.to raise_error(StandardError, 'nil argument not allowed!')
expected StandardError with "nil argument not allowed!", got #<NoMethodError: undefined method 'bytesize' for nil> with backtrace:
# ./lib/thrift/protocol/binary_protocol.rb:115:in 'Thrift::BinaryProtocol#write_binary'
# ./spec/binary_protocol_spec_shared.rb:254:in 'block (3 levels) in <top (required)>'
# ./spec/binary_protocol_spec_shared.rb:254:in 'block (2 levels) in <top (required)>'
Shared Example Group: "a binary protocol" called from ./spec/binary_protocol_spec.rb:25
# ./spec/binary_protocol_spec_shared.rb:254:in 'block (2 levels) in <top (required)>'
- [x] Did you create an Apache Jira ticket? THRIFT-5909
- [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [x] Did you squash your changes to a single commit? (not required, but preferred)
- [x] 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.