summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2017-12-14 13:31:41 +0000
committernagachika <nagachika@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2017-12-14 13:31:41 +0000
commit2e728d51e70ed3756ad760c687a08b8487b0112f (patch)
tree75dcef6d863b26b1bfe1be64362c1ab2a6524ae7
parent096db35abad5e9288b2563c29a98fcf7ebce7715 (diff)
merge revision(s) 60123,60172,60189,60208,60210,60211: [Backport #14005]
webrick: avoid unnecessary IO#sync= call Sockets and pipes are always created with FMODE_SYNC flag already set (otherwise many things would be broken). * lib/webrick/server.rb (accept_client): remove unnecessary IO#sync= call 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] webrick: fix up r60172 By making the socket non-blocking in r60172, TLS/SSL negotiation via the SSL_accept function must handle non-blocking sockets properly and retry on SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE. OpenSSL::SSL::SSLSocket#accept cannot do that properly with a non-blocking socket, so it must use non-blocking logic of OpenSSL::SSL::SSLSocket#accept_nonblock. Thanks to MSP-Greg (Greg L) for finding this. * lib/webrick/server.rb (start_thread): use SSL_accept properly with non-blocking socket. [Bug #14013] [Bug #14005] webrick: fix up r60172 and revert r60189 Thanks to MSP-Greg (Greg L) for helping with this. * lib/webrick/server.rb (start_thread): ignore ECONNRESET, ECONNABORTED, EPROTO, and EINVAL on TLS negotiation errors the same way they were ignored before r60172 in the accept_client method of the main acceptor thread. [Bug #14013] [Bug #14005] webrick: fix up r60172 and r60208 Thanks to MSP-Greg (Greg L) for helping with this. * lib/webrick/server.rb (start_thread): fix non-local return introduced in r60208 webrick: fix up r60172 and r60210 Thanks to MSP-Greg (Greg L) for helping with this. * lib/webrick/server.rb (start_thread): properly fix non-local return introduced in r60208 and r60210 git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@61239 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--lib/webrick/server.rb40
-rw-r--r--test/webrick/test_ssl_server.rb27
-rw-r--r--version.h2
3 files changed, 57 insertions, 12 deletions
diff --git a/lib/webrick/server.rb b/lib/webrick/server.rb
index 2e7654d9bb..924ec521be 100644
--- a/lib/webrick/server.rb
+++ b/lib/webrick/server.rb
@@ -252,18 +252,26 @@ module WEBrick
# the client socket.
def accept_client(svr)
- sock = nil
- begin
- sock = svr.accept
- sock.sync = true
- 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
##
@@ -286,6 +294,16 @@ module WEBrick
@logger.debug "accept: <address unknown>"
raise
end
+ if sock.respond_to?(:sync_close=) && @config[:SSLStartImmediately]
+ WEBrick::Utils.timeout(@config[:RequestTimeout]) do
+ begin
+ sock.accept # OpenSSL::SSL::SSLSocket#accept
+ rescue Errno::ECONNRESET, Errno::ECONNABORTED,
+ Errno::EPROTO, Errno::EINVAL
+ Thread.exit
+ end
+ end
+ end
call_callback(:AcceptCallback, sock)
block ? block.call(sock) : run(sock)
rescue Errno::ENOTCONN
diff --git a/test/webrick/test_ssl_server.rb b/test/webrick/test_ssl_server.rb
index f6d5250365..64cf25f78b 100644
--- a/test/webrick/test_ssl_server.rb
+++ b/test/webrick/test_ssl_server.rb
@@ -2,6 +2,7 @@ require "test/unit"
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::TestCase
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
diff --git a/version.h b/version.h
index a2401e6ec6..4ca616a216 100644
--- a/version.h
+++ b/version.h
@@ -1,6 +1,6 @@
#define RUBY_VERSION "2.4.3"
#define RUBY_RELEASE_DATE "2017-12-14"
-#define RUBY_PATCHLEVEL 203
+#define RUBY_PATCHLEVEL 204
#define RUBY_RELEASE_YEAR 2017
#define RUBY_RELEASE_MONTH 12