Author: bryanduxbury
Date: Wed Jan  6 23:12:09 2010
New Revision: 896712

URL: http://svn.apache.org/viewvc?rev=896712&view=rev
Log:
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.

Modified:
    incubator/thrift/trunk/lib/rb/ext/compact_protocol.c
    incubator/thrift/trunk/lib/rb/lib/thrift/protocol/compact_protocol.rb
    incubator/thrift/trunk/lib/rb/spec/compact_protocol_spec.rb
    incubator/thrift/trunk/test/DebugProtoTest.thrift

Modified: incubator/thrift/trunk/lib/rb/ext/compact_protocol.c
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/ext/compact_protocol.c?rev=896712&r1=896711&r2=896712&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/ext/compact_protocol.c (original)
+++ incubator/thrift/trunk/lib/rb/ext/compact_protocol.c Wed Jan  6 23:12:09 
2010
@@ -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.

Modified: incubator/thrift/trunk/lib/rb/lib/thrift/protocol/compact_protocol.rb
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/lib/thrift/protocol/compact_protocol.rb?rev=896712&r1=896711&r2=896712&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/lib/thrift/protocol/compact_protocol.rb 
(original)
+++ incubator/thrift/trunk/lib/rb/lib/thrift/protocol/compact_protocol.rb Wed 
Jan  6 23:12:09 2010
@@ -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.

Modified: incubator/thrift/trunk/lib/rb/spec/compact_protocol_spec.rb
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/spec/compact_protocol_spec.rb?rev=896712&r1=896711&r2=896712&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/spec/compact_protocol_spec.rb (original)
+++ incubator/thrift/trunk/lib/rb/spec/compact_protocol_spec.rb Wed Jan  6 
23:12:09 2010
@@ -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

Modified: incubator/thrift/trunk/test/DebugProtoTest.thrift
URL: 
http://svn.apache.org/viewvc/incubator/thrift/trunk/test/DebugProtoTest.thrift?rev=896712&r1=896711&r2=896712&view=diff
==============================================================================
--- incubator/thrift/trunk/test/DebugProtoTest.thrift (original)
+++ incubator/thrift/trunk/test/DebugProtoTest.thrift Wed Jan  6 23:12:09 2010
@@ -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


Reply via email to