[jira] Reopened: (THRIFT-1047) rb_thrift_memory_buffer_write treats arg as string without check, segfaults if you pass non-string

[Jake Donham (JIRA)]

     [ 
https://issues.apache.org/jira/browse/THRIFT-1047?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jake Donham reopened THRIFT-1047:
---------------------------------


I did a micro benchmark showing that this change is < 0.5% slower. You could 
argue that this is a worst-case test, when all the data is in cache, since the 
benchmark writes the same string over and over.

Here are some test runs

|| | user | system | total | real ||
| without patch | 3.232000 | 0.050000 | 3.282000 | 3.416818 |
| with patch | 3.248000 | 0.050000 | 3.298000 | 3.432130 |

Here is the benchmark code:
{code}
require 'rubygems'
gem 'thrift', '=0.7.0'
require 'benchmark'
require 'thrift'

def run(avg_over, size)
  runs = []
  avg_over.times do
    run = Benchmark.measure do
      trans = Thrift::MemoryBufferTransport.new
      size.times { trans.write "foo" }
    end
    runs << run
  end
  runs.inject {|x,y| x + y} / avg_over
end

Benchmark.benchmark {|x|
  [ run(5, 10000000) ]
}
{code}

Here is the patch:
{code}
--- ext/memory_buffer.c.orig    2011-01-28 15:51:28.000000000 -0800
+++ ext/memory_buffer.c 2011-01-28 15:51:41.000000000 -0800
@@ -32,6 +32,8 @@
 
 VALUE rb_thrift_memory_buffer_write(VALUE self, VALUE str) {
   VALUE buf = GET_BUF(self);
+  if (IMMEDIATE_P(str) || BUILTIN_TYPE(str) != T_STRING)
+    Check_Type(str, T_STRING);
   rb_str_buf_cat(buf, RSTRING_PTR(str), RSTRING_LEN(str));
   return Qnil;
 }
{code}

> rb_thrift_memory_buffer_write treats arg as string without check, segfaults 
> if you pass non-string
> --------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-1047
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1047
>             Project: Thrift
>          Issue Type: Bug
>          Components: Ruby - Library
>    Affects Versions: 0.5
>            Reporter: Jake Donham
>
> I think there should be a call to Check_Type(str, T_STRING) in there
> VALUE rb_thrift_memory_buffer_write(VALUE self, VALUE str) {
>   VALUE buf = GET_BUF(self);
>   rb_str_buf_cat(buf, RSTRING_PTR(str), RSTRING_LEN(str));
>   return Qnil;
> }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.