THRIFT-671. rb: Ruby compact protocol implementation gets mixed up when there are fields that don't fit in the delta space
This patch adds a test and a fix for the problem.
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@896712 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/rb/ext/compact_protocol.c b/lib/rb/ext/compact_protocol.c
index 33a1f9f..768b2e5 100644
--- a/lib/rb/ext/compact_protocol.c
+++ b/lib/rb/ext/compact_protocol.c
@@ -458,6 +458,7 @@
if (modifier == 0) {
// not a delta. look ahead for the zigzag varint field id.
+ LAST_ID(self);
field_id = read_i16(self);
} else {
// has a delta. add the delta to the last read field id.
diff --git a/lib/rb/lib/thrift/protocol/compact_protocol.rb b/lib/rb/lib/thrift/protocol/compact_protocol.rb
index c8f4365..3be2a90 100644
--- a/lib/rb/lib/thrift/protocol/compact_protocol.rb
+++ b/lib/rb/lib/thrift/protocol/compact_protocol.rb
@@ -252,6 +252,7 @@
modifier = (type & 0xf0) >> 4
if modifier == 0
# not a delta. look ahead for the zigzag varint field id.
+ @last_field.pop
field_id = read_i16()
else
# has a delta. add the delta to the last read field id.
diff --git a/lib/rb/spec/compact_protocol_spec.rb b/lib/rb/spec/compact_protocol_spec.rb
index b9a7981..1fa2182 100644
--- a/lib/rb/spec/compact_protocol_spec.rb
+++ b/lib/rb/spec/compact_protocol_spec.rb
@@ -86,12 +86,12 @@
it "should make method calls correctly" do
client_out_trans = Thrift::MemoryBufferTransport.new
client_out_proto = Thrift::CompactProtocol.new(client_out_trans)
-
+
client_in_trans = Thrift::MemoryBufferTransport.new
client_in_proto = Thrift::CompactProtocol.new(client_in_trans)
-
+
processor = Srv::Processor.new(JankyHandler.new)
-
+
client = Srv::Client.new(client_in_proto, client_out_proto)
client.send_Janky(1)
# puts client_out_trans.inspect_buffer
@@ -99,6 +99,22 @@
client.recv_Janky.should == 2
end
+ it "should deal with fields following fields that have non-delta ids" do
+ brcp = BreaksRubyCompactProtocol.new(
+ :field1 => "blah",
+ :field2 => BigFieldIdStruct.new(
+ :field1 => "string1",
+ :field2 => "string2"),
+ :field3 => 3)
+ ser = Thrift::Serializer.new(Thrift::CompactProtocolFactory.new)
+ bytes = ser.serialize(brcp)
+
+ deser = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
+ brcp2 = BreaksRubyCompactProtocol.new
+ deser.deserialize(brcp2, bytes)
+ brcp2.should == brcp
+ end
+
class JankyHandler
def Janky(i32arg)
i32arg * 2
diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift
index 9cdb16d..dbce93e 100644
--- a/test/DebugProtoTest.thrift
+++ b/test/DebugProtoTest.thrift
@@ -327,3 +327,14 @@
struct StructWithASomemap {
1: required SomeMap somemap_field;
}
+
+struct BigFieldIdStruct {
+ 1: string field1;
+ 45: string field2;
+}
+
+struct BreaksRubyCompactProtocol {
+ 1: string field1;
+ 2: BigFieldIdStruct field2;
+ 3: i32 field3;
+}
\ No newline at end of file