You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Nevo Hed (JIRA)" <ji...@apache.org> on 2012/06/20 03:54:43 UTC

[jira] [Created] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Nevo Hed created THRIFT-1632:
--------------------------------

             Summary: ruby: data corruption in thrift_native implementation of MemoryBufferTransport
                 Key: THRIFT-1632
                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
             Project: Thrift
          Issue Type: Bug
          Components: Ruby - Library
    Affects Versions: 0.8, 0.7, 0.9
         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
            Reporter: Nevo Hed
            Assignee: Nevo Hed


Detected a failure when serializing, then deserializing a specific object
(I think the object needs to be large enough, AND probably must have non zero data at a specific offset)


$ /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


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403646#comment-13403646 ] 

Hudson commented on THRIFT-1632:
--------------------------------

Integrated in Thrift #508 (See [https://builds.apache.org/job/Thrift/508/])
    THRIFT-1632. rb: ruby: data corruption in thrift_native implementation of MemoryBufferTransport

This patch fixes a subtle bug whereby the read buffer was being resized but the method continued to read from the original, unresized buffer but at the wrong location. (Revision 1355198)

     Result = SUCCESS
bryanduxbury : http://svn.apache.org/viewvc/?view=rev&rev=1355198
Files : 
* /thrift/trunk/lib/rb/ext/memory_buffer.c

                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>             Fix For: 0.9
>
>         Attachments: patch, test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403216#comment-13403216 ] 

Bryan Duxbury commented on THRIFT-1632:
---------------------------------------

After staring at this for a bit, I think I've figured out the bug in this function and why your patch solves the issue.

The way that this is supposed to work is that when we've consumed enough of the memory buffer, we reallocate a new one without the used-up space in the front. This is intended to save memory. However, the issue is that when we decide to resize the buffer, we don't reassign the "buf" variable to the new buffer - we've still got a pointer to the old one. But we reset the index pointer as though we have switched, which means that when you start reading again, you'll be getting the wrong data.

Your patch fixes this issue by only doing a garbage resize once after all the reading is done, thus guaranteeing that the buffer pointer and its index remain valid throughout the lifetime of the method. 
                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: patch, test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Nevo Hed (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13403298#comment-13403298 ] 

Nevo Hed commented on THRIFT-1632:
----------------------------------

Bryan,

Thanks for looking at this.  I definitely approached this from an outsider perspective looking at the two versions (C-ruby-lib vs ruby module) and attempted to make the broken one that does not seem to obliterate the data.

So my question is - is my change functionaly different than what is already in the ruby read_into_buffer() method? [lib/rb/lib/thrift/transport/memory_buffer_transport.rb]


                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: patch, test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Nevo Hed (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nevo Hed updated THRIFT-1632:
-----------------------------

    Attachment: test.rb

Updated test.rb to also compare the original and the deserialized objects.  Also increased the number of hash elements for roubustness
                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: patch, test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Nevo Hed (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nevo Hed updated THRIFT-1632:
-----------------------------

    Attachment:     (was: test.rb)
    
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: patch, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Closed] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bryan Duxbury closed THRIFT-1632.
---------------------------------

       Resolution: Fixed
    Fix Version/s: 0.9

I just committed a slightly modified version of this to TRUNK. Thanks for finding the bug and for drafting the original patch, Nevo!
                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>             Fix For: 0.9
>
>         Attachments: patch, test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Nevo Hed (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nevo Hed updated THRIFT-1632:
-----------------------------

    Attachment: test.thrift

Data definition used for test.rb
                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Nevo Hed (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nevo Hed updated THRIFT-1632:
-----------------------------

    Attachment: test.rb

Test showing the failure

/usr/bin/thrift --gen rb test.thrift
ruby test.rb
echo $?


                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (THRIFT-1632) ruby: data corruption in thrift_native implementation of MemoryBufferTransport

Posted by "Nevo Hed (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-1632?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nevo Hed updated THRIFT-1632:
-----------------------------

    Attachment: patch

1) I confirmed this problem to 
   occur with 0.8.0 & trunk from svn

2) I also narrowed it down to 
   rb_thrift_memory_buffer_read_into_buffer
   in lib/rb/ext/memory_buffer.c
   (disable binding this function and the
   test passes, i.e. the ruby
   implementation, rather than the 
   native, is executed)

3) without actually trying to understand
   the actual code, I noticed a difference
   between the ruby & C versions where the
   "index" vs "GARBAGE_BUFFER_SIZE"
   comparison (and what I assume is a
   flush) was outside of the loop in Ruby,
   and inside the loop in "C".


The attached patch makes the C side behave more like the well-behaving ruby side.  It moves the the above mentioned comparison after the loop.  Also note that I moved the  initialization of index outside of the loop to resolve a compiler warning.

                
> ruby: data corruption in thrift_native implementation of MemoryBufferTransport
> ------------------------------------------------------------------------------
>
>                 Key: THRIFT-1632
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1632
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.7, 0.8, 0.9
>         Environment: Tested on Linux/Centos 6.0, with thrift_native.so installed
>            Reporter: Nevo Hed
>            Assignee: Nevo Hed
>              Labels: newbie, patch
>         Attachments: patch, test.rb, test.thrift
>
>
> Detected a failure when serializing, then deserializing a specific object
> (I think the object needs to be large enough, AND probably must have non zero data at a specific offset)
> $ /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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira