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
