Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-5310

Ruby BinaryProtocol has invalid range checks for byte and i64

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 0.13.0
    • None
    • Ruby - Library
    • None

    Description

      Hello,

      The range checks in https://github.com/apache/thrift/blob/master/lib/rb/lib/thrift/protocol/binary_protocol.rb are wrong.

      Right now, if for example, someone accidentally writes an unsigned byte like 255, it would read as -1 on the other end of the line.

       

      For write_byte, it should be:

       byte < -2**7 || byte >= 2**7 

      For write_i64, it should be:

       i64 < -2**63 || i64 >= 2**63 

      And i just noticed that for write_i16 there is no range check at all.

       

      It is probably also better to put the calculated min/max values into consts. I did a quick benchmark:

                                 user     system      total        real
      with const:            0.526762   0.000562   0.527324 (  0.528134)
      with calculation:      2.384387   0.002364   2.386751 (  2.389461)

      Here is the code:

      require 'benchmark'
      
      MIN_I64 = -2**63
      MAX_I64 = 2**63 - 1
      def check_with_const(i64)
        i64 < MIN_I64 || i64 > MAX_I64
      end
      
      def check_with_calc(i64)
        i64 < -2**63 || i64 >= 2**63
      end
      
      n = 5_000_000
      Benchmark.bm(20) do |x|
        x.report('with const:') { (1..n).each { |i|; check_with_const(i) } }
        x.report('with calculation:') { (1..n).each { |i|; check_with_calc(i) } }
      end
      

       

      If PR's are welcome i'm more than happy to provide one.

       

      Best regards,
      Eric

       

       

      Attachments

        Activity

          People

            Unassigned Unassigned
            Eric Koch Eric-Christian Koch
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated: