Fixed en error preventing Thrift::NonblockingServer from working with SSL

Author: kpumuk

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

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})"

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

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

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