Repository: thrift Updated Branches: refs/heads/master 96409d9df -> 123258ba6
THRIFT-3364 Fix ruby binary field encoding in TJSONProtocol Client: Ruby Patch: Nobuaki Sukegawa <[email protected]> This closes #633 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/123258ba Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/123258ba Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/123258ba Branch: refs/heads/master Commit: 123258ba60facd8581d868c71a543487b2acff3c Parents: 96409d9 Author: Jens Geyer <[email protected]> Authored: Fri Oct 2 00:38:17 2015 +0200 Committer: Jens Geyer <[email protected]> Committed: Fri Oct 2 00:38:17 2015 +0200 ---------------------------------------------------------------------- lib/rb/lib/thrift/processor.rb | 2 -- lib/rb/lib/thrift/protocol/json_protocol.rb | 13 +++++++++++-- lib/rb/spec/binary_protocol_spec_shared.rb | 4 ++-- lib/rb/spec/compact_protocol_spec.rb | 18 +++++++++--------- lib/rb/spec/json_protocol_spec.rb | 14 ++++++++++++-- lib/rb/spec/spec_helper.rb | 2 +- test/DebugProtoTest.thrift | 1 + test/known_failures_Linux.json | 12 ++---------- test/rb/Gemfile | 1 + test/rb/integration/TestClient.rb | 2 +- test/rb/integration/TestServer.rb | 2 ++ 11 files changed, 42 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/lib/rb/lib/thrift/processor.rb ---------------------------------------------------------------------- diff --git a/lib/rb/lib/thrift/processor.rb b/lib/rb/lib/thrift/processor.rb index b96fb43..fd312ee 100644 --- a/lib/rb/lib/thrift/processor.rb +++ b/lib/rb/lib/thrift/processor.rb @@ -57,12 +57,10 @@ module Thrift end def write_error(err, oprot, name, seqid) - p 'write_error' oprot.write_message_begin(name, MessageTypes::EXCEPTION, seqid) err.write(oprot) oprot.write_message_end oprot.trans.flush - p 'write_error end' end end end http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/lib/rb/lib/thrift/protocol/json_protocol.rb ---------------------------------------------------------------------- diff --git a/lib/rb/lib/thrift/protocol/json_protocol.rb b/lib/rb/lib/thrift/protocol/json_protocol.rb index 9f0069d..4d6186c 100644 --- a/lib/rb/lib/thrift/protocol/json_protocol.rb +++ b/lib/rb/lib/thrift/protocol/json_protocol.rb @@ -18,6 +18,7 @@ # under the License. # +require 'base64' module Thrift class LookaheadReader @@ -310,7 +311,7 @@ module Thrift def write_json_base64(str) @context.write(trans) trans.write(@@kJSONStringDelimiter) - write_json_string([str].pack("m")) + trans.write(Base64.strict_encode64(str)) trans.write(@@kJSONStringDelimiter) end @@ -546,7 +547,15 @@ module Thrift # Reads a block of base64 characters, decoding it, and returns via str def read_json_base64 - read_json_string.unpack("m")[0] + str = read_json_string + m = str.length % 4 + if m != 0 + # Add missing padding + (4 - m).times do + str += '=' + end + end + Base64.strict_decode64(str) end # Reads a sequence of characters, stopping at the first one that is not http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/lib/rb/spec/binary_protocol_spec_shared.rb ---------------------------------------------------------------------- diff --git a/lib/rb/spec/binary_protocol_spec_shared.rb b/lib/rb/spec/binary_protocol_spec_shared.rb index c615b58..7a9d028 100644 --- a/lib/rb/spec/binary_protocol_spec_shared.rb +++ b/lib/rb/spec/binary_protocol_spec_shared.rb @@ -423,9 +423,9 @@ shared_examples_for 'a binary protocol' do clientproto = protocol_class.new(clientside) serverproto = protocol_class.new(serverside) - processor = Srv::Processor.new(SrvHandler.new) + processor = Thrift::Test::Srv::Processor.new(SrvHandler.new) - client = Srv::Client.new(clientproto, clientproto) + client = Thrift::Test::Srv::Client.new(clientproto, clientproto) # first block firstblock.call(client) http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/lib/rb/spec/compact_protocol_spec.rb ---------------------------------------------------------------------- diff --git a/lib/rb/spec/compact_protocol_spec.rb b/lib/rb/spec/compact_protocol_spec.rb index daad583..8a1a228 100644 --- a/lib/rb/spec/compact_protocol_spec.rb +++ b/lib/rb/spec/compact_protocol_spec.rb @@ -75,11 +75,11 @@ describe Thrift::CompactProtocol do trans = Thrift::MemoryBufferTransport.new proto = Thrift::CompactProtocol.new(trans) - struct = CompactProtoTestStruct.new + struct = Thrift::Test::CompactProtoTestStruct.new # sets and maps don't hash well... not sure what to do here. struct.write(proto) - struct2 = CompactProtoTestStruct.new + struct2 = Thrift::Test::CompactProtoTestStruct.new struct2.read(proto) struct2.should == struct end @@ -91,9 +91,9 @@ describe Thrift::CompactProtocol do client_in_trans = Thrift::MemoryBufferTransport.new client_in_proto = Thrift::CompactProtocol.new(client_in_trans) - processor = Srv::Processor.new(JankyHandler.new) + processor = Thrift::Test::Srv::Processor.new(JankyHandler.new) - client = Srv::Client.new(client_in_proto, client_out_proto) + client = Thrift::Test::Srv::Client.new(client_in_proto, client_out_proto) client.send_Janky(1) # puts client_out_trans.inspect_buffer processor.process(client_out_proto, client_in_proto) @@ -101,9 +101,9 @@ describe Thrift::CompactProtocol do end it "should deal with fields following fields that have non-delta ids" do - brcp = BreaksRubyCompactProtocol.new( + brcp = Thrift::Test::BreaksRubyCompactProtocol.new( :field1 => "blah", - :field2 => BigFieldIdStruct.new( + :field2 => Thrift::Test::BigFieldIdStruct.new( :field1 => "string1", :field2 => "string2"), :field3 => 3) @@ -111,18 +111,18 @@ describe Thrift::CompactProtocol do bytes = ser.serialize(brcp) deser = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new) - brcp2 = BreaksRubyCompactProtocol.new + brcp2 = Thrift::Test::BreaksRubyCompactProtocol.new deser.deserialize(brcp2, bytes) brcp2.should == brcp end it "should deserialize an empty map to an empty hash" do - struct = SingleMapTestStruct.new(:i32_map => {}) + struct = Thrift::Test::SingleMapTestStruct.new(:i32_map => {}) ser = Thrift::Serializer.new(Thrift::CompactProtocolFactory.new) bytes = ser.serialize(struct) deser = Thrift::Deserializer.new(Thrift::CompactProtocolFactory.new) - struct2 = SingleMapTestStruct.new + struct2 = Thrift::Test::SingleMapTestStruct.new deser.deserialize(struct2, bytes) struct.should == struct2 end http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/lib/rb/spec/json_protocol_spec.rb ---------------------------------------------------------------------- diff --git a/lib/rb/spec/json_protocol_spec.rb b/lib/rb/spec/json_protocol_spec.rb index 2f7f1e6..9fb6b7b 100644 --- a/lib/rb/spec/json_protocol_spec.rb +++ b/lib/rb/spec/json_protocol_spec.rb @@ -57,7 +57,7 @@ describe 'JsonProtocol' do it "should write json base64" do @prot.write_json_base64("this is a base64 string") - @trans.read(@trans.available).should == "\"\"dGhpcyBpcyBhIGJhc2U2NCBzdHJpbmc=\\n\"\"" + @trans.read(@trans.available).should == "\"dGhpcyBpcyBhIGJhc2U2NCBzdHJpbmc=\"" end it "should write json integer" do @@ -244,7 +244,12 @@ describe 'JsonProtocol' do it "should write binary" do @prot.write_binary("this is a base64 string") - @trans.read(@trans.available).should == "\"\"dGhpcyBpcyBhIGJhc2U2NCBzdHJpbmc=\\n\"\"" + @trans.read(@trans.available).should == "\"dGhpcyBpcyBhIGJhc2U2NCBzdHJpbmc=\"" + end + + it "should write long binary" do + @prot.write_binary((0...256).to_a.pack('C*')) + @trans.read(@trans.available).should == "\"AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6PkJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v8PHy8/T19vf4+fr7/P3+/w==\"" end it "should get type name for type id" do @@ -503,6 +508,11 @@ describe 'JsonProtocol' do @trans.write("\"dGhpcyBpcyBhIHRlc3Qgc3RyaW5n\"") @prot.read_binary.should == "this is a test string" end + + it "should read long binary" do + @trans.write("\"AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6PkJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v8PHy8/T19vf4+fr7/P3+/w==\"") + @prot.read_binary.bytes.to_a.should == (0...256).to_a + end end describe Thrift::JsonProtocolFactory do http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/lib/rb/spec/spec_helper.rb ---------------------------------------------------------------------- diff --git a/lib/rb/spec/spec_helper.rb b/lib/rb/spec/spec_helper.rb index 3672bf0..5bf98d0 100644 --- a/lib/rb/spec/spec_helper.rb +++ b/lib/rb/spec/spec_helper.rb @@ -54,7 +54,7 @@ require 'thrift_spec_types' require 'nonblocking_service' module Fixtures - COMPACT_PROTOCOL_TEST_STRUCT = COMPACT_TEST.dup + COMPACT_PROTOCOL_TEST_STRUCT = Thrift::Test::COMPACT_TEST.dup COMPACT_PROTOCOL_TEST_STRUCT.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 http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/test/DebugProtoTest.thrift ---------------------------------------------------------------------- diff --git a/test/DebugProtoTest.thrift b/test/DebugProtoTest.thrift index 4e9fb47..50ae4c1 100644 --- a/test/DebugProtoTest.thrift +++ b/test/DebugProtoTest.thrift @@ -20,6 +20,7 @@ namespace c_glib TTest namespace cpp thrift.test.debug namespace java thrift.test +namespace rb thrift.test struct Doubles { 1: double nan, http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/test/known_failures_Linux.json ---------------------------------------------------------------------- diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json index 5685efe..c22c906 100644 --- a/test/known_failures_Linux.json +++ b/test/known_failures_Linux.json @@ -96,8 +96,6 @@ "go-nodejs_json_framed-ip-ssl", "go-perl_binary_buffered-ip-ssl", "go-perl_binary_framed-ip-ssl", - "go-rb_json_buffered-ip", - "go-rb_json_framed-ip", "hs-cpp_json_buffered-ip", "hs-cpp_json_framed-ip", "hs-csharp_binary_framed-ip", @@ -220,15 +218,9 @@ "py-rb_json_framed-ip", "rb-cpp_json_buffered-ip", "rb-cpp_json_framed-ip", - "rb-csharp_json_buffered-ip", - "rb-csharp_json_framed-ip", - "rb-hs_json_buffered-ip", - "rb-hs_json_framed-ip", "rb-java_json_buffered-ip", "rb-java_json_framed-fastframed-ip", "rb-java_json_framed-ip", "rb-nodejs_json_buffered-ip", - "rb-nodejs_json_framed-ip", - "rb-rb_json_buffered-ip", - "rb-rb_json_framed-ip" -] + "rb-nodejs_json_framed-ip" +] \ No newline at end of file http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/test/rb/Gemfile ---------------------------------------------------------------------- diff --git a/test/rb/Gemfile b/test/rb/Gemfile index 8301e44..58c04aa 100644 --- a/test/rb/Gemfile +++ b/test/rb/Gemfile @@ -4,3 +4,4 @@ require "rubygems" gem "rack", "~> 1.5.2" gem "thin", "~> 1.5.0" +gem "test-unit" http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/test/rb/integration/TestClient.rb ---------------------------------------------------------------------- diff --git a/test/rb/integration/TestClient.rb b/test/rb/integration/TestClient.rb index b31a024..fb339c4 100755 --- a/test/rb/integration/TestClient.rb +++ b/test/rb/integration/TestClient.rb @@ -125,7 +125,7 @@ class SimpleClientTest < Test::Unit::TestCase def test_binary p 'test_binary' - val = [42, 0, 142, 242] + val = (0...256).reverse_each.to_a ret = @client.testBinary(val.pack('C*')) assert_equal(val, ret.bytes.to_a) end http://git-wip-us.apache.org/repos/asf/thrift/blob/123258ba/test/rb/integration/TestServer.rb ---------------------------------------------------------------------- diff --git a/test/rb/integration/TestServer.rb b/test/rb/integration/TestServer.rb index 0021e2a..bab723a 100755 --- a/test/rb/integration/TestServer.rb +++ b/test/rb/integration/TestServer.rb @@ -32,6 +32,8 @@ class SimpleHandler :testEnum, :testTypedef, :testMultiException].each do |meth| define_method(meth) do |thing| + p meth + p thing thing end
