Skip to content

Commit d6a2fd5

Browse files
esiegelbyroot
authored andcommitted
Use nonblocking IO for SSL socket connect.
Attempting to use SSL when the server isn't configured this way will lead to the connection hanging forever. By using nonblocking IO we can respect a timeout. fixes #835
1 parent 331fadc commit d6a2fd5

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

lib/redis/connection/ruby.rb

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,28 @@ def self.connect(host, port, timeout, ssl_params)
266266

267267
ssl_sock = new(tcp_sock, ctx)
268268
ssl_sock.hostname = host
269-
ssl_sock.connect
269+
270+
begin
271+
# Initiate the socket connection in the background. If it doesn't fail
272+
# immediately it will raise an IO::WaitWritable (Errno::EINPROGRESS)
273+
# indicating the connection is in progress.
274+
# Unlike waiting for a tcp socket to connect, you can't time out ssl socket
275+
# connections during the connect phase properly, because IO.select only partially works.
276+
# Instead, you have to retry.
277+
ssl_sock.connect_nonblock
278+
rescue Errno::EAGAIN, Errno::EWOULDBLOCK, IO::WaitReadable
279+
if IO.select([ssl_sock], nil, nil, timeout)
280+
retry
281+
else
282+
raise TimeoutError
283+
end
284+
rescue IO::WaitWritable
285+
if IO.select(nil, [ssl_sock], nil, timeout)
286+
retry
287+
else
288+
raise TimeoutError
289+
end
290+
end
270291

271292
unless ctx.verify_mode == OpenSSL::SSL::VERIFY_NONE
272293
ssl_sock.post_connection_check(host)

test/ssl_test.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ class SslTest < Minitest::Test
66

77
driver(:ruby) do
88

9+
def test_connection_to_non_ssl_server
10+
assert_raises(Redis::CannotConnectError) do
11+
redis = Redis.new(OPTIONS.merge(ssl: true))
12+
redis.ping
13+
end
14+
end
15+
916
def test_verified_ssl_connection
1017
RedisMock.start({ :ping => proc { "+PONG" } }, ssl_server_opts("trusted")) do |port|
1118
redis = Redis.new(:port => port, :ssl => true, :ssl_params => { :ca_file => ssl_ca_file })

0 commit comments

Comments
 (0)