From 14fe791ef36d84a16abdafd67d87ff85e8780924 Mon Sep 17 00:00:00 2001 From: Kevin Clark Date: Mon, 4 Aug 2008 18:46:19 +0000 Subject: [PATCH] Merge branch 'THRIFT-103' git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@682458 13f79535-47bb-0310-9956-ffa450edef68 --- lib/rb/ext/binaryprotocolaccelerated.c | 25 ++++++--- lib/rb/spec/ThriftSpec.thrift | 14 +++++ lib/rb/spec/binaryprotocol_spec_shared.rb | 29 ++++++++++ lib/rb/spec/binaryprotocolaccelerated_spec.rb | 53 +++++++++++++++++++ lib/rb/spec/gen-rb/ThriftSpec_types.rb | 18 +++++++ 5 files changed, 131 insertions(+), 8 deletions(-) diff --git a/lib/rb/ext/binaryprotocolaccelerated.c b/lib/rb/ext/binaryprotocolaccelerated.c index 6e295ed2..1da7a3de 100644 --- a/lib/rb/ext/binaryprotocolaccelerated.c +++ b/lib/rb/ext/binaryprotocolaccelerated.c @@ -186,7 +186,7 @@ static field_spec* parse_field_spec(VALUE field_data) { spec->type = type; - if (Qnil != name) { + if (!NIL_P(name)) { spec->name = StringValuePtr(name); } else { spec->name = NULL; @@ -317,6 +317,8 @@ static void write_container(VALUE buf, VALUE value, field_spec* spec) { VALUE keys; VALUE key; VALUE val; + + Check_Type(value, T_HASH); keys = rb_funcall(value, keys_id, 0); @@ -347,14 +349,17 @@ static void write_container(VALUE buf, VALUE value, field_spec* spec) { } case T_LIST: { + Check_Type(value, T_ARRAY); + sz = RARRAY(value)->len; write_list_begin(buf, spec->data.element->type, sz); for (i = 0; i < sz; ++i) { + VALUE val = rb_ary_entry(value, i); if (IS_CONTAINER(spec->data.element->type)) { - write_container(buf, rb_ary_entry(value, i), spec->data.element); + write_container(buf, val, spec->data.element); } else { - binary_encoding(buf, rb_ary_entry(value, i), spec->data.element->type); + binary_encoding(buf, val, spec->data.element->type); } } write_list_end(buf); @@ -380,10 +385,11 @@ static void write_container(VALUE buf, VALUE value, field_spec* spec) { write_set_begin(buf, spec->data.element->type, sz); for (i = 0; i < sz; i++) { + VALUE val = rb_ary_entry(items, i); if (IS_CONTAINER(spec->data.element->type)) { - write_container(buf, rb_ary_entry(items, i), spec->data.element); + write_container(buf, val, spec->data.element); } else { - binary_encoding(buf, rb_ary_entry(items, i), spec->data.element->type); + binary_encoding(buf, val, spec->data.element->type); } } @@ -409,7 +415,7 @@ static int encode_field(VALUE fid, VALUE data, VALUE ary) { VALUE value = rb_ivar_get(obj, rb_intern(name_buf)); - if (Qnil == value) { + if (NIL_P(value)) { free_field_spec(spec); return 0; } @@ -471,9 +477,12 @@ static void binary_encoding(VALUE buf, VALUE obj, int type) { write_double(buf, NUM2DBL(obj)); break; - case T_STR: - write_string(buf, StringValuePtr(obj), RSTRING(obj)->len); + case T_STR: { + // make sure to call StringValuePtr before calling RSTRING + char *ptr = StringValuePtr(obj); + write_string(buf, ptr, RSTRING(obj)->len); break; + } case T_STRCT: { // rb_hash_foreach has to take args as a ruby array diff --git a/lib/rb/spec/ThriftSpec.thrift b/lib/rb/spec/ThriftSpec.thrift index 4309829f..022c786b 100644 --- a/lib/rb/spec/ThriftSpec.thrift +++ b/lib/rb/spec/ThriftSpec.thrift @@ -17,6 +17,20 @@ struct BoolStruct { 1: bool yesno = 1 } +struct SimpleList { + 1: list bools, + 2: list bytes, + 3: list i16s, + 4: list i32s, + 5: list i64s, + 6: list doubles, + 7: list strings, + 8: list> maps, + 9: list> lists, + 10: list> sets, + 11: list hellos +} + service NonblockingService { Hello greeting(1:bool english) bool block() diff --git a/lib/rb/spec/binaryprotocol_spec_shared.rb b/lib/rb/spec/binaryprotocol_spec_shared.rb index 9a4af6bc..ced15b42 100644 --- a/lib/rb/spec/binaryprotocol_spec_shared.rb +++ b/lib/rb/spec/binaryprotocol_spec_shared.rb @@ -63,6 +63,11 @@ shared_examples_for 'a binary protocol' do @prot.write_bool(false) end + it "should treat a nil bool as false" do + @prot.should_receive(:write_byte).with(0) + @prot.write_bool(nil) + end + it "should write a byte" do # byte is small enough, let's check -128..127 (-128..127).each do |i| @@ -81,6 +86,10 @@ shared_examples_for 'a binary protocol' do lambda { @prot.write_byte(2**65) }.should raise_error(RangeError) end + it "should error gracefully when trying to write a nil byte" do + lambda { @prot.write_byte(nil) }.should raise_error + end + it "should write an i16" do # try a random scattering of values # include the signed i16 minimum/maximum @@ -101,6 +110,10 @@ shared_examples_for 'a binary protocol' do # lambda { @prot.write_i16(2**65) }.should raise_error(RangeError) end + it "should error gracefully when trying to write a nil i16" do + lambda { @prot.write_i16(nil) }.should raise_error + end + it "should write an i32" do # try a random scattering of values # include the signed i32 minimum/maximum @@ -121,6 +134,10 @@ shared_examples_for 'a binary protocol' do # lambda { @prot.write_i32(2 ** 65 + 5) }.should raise_error(RangeError) end + it "should error gracefully when trying to write a nil i32" do + lambda { @prot.write_i32(nil) }.should raise_error + end + it "should write an i64" do # try a random scattering of values # try the signed i64 minimum/maximum @@ -142,6 +159,10 @@ shared_examples_for 'a binary protocol' do # lambda { @prot.write_i64(2 ** 65 + 5) }.should raise_error(RangeError) end + it "should error gracefully when trying to write a nil i64" do + lambda { @prot.write_i64(nil) }.should raise_error + end + it "should write a double" do # try a random scattering of values, including min/max @trans.should_receive(:write).with([Float::MIN].pack('G')).ordered @@ -157,6 +178,10 @@ shared_examples_for 'a binary protocol' do end end + it "should error gracefully when trying to write a nil double" do + lambda { @prot.write_double(nil) }.should raise_error + end + it "should write a string" do str = "hello world" @prot.should_receive(:write_i32).with(str.length).ordered @@ -164,6 +189,10 @@ shared_examples_for 'a binary protocol' do @prot.write_string(str) end + it "should error gracefully when trying to write a nil string" do + lambda { @prot.write_string(nil) }.should raise_error + end + # message footer is a noop it "should read a field header" do diff --git a/lib/rb/spec/binaryprotocolaccelerated_spec.rb b/lib/rb/spec/binaryprotocolaccelerated_spec.rb index 86101f81..6a3cc393 100644 --- a/lib/rb/spec/binaryprotocolaccelerated_spec.rb +++ b/lib/rb/spec/binaryprotocolaccelerated_spec.rb @@ -80,6 +80,59 @@ class ThriftBinaryProtocolAcceleratedSpec < Spec::ExampleGroup trans = Thrift::MemoryBuffer.new("\v\000\001\000\000\000\fHello\000World!\000") @prot.decode_binary(SpecNamespace::Hello.new, trans).should == SpecNamespace::Hello.new(:greeting => "Hello\000World!") end + + it "should error when encoding a struct with a nil value in a list" do + Thrift.type_checking = false + sl = SpecNamespace::SimpleList + hello = SpecNamespace::Hello + # nil counts as false for bools + # lambda { @prot.encode_binary(sl.new(:bools => [true, false, nil, false])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:bytes => [1, 2, nil, 3])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:i16s => [1, 2, nil, 3])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:i32s => [1, 2, nil, 3])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:i64s => [1, 2, nil, 3])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:doubles => [1.0, 2.0, nil, 3.0])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:strings => ["one", "two", nil, "three"])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:lists => [[1, 2], nil, [3, 4]])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:maps => [{1 => 2}, nil, {3 => 4}])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:sets => [Set.new([1, 2]), nil, Set.new([3, 4])])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:structs => [hello.new, nil, hello.new(:greeting => "hi")])) }.should raise_error + end + + it "should error when encoding a non-nil, non-correctly-typed value in a list" do + Thrift.type_checking = false + sl = SpecNamespace::SimpleList + hello = SpecNamespace::Hello + # bool should accept any value + # lambda { @prot.encode_binary(sl.new(:bools => [true, false, 3])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:bytes => [1, 2, "3", 5])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:i16s => ["one", 2, 3])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:i32s => [[1,2], 3, 4])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:i64s => [{1 => 2}, 3, 4])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:doubles => ["one", 2.3, 3.4])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:strings => ["one", "two", 3, 4])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:lists => [{1 => 2}, [3, 4]])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:maps => [{1 => 2}, [3, 4]])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:sets => [Set.new([1, 2]), 3, 4])) }.should raise_error + lambda { @prot.encode_binary(sl.new(:structs => [3, "four"])) }.should raise_error + end + + it "should error when given nil to encode" do + lambda { @prot.encode_binary(nil) }.should raise_error + end + + it "should error when encoding an improper object where a container is expected" do + Thrift.type_checking = false + sl = SpecNamespace::SimpleList + lambda { @prot.encode_binary(sl.new(:strings => {"one" => "two", nil => "three"})) }.should raise_error + lambda { @prot.encode_binary(sl.new(:maps => [[1, 2]])) }.should raise_error + end + + it "should accept arrays and hashes as sets" do + Thrift.type_checking = false + sl = SpecNamespace::SimpleList + lambda { @prot.encode_binary(sl.new(:sets => [[1, 2], {3 => true, 4 => true}])) }.should_not raise_error + end end describe BinaryProtocolAcceleratedFactory do diff --git a/lib/rb/spec/gen-rb/ThriftSpec_types.rb b/lib/rb/spec/gen-rb/ThriftSpec_types.rb index b98ae7de..24358dad 100644 --- a/lib/rb/spec/gen-rb/ThriftSpec_types.rb +++ b/lib/rb/spec/gen-rb/ThriftSpec_types.rb @@ -46,4 +46,22 @@ module SpecNamespace } end + class SimpleList + include Thrift::Struct + Thrift::Struct.field_accessor self, :bools, :bytes, :i16s, :i32s, :i64s, :doubles, :strings, :maps, :lists, :sets, :hellos + FIELDS = { + 1 => {:type => Thrift::Types::LIST, :name => 'bools', :element => {:type => Thrift::Types::BOOL}}, + 2 => {:type => Thrift::Types::LIST, :name => 'bytes', :element => {:type => Thrift::Types::BYTE}}, + 3 => {:type => Thrift::Types::LIST, :name => 'i16s', :element => {:type => Thrift::Types::I16}}, + 4 => {:type => Thrift::Types::LIST, :name => 'i32s', :element => {:type => Thrift::Types::I32}}, + 5 => {:type => Thrift::Types::LIST, :name => 'i64s', :element => {:type => Thrift::Types::I64}}, + 6 => {:type => Thrift::Types::LIST, :name => 'doubles', :element => {:type => Thrift::Types::DOUBLE}}, + 7 => {:type => Thrift::Types::LIST, :name => 'strings', :element => {:type => Thrift::Types::STRING}}, + 8 => {:type => Thrift::Types::LIST, :name => 'maps', :element => {:type => Thrift::Types::MAP, :key => {:type => Thrift::Types::I16}, :value => {:type => Thrift::Types::I16}}}, + 9 => {:type => Thrift::Types::LIST, :name => 'lists', :element => {:type => Thrift::Types::LIST, :element => {:type => Thrift::Types::I16}}}, + 10 => {:type => Thrift::Types::LIST, :name => 'sets', :element => {:type => Thrift::Types::SET, :element => {:type => Thrift::Types::I16}}}, + 11 => {:type => Thrift::Types::LIST, :name => 'hellos', :element => {:type => Thrift::Types::STRUCT, :class => Hello}} + } + end + end -- 2.17.1