You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by br...@apache.org on 2011/09/20 20:45:56 UTC

svn commit: r1173300 - in /thrift/trunk: lib/rb/ext/struct.c lib/rb/lib/thrift/struct_union.rb lib/rb/spec/compact_protocol_spec.rb test/DebugProtoTest.thrift

Author: bryanduxbury
Date: Tue Sep 20 18:45:56 2011
New Revision: 1173300

URL: http://svn.apache.org/viewvc?rev=1173300&view=rev
Log:
THRIFT-1331. rb: Ruby library deserializes an empty map to nil

This patch properly detects when the metadata is omitted in Compact Protocol messages.

Patch: Armaan Sarkar

Modified:
    thrift/trunk/lib/rb/ext/struct.c
    thrift/trunk/lib/rb/lib/thrift/struct_union.rb
    thrift/trunk/lib/rb/spec/compact_protocol_spec.rb
    thrift/trunk/test/DebugProtoTest.thrift

Modified: thrift/trunk/lib/rb/ext/struct.c
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/rb/ext/struct.c?rev=1173300&r1=1173299&r2=1173300&view=diff
==============================================================================
--- thrift/trunk/lib/rb/ext/struct.c (original)
+++ thrift/trunk/lib/rb/ext/struct.c Tue Sep 20 18:45:56 2011
@@ -484,7 +484,7 @@ static VALUE read_anything(VALUE protoco
     if (!NIL_P(key_info) && !NIL_P(value_info)) {
       int specified_key_type = FIX2INT(rb_hash_aref(key_info, type_sym));
       int specified_value_type = FIX2INT(rb_hash_aref(value_info, type_sym));
-      if (specified_key_type == key_ttype && specified_value_type == value_ttype) {
+      if (num_entries == 0 || (specified_key_type == key_ttype && specified_value_type == value_ttype)) {
         result = rb_hash_new();
 
         for (i = 0; i < num_entries; ++i) {

Modified: thrift/trunk/lib/rb/lib/thrift/struct_union.rb
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/rb/lib/thrift/struct_union.rb?rev=1173300&r1=1173299&r2=1173300&view=diff
==============================================================================
--- thrift/trunk/lib/rb/lib/thrift/struct_union.rb (original)
+++ thrift/trunk/lib/rb/lib/thrift/struct_union.rb Tue Sep 20 18:45:56 2011
@@ -56,7 +56,7 @@ module Thrift
       when Types::MAP
         key_type, val_type, size = iprot.read_map_begin
         # Skip the map contents if the declared key or value types don't match the expected ones.
-        if (key_type != field[:key][:type] || val_type != field[:value][:type])
+        if (size != 0 && (key_type != field[:key][:type] || val_type != field[:value][:type]))
           size.times do
             iprot.skip(key_type)
             iprot.skip(val_type)

Modified: thrift/trunk/lib/rb/spec/compact_protocol_spec.rb
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/rb/spec/compact_protocol_spec.rb?rev=1173300&r1=1173299&r2=1173300&view=diff
==============================================================================
--- thrift/trunk/lib/rb/spec/compact_protocol_spec.rb (original)
+++ thrift/trunk/lib/rb/spec/compact_protocol_spec.rb Tue Sep 20 18:45:56 2011
@@ -115,6 +115,17 @@ describe Thrift::CompactProtocol do
     brcp2.should == brcp
   end
   
+  it "should deserialize an empty map to an empty hash" do
+    struct = SingleMapTestStruct.new(:i32_map => {})
+    ser = Thrift::Serializer.new(Thrift::CompactProtocolFactory.new)
+    bytes = ser.serialize(struct)
+
+    deser = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new)
+    struct2 = SingleMapTestStruct.new
+    deser.deserialize(struct2, bytes)
+    struct.should == struct2
+  end
+  
   class JankyHandler
     def Janky(i32arg)
       i32arg * 2

Modified: thrift/trunk/test/DebugProtoTest.thrift
URL: http://svn.apache.org/viewvc/thrift/trunk/test/DebugProtoTest.thrift?rev=1173300&r1=1173299&r2=1173300&view=diff
==============================================================================
--- thrift/trunk/test/DebugProtoTest.thrift (original)
+++ thrift/trunk/test/DebugProtoTest.thrift Tue Sep 20 18:45:56 2011
@@ -161,6 +161,10 @@ struct CompactProtoTestStruct {
   49: map<byte, list<byte>>       byte_list_map;
 }
 
+// To be used to test the serialization of an empty map
+struct SingleMapTestStruct {
+  1: required map<i32, i32>       i32_map;
+}
 
 const CompactProtoTestStruct COMPACT_TEST = {
   'a_byte'             : 127,