From a887f272a9c8cacfff6be208ae7574746feb9f29 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Wed, 19 Nov 2025 18:52:54 -0500 Subject: [PATCH 1/7] Removed base64 and added logger as an explicit add_dependency Starting with Ruby 3.4, base64 is no longer a bundled gem, and now Thrift does not load in modern Ruby versions: An error occurred while loading ./spec/types_spec.rb. Failure/Error: require 'base64' LoadError: cannot load such file -- base64 # ./lib/thrift/protocol/json_protocol.rb:21:in '' # ./lib/thrift.rb:45:in '' # ./spec/spec_helper.rb:30:in '' # ./spec/types_spec.rb:20:in '' Ruby already has ability to serialize and deserialize base64 without requiring the base64 gem, which is a thin wrapper for syntactic sugar. Additionally, the code throws a warning at the moment, which will become an error in Ruby 3.5: /code/lib/thrift/processor.rb:20: warning: logger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. You can add logger to your Gemfile or gemspec to silence this warning. Added logger as an explicit dependency to avoid this warning. --- lib/rb/lib/thrift/protocol/json_protocol.rb | 8 +++----- lib/rb/thrift.gemspec | 5 +++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/rb/lib/thrift/protocol/json_protocol.rb b/lib/rb/lib/thrift/protocol/json_protocol.rb index 91e74e46bf8..d03357e61b2 100644 --- a/lib/rb/lib/thrift/protocol/json_protocol.rb +++ b/lib/rb/lib/thrift/protocol/json_protocol.rb @@ -16,9 +16,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -# - -require 'base64' +# module Thrift class LookaheadReader @@ -311,7 +309,7 @@ def write_json_string(str) def write_json_base64(str) @context.write(trans) trans.write(@@kJSONStringDelimiter) - trans.write(Base64.strict_encode64(str)) + trans.write([str].pack('m0')) trans.write(@@kJSONStringDelimiter) end @@ -555,7 +553,7 @@ def read_json_base64 str += '=' end end - Base64.strict_decode64(str) + str.unpack1('m0') end # Reads a sequence of characters, stopping at the first one that is not diff --git a/lib/rb/thrift.gemspec b/lib/rb/thrift.gemspec index 68bb4e19b23..b128c0e1ab4 100644 --- a/lib/rb/thrift.gemspec +++ b/lib/rb/thrift.gemspec @@ -26,15 +26,16 @@ Gem::Specification.new do |s| s.require_paths = %w[lib ext] + s.add_dependency 'logger' + s.add_development_dependency 'bundler', '>= 1.11' s.add_development_dependency 'pry', '~> 0.11.3' s.add_development_dependency 'pry-byebug', '~> 3.6' s.add_development_dependency 'pry-stack_explorer', '~> 0.4.9.2' s.add_development_dependency 'rack', '= 2.2.6.4' s.add_development_dependency 'rack-test', '~> 0.8.3' - s.add_development_dependency 'rake', '~> 12.3' + s.add_development_dependency 'rake', '~> 13.3' s.add_development_dependency 'rspec', '~> 3.7' s.add_development_dependency 'srv', '~> 1.0' s.add_development_dependency 'thin', '~> 1.7' end - From 6785e2cf9caaa4726852228f76a9a822757fd18a Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Thu, 20 Nov 2025 18:09:21 -0500 Subject: [PATCH 2/7] Remove Fixnum references to support modern Ruby versions Fixnum type has been deprecated since Ruby 2.4 and removed in Ruby 3.0, which makes BaseProtocol incompatible with modern Ruby versions. This change removes the Fixnum reference, as well as a monkey-patch that introduces Fixnum#ord() to Ruby versions below 1.8.7. **This change essentially makes it officially required to use Ruby 2.4 or newer.** Fixes the following test failures: 1) BaseProtocol Thrift::BaseProtocol should write out the different types (deprecated write_type signature) Failure/Error: if field_info.is_a? Fixnum NameError: uninitialized constant Thrift::BaseProtocol::Fixnum # ./lib/thrift/protocol/base_protocol.rb:256:in 'Thrift::BaseProtocol#write_type' # ./spec/base_protocol_spec.rb:65:in 'block (3 levels) in ' 2) BaseProtocol Thrift::BaseProtocol should write out the different types Failure/Error: if field_info.is_a? Fixnum NameError: uninitialized constant Thrift::BaseProtocol::Fixnum # ./lib/thrift/protocol/base_protocol.rb:256:in 'Thrift::BaseProtocol#write_type' # ./spec/base_protocol_spec.rb:90:in 'block (3 levels) in ' 3) BaseProtocol Thrift::BaseProtocol should read the different types (deprecated read_type signature) Failure/Error: if field_info.is_a? Fixnum NameError: uninitialized constant Thrift::BaseProtocol::Fixnum # ./lib/thrift/protocol/base_protocol.rb:296:in 'Thrift::BaseProtocol#read_type' # ./spec/base_protocol_spec.rb:113:in 'block (3 levels) in ' 4) BaseProtocol Thrift::BaseProtocol should read the different types Failure/Error: if field_info.is_a? Fixnum NameError: uninitialized constant Thrift::BaseProtocol::Fixnum # ./lib/thrift/protocol/base_protocol.rb:296:in 'Thrift::BaseProtocol#read_type' # ./spec/base_protocol_spec.rb:136:in 'block (3 levels) in ' --- LANGUAGES.md | 2 +- lib/rb/lib/thrift.rb | 1 - lib/rb/lib/thrift/core_ext.rb | 23 --------------- lib/rb/lib/thrift/core_ext/fixnum.rb | 29 ------------------- lib/rb/lib/thrift/protocol/base_protocol.rb | 8 ++--- lib/rb/lib/thrift/transport/base_transport.rb | 2 +- lib/rb/spec/struct_spec.rb | 4 +-- lib/rb/spec/types_spec.rb | 4 +-- lib/rb/thrift.gemspec | 2 ++ test/rb/integration/TestClient.rb | 4 +-- 10 files changed, 14 insertions(+), 65 deletions(-) delete mode 100644 lib/rb/lib/thrift/core_ext.rb delete mode 100644 lib/rb/lib/thrift/core_ext/fixnum.rb diff --git a/LANGUAGES.md b/LANGUAGES.md index baed8cec302..e70b0657ee9 100644 --- a/LANGUAGES.md +++ b/LANGUAGES.md @@ -312,7 +312,7 @@ Thrift's core protocol is TBinary, supported by all languages except for JavaScr Ruby 0.2.0 Yes -2.3.1p1122.5.1p57 +2.4.02.5.1p57 YesYesYesYes YesYes diff --git a/lib/rb/lib/thrift.rb b/lib/rb/lib/thrift.rb index 0f581229c15..5d8369501ea 100644 --- a/lib/rb/lib/thrift.rb +++ b/lib/rb/lib/thrift.rb @@ -23,7 +23,6 @@ $:.unshift File.dirname(__FILE__) require 'thrift/bytes' -require 'thrift/core_ext' require 'thrift/exceptions' require 'thrift/types' require 'thrift/processor' diff --git a/lib/rb/lib/thrift/core_ext.rb b/lib/rb/lib/thrift/core_ext.rb deleted file mode 100644 index f763cd5344d..00000000000 --- a/lib/rb/lib/thrift/core_ext.rb +++ /dev/null @@ -1,23 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# - -Dir[File.dirname(__FILE__) + "/core_ext/*.rb"].each do |file| - name = File.basename(file, '.rb') - require "thrift/core_ext/#{name}" -end diff --git a/lib/rb/lib/thrift/core_ext/fixnum.rb b/lib/rb/lib/thrift/core_ext/fixnum.rb deleted file mode 100644 index b4fc90dd697..00000000000 --- a/lib/rb/lib/thrift/core_ext/fixnum.rb +++ /dev/null @@ -1,29 +0,0 @@ -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# - -# Versions of ruby pre 1.8.7 do not have an .ord method available in the Fixnum -# class. -# -if RUBY_VERSION < "1.8.7" - class Fixnum - def ord - self - end - end -end \ No newline at end of file diff --git a/lib/rb/lib/thrift/protocol/base_protocol.rb b/lib/rb/lib/thrift/protocol/base_protocol.rb index 4d83a21ddb7..e9ea7c29bd5 100644 --- a/lib/rb/lib/thrift/protocol/base_protocol.rb +++ b/lib/rb/lib/thrift/protocol/base_protocol.rb @@ -251,9 +251,9 @@ def write_field(*args) # # Returns nothing. def write_type(field_info, value) - # if field_info is a Fixnum, assume it is a Thrift::Types constant + # if field_info is a Integer, assume it is a Thrift::Types constant # convert it into a field_info Hash for backwards compatibility - if field_info.is_a? Fixnum + if field_info.is_a? Integer field_info = {:type => field_info} end @@ -291,9 +291,9 @@ def write_type(field_info, value) # # Returns the value read; object type varies based on field_info[:type]. def read_type(field_info) - # if field_info is a Fixnum, assume it is a Thrift::Types constant + # if field_info is a Integer, assume it is a Thrift::Types constant # convert it into a field_info Hash for backwards compatibility - if field_info.is_a? Fixnum + if field_info.is_a? Integer field_info = {:type => field_info} end diff --git a/lib/rb/lib/thrift/transport/base_transport.rb b/lib/rb/lib/thrift/transport/base_transport.rb index 97e59352af7..12dd6731264 100644 --- a/lib/rb/lib/thrift/transport/base_transport.rb +++ b/lib/rb/lib/thrift/transport/base_transport.rb @@ -62,7 +62,7 @@ def read(sz) raise NotImplementedError end - # Returns an unsigned byte as a Fixnum in the range (0..255). + # Returns an unsigned byte as a Integer in the range (0..255). def read_byte buf = read_all(1) return Bytes.get_string_byte(buf, 0) diff --git a/lib/rb/spec/struct_spec.rb b/lib/rb/spec/struct_spec.rb index bbd502b6273..9a4baa89f87 100644 --- a/lib/rb/spec/struct_spec.rb +++ b/lib/rb/spec/struct_spec.rb @@ -227,7 +227,7 @@ def validate_default_arguments(object) it "should support optional type-checking in Thrift::Struct.new" do Thrift.type_checking = true begin - expect { SpecNamespace::Hello.new(:greeting => 3) }.to raise_error(Thrift::TypeError, /Expected Types::STRING, received (Integer|Fixnum) for field greeting/) + expect { SpecNamespace::Hello.new(:greeting => 3) }.to raise_error(Thrift::TypeError, "Expected Types::STRING, received Integer for field greeting") ensure Thrift.type_checking = false end @@ -238,7 +238,7 @@ def validate_default_arguments(object) Thrift.type_checking = true begin hello = SpecNamespace::Hello.new - expect { hello.greeting = 3 }.to raise_error(Thrift::TypeError, /Expected Types::STRING, received (Integer|Fixnum) for field greeting/) + expect { hello.greeting = 3 }.to raise_error(Thrift::TypeError, "Expected Types::STRING, received Integer for field greeting") ensure Thrift.type_checking = false end diff --git a/lib/rb/spec/types_spec.rb b/lib/rb/spec/types_spec.rb index d595ab56396..a385367f39d 100644 --- a/lib/rb/spec/types_spec.rb +++ b/lib/rb/spec/types_spec.rb @@ -100,9 +100,9 @@ end it "should give the Thrift::TypeError a readable message" do - msg = /Expected Types::STRING, received (Integer|Fixnum) for field foo/ + msg = "Expected Types::STRING, received Integer for field foo" expect { Thrift.check_type(3, {:type => Thrift::Types::STRING}, :foo) }.to raise_error(Thrift::TypeError, msg) - msg = /Expected Types::STRING, received (Integer|Fixnum) for field foo.element/ + msg = "Expected Types::STRING, received Integer for field foo.element" field = {:type => Thrift::Types::LIST, :element => {:type => Thrift::Types::STRING}} expect { Thrift.check_type([3], field, :foo) }.to raise_error(Thrift::TypeError, msg) msg = "Expected Types::I32, received NilClass for field foo.element.key" diff --git a/lib/rb/thrift.gemspec b/lib/rb/thrift.gemspec index b128c0e1ab4..95a9b291d98 100644 --- a/lib/rb/thrift.gemspec +++ b/lib/rb/thrift.gemspec @@ -12,6 +12,8 @@ Gem::Specification.new do |s| s.license = 'Apache-2.0' s.extensions = ['ext/extconf.rb'] + s.required_ruby_version = '>= 2.4.0' + s.rdoc_options = %w[--line-numbers --inline-source --title Thrift --main README] s.rubyforge_project = 'thrift' diff --git a/test/rb/integration/TestClient.rb b/test/rb/integration/TestClient.rb index 639aca99ee9..403cc4cea21 100755 --- a/test/rb/integration/TestClient.rb +++ b/test/rb/integration/TestClient.rb @@ -225,14 +225,14 @@ def test_enum ret = @client.testEnum(val) assert_equal(ret, 6) - assert_kind_of(Fixnum, ret) + assert_kind_of(Integer, ret) end def test_typedef p 'test_typedef' #UserId testTypedef(1: UserId thing), assert_equal(@client.testTypedef(309858235082523), 309858235082523) - assert_kind_of(Fixnum, @client.testTypedef(309858235082523)) + assert_kind_of(Integer, @client.testTypedef(309858235082523)) true end From 90b4e3fc59f81cdfb6a46fb4a887b65457ecd337 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Wed, 19 Nov 2025 18:40:19 -0500 Subject: [PATCH 3/7] Rack response needs to be finished when returned from the Rack application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This merge request addresses the following test failures: ``` 1) Thrift::ThinHTTPServer::RackApplication 404 response receives a non-POST Failure/Error: expect(last_response.status).to be 404 expected # => 404 got # => 500 Compared using equal?, which compares object identity, but expected and actual are not the same object. Use `expect(actual).to eq(expected)` if you don't care about object identity in this example. # ./spec/thin_http_server_spec.rb:102:in 'block (3 levels) in ' 2) Thrift::ThinHTTPServer::RackApplication 404 response receives a header other than application/x-thrift Failure/Error: expect(last_response.status).to be 404 expected # => 404 got # => 500 Compared using equal?, which compares object identity, but expected and actual are not the same object. Use `expect(actual).to eq(expected)` if you don't care about object identity in this example. # ./spec/thin_http_server_spec.rb:108:in 'block (3 levels) in ' 3) Thrift::ThinHTTPServer::RackApplication 200 response status code 200 Failure/Error: expect(last_response.ok?).to be_truthy expected: truthy value got: false # ./spec/thin_http_server_spec.rb:135:in 'block (3 levels) in ' ``` From the [Rack documentation](https://rack.github.io/rack/2.2/Rack/Response.html), > Your application’s call should end returning [Response#finish](https://rack.github.io/rack/2.2/Rack/Response.html#method-i-finish). Additionally: * using identity checks on integers produces weird expectation "expected # => 404", which is not necessary. Switched to using `eq` matcher * `be_truthy` matcher is overly generic and identity matching on `true`/`false` is preferred: "expected true" output will be displayed on failure --- lib/rb/lib/thrift/server/thin_http_server.rb | 4 ++-- lib/rb/spec/thin_http_server_spec.rb | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/rb/lib/thrift/server/thin_http_server.rb b/lib/rb/lib/thrift/server/thin_http_server.rb index 4a81c6d1776..bbd11755c32 100644 --- a/lib/rb/lib/thrift/server/thin_http_server.rb +++ b/lib/rb/lib/thrift/server/thin_http_server.rb @@ -60,9 +60,9 @@ def self.for(path, processor, protocol_factory) run lambda { |env| request = Rack::Request.new(env) if RackApplication.valid_thrift_request?(request) - RackApplication.successful_request(request, processor, protocol_factory) + RackApplication.successful_request(request, processor, protocol_factory).finish else - RackApplication.failed_request + RackApplication.failed_request.finish end } end diff --git a/lib/rb/spec/thin_http_server_spec.rb b/lib/rb/spec/thin_http_server_spec.rb index 665391b7dc6..6f8f8e2cc22 100644 --- a/lib/rb/spec/thin_http_server_spec.rb +++ b/lib/rb/spec/thin_http_server_spec.rb @@ -99,13 +99,13 @@ def app it 'receives a non-POST' do header('Content-Type', "application/x-thrift") get "/" - expect(last_response.status).to be 404 + expect(last_response.status).to eq 404 end it 'receives a header other than application/x-thrift' do header('Content-Type', "application/json") post "/" - expect(last_response.status).to be 404 + expect(last_response.status).to eq 404 end end @@ -132,10 +132,9 @@ def app it 'status code 200' do header('Content-Type', "application/x-thrift") post "/" - expect(last_response.ok?).to be_truthy + expect(last_response.ok?).to be true end end end - From 54e73b9e4e3379445fbc9639fee168f2ceac71c9 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Wed, 19 Nov 2025 18:20:45 -0500 Subject: [PATCH 4/7] Added Ruby library tests to the GitHub workflow --- .github/workflows/build.yml | 39 +++++++++++++++++++++++++++++++++++++ lib/rb/thrift.gemspec | 4 ++++ 2 files changed, 43 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2da3a59d3a5..1353f603e7e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -641,6 +641,45 @@ jobs: path: lib/cpp/test/*.xml retention-days: 3 + lib-ruby: + needs: compiler + runs-on: ubuntu-24.04 + strategy: + matrix: + ruby-version: ["2.7", "3.0", "3.1", "3.2", "3.3", "3.4", "4.0.0-preview2"] + fail-fast: false + steps: + - uses: actions/checkout@v5 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby-version }} + bundler-cache: true + working-directory: lib/rb + + - name: Run bootstrap + run: ./bootstrap.sh + + - name: Run configure + run: | + ./configure $(echo $CONFIG_ARGS_FOR_LIBS | sed 's/without-ruby/with-ruby/') + + - uses: actions/download-artifact@v6 + with: + name: thrift-compiler + path: compiler/cpp + + - name: Run thrift-compiler + run: | + chmod a+x compiler/cpp/thrift + compiler/cpp/thrift -version + + - name: Run tests + run: | + cd lib/rb + bundle exec rake spec + cross-test: needs: - lib-java-kotlin diff --git a/lib/rb/thrift.gemspec b/lib/rb/thrift.gemspec index 95a9b291d98..3d342a60b1b 100644 --- a/lib/rb/thrift.gemspec +++ b/lib/rb/thrift.gemspec @@ -40,4 +40,8 @@ Gem::Specification.new do |s| s.add_development_dependency 'rspec', '~> 3.7' s.add_development_dependency 'srv', '~> 1.0' s.add_development_dependency 'thin', '~> 1.7' + + # Only required for tests with Rack 2.x on Ruby 4.0+ + s.add_development_dependency 'cgi' + s.add_development_dependency 'ostruct' end From 07dd1bda2b5217512edad9485542279dac86a318 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Wed, 19 Nov 2025 18:41:08 -0500 Subject: [PATCH 5/7] Always pass a floating point number to sleep() --- lib/rb/spec/nonblocking_server_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rb/spec/nonblocking_server_spec.rb b/lib/rb/spec/nonblocking_server_spec.rb index 613d883904a..ad69945bd1d 100644 --- a/lib/rb/spec/nonblocking_server_spec.rb +++ b/lib/rb/spec/nonblocking_server_spec.rb @@ -157,7 +157,7 @@ def setup_client_thread(result) when :hello result << client.greeting(true) # ignore result when :sleep - client.sleep(args[0] || 0.5) + client.sleep((args[0] || 0.5).to_f) result << :slept when :shutdown client.shutdown From 2fec1533475482b34170f9eabf5327914cdbae83 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Wed, 19 Nov 2025 19:29:09 -0500 Subject: [PATCH 6/7] Fixed '#to_io gives NilClass' error --- lib/rb/lib/thrift/server/nonblocking_server.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/rb/lib/thrift/server/nonblocking_server.rb b/lib/rb/lib/thrift/server/nonblocking_server.rb index 740f3417e20..f6ab54b787f 100644 --- a/lib/rb/lib/thrift/server/nonblocking_server.rb +++ b/lib/rb/lib/thrift/server/nonblocking_server.rb @@ -46,6 +46,10 @@ def serve break if @server_transport.closed? begin rd, = select([@server_transport], nil, nil, 0.1) + rescue ::TypeError => e + # When select() is called with a closed transport with handler set to nil in + # shutdown paths, Ruby raises a TypeError ("#to_io gives NilClass"). + break rescue Errno::EBADF => e # In Ruby 1.9, calling @server_transport.close in shutdown paths causes the select() to raise an # Errno::EBADF. If this happens, ignore it and retry the loop. From db032b6a914b6df40f8f82a101656e78a2d8485a Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Wed, 19 Nov 2025 23:33:23 -0500 Subject: [PATCH 7/7] Added UUID support in Ruby library --- LANGUAGES.md | 4 +- .../cpp/src/thrift/generate/t_rb_generator.cc | 5 + lib/rb/ext/binary_protocol_accelerated.c | 87 ++++++++ lib/rb/ext/compact_protocol.c | 5 + lib/rb/ext/constants.h | 3 + lib/rb/ext/struct.c | 13 ++ lib/rb/ext/thrift_native.c | 10 +- lib/rb/lib/thrift/protocol/base_protocol.rb | 22 ++ lib/rb/lib/thrift/protocol/binary_protocol.rb | 26 ++- .../lib/thrift/protocol/compact_protocol.rb | 41 +++- lib/rb/lib/thrift/protocol/json_protocol.rb | 12 + .../lib/thrift/protocol/protocol_decorator.rb | 8 + lib/rb/lib/thrift/types.rb | 3 + lib/rb/spec/ThriftSpec.thrift | 6 +- lib/rb/spec/binary_protocol_spec_shared.rb | 28 ++- lib/rb/spec/compact_protocol_spec.rb | 24 ++ lib/rb/spec/json_protocol_spec.rb | 12 +- lib/rb/spec/struct_spec.rb | 77 +++++++ lib/rb/spec/union_spec.rb | 65 +++++- lib/rb/spec/uuid_validation_spec.rb | 210 ++++++++++++++++++ test/rb/Makefile.am | 4 +- test/rb/integration/TestClient.rb | 8 + test/rb/integration/TestServer.rb | 2 +- 23 files changed, 645 insertions(+), 30 deletions(-) create mode 100644 lib/rb/spec/uuid_validation_spec.rb diff --git a/LANGUAGES.md b/LANGUAGES.md index e70b0657ee9..1fd76ea1ed7 100644 --- a/LANGUAGES.md +++ b/LANGUAGES.md @@ -312,8 +312,8 @@ Thrift's core protocol is TBinary, supported by all languages except for JavaScr Ruby 0.2.0 Yes -2.4.02.5.1p57 - +2.7.84.0.0 +Yes YesYesYesYes YesYes YesYesYesYes diff --git a/compiler/cpp/src/thrift/generate/t_rb_generator.cc b/compiler/cpp/src/thrift/generate/t_rb_generator.cc index e9465736692..9b093143b88 100644 --- a/compiler/cpp/src/thrift/generate/t_rb_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_rb_generator.cc @@ -430,6 +430,9 @@ t_rb_ofstream& t_rb_generator::render_const_value(t_rb_ofstream& out, case t_base_type::TYPE_STRING: out << "%q\"" << get_escaped_string(value) << '"'; break; + case t_base_type::TYPE_UUID: + out << "%q\"" << get_escaped_string(value) << '"'; + break; case t_base_type::TYPE_BOOL: out << (value->get_integer() > 0 ? "true" : "false"); break; @@ -1175,6 +1178,8 @@ string t_rb_generator::type_to_enum(t_type* type) { return "::Thrift::Types::I64"; case t_base_type::TYPE_DOUBLE: return "::Thrift::Types::DOUBLE"; + case t_base_type::TYPE_UUID: + return "::Thrift::Types::UUID"; default: throw "compiler error: unhandled type"; } diff --git a/lib/rb/ext/binary_protocol_accelerated.c b/lib/rb/ext/binary_protocol_accelerated.c index c9ae4d299bb..6c094bc395d 100644 --- a/lib/rb/ext/binary_protocol_accelerated.c +++ b/lib/rb/ext/binary_protocol_accelerated.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +87,19 @@ static void write_string_direct(VALUE trans, VALUE str) { rb_funcall(trans, write_method_id, 1, str); } +// Efficient hex character to integer conversion +static inline int hex_char_to_int(char c) { + if (c >= '0' && c <= '9') return c - '0'; + if (c >= 'a' && c <= 'f') return c - 'a' + 10; + if (c >= 'A' && c <= 'F') return c - 'A' + 10; + return -1; // invalid hex character +} + +// Efficient integer to hex character conversion +static inline char int_to_hex_char(int val) { + return val < 10 ? ('0' + val) : ('a' + val - 10); +} + //-------------------------------- // interface writing methods //-------------------------------- @@ -228,6 +242,55 @@ VALUE rb_thrift_binary_proto_write_binary(VALUE self, VALUE buf) { return Qnil; } +VALUE rb_thrift_binary_proto_write_uuid(VALUE self, VALUE uuid) { + CHECK_NIL(uuid); + + if (TYPE(uuid) != T_STRING) { + rb_raise(rb_eStandardError, "UUID must be a string"); + } + + VALUE trans = GET_TRANSPORT(self); + char bytes[16]; + const char* str = RSTRING_PTR(uuid); + long len = RSTRING_LEN(uuid); + + // Parse UUID string (format: "550e8400-e29b-41d4-a716-446655440000") + // Expected length: 36 characters (32 hex + 4 hyphens) + if (len != 36 || str[8] != '-' || str[13] != '-' || str[18] != '-' || str[23] != '-') { + VALUE exception_class = rb_const_get(thrift_module, rb_intern("ProtocolException")); + VALUE invalid_data = rb_const_get(exception_class, rb_intern("INVALID_DATA")); + VALUE args[2]; + args[0] = invalid_data; + args[1] = rb_str_new2("Invalid UUID format"); + rb_exc_raise(rb_class_new_instance(2, args, exception_class)); + } + + // Parse hex string to bytes using direct conversion, skipping hyphens + int byte_idx = 0; + for (int i = 0; i < len && byte_idx < 16; i++) { + if (str[i] == '-') continue; + + // Convert two hex characters to one byte + int high = hex_char_to_int(str[i]); + int low = hex_char_to_int(str[i + 1]); + + if (high < 0 || low < 0) { + VALUE exception_class = rb_const_get(thrift_module, rb_intern("ProtocolException")); + VALUE invalid_data = rb_const_get(exception_class, rb_intern("INVALID_DATA")); + VALUE args[2]; + args[0] = invalid_data; + args[1] = rb_str_new2("Invalid hex character in UUID"); + rb_exc_raise(rb_class_new_instance(2, args, exception_class)); + } + + bytes[byte_idx++] = (unsigned char)((high << 4) | low); + i++; // skip next char since we processed two + } + + WRITE(trans, bytes, 16); + return Qnil; +} + //--------------------------------------- // interface reading methods //--------------------------------------- @@ -400,6 +463,28 @@ VALUE rb_thrift_binary_proto_read_binary(VALUE self) { return READ(self, size); } +VALUE rb_thrift_binary_proto_read_uuid(VALUE self) { + unsigned char bytes[16]; + VALUE data = READ(self, 16); + memcpy(bytes, RSTRING_PTR(data), 16); + + // Format as UUID string: "XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX" + char uuid_str[37]; + char* p = uuid_str; + + for (int i = 0; i < 16; i++) { + *p++ = int_to_hex_char((bytes[i] >> 4) & 0x0F); + *p++ = int_to_hex_char(bytes[i] & 0x0F); + if (i == 3 || i == 5 || i == 7 || i == 9) { + *p++ = '-'; + } + } + + *p = '\0'; + + return rb_str_new(uuid_str, 36); +} + void Init_binary_protocol_accelerated() { VALUE thrift_binary_protocol_class = rb_const_get(thrift_module, rb_intern("BinaryProtocol")); @@ -425,6 +510,7 @@ void Init_binary_protocol_accelerated() { rb_define_method(bpa_class, "write_double", rb_thrift_binary_proto_write_double, 1); rb_define_method(bpa_class, "write_string", rb_thrift_binary_proto_write_string, 1); rb_define_method(bpa_class, "write_binary", rb_thrift_binary_proto_write_binary, 1); + rb_define_method(bpa_class, "write_uuid", rb_thrift_binary_proto_write_uuid, 1); // unused methods rb_define_method(bpa_class, "write_message_end", rb_thrift_binary_proto_write_message_end, 0); rb_define_method(bpa_class, "write_struct_begin", rb_thrift_binary_proto_write_struct_begin, 1); @@ -447,6 +533,7 @@ void Init_binary_protocol_accelerated() { rb_define_method(bpa_class, "read_double", rb_thrift_binary_proto_read_double, 0); rb_define_method(bpa_class, "read_string", rb_thrift_binary_proto_read_string, 0); rb_define_method(bpa_class, "read_binary", rb_thrift_binary_proto_read_binary, 0); + rb_define_method(bpa_class, "read_uuid", rb_thrift_binary_proto_read_uuid, 0); // unused methods rb_define_method(bpa_class, "read_message_end", rb_thrift_binary_proto_read_message_end, 0); rb_define_method(bpa_class, "read_struct_begin", rb_thrift_binary_proto_read_struct_begin, 0); diff --git a/lib/rb/ext/compact_protocol.c b/lib/rb/ext/compact_protocol.c index c98c09eb491..7feaf748b40 100644 --- a/lib/rb/ext/compact_protocol.c +++ b/lib/rb/ext/compact_protocol.c @@ -58,6 +58,7 @@ static int CTYPE_LIST = 0x09; static int CTYPE_SET = 0x0A; static int CTYPE_MAP = 0x0B; static int CTYPE_STRUCT = 0x0C; +static int CTYPE_UUID = 0x0D; VALUE rb_thrift_compact_proto_write_i16(VALUE self, VALUE i16); @@ -86,6 +87,8 @@ static int get_compact_type(VALUE type_value) { return CTYPE_MAP; } else if (type == TTYPE_STRUCT) { return CTYPE_STRUCT; + } else if (type == TTYPE_UUID) { + return CTYPE_UUID; } else { char str[50]; sprintf(str, "don't know what type: %d", type); @@ -357,6 +360,8 @@ static int8_t get_ttype(int8_t ctype) { return TTYPE_MAP; } else if (ctype == CTYPE_STRUCT) { return TTYPE_STRUCT; + } else if (ctype == CTYPE_UUID) { + return TTYPE_UUID; } else { char str[50]; sprintf(str, "don't know what type: %d", ctype); diff --git a/lib/rb/ext/constants.h b/lib/rb/ext/constants.h index e7aec44787a..4fda2c0a345 100644 --- a/lib/rb/ext/constants.h +++ b/lib/rb/ext/constants.h @@ -29,6 +29,7 @@ extern int TTYPE_MAP; extern int TTYPE_SET; extern int TTYPE_LIST; extern int TTYPE_STRUCT; +extern int TTYPE_UUID; extern ID validate_method_id; extern ID write_struct_begin_method_id; @@ -49,6 +50,7 @@ extern ID write_list_begin_method_id; extern ID write_list_end_method_id; extern ID write_set_begin_method_id; extern ID write_set_end_method_id; +extern ID write_uuid_method_id; extern ID read_bool_method_id; extern ID read_byte_method_id; extern ID read_i16_method_id; @@ -63,6 +65,7 @@ extern ID read_list_begin_method_id; extern ID read_list_end_method_id; extern ID read_set_begin_method_id; extern ID read_set_end_method_id; +extern ID read_uuid_method_id; extern ID read_struct_begin_method_id; extern ID read_struct_end_method_id; extern ID read_field_begin_method_id; diff --git a/lib/rb/ext/struct.c b/lib/rb/ext/struct.c index e8255a9cbbf..f385a9f2c5a 100644 --- a/lib/rb/ext/struct.c +++ b/lib/rb/ext/struct.c @@ -75,6 +75,11 @@ VALUE default_write_string(VALUE protocol, VALUE value) { return Qnil; } +VALUE default_write_uuid(VALUE protocol, VALUE value) { + rb_funcall(protocol, write_uuid_method_id, 1, value); + return Qnil; +} + VALUE default_write_binary(VALUE protocol, VALUE value) { rb_funcall(protocol, write_binary_method_id, 1, value); return Qnil; @@ -195,6 +200,10 @@ VALUE default_read_string(VALUE protocol) { return rb_funcall(protocol, read_string_method_id, 0); } +VALUE default_read_uuid(VALUE protocol) { + return rb_funcall(protocol, read_uuid_method_id, 0); +} + VALUE default_read_binary(VALUE protocol) { return rb_funcall(protocol, read_binary_method_id, 0); } @@ -342,6 +351,8 @@ static void write_anything(int ttype, VALUE value, VALUE protocol, VALUE field_i } else { default_write_binary(protocol, value); } + } else if (ttype == TTYPE_UUID) { + default_write_uuid(protocol, value); } else if (IS_CONTAINER(ttype)) { write_container(ttype, field_info, value, protocol); } else if (ttype == TTYPE_STRUCT) { @@ -452,6 +463,8 @@ static VALUE read_anything(VALUE protocol, int ttype, VALUE field_info) { } } else if (ttype == TTYPE_DOUBLE) { result = default_read_double(protocol); + } else if (ttype == TTYPE_UUID) { + result = default_read_uuid(protocol); } else if (ttype == TTYPE_STRUCT) { VALUE klass = rb_hash_aref(field_info, class_sym); result = rb_class_new_instance(0, NULL, klass); diff --git a/lib/rb/ext/thrift_native.c b/lib/rb/ext/thrift_native.c index d53545473a9..16a5ea4dd47 100644 --- a/lib/rb/ext/thrift_native.c +++ b/lib/rb/ext/thrift_native.c @@ -43,6 +43,7 @@ int TTYPE_MAP; int TTYPE_SET; int TTYPE_LIST; int TTYPE_STRUCT; +int TTYPE_UUID; // method ids ID validate_method_id; @@ -57,6 +58,7 @@ ID write_i32_method_id; ID write_i64_method_id; ID write_double_method_id; ID write_string_method_id; +ID write_uuid_method_id; ID write_binary_method_id; ID write_map_begin_method_id; ID write_map_end_method_id; @@ -70,6 +72,7 @@ ID read_i16_method_id; ID read_i32_method_id; ID read_i64_method_id; ID read_string_method_id; +ID read_uuid_method_id; ID read_binary_method_id; ID read_double_method_id; ID read_map_begin_method_id; @@ -138,6 +141,7 @@ void Init_thrift_native() { TTYPE_SET = FIX2INT(rb_const_get(thrift_types_module, rb_intern("SET"))); TTYPE_LIST = FIX2INT(rb_const_get(thrift_types_module, rb_intern("LIST"))); TTYPE_STRUCT = FIX2INT(rb_const_get(thrift_types_module, rb_intern("STRUCT"))); + TTYPE_UUID = FIX2INT(rb_const_get(thrift_types_module, rb_intern("UUID"))); // method ids validate_method_id = rb_intern("validate"); @@ -152,6 +156,7 @@ void Init_thrift_native() { write_i64_method_id = rb_intern("write_i64"); write_double_method_id = rb_intern("write_double"); write_string_method_id = rb_intern("write_string"); + write_uuid_method_id = rb_intern("write_uuid"); write_binary_method_id = rb_intern("write_binary"); write_map_begin_method_id = rb_intern("write_map_begin"); write_map_end_method_id = rb_intern("write_map_end"); @@ -165,10 +170,11 @@ void Init_thrift_native() { read_i32_method_id = rb_intern("read_i32"); read_i64_method_id = rb_intern("read_i64"); read_string_method_id = rb_intern("read_string"); + read_uuid_method_id = rb_intern("read_uuid"); read_binary_method_id = rb_intern("read_binary"); read_double_method_id = rb_intern("read_double"); read_map_begin_method_id = rb_intern("read_map_begin"); - read_map_end_method_id = rb_intern("read_map_end"); + read_map_end_method_id = rb_intern("read_map_end"); read_list_begin_method_id = rb_intern("read_list_begin"); read_list_end_method_id = rb_intern("read_list_end"); read_set_begin_method_id = rb_intern("read_set_begin"); @@ -192,7 +198,7 @@ void Init_thrift_native() { fields_const_id = rb_intern("FIELDS"); transport_ivar_id = rb_intern("@trans"); strict_read_ivar_id = rb_intern("@strict_read"); - strict_write_ivar_id = rb_intern("@strict_write"); + strict_write_ivar_id = rb_intern("@strict_write"); // cached symbols type_sym = ID2SYM(rb_intern("type")); diff --git a/lib/rb/lib/thrift/protocol/base_protocol.rb b/lib/rb/lib/thrift/protocol/base_protocol.rb index e9ea7c29bd5..dfce44a3bc0 100644 --- a/lib/rb/lib/thrift/protocol/base_protocol.rb +++ b/lib/rb/lib/thrift/protocol/base_protocol.rb @@ -137,6 +137,15 @@ def write_binary(buf) raise NotImplementedError end + # Writes a UUID as 16 bytes. + # + # uuid - The UUID string to write (e.g. "550e8400-e29b-41d4-a716-446655440000"). + # + # Returns nothing. + def write_uuid(uuid) + raise NotImplementedError + end + def read_message_begin raise NotImplementedError end @@ -212,6 +221,13 @@ def read_binary raise NotImplementedError end + # Reads a UUID as 16 bytes and returns it as a string. + # + # Returns a String (e.g. "550e8400-e29b-41d4-a716-446655440000"). + def read_uuid + raise NotImplementedError + end + # Writes a field based on the field information, field ID and value. # # field_info - A Hash containing the definition of the field: @@ -276,6 +292,8 @@ def write_type(field_info, value) else write_string(value) end + when Types::UUID + write_uuid(value) when Types::STRUCT value.write(self) else @@ -316,6 +334,8 @@ def read_type(field_info) else read_string end + when Types::UUID + read_uuid else raise NotImplementedError end @@ -337,6 +357,8 @@ def skip(type) read_double when Types::STRING read_string + when Types::UUID + read_uuid when Types::STRUCT read_struct_begin while true diff --git a/lib/rb/lib/thrift/protocol/binary_protocol.rb b/lib/rb/lib/thrift/protocol/binary_protocol.rb index d8279dbe6b5..780a9d3ed1f 100644 --- a/lib/rb/lib/thrift/protocol/binary_protocol.rb +++ b/lib/rb/lib/thrift/protocol/binary_protocol.rb @@ -116,6 +116,24 @@ def write_binary(buf) trans.write(buf) end + def write_uuid(uuid) + # Validate UUID format: "550e8400-e29b-41d4-a716-446655440000" + # Expected length: 36 characters (32 hex + 4 hyphens) + unless uuid.is_a?(String) && uuid.length == 36 && + uuid[8] == '-' && uuid[13] == '-' && uuid[18] == '-' && uuid[23] == '-' + raise ProtocolException.new(ProtocolException::INVALID_DATA, 'Invalid UUID format') + end + + # Pack hex directly without creating intermediate string + # Check for valid hex characters during packing + hex_str = uuid.delete('-') + unless hex_str =~ /\A[0-9a-fA-F]{32}\z/ + raise ProtocolException.new(ProtocolException::INVALID_DATA, 'Invalid hex characters in UUID') + end + + trans.write([hex_str].pack('H*')) + end + def read_message_begin version = read_i32 if version < 0 @@ -226,7 +244,13 @@ def read_binary size = read_i32 trans.read_all(size) end - + + def read_uuid + bytes = trans.read_all(16) + hex = bytes.unpack('H*')[0] + "#{hex[0, 8]}-#{hex[8, 4]}-#{hex[12, 4]}-#{hex[16, 4]}-#{hex[20, 12]}" + end + def to_s "binary(#{super.to_s})" end diff --git a/lib/rb/lib/thrift/protocol/compact_protocol.rb b/lib/rb/lib/thrift/protocol/compact_protocol.rb index 1f9bd30608f..b93c6e7f1ea 100644 --- a/lib/rb/lib/thrift/protocol/compact_protocol.rb +++ b/lib/rb/lib/thrift/protocol/compact_protocol.rb @@ -45,7 +45,8 @@ class CompactTypes SET = 0x0A MAP = 0x0B STRUCT = 0x0C - + UUID = 0x0D + def self.is_bool_type?(b) (b & 0x0f) == BOOLEAN_TRUE || (b & 0x0f) == BOOLEAN_FALSE end @@ -63,7 +64,8 @@ def self.is_bool_type?(b) LIST => Types::LIST, SET => Types::SET, MAP => Types::MAP, - STRUCT => Types::STRUCT + STRUCT => Types::STRUCT, + UUID => Types::UUID } TTYPE_TO_COMPACT = { @@ -78,7 +80,8 @@ def self.is_bool_type?(b) Types::LIST => LIST, Types::SET => SET, Types::MAP => MAP, - Types::STRUCT => STRUCT + Types::STRUCT => STRUCT, + Types::UUID => UUID } def self.get_ttype(compact_type) @@ -220,6 +223,22 @@ def write_binary(buf) @trans.write(buf) end + def write_uuid(uuid) + # Validate UUID format: "550e8400-e29b-41d4-a716-446655440000" + unless uuid.is_a?(String) && uuid.length == 36 && + uuid[8] == '-' && uuid[13] == '-' && uuid[18] == '-' && uuid[23] == '-' + raise ProtocolException.new(ProtocolException::INVALID_DATA, 'Invalid UUID format') + end + + # Pack hex directly without creating intermediate string + hex_str = uuid.delete('-') + unless hex_str =~ /\A[0-9a-fA-F]{32}\z/ + raise ProtocolException.new(ProtocolException::INVALID_DATA, 'Invalid hex characters in UUID') + end + + @trans.write([hex_str].pack('H*')) + end + def read_message_begin protocol_id = read_byte() if protocol_id != PROTOCOL_ID @@ -345,17 +364,23 @@ def read_binary size = read_varint32() trans.read_all(size) end - + + def read_uuid + bytes = trans.read_all(16) + hex = bytes.unpack('H*')[0] + "#{hex[0, 8]}-#{hex[8, 4]}-#{hex[12, 4]}-#{hex[16, 4]}-#{hex[20, 12]}" + end + def to_s "compact(#{super.to_s})" end private - - # - # Abstract method for writing the start of lists and sets. List and sets on + + # + # Abstract method for writing the start of lists and sets. List and sets on # the wire differ only by the type indicator. - # + # def write_collection_begin(elem_type, size) if size <= 14 write_byte(size << 4 | CompactTypes.get_compact_type(elem_type)) diff --git a/lib/rb/lib/thrift/protocol/json_protocol.rb b/lib/rb/lib/thrift/protocol/json_protocol.rb index d03357e61b2..703e88d9134 100644 --- a/lib/rb/lib/thrift/protocol/json_protocol.rb +++ b/lib/rb/lib/thrift/protocol/json_protocol.rb @@ -179,6 +179,8 @@ def get_type_name_for_type_id(id) "set" when Types::LIST "lst" + when Types::UUID + "uid" else raise NotImplementedError end @@ -207,6 +209,8 @@ def get_type_id_for_type_name(name) result = Types::SET elsif (name == "lst") result = Types::LIST + elsif (name == "uid") + result = Types::UUID else result = Types::STOP end @@ -475,6 +479,10 @@ def write_binary(str) write_json_base64(str) end + def write_uuid(uuid) + write_json_string(uuid) + end + ## # Reading functions ## @@ -767,6 +775,10 @@ def read_binary read_json_base64 end + def read_uuid + read_json_string + end + def to_s "json(#{super.to_s})" end diff --git a/lib/rb/lib/thrift/protocol/protocol_decorator.rb b/lib/rb/lib/thrift/protocol/protocol_decorator.rb index b1e3c155d44..a080917fbb4 100644 --- a/lib/rb/lib/thrift/protocol/protocol_decorator.rb +++ b/lib/rb/lib/thrift/protocol/protocol_decorator.rb @@ -111,6 +111,10 @@ def write_binary(buf) @protocol.write_binary(buf) end + def write_uuid(uuid) + @protocol.write_uuid(uuid) + end + def read_message_begin @protocol.read_message_begin end @@ -190,5 +194,9 @@ def read_string def read_binary @protocol.read_binary end + + def read_uuid + @protocol.read_uuid + end end end \ No newline at end of file diff --git a/lib/rb/lib/thrift/types.rb b/lib/rb/lib/thrift/types.rb index cac52691a7f..0f6ab42f81e 100644 --- a/lib/rb/lib/thrift/types.rb +++ b/lib/rb/lib/thrift/types.rb @@ -34,6 +34,7 @@ module Types MAP = 13 SET = 14 LIST = 15 + UUID = 16 end class << self @@ -56,6 +57,8 @@ def self.check_type(value, field, name, skip_nil=true) Float when Types::STRING String + when Types::UUID + String when Types::STRUCT [Struct, Union] when Types::MAP diff --git a/lib/rb/spec/ThriftSpec.thrift b/lib/rb/spec/ThriftSpec.thrift index b42481b32d3..3c2f3e0a875 100644 --- a/lib/rb/spec/ThriftSpec.thrift +++ b/lib/rb/spec/ThriftSpec.thrift @@ -60,6 +60,7 @@ union TestUnion { 3: i32 other_i32_field; 4: SomeEnum enum_field; 5: binary binary_field; + 6: uuid uuid_field; } struct Foo { @@ -71,6 +72,7 @@ struct Foo { 6: set shorts = [5, 17, 239], 7: optional string opt_string 8: bool my_bool + 9: optional uuid opt_uuid } struct Foo2 { @@ -92,7 +94,8 @@ struct SimpleList { 8: list> maps, 9: list> lists, 10: list> sets, - 11: list hellos + 11: list hellos, + 12: list uuids } exception Xception { @@ -119,6 +122,7 @@ union My_union { 8: i32 other_i32 9: SomeEnum some_enum; 10: map> my_map; + 11: uuid unique_id; } struct Struct_with_union { diff --git a/lib/rb/spec/binary_protocol_spec_shared.rb b/lib/rb/spec/binary_protocol_spec_shared.rb index 58d65f0401d..dacb64a5257 100644 --- a/lib/rb/spec/binary_protocol_spec_shared.rb +++ b/lib/rb/spec/binary_protocol_spec_shared.rb @@ -249,28 +249,44 @@ it "should error gracefully when trying to write a nil string" do expect { @prot.write_string(nil) }.to raise_error end - + + it "should write a uuid" do + @prot.write_uuid("00112233-4455-6677-8899-aabbccddeeff") + a = @trans.read(@trans.available) + expect(a.unpack('C*')).to eq([0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]) + end + + it "should read a uuid" do + @trans.write([0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff].pack('C*')) + uuid = @prot.read_uuid + expect(uuid).to eq("00112233-4455-6677-8899-aabbccddeeff") + end + + it "should error gracefully when trying to write an invalid uuid" do + expect { @prot.write_uuid("invalid") }.to raise_error(Thrift::ProtocolException) + end + it "should write the message header without version when writes are not strict" do @prot = protocol_class.new(@trans, true, false) # no strict write @prot.write_message_begin('testMessage', Thrift::MessageTypes::CALL, 17) expect(@trans.read(@trans.available)).to eq("\000\000\000\vtestMessage\001\000\000\000\021") end - + it "should write the message header with a version when writes are strict" do @prot = protocol_class.new(@trans) # strict write @prot.write_message_begin('testMessage', Thrift::MessageTypes::CALL, 17) expect(@trans.read(@trans.available)).to eq("\200\001\000\001\000\000\000\vtestMessage\000\000\000\021") end - + # message footer is a noop - + it "should read a field header" do @trans.write([Thrift::Types::STRING, 3].pack("cn")) expect(@prot.read_field_begin).to eq([nil, Thrift::Types::STRING, 3]) end - + # field footer is a noop - + it "should read a stop field" do @trans.write([Thrift::Types::STOP].pack("c")); expect(@prot.read_field_begin).to eq([nil, Thrift::Types::STOP, 0]) diff --git a/lib/rb/spec/compact_protocol_spec.rb b/lib/rb/spec/compact_protocol_spec.rb index 513dd69cfb3..7519810ec30 100644 --- a/lib/rb/spec/compact_protocol_spec.rb +++ b/lib/rb/spec/compact_protocol_spec.rb @@ -71,6 +71,30 @@ end end + it "should write a uuid" do + trans = Thrift::MemoryBufferTransport.new + proto = Thrift::CompactProtocol.new(trans) + + proto.write_uuid("00112233-4455-6677-8899-aabbccddeeff") + a = trans.read(trans.available) + expect(a.unpack('C*')).to eq([0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]) + end + + it "should read a uuid" do + trans = Thrift::MemoryBufferTransport.new + proto = Thrift::CompactProtocol.new(trans) + + trans.write([0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff].pack('C*')) + uuid = proto.read_uuid + expect(uuid).to eq("00112233-4455-6677-8899-aabbccddeeff") + end + + it "should error gracefully when trying to write an invalid uuid" do + trans = Thrift::MemoryBufferTransport.new + proto = Thrift::CompactProtocol.new(trans) + expect { proto.write_uuid("invalid") }.to raise_error(Thrift::ProtocolException) + end + it "should encode and decode a monster struct correctly" do trans = Thrift::MemoryBufferTransport.new proto = Thrift::CompactProtocol.new(trans) diff --git a/lib/rb/spec/json_protocol_spec.rb b/lib/rb/spec/json_protocol_spec.rb index fe1af7bb2ea..6c0d6a75a70 100644 --- a/lib/rb/spec/json_protocol_spec.rb +++ b/lib/rb/spec/json_protocol_spec.rb @@ -252,6 +252,11 @@ expect(@trans.read(@trans.available)).to eq("\"AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6PkJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v8PHy8/T19vf4+fr7/P3+/w==\"") end + it "should write a uuid" do + @prot.write_uuid("00112233-4455-6677-8899-aabbccddeeff") + expect(@trans.read(@trans.available)).to eq("\"00112233-4455-6677-8899-aabbccddeeff\"") + end + it "should get type name for type id" do expect {@prot.get_type_name_for_type_id(Thrift::Types::STOP)}.to raise_error(NotImplementedError) expect {@prot.get_type_name_for_type_id(Thrift::Types::VOID)}.to raise_error(NotImplementedError) @@ -534,7 +539,12 @@ @trans.write("\"AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6PkJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v8PHy8/T19vf4+fr7/P3+/w==\"") expect(@prot.read_binary.bytes.to_a).to eq((0...256).to_a) end - + + it "should read a uuid" do + @trans.write("\"00112233-4455-6677-8899-aabbccddeeff\"") + expect(@prot.read_uuid).to eq("00112233-4455-6677-8899-aabbccddeeff") + end + it "should provide a reasonable to_s" do expect(@prot.to_s).to eq("json(memory)") end diff --git a/lib/rb/spec/struct_spec.rb b/lib/rb/spec/struct_spec.rb index 9a4baa89f87..9349686ba1f 100644 --- a/lib/rb/spec/struct_spec.rb +++ b/lib/rb/spec/struct_spec.rb @@ -289,5 +289,82 @@ def validate_default_arguments(object) e.write(prot) end end + + it "should handle UUID fields in structs" do + struct = SpecNamespace::Foo.new( + simple: 42, + words: 'test', + opt_uuid: '550e8400-e29b-41d4-a716-446655440000' + ) + + trans = Thrift::MemoryBufferTransport.new + prot = Thrift::BinaryProtocol.new(trans) + + struct.write(prot) + + result = SpecNamespace::Foo.new + result.read(prot) + + expect(result.simple).to eq(42) + expect(result.words).to eq('test') + expect(result.opt_uuid).to eq('550e8400-e29b-41d4-a716-446655440000') + end + + it "should handle optional UUID fields when unset" do + struct = SpecNamespace::Foo.new(simple: 42, words: 'test') + expect(struct.opt_uuid).to be_nil + expect(struct.opt_uuid?).to be_falsey + end + + it "should handle list of UUIDs in SimpleList" do + uuids = ['550e8400-e29b-41d4-a716-446655440000', '6ba7b810-9dad-11d1-80b4-00c04fd430c8'] + struct = SpecNamespace::SimpleList.new(uuids: uuids) + + trans = Thrift::MemoryBufferTransport.new + prot = Thrift::CompactProtocol.new(trans) + + struct.write(prot) + + result = SpecNamespace::SimpleList.new + result.read(prot) + + expect(result.uuids).to eq(uuids) + end + + it "should normalize UUID case to lowercase" do + struct = SpecNamespace::Foo.new(opt_uuid: '550E8400-E29B-41D4-A716-446655440000') + + trans = Thrift::MemoryBufferTransport.new + prot = Thrift::BinaryProtocol.new(trans) + + struct.write(prot) + + result = SpecNamespace::Foo.new + result.read(prot) + + expect(result.opt_uuid).to eq('550e8400-e29b-41d4-a716-446655440000') + end + + it "should handle UUID alongside other types in SimpleList" do + struct = SpecNamespace::SimpleList.new( + bools: [true, false], + i32s: [1, 2, 3], + strings: ['hello', 'world'], + uuids: ['550e8400-e29b-41d4-a716-446655440000', '00000000-0000-0000-0000-000000000000'] + ) + + trans = Thrift::MemoryBufferTransport.new + prot = Thrift::BinaryProtocol.new(trans) + + struct.write(prot) + + result = SpecNamespace::SimpleList.new + result.read(prot) + + expect(result.bools).to eq([true, false]) + expect(result.i32s).to eq([1, 2, 3]) + expect(result.strings).to eq(['hello', 'world']) + expect(result.uuids).to eq(['550e8400-e29b-41d4-a716-446655440000', '00000000-0000-0000-0000-000000000000']) + end end end diff --git a/lib/rb/spec/union_spec.rb b/lib/rb/spec/union_spec.rb index efb38534638..7bf4ca04091 100644 --- a/lib/rb/spec/union_spec.rb +++ b/lib/rb/spec/union_spec.rb @@ -91,6 +91,24 @@ expect(union).not_to eq(other_union) end + it "should equate two unions with the same UUID value" do + union = SpecNamespace::My_union.new(:unique_id, '550e8400-e29b-41d4-a716-446655440000') + other_union = SpecNamespace::My_union.new(:unique_id, '550e8400-e29b-41d4-a716-446655440000') + expect(union).to eq(other_union) + end + + it "should not equate two unions with different UUID values" do + union = SpecNamespace::My_union.new(:unique_id, '550e8400-e29b-41d4-a716-446655440000') + other_union = SpecNamespace::My_union.new(:unique_id, '6ba7b810-9dad-11d1-80b4-00c04fd430c8') + expect(union).not_to eq(other_union) + end + + it "should not equate UUID union with different field type" do + union = SpecNamespace::My_union.new(:unique_id, '550e8400-e29b-41d4-a716-446655440000') + other_union = SpecNamespace::My_union.new(:some_characters, '550e8400-e29b-41d4-a716-446655440000') + expect(union).not_to eq(other_union) + end + it "should inspect properly" do union = SpecNamespace::My_union.new(:integer32, 25) expect(union.inspect).to eq("") @@ -192,23 +210,58 @@ it "should be comparable" do relationships = [ - [0, -1, -1, -1], - [1, 0, -1, -1], - [1, 1, 0, -1], - [1, 1, 1, 0]] + [0, -1, -1, -1, -1, -1], + [1, 0, -1, -1, -1, -1], + [1, 1, 0, -1, -1, -1], + [1, 1, 1, 0, -1, -1], + [1, 1, 1, 1, 0, -1], + [1, 1, 1, 1, 1, 0]] objs = [ SpecNamespace::TestUnion.new(:string_field, "blah"), SpecNamespace::TestUnion.new(:string_field, "blahblah"), SpecNamespace::TestUnion.new(:i32_field, 1), + SpecNamespace::TestUnion.new(:uuid_field, '550e8400-e29b-41d4-a716-446655440000'), + SpecNamespace::TestUnion.new(:uuid_field, '6ba7b810-9dad-11d1-80b4-00c04fd430c8'), SpecNamespace::TestUnion.new()] - for y in 0..3 - for x in 0..3 + objs.size.times do |y| + objs.size.times do |x| # puts "#{objs[y].inspect} <=> #{objs[x].inspect} should == #{relationships[y][x]}" expect(objs[y] <=> objs[x]).to eq(relationships[y][x]) end end end + + it "should handle UUID as union value" do + union = SpecNamespace::My_union.new + union.unique_id = 'ffffffff-ffff-ffff-ffff-ffffffffffff' + + trans = Thrift::MemoryBufferTransport.new + prot = Thrift::CompactProtocol.new(trans) + + union.write(prot) + + result = SpecNamespace::My_union.new + result.read(prot) + + expect(result.unique_id).to eq('ffffffff-ffff-ffff-ffff-ffffffffffff') + expect(result.get_set_field).to eq(:unique_id) + end + + it "should normalize UUID case in union" do + union = SpecNamespace::My_union.new + union.unique_id = '550E8400-E29B-41D4-A716-446655440000' + + trans = Thrift::MemoryBufferTransport.new + prot = Thrift::BinaryProtocol.new(trans) + + union.write(prot) + + result = SpecNamespace::My_union.new + result.read(prot) + + expect(result.unique_id).to eq('550e8400-e29b-41d4-a716-446655440000') + end end end diff --git a/lib/rb/spec/uuid_validation_spec.rb b/lib/rb/spec/uuid_validation_spec.rb new file mode 100644 index 00000000000..1d796e55e92 --- /dev/null +++ b/lib/rb/spec/uuid_validation_spec.rb @@ -0,0 +1,210 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +require 'spec_helper' + +describe 'UUID Validation' do + protocols = [ + ['BinaryProtocol', Thrift::BinaryProtocol], + ] + + if defined?(Thrift::BinaryProtocolAccelerated) + protocols << ['BinaryProtocolAccelerated', Thrift::BinaryProtocolAccelerated] + end + + protocols << ['CompactProtocol', Thrift::CompactProtocol] + + protocols.each do |protocol_name, protocol_class| + describe protocol_name do + before(:each) do + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + end + + context 'valid UUIDs' do + it 'should accept lowercase UUIDs' do + uuid = '550e8400-e29b-41d4-a716-446655440000' + expect { @prot.write_uuid(uuid) }.not_to raise_error + result = @prot.read_uuid + expect(result).to eq(uuid) + end + + it 'should accept uppercase UUIDs' do + uuid = '550E8400-E29B-41D4-A716-446655440000' + expect { @prot.write_uuid(uuid) }.not_to raise_error + result = @prot.read_uuid + # Result should be lowercase + expect(result).to eq('550e8400-e29b-41d4-a716-446655440000') + end + + it 'should accept mixed case UUIDs' do + uuid = '550e8400-E29B-41d4-A716-446655440000' + expect { @prot.write_uuid(uuid) }.not_to raise_error + result = @prot.read_uuid + expect(result).to eq('550e8400-e29b-41d4-a716-446655440000') + end + + it 'should accept all zeros' do + uuid = '00000000-0000-0000-0000-000000000000' + expect { @prot.write_uuid(uuid) }.not_to raise_error + result = @prot.read_uuid + expect(result).to eq(uuid) + end + + it 'should accept all fs' do + uuid = 'ffffffff-ffff-ffff-ffff-ffffffffffff' + expect { @prot.write_uuid(uuid) }.not_to raise_error + result = @prot.read_uuid + expect(result).to eq(uuid) + end + end + + context 'invalid UUIDs' do + it 'should reject nil' do + expect { @prot.write_uuid(nil) }.to raise_error + end + + it 'should reject non-string' do + expect { @prot.write_uuid(12345) }.to raise_error + end + + it 'should reject wrong length' do + expect { @prot.write_uuid('550e8400-e29b-41d4-a716') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject missing hyphens' do + expect { @prot.write_uuid('550e8400e29b41d4a716446655440000') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject hyphens in wrong positions' do + expect { @prot.write_uuid('550e840-0e29b-41d4-a716-446655440000') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject invalid hex characters (g)' do + expect { @prot.write_uuid('550e8400-e29b-41d4-a716-44665544000g') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject invalid hex characters (z)' do + expect { @prot.write_uuid('z50e8400-e29b-41d4-a716-446655440000') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject invalid hex characters (space)' do + expect { @prot.write_uuid('550e8400-e29b-41d4-a716-44665544000 ') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject empty string' do + expect { @prot.write_uuid('') }.to raise_error(Thrift::ProtocolException) + end + + it 'should reject UUID with extra characters' do + expect { @prot.write_uuid('550e8400-e29b-41d4-a716-446655440000x') }.to raise_error(Thrift::ProtocolException) + end + end + + context 'malformed binary data on read' do + it 'should raise error on truncated data' do + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + + # Write only 10 bytes instead of 16 + @trans.write("\x00" * 10) + + expect { @prot.read_uuid }.to raise_error(EOFError) + end + + it 'should raise error on 15 bytes (one byte short)' do + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + + @trans.write("\x00" * 15) + + expect { @prot.read_uuid }.to raise_error(EOFError) + end + + it 'should raise error on empty buffer' do + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + + expect { @prot.read_uuid }.to raise_error(EOFError) + end + end + + context 'multiple UUIDs in sequence' do + it 'should handle 10 UUIDs in sequence' do + uuids = 10.times.map { |i| sprintf('%08x-0000-0000-0000-000000000000', i) } + + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + + uuids.each { |uuid| @prot.write_uuid(uuid) } + + results = 10.times.map { @prot.read_uuid } + expect(results).to eq(uuids) + end + + it 'should handle UUIDs interleaved with other types' do + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + + @prot.write_i32(42) + @prot.write_uuid('550e8400-e29b-41d4-a716-446655440000') + @prot.write_string('test') + @prot.write_uuid('6ba7b810-9dad-11d1-80b4-00c04fd430c8') + @prot.write_i64(123456789) + + expect(@prot.read_i32).to eq(42) + expect(@prot.read_uuid).to eq('550e8400-e29b-41d4-a716-446655440000') + expect(@prot.read_string).to eq('test') + expect(@prot.read_uuid).to eq('6ba7b810-9dad-11d1-80b4-00c04fd430c8') + expect(@prot.read_i64).to eq(123456789) + end + + it 'should handle UUIDs in struct fields context' do + @trans = Thrift::MemoryBufferTransport.new + @prot = protocol_class.new(@trans) + + # Simulate struct field headers + @prot.write_struct_begin('test') + @prot.write_field_begin('uuid1', Thrift::Types::UUID, 1) + @prot.write_uuid('550e8400-e29b-41d4-a716-446655440000') + @prot.write_field_end + @prot.write_field_begin('uuid2', Thrift::Types::UUID, 2) + @prot.write_uuid('6ba7b810-9dad-11d1-80b4-00c04fd430c8') + @prot.write_field_end + @prot.write_field_stop + @prot.write_struct_end + + @prot.read_struct_begin + name, type, id = @prot.read_field_begin + expect(type).to eq(Thrift::Types::UUID) + expect(@prot.read_uuid).to eq('550e8400-e29b-41d4-a716-446655440000') + @prot.read_field_end + + name, type, id = @prot.read_field_begin + expect(type).to eq(Thrift::Types::UUID) + expect(@prot.read_uuid).to eq('6ba7b810-9dad-11d1-80b4-00c04fd430c8') + @prot.read_field_end + + name, type, id = @prot.read_field_begin + expect(type).to eq(Thrift::Types::STOP) + end + end + end + end +end diff --git a/test/rb/Makefile.am b/test/rb/Makefile.am index 9c5d557fd0d..5628a238293 100644 --- a/test/rb/Makefile.am +++ b/test/rb/Makefile.am @@ -17,8 +17,8 @@ # under the License. # -stubs: $(THRIFT) ../v0.16/ThriftTest.thrift ../SmallTest.thrift - $(THRIFT) --gen rb ../v0.16/ThriftTest.thrift +stubs: $(THRIFT) ../ThriftTest.thrift ../SmallTest.thrift + $(THRIFT) --gen rb ../ThriftTest.thrift $(THRIFT) --gen rb ../SmallTest.thrift $(THRIFT) --gen rb ../Recursive.thrift diff --git a/test/rb/integration/TestClient.rb b/test/rb/integration/TestClient.rb index 403cc4cea21..187652988d1 100755 --- a/test/rb/integration/TestClient.rb +++ b/test/rb/integration/TestClient.rb @@ -247,6 +247,14 @@ def get_struct Thrift::Test::Xtruct.new({'string_thing' => 'hi!', 'i32_thing' => 4 }) end + def test_uuid + p 'test_uuid' + val = '00112233-4455-6677-8899-aabbccddeeff' + ret = @client.testUuid(val) + assert_equal(ret, val) + assert_kind_of(String, ret) + end + def test_struct p 'test_struct' ret = @client.testStruct(get_struct) diff --git a/test/rb/integration/TestServer.rb b/test/rb/integration/TestServer.rb index 7caf6a8cb58..3debaa304b4 100755 --- a/test/rb/integration/TestServer.rb +++ b/test/rb/integration/TestServer.rb @@ -29,7 +29,7 @@ class SimpleHandler [:testVoid, :testString, :testBool, :testByte, :testI32, :testI64, :testDouble, :testBinary, :testStruct, :testMap, :testStringMap, :testSet, :testList, :testNest, :testEnum, :testTypedef, - :testEnum, :testTypedef, :testMultiException].each do |meth| + :testEnum, :testTypedef, :testMultiException, :testUuid].each do |meth| define_method(meth) do |thing| p meth