I have posted it, it is THRIFT-569. This is my first ever patch to an open source project, so I'm not sure I did it right. If there is anything else I need to do, please let me know.
-- Sinan On Thu, Aug 27, 2009 at 2:14 AM, Bryan Duxbury <[email protected]> wrote: > Sinan - This is a great find. Can you open a JIRA ticket (http:// > issues.apache.org/jira/browse/THRIFT) and attach the patch so all the > legal stuff is squared away? Thanks! > > -Bryan > > > On Aug 26, 2009, at 2:13 PM, Sinan Taifour wrote: > > Greetings! >> >> I have noticed that when using the BinaryProtocolAccelerated in Ruby, and >> expecting a map<string,string> and getting something else, a segmentation >> fault occurs. I was able to trace the segmentation fault back to >> binary_protocol_accelerated.c, in r807972, the problem is in >> RSTRING_LEN(str) inside write_string_direct(...). >> >> Notice that, in the Python implementation, or when using BinaryProtocol as >> opposed to BinaryProtocolAccelerated, an exception is raised when a value >> has a type other than string in a map<string, string>. >> >> Below is a patch that solved the problem for me, please advise on whether >> or >> not this would be the right way to solve it (it appears to work properly >> in >> different cases). >> >> Index: lib/rb/ext/binary_protocol_accelerated.c >> =================================================================== >> --- lib/rb/ext/binary_protocol_accelerated.c (revision 807972) >> +++ lib/rb/ext/binary_protocol_accelerated.c (working copy) >> @@ -76,6 +76,9 @@ >> } >> >> static void write_string_direct(VALUE trans, VALUE str) { >> + if (TYPE(str) != T_STRING) { >> + rb_raise(rb_eStandardError, "Value should be a string"); >> + } >> write_i32_direct(trans, RSTRING_LEN(str)); >> rb_funcall(trans, write_method_id, 1, str); >> } >> >> Also, below you can find a set of files that can be used to reproduce the >> segmentation fault mentioned: >> >> test.thrift >> =================================================================== >> service Test { >> i32 c1(1: i32 p1, 2: i32 p2, 3: map<string,string> p3) >> } >> >> server.rb >> =================================================================== >> require 'rubygems' >> require 'thrift' >> require 'extlib' >> CURDIR = File.dirname(__FILE__) >> $: << CURDIR / "gen-rb" >> Dir[CURDIR / "gen-rb" / "**" / "*"].each { |f| require f unless >> File.directory?(f) } >> >> class TestHandler >> def c1(p1, p2, p3) >> p1 + p2 >> end >> end >> >> handler = TestHandler.new() >> processor = Test::Processor.new(handler) >> transport = Thrift::ServerSocket.new(9090) >> transportFactory = Thrift::BufferedTransportFactory.new() >> protocolFactory = Thrift::BinaryProtocolAcceleratedFactory.new >> server = Thrift::ThreadPoolServer.new(processor, transport, >> transportFactory, protocolFactory) >> >> server.serve() >> >> client.rb >> =================================================================== >> require 'rubygems' >> require 'thrift' >> require 'extlib' >> CURDIR = File.dirname(__FILE__) >> $: << CURDIR / "gen-rb" >> Dir[CURDIR / "gen-rb" / "**" / "*"].each { |f| require f unless >> File.directory?(f) } >> >> transport = Thrift::BufferedTransport.new(Thrift::Socket.new('localhost', >> 9090)) >> protocol = Thrift::BinaryProtocolAccelerated.new(transport) >> srv = Test::Client.new(protocol) >> transport.open() >> >> p srv.c1(1,1,{'asfd' => true}) # Causes segmentaion fault without the >> patch, >> raises an error with the patch >> >> >> Thank you! >> >> -- Sinan Taifour >> > >
