THRIFT-417. rb: BufferedTransport can enter an infinite loop
Switch native proto implementations to use read_all instead of read. Add a bunch of tests to verify.
Also:
- removed some commented code in binary_protocol_accelerated.c
- struct.c was missing one of the possible native method calls
- updates gem manifest (included files that didn't exist)
- fixed svn:ignores of test/rb/gen-rb and lib/java/gen-java
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@761037 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/rb/Manifest b/lib/rb/Manifest
index eacaaee..17439ed 100644
--- a/lib/rb/Manifest
+++ b/lib/rb/Manifest
@@ -7,7 +7,6 @@
benchmark/server.rb
benchmark/thin_server.rb
CHANGELOG
-COPYING
ext/binary_protocol_accelerated.c
ext/binary_protocol_accelerated.h
ext/compact_protocol.c
@@ -43,7 +42,6 @@
lib/thrift/transport.rb
lib/thrift/types.rb
lib/thrift.rb
-LICENSE
Manifest
README
spec/binaryprotocol_spec.rb
diff --git a/lib/rb/ext/binary_protocol_accelerated.c b/lib/rb/ext/binary_protocol_accelerated.c
index 751281e..728a057 100644
--- a/lib/rb/ext/binary_protocol_accelerated.c
+++ b/lib/rb/ext/binary_protocol_accelerated.c
@@ -336,7 +336,7 @@
VALUE rb_thrift_binary_proto_read_bool(VALUE self) {
char byte = read_byte_direct(self);
- return byte == 1 ? Qtrue : Qfalse;
+ return byte != 0 ? Qtrue : Qfalse;
}
VALUE rb_thrift_binary_proto_read_byte(VALUE self) {
diff --git a/lib/rb/ext/constants.h b/lib/rb/ext/constants.h
index 2b18081..57df544 100644
--- a/lib/rb/ext/constants.h
+++ b/lib/rb/ext/constants.h
@@ -73,7 +73,7 @@
extern ID write_field_stop_method_id;
extern ID skip_method_id;
extern ID write_method_id;
-extern ID read_method_id;
+extern ID read_all_method_id;
extern ID native_qmark_method_id;
extern ID fields_const_id;
diff --git a/lib/rb/ext/macros.h b/lib/rb/ext/macros.h
index 300b1cc..265f693 100644
--- a/lib/rb/ext/macros.h
+++ b/lib/rb/ext/macros.h
@@ -22,7 +22,7 @@
#define GET_STRICT_WRITE(obj) rb_ivar_get(obj, strict_write_ivar_id)
#define WRITE(obj, data, length) rb_funcall(obj, write_method_id, 1, rb_str_new(data, length))
#define CHECK_NIL(obj) if (NIL_P(obj)) { rb_raise(rb_eStandardError, "nil argument not allowed!");}
-#define READ(obj, length) rb_funcall(GET_TRANSPORT(obj), read_method_id, 1, INT2FIX(length))
+#define READ(obj, length) rb_funcall(GET_TRANSPORT(obj), read_all_method_id, 1, INT2FIX(length))
#ifndef RFLOAT_VALUE
# define RFLOAT_VALUE(v) RFLOAT(rb_Float(v))->value
diff --git a/lib/rb/ext/struct.c b/lib/rb/ext/struct.c
index 8dd5c3e..fee285e 100644
--- a/lib/rb/ext/struct.c
+++ b/lib/rb/ext/struct.c
@@ -416,11 +416,6 @@
// call validate
rb_funcall(self, validate_method_id, 0);
- // if (rb_funcall(protocol, native_qmark_method_id, 0) == Qtrue) {
- // set_native_proto_function_pointers(protocol);
- // } else {
- // set_default_proto_function_pointers();
- // }
check_native_proto_method_table(protocol);
// write struct begin
@@ -565,7 +560,7 @@
// read each field
while (true) {
- VALUE field_header = rb_funcall(protocol, read_field_begin_method_id, 0);
+ VALUE field_header = mt->read_field_begin(protocol);
VALUE field_type_value = rb_ary_entry(field_header, 1);
int field_type = FIX2INT(field_type_value);
diff --git a/lib/rb/ext/thrift_native.c b/lib/rb/ext/thrift_native.c
index 7e71ad3..effa202 100644
--- a/lib/rb/ext/thrift_native.c
+++ b/lib/rb/ext/thrift_native.c
@@ -87,7 +87,7 @@
ID write_field_stop_method_id;
ID skip_method_id;
ID write_method_id;
-ID read_method_id;
+ID read_all_method_id;
ID native_qmark_method_id;
// constant ids
@@ -169,7 +169,7 @@
write_field_stop_method_id = rb_intern("write_field_stop");
skip_method_id = rb_intern("skip");
write_method_id = rb_intern("write");
- read_method_id = rb_intern("read");
+ read_all_method_id = rb_intern("read_all");
native_qmark_method_id = rb_intern("native?");
// constant ids
diff --git a/lib/rb/spec/binaryprotocol_spec_shared.rb b/lib/rb/spec/binaryprotocol_spec_shared.rb
index 90371b3..18ea8e8 100644
--- a/lib/rb/spec/binaryprotocol_spec_shared.rb
+++ b/lib/rb/spec/binaryprotocol_spec_shared.rb
@@ -299,4 +299,77 @@
@trans.write([str.size].pack("N") + str)
@prot.read_string.should == str
end
+
+ it "should perform a complete rpc with no args or return" do
+ srv_test(
+ proc {|client| client.send_voidMethod()},
+ proc {|client| client.recv_voidMethod.should == nil}
+ )
+ end
+
+ it "should perform a complete rpc with a primitive return type" do
+ srv_test(
+ proc {|client| client.send_primitiveMethod()},
+ proc {|client| client.recv_primitiveMethod.should == 1}
+ )
+ end
+
+ it "should perform a complete rpc with a struct return type" do
+ srv_test(
+ proc {|client| client.send_structMethod()},
+ proc {|client|
+ result = client.recv_structMethod
+ result.set_byte_map = nil
+ result.map_byte_map = nil
+ result.should == Fixtures::COMPACT_PROTOCOL_TEST_STRUCT
+ }
+ )
+ end
+
+ def get_socket_connection
+ server = Thrift::ServerSocket.new("localhost", 9090)
+ server.listen
+
+ clientside = Thrift::Socket.new("localhost", 9090)
+ clientside.open
+ serverside = server.accept
+ [clientside, serverside, server]
+ end
+
+ def srv_test(firstblock, secondblock)
+ clientside, serverside, server = get_socket_connection
+
+ clientproto = protocol_class.new(clientside)
+ serverproto = protocol_class.new(serverside)
+
+ processor = Srv::Processor.new(SrvHandler.new)
+
+ client = Srv::Client.new(clientproto, clientproto)
+
+ # first block
+ firstblock.call(client)
+
+ processor.process(serverproto, serverproto)
+
+ # second block
+ secondblock.call(client)
+ ensure
+ clientside.close
+ serverside.close
+ server.close
+ end
+
+ class SrvHandler
+ def voidMethod()
+ end
+
+ def primitiveMethod
+ 1
+ end
+
+ def structMethod
+ Fixtures::COMPACT_PROTOCOL_TEST_STRUCT
+ end
+ end
+
end
diff --git a/lib/rb/spec/spec_helper.rb b/lib/rb/spec/spec_helper.rb
index 19db02a..d099491 100644
--- a/lib/rb/spec/spec_helper.rb
+++ b/lib/rb/spec/spec_helper.rb
@@ -48,4 +48,6 @@
module Fixtures
COMPACT_PROTOCOL_TEST_STRUCT = CompactProtoTestStruct.new(:a_binary => [0,1,2,3,4,5,6,7,8].pack('c*'))
+ COMPACT_PROTOCOL_TEST_STRUCT.set_byte_map = nil
+ COMPACT_PROTOCOL_TEST_STRUCT.map_byte_map = nil
end
\ No newline at end of file