Merge branch 'THRIFT-103'
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@682458 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/rb/ext/binaryprotocolaccelerated.c b/lib/rb/ext/binaryprotocolaccelerated.c
index 6e295ed..1da7a3d 100644
--- a/lib/rb/ext/binaryprotocolaccelerated.c
+++ b/lib/rb/ext/binaryprotocolaccelerated.c
@@ -186,7 +186,7 @@
spec->type = type;
- if (Qnil != name) {
+ if (!NIL_P(name)) {
spec->name = StringValuePtr(name);
} else {
spec->name = NULL;
@@ -317,6 +317,8 @@
VALUE keys;
VALUE key;
VALUE val;
+
+ Check_Type(value, T_HASH);
keys = rb_funcall(value, keys_id, 0);
@@ -347,14 +349,17 @@
}
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 @@
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 @@
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 @@
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 4309829..022c786 100644
--- a/lib/rb/spec/ThriftSpec.thrift
+++ b/lib/rb/spec/ThriftSpec.thrift
@@ -17,6 +17,20 @@
1: bool yesno = 1
}
+struct SimpleList {
+ 1: list<bool> bools,
+ 2: list<byte> bytes,
+ 3: list<i16> i16s,
+ 4: list<i32> i32s,
+ 5: list<i64> i64s,
+ 6: list<double> doubles,
+ 7: list<string> strings,
+ 8: list<map<i16, i16>> maps,
+ 9: list<list<i16>> lists,
+ 10: list<set<i16>> sets,
+ 11: list<Hello> 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 9a4af6b..ced15b4 100644
--- a/lib/rb/spec/binaryprotocol_spec_shared.rb
+++ b/lib/rb/spec/binaryprotocol_spec_shared.rb
@@ -63,6 +63,11 @@
@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 @@
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 @@
# 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 @@
# 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 @@
# 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 @@
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 @@
@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 86101f8..6a3cc39 100644
--- a/lib/rb/spec/binaryprotocolaccelerated_spec.rb
+++ b/lib/rb/spec/binaryprotocolaccelerated_spec.rb
@@ -80,6 +80,59 @@
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 b98ae7d..24358da 100644
--- a/lib/rb/spec/gen-rb/ThriftSpec_types.rb
+++ b/lib/rb/spec/gen-rb/ThriftSpec_types.rb
@@ -46,4 +46,22 @@
}
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