diff --git a/lib/rb/lib/thrift/protocol/json_protocol.rb b/lib/rb/lib/thrift/protocol/json_protocol.rb index 23ababf5ca..f87ee3ba80 100644 --- a/lib/rb/lib/thrift/protocol/json_protocol.rb +++ b/lib/rb/lib/thrift/protocol/json_protocol.rb @@ -774,7 +774,6 @@ def read_binary def read_uuid uuid = read_json_string - raise EOFError.new if uuid.length < 36 UUID.validate_uuid!(uuid) uuid.tap(&:downcase!) end diff --git a/lib/rb/spec/server_spec.rb b/lib/rb/spec/server_spec.rb index 76f21a289b..b46bc69f65 100644 --- a/lib/rb/spec/server_spec.rb +++ b/lib/rb/spec/server_spec.rb @@ -133,17 +133,18 @@ def voidMethod expect { @server.serve }.to throw_symbol(:stop) end - { - Thrift::CompactProtocolFactory.new => proc do + [ + ["compact unknown type", Thrift::CompactProtocolFactory.new, proc do trans = Thrift::MemoryBufferTransport.new prot = Thrift::CompactProtocol.new(trans) prot.write_message_begin('unknown', Thrift::MessageTypes::CALL, 1) trans.write([0x1e, 0].pack('C*')) trans.read(trans.available) - end, - Thrift::JsonProtocolFactory.new => proc { '[1,"unknown",1,1,{"1":{"wat":0}}]' } - }.each do |protocol_factory, malformed_request| - it "closes a malformed #{protocol_factory} connection and continues accepting clients" do + end], + ["JSON unknown type", Thrift::JsonProtocolFactory.new, proc { '[1,"unknown",1,1,{"1":{"wat":0}}]' }], + ["short JSON UUID", Thrift::JsonProtocolFactory.new, proc { '[1,"unknown",1,1,{"1":{"uid":"x"}}]' }] + ].each do |failure_type, protocol_factory, malformed_request| + it "closes a malformed #{failure_type} connection and continues accepting clients" do ready = Queue.new errors = Queue.new server_transport = EphemeralServerSocket.new(ready) diff --git a/lib/rb/spec/uuid_validation_spec.rb b/lib/rb/spec/uuid_validation_spec.rb index 83f08b095b..cacb87779d 100644 --- a/lib/rb/spec/uuid_validation_spec.rb +++ b/lib/rb/spec/uuid_validation_spec.rb @@ -21,15 +21,16 @@ require 'spec_helper' describe 'UUID Validation' do - protocols = [ + fixed_width_protocols = [ ['BinaryProtocol', Thrift::BinaryProtocol], ] if defined?(Thrift::BinaryProtocolAccelerated) - protocols << ['BinaryProtocolAccelerated', Thrift::BinaryProtocolAccelerated] + fixed_width_protocols << ['BinaryProtocolAccelerated', Thrift::BinaryProtocolAccelerated] end - protocols << ['CompactProtocol', Thrift::CompactProtocol] + fixed_width_protocols << ['CompactProtocol', Thrift::CompactProtocol] + protocols = fixed_width_protocols.dup protocols << ['JsonProtocol', Thrift::JsonProtocol] protocols.each do |protocol_name, protocol_class| @@ -134,42 +135,6 @@ def expect_invalid_uuid(value, message) end end - context 'malformed binary data on read' do - it 'should raise error on truncated data' do - @trans = Thrift::MemoryBufferTransport.new - @prot = protocol_class.new(@trans) - - # Write only 10 bytes instead of 16 - if protocol_class == Thrift::JsonProtocol - @trans.write('"00000000-0000-0000-0000"') - else - @trans.write("\x00" * 10) - end - - expect { @prot.read_uuid }.to raise_error(EOFError) - end - - it 'should raise error on 15 bytes (one byte short)' do - @trans = Thrift::MemoryBufferTransport.new - @prot = protocol_class.new(@trans) - - if protocol_class == Thrift::JsonProtocol - @trans.write('"00000000-0000-0000-0000-000000000"') - else - @trans.write("\x00" * 15) - end - - expect { @prot.read_uuid }.to raise_error(EOFError) - end - - it 'should raise error on empty buffer' do - @trans = Thrift::MemoryBufferTransport.new - @prot = protocol_class.new(@trans) - - expect { @prot.read_uuid }.to raise_error(EOFError) - end - end - context 'multiple UUIDs in sequence' do it 'should handle 10 UUIDs in sequence' do uuids = 10.times.map { |i| sprintf('%08x-0000-0000-0000-000000000000', i) } @@ -236,4 +201,63 @@ def expect_invalid_uuid(value, message) end end end + + describe 'fixed-width UUID protocols' do + fixed_width_protocols.each do |protocol_name, protocol_class| + describe protocol_name do + [10, 15].each do |available_bytes| + it "raises EOFError when only #{available_bytes} of 16 UUID bytes are available" do + trans = Thrift::MemoryBufferTransport.new("\x00" * available_bytes) + prot = protocol_class.new(trans) + + expect { prot.read_uuid }.to raise_error(EOFError) + end + end + + it 'raises EOFError when no UUID bytes are available' do + trans = Thrift::MemoryBufferTransport.new + prot = protocol_class.new(trans) + + expect { prot.read_uuid }.to raise_error(EOFError) + end + end + end + end + + describe Thrift::JsonProtocol do + it 'raises EOFError when the JSON string is missing its closing quote' do + uuid = '00000000-0000-0000-0000-000000000000' + json_without_closing_quote = '"' + uuid + trans = Thrift::MemoryBufferTransport.new(json_without_closing_quote) + prot = described_class.new(trans) + + expect { prot.read_uuid }.to raise_error(EOFError) + end + + it 'raises EOFError when no JSON UUID data is available' do + trans = Thrift::MemoryBufferTransport.new + prot = described_class.new(trans) + + expect { prot.read_uuid }.to raise_error(EOFError) + end + + context 'with a complete malformed UUID string' do + [ + '00000000-0000-0000-0000', + '00000000-0000-0000-0000-000000000' + ].each do |uuid| + it "rejects a #{uuid.length}-character UUID" do + trans = Thrift::MemoryBufferTransport.new("\"#{uuid}\"") + prot = described_class.new(trans) + + expect { prot.read_uuid }.to raise_error( + Thrift::ProtocolException, + 'Invalid UUID format' + ) do |error| + expect(error.type).to eq(Thrift::ProtocolException::INVALID_DATA) + end + end + end + end + end end