normal	2017-10-13 03:50:07 +0900 (Fri, 13 Oct 2017)

  New Revision: 60172

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=60172

  Log:
    webrick: do not hang acceptor on slow TLS connections
    
    OpenSSL::SSL::SSLSocket#accept may block indefinitely on clients
    which negotiate the TCP connection, but fail (or are slow) to
    negotiate the subsequent TLS handshake.  This prevents the
    multi-threaded WEBrick server from accepting other connections.
    
    Since the TLS handshake (via OpenSSL::SSL::SSLSocket#accept)
    consists of normal read/write traffic over TCP, handle it in the
    per-client thread, instead.
    
    Furthermore, using non-blocking accept() is useful for non-TLS
    sockets anyways because spurious wakeups are possible from
    select(2).
    
    * lib/webrick/server.rb (accept_client): use TCPServer#accept_nonblock
      and remove OpenSSL::SSL::SSLSocket#accept call
    * lib/webrick/server.rb (start_thread): call OpenSSL::SSL::SSLSocket#accept
    * test/webrick/test_ssl_server.rb (test_slow_connect): new test
      [ruby-core:83221] [Bug #14005]

  Modified files:
    trunk/lib/webrick/server.rb
    trunk/test/webrick/test_ssl_server.rb
Index: lib/webrick/server.rb
===================================================================
--- lib/webrick/server.rb	(revision 60171)
+++ lib/webrick/server.rb	(revision 60172)
@@ -251,17 +251,26 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L251
     # the client socket.
 
     def accept_client(svr)
-      sock = nil
-      begin
-        sock = svr.accept
-        Utils::set_non_blocking(sock)
-      rescue Errno::ECONNRESET, Errno::ECONNABORTED,
-             Errno::EPROTO, Errno::EINVAL
-      rescue StandardError => ex
-        msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}"
-        @logger.error msg
+      case sock = svr.to_io.accept_nonblock(exception: false)
+      when :wait_readable
+        nil
+      else
+        if svr.respond_to?(:start_immediately)
+          sock = OpenSSL::SSL::SSLSocket.new(sock, ssl_context)
+          sock.sync_close = true
+          # we cannot do OpenSSL::SSL::SSLSocket#accept here because
+          # a slow client can prevent us from accepting connections
+          # from other clients
+        end
+        sock
       end
-      return sock
+    rescue Errno::ECONNRESET, Errno::ECONNABORTED,
+           Errno::EPROTO, Errno::EINVAL
+      nil
+    rescue StandardError => ex
+      msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}"
+      @logger.error msg
+      nil
     end
 
     ##
@@ -284,6 +293,11 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L293
             @logger.debug "accept: <address unknown>"
             raise
           end
+          if sock.respond_to?(:sync_close=) && @config[:SSLStartImmediately]
+            WEBrick::Utils.timeout(@config[:RequestTimeout]) do
+              sock.accept # OpenSSL::SSL::SSLSocket#accept
+            end
+          end
           call_callback(:AcceptCallback, sock)
           block ? block.call(sock) : run(sock)
         rescue Errno::ENOTCONN
Index: test/webrick/test_ssl_server.rb
===================================================================
--- test/webrick/test_ssl_server.rb	(revision 60171)
+++ test/webrick/test_ssl_server.rb	(revision 60172)
@@ -2,6 +2,7 @@ require "test/unit" https://github.com/ruby/ruby/blob/trunk/test/webrick/test_ssl_server.rb#L2
 require "webrick"
 require "webrick/ssl"
 require_relative "utils"
+require 'timeout'
 
 class TestWEBrickSSLServer < Test::Unit::TestCase
   class Echo < WEBrick::GenericServer
@@ -37,4 +38,30 @@ class TestWEBrickSSLServer < Test::Unit: https://github.com/ruby/ruby/blob/trunk/test/webrick/test_ssl_server.rb#L38
       io.close
     }
   end
+
+  def test_slow_connect
+    poke = lambda do |io, msg|
+      begin
+        sock = OpenSSL::SSL::SSLSocket.new(io)
+        sock.connect
+        sock.puts(msg)
+        assert_equal "#{msg}\n", sock.gets, msg
+      ensure
+        sock&.close
+        io.close
+      end
+    end
+    config = {
+      :SSLEnable => true,
+      :SSLCertName => "/C=JP/O=www.ruby-lang.org/CN=Ruby",
+    }
+    Timeout.timeout(10) do
+      TestWEBrick.start_server(Echo, config) do |server, addr, port, log|
+        outer = TCPSocket.new(addr, port)
+        inner = TCPSocket.new(addr, port)
+        poke.call(inner, 'fast TLS negotiation')
+        poke.call(outer, 'slow TLS negotiation')
+      end
+    end
+  end
 end

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/