Re: Deserialization failure with ruby [THRIFT-1632]

[Nevo Hed <nh...@aereo.com>]

I have created a Jira bug report for this and submitted a patch (fix tested
with 0.7.0 & svn trunk)

https://issues.apache.org/jira/browse/THRIFT-1632

  -Nevo


$ svn diff
Index: memory_buffer.c
===================================================================
--- memory_buffer.c (revision 1351768)
+++ memory_buffer.c (working copy)
@@ -93,17 +93,13 @@
   int index;
   VALUE buf = GET_BUF(self);

+  index = FIX2INT(rb_ivar_get(self, index_ivar_id));
   while (i < size) {
-    index = FIX2INT(rb_ivar_get(self, index_ivar_id));
     if (index >= RSTRING_LEN(buf)) {
       rb_raise(rb_eEOFError, "Not enough bytes remain in memory buffer");
     }
     char byte = RSTRING_PTR(buf)[index++];

-    if (index >= GARBAGE_BUFFER_SIZE) {
-      rb_ivar_set(self, buf_ivar_id, rb_funcall(buf, slice_method_id, 2,
INT2FIX(index), INT2FIX(RSTRING_LEN(buf) - 1)));
-      index = 0;
-    }
     rb_ivar_set(self, index_ivar_id, INT2FIX(index));

     if (i >= RSTRING_LEN(buffer_value)) {
@@ -112,6 +108,12 @@
     ((char*)RSTRING_PTR(buffer_value))[i] = byte;
     i++;
   }
+  if (index >= GARBAGE_BUFFER_SIZE) {
+    rb_ivar_set(self, buf_ivar_id, rb_funcall(buf, slice_method_id, 2,
INT2FIX(index), INT2FIX(RSTRING_LEN(buf) - 1)));
+    index = 0;
+  }
+  rb_ivar_set(self, index_ivar_id, INT2FIX(index));
+
   return INT2FIX(i);
 }




On Tue, Jun 19, 2012 at 12:38 AM, Nevo Hed <nh...@aereo.com> wrote:

>
> This is tested with thrift 0.7.0, and as it is pretty sensitive (removing
> or changing the type of any of these definitions seems to make the problem
> go away)
> The failures are typically those of validation errors as you see below,
> but other have been seen as well.
>
> Has anyone seen anything like this?
>
> Thanks
>    -Nevo
>
> --8<------------test.thrift-----------------------------------------------
> enum En1 {
>   EN1_0 = 1
> }
>
> struct MapEl {
>   1: required En1 x1 = En1.EN1_0,
>   2: required i32 x2 = 2,
>   7: optional i32 x3 = 3,
>   8: optional i32 x4 = 4,
> }
>
> struct topStruct {
>   1: required i32            y1 = 1,
>   2: required i16            y2 = 2,
>   3: required i32            y3 = 3,
>   4: optional map<i32,MapEl> y4 = {},
> }
>
> --8<------------test.rb----------------------------------------------------
>
> #!/usr/bin/ruby
> require 'thrift'
> require 'gen-rb/test_types'
>
> begin
>   v=TopStruct.new
>
>   for i in 1..124 do
>     v.y4[i]=MapEl.new()
>   end
>
>   ruby_binary_protocol =
> Thrift::BinaryProtocol.new(Thrift::MemoryBufferTransport.new)
>
>   v.write(ruby_binary_protocol)
>   TopStruct.new.read(ruby_binary_protocol)
>   # drop read data on the floor
>
> rescue Exception => tx
>   print "Caught #{tx.class} exception: #{tx.message}\n"
>   print "Trace:\n  ", tx.backtrace.join("\n  "), "\n"
> end
> nil
>
> --8<----------------------------------------------------------------------
>
> $ /usr/bin/thrift --gen rb test.thrift && ruby test.rb
> Caught Thrift::ProtocolException exception: Invalid value of field x1!
> Trace:
>   ./gen-rb/test_types.rb:34:in `validate'
>   test.rb:15:in `read'
>   test.rb:15
>
>