THRIFT-6149: Avoid decoding skipped Ruby strings

lib/rb/lib/thrift/protocol/base_protocol.rb

@@ -231,6 +231,10 @@ def read_uuid
       raise NotImplementedError
     end
 
+    def skip_string
+      read_string
+    end
+
     # Writes a field based on the field information, field ID and value.
     #
     # field_info - A Hash containing the definition of the field:
@@ -359,7 +363,7 @@ def skip(type, max_depth = 64)
       when Types::DOUBLE
         read_double
       when Types::STRING
-        read_string
+        skip_string
       when Types::UUID
         read_uuid
       when Types::STRUCT

lib/rb/lib/thrift/protocol/binary_protocol.rb

@@ -262,6 +262,10 @@ def read_binary
       end
     end
 
+    def skip_string
+      read_binary
+    end
+
     def read_uuid
       UUID.uuid_from_bytes(trans.read_all(16))
     end

lib/rb/lib/thrift/protocol/compact_protocol.rb

@@ -428,6 +428,10 @@ def read_binary
       trans.read_all(size)
     end
 
+    def skip_string
+      read_binary
+    end
+
     def read_uuid
       UUID.uuid_from_bytes(trans.read_all(16))
     end

lib/rb/lib/thrift/protocol/header_protocol.rb

@@ -265,6 +265,10 @@ def read_binary
       @protocol.read_binary
     end
 
+    def skip_string
+      @protocol.skip_string
+    end
+
     def read_uuid
       @protocol.read_uuid
     end

lib/rb/lib/thrift/protocol/json_protocol.rb

@@ -772,6 +772,10 @@ def read_binary
       read_json_base64
     end
 
+    def skip_string
+      read_string
+    end
+
     def read_uuid
       uuid = read_json_string
       UUID.validate_uuid!(uuid)

lib/rb/lib/thrift/protocol/protocol_decorator.rb

@@ -196,6 +196,10 @@ def read_binary
       @protocol.read_binary
     end
 
+    def skip_string
+      @protocol.skip_string
+    end
+
     def read_uuid
       @protocol.read_uuid
     end

lib/rb/spec/binary_protocol_spec_shared.rb

@@ -255,6 +255,16 @@
     expect(a.unpack('C*')).to eq([0x00, 0x00, 0x00, 0x04, 0x00, 0x01, 0x02, 0x03])
   end
 
+  it 'should skip strings as binary values' do
+    @prot.write_binary('value')
+    expect(@prot).to receive(:read_binary).and_call_original
+    expect(@prot).not_to receive(:read_string)
+
+    @prot.skip(Thrift::Types::STRING)
+
+    expect(@trans.available).to eq(0)
+  end
+
   it 'should write a frozen non-binary string without mutating the input' do
     buffer = "abc \u20AC".encode('UTF-8').freeze
     @prot.write_binary(buffer)

lib/rb/spec/compact_protocol_spec.rb

@@ -66,6 +66,18 @@
     end
   end
 
+  it "should skip strings as binary values" do
+    trans = Thrift::MemoryBufferTransport.new
+    proto = Thrift::CompactProtocol.new(trans)
+    proto.write_binary("value")
+    expect(proto).to receive(:read_binary).and_call_original
+    expect(proto).not_to receive(:read_string)
+
+    proto.skip(Thrift::Types::STRING)
+
+    expect(trans.available).to eq(0)
+  end
+
   it "should encode and decode primitives in fields correctly" do
     TESTS.each_pair do |primitive_type, test_values|
       final_primitive_type = primitive_type == :binary ? :string : primitive_type

lib/rb/spec/header_protocol_spec.rb

@@ -44,6 +44,13 @@
       expect(protocol.trans).to equal(header_trans)
     end
 
+    it "should delegate skipped strings to the selected protocol" do
+      selected_protocol = @protocol.instance_variable_get(:@protocol)
+      expect(selected_protocol).to receive(:skip_string)
+
+      @protocol.skip(Thrift::Types::STRING)
+    end
+
     describe "header management delegation" do
       it "should delegate get_headers" do
         # Write with headers and read back to populate read headers

lib/rb/spec/json_protocol_spec.rb

@@ -583,6 +583,27 @@
       expect(@prot.read_set_end).to eq(nil)
     end
 
+    it "should skip unknown string fields without base64 decoding" do
+      prot = Thrift::JsonProtocol.new(Thrift::MemoryBufferTransport.new('{"2":{"str":"%"}}'))
+      hello = SpecNamespace::Hello.new
+
+      expect {
+        hello.read(prot)
+      }.not_to raise_error
+      expect(hello.greeting).to eq("hello world")
+    end
+
+    it "should skip unknown string fields through protocol decorators" do
+      trans = Thrift::MemoryBufferTransport.new('{"2":{"str":"%"}}')
+      prot = Thrift::MultiplexedProtocol.new(Thrift::JsonProtocol.new(trans), "service")
+      hello = SpecNamespace::Hello.new
+
+      expect {
+        hello.read(prot)
+      }.not_to raise_error
+      expect(hello.greeting).to eq("hello world")
+    end
+
     it "should read bool" do
       @trans.write("0\"\"")
       expect(@prot.read_bool).to eq(false)

lib/rb/spec/protocol_decorator_spec.rb

@@ -30,4 +30,15 @@
     expect(protocol).to receive(:write_message_begin).with('method', Thrift::MessageTypes::CALL, 42)
     decorator.write_message_begin('method', Thrift::MessageTypes::CALL, 42)
   end
+
+  it 'forwards skipped strings to the decorated protocol' do
+    decorator_class = Class.new(Thrift::BaseProtocol) do
+      include Thrift::ProtocolDecorator
+    end
+    protocol = double('Protocol')
+    decorator = decorator_class.new(protocol)
+
+    expect(protocol).to receive(:skip_string)
+    decorator.skip(Thrift::Types::STRING)
+  end
 end