From 0399aee69f09af7c9c13fe1436447133d3a1b440 Mon Sep 17 00:00:00 2001 From: Dmytro Shteflyuk Date: Fri, 13 Feb 2026 14:12:43 -0500 Subject: [PATCH] Fixed en error preventing Thrift::NonblockingServer from working with SSL --- lib/rb/lib/thrift/transport/server_socket.rb | 2 +- lib/rb/lib/thrift/transport/socket.rb | 4 +- lib/rb/spec/nonblocking_server_spec.rb | 99 ++++++++++++++++++++ lib/rb/spec/ssl_server_socket_spec.rb | 12 +++ lib/rb/spec/ssl_socket_spec.rb | 10 ++ 5 files changed, 125 insertions(+), 2 deletions(-) diff --git a/lib/rb/lib/thrift/transport/server_socket.rb b/lib/rb/lib/thrift/transport/server_socket.rb index 60e4b8a105e..b21c65fd933 100644 --- a/lib/rb/lib/thrift/transport/server_socket.rb +++ b/lib/rb/lib/thrift/transport/server_socket.rb @@ -60,7 +60,7 @@ def closed? end def to_io - @handle || raise(IOError, 'closed stream') + @handle&.to_io || raise(IOError, 'closed stream') end def to_s diff --git a/lib/rb/lib/thrift/transport/socket.rb b/lib/rb/lib/thrift/transport/socket.rb index b3476ea5533..7d01e2e3b03 100644 --- a/lib/rb/lib/thrift/transport/socket.rb +++ b/lib/rb/lib/thrift/transport/socket.rb @@ -134,7 +134,9 @@ def close @handle = nil end - alias to_io handle + def to_io + @handle&.to_io || raise(IOError, 'closed stream') + end def to_s "socket(#{@host}:#{@port})" diff --git a/lib/rb/spec/nonblocking_server_spec.rb b/lib/rb/spec/nonblocking_server_spec.rb index abf1597ea52..cfc60ff204b 100644 --- a/lib/rb/spec/nonblocking_server_spec.rb +++ b/lib/rb/spec/nonblocking_server_spec.rb @@ -18,6 +18,7 @@ # require 'spec_helper' +require 'timeout' describe 'NonblockingServer' do @@ -260,4 +261,102 @@ def setup_client_thread(result) expect(@server_thread.join(2)).not_to be_nil end end + + describe "#{Thrift::NonblockingServer} with TLS transport" do + before(:each) do + @port = available_port + handler = Handler.new + processor = SpecNamespace::NonblockingService::Processor.new(handler) + @transport = Thrift::SSLServerSocket.new('localhost', @port, create_server_ssl_context) + transport_factory = Thrift::FramedTransportFactory.new + logger = Logger.new(STDERR) + logger.level = Logger::WARN + @server = Thrift::NonblockingServer.new(processor, @transport, transport_factory, nil, 5, logger) + handler.server = @server + + @server_thread = Thread.new(Thread.current) do |master_thread| + begin + @server.serve + rescue => e + master_thread.raise e + end + end + + @clients = [] + wait_until_listening + end + + after(:each) do + @clients.each(&:close) + @server.shutdown if @server + @server_thread.join(2) if @server_thread + @transport.close if @transport + end + + it "should handle requests over TLS" do + expect(@server_thread).to be_alive + + client = setup_tls_client + expect(client.greeting(true)).to eq(SpecNamespace::Hello.new) + + @server.shutdown + expect(@server_thread.join(2)).to be_an_instance_of(Thread) + end + + def setup_tls_client + transport = Thrift::FramedTransport.new( + Thrift::SSLSocket.new('localhost', @port, nil, create_client_ssl_context) + ) + protocol = Thrift::BinaryProtocol.new(transport) + client = SpecNamespace::NonblockingService::Client.new(protocol) + transport.open + @clients << transport + client + end + + def wait_until_listening + Timeout.timeout(2) do + until @transport.handle + raise "Server thread exited unexpectedly" unless @server_thread.alive? + sleep 0.01 + end + end + end + + def available_port + TCPServer.open('localhost', 0) { |server| server.addr[1] } + end + + def ssl_keys_dir + File.expand_path('../../../test/keys', __dir__) + end + + def create_server_ssl_context + OpenSSL::SSL::SSLContext.new.tap do |ctx| + ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + if ctx.respond_to?(:min_version=) && OpenSSL::SSL.const_defined?(:TLS1_2_VERSION) + ctx.min_version = OpenSSL::SSL::TLS1_2_VERSION + end + ctx.ca_file = File.join(ssl_keys_dir, 'CA.pem') + ctx.cert = OpenSSL::X509::Certificate.new(File.read(File.join(ssl_keys_dir, 'server.crt'))) + ctx.cert_store = OpenSSL::X509::Store.new + ctx.cert_store.add_file(File.join(ssl_keys_dir, 'client.pem')) + ctx.key = OpenSSL::PKey::RSA.new(File.read(File.join(ssl_keys_dir, 'server.key'))) + end + end + + def create_client_ssl_context + OpenSSL::SSL::SSLContext.new.tap do |ctx| + ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER + if ctx.respond_to?(:min_version=) && OpenSSL::SSL.const_defined?(:TLS1_2_VERSION) + ctx.min_version = OpenSSL::SSL::TLS1_2_VERSION + end + ctx.ca_file = File.join(ssl_keys_dir, 'CA.pem') + ctx.cert = OpenSSL::X509::Certificate.new(File.read(File.join(ssl_keys_dir, 'client.crt'))) + ctx.cert_store = OpenSSL::X509::Store.new + ctx.cert_store.add_file(File.join(ssl_keys_dir, 'server.pem')) + ctx.key = OpenSSL::PKey::RSA.new(File.read(File.join(ssl_keys_dir, 'client.key'))) + end + end + end end diff --git a/lib/rb/spec/ssl_server_socket_spec.rb b/lib/rb/spec/ssl_server_socket_spec.rb index 82e65184326..20cfdf485a2 100644 --- a/lib/rb/spec/ssl_server_socket_spec.rb +++ b/lib/rb/spec/ssl_server_socket_spec.rb @@ -27,6 +27,18 @@ @socket = Thrift::SSLServerSocket.new(1234) end + it "should delegate to_io to the underlying SSL server handle" do + tcp_server = double("TCPServer") + ssl_server = double("SSLServer") + + allow(TCPServer).to receive(:new).with(nil, 1234).and_return(tcp_server) + allow(OpenSSL::SSL::SSLServer).to receive(:new).with(tcp_server, nil).and_return(ssl_server) + allow(ssl_server).to receive(:to_io).and_return(tcp_server) + + @socket.listen + expect(@socket.to_io).to eq(tcp_server) + end + it "should provide a reasonable to_s" do expect(@socket.to_s).to eq("ssl(socket(:1234))") end diff --git a/lib/rb/spec/ssl_socket_spec.rb b/lib/rb/spec/ssl_socket_spec.rb index 808d8d512ee..eb68bcff770 100644 --- a/lib/rb/spec/ssl_socket_spec.rb +++ b/lib/rb/spec/ssl_socket_spec.rb @@ -35,6 +35,7 @@ allow(@handle).to receive(:connect_nonblock) allow(@handle).to receive(:close) allow(@handle).to receive(:post_connection_check) + allow(@handle).to receive(:to_io).and_return(@simple_socket_handle) allow(::Socket).to receive(:new).and_return(@simple_socket_handle) allow(OpenSSL::SSL::SSLSocket).to receive(:new).and_return(@handle) @@ -71,6 +72,15 @@ expect(Thrift::SSLSocket.new('localhost', 8080, 5, @context).ssl_context).to eq(@context) end + it "should delegate to_io to the underlying SSL socket handle" do + @socket.open + expect(@socket.to_io).to eq(@simple_socket_handle) + end + + it "should raise IOError when to_io is called on a closed stream" do + expect { @socket.to_io }.to raise_error(IOError, 'closed stream') + end + it "should provide a reasonable to_s" do expect(Thrift::SSLSocket.new('myhost', 8090).to_s).to eq("ssl(socket(myhost:8090))") end