diff options
author | Chris Donati <chris.donati@appscale.com> | 2018-11-09 12:47:42 -0800 |
---|---|---|
committer | Stephen SORRIAUX <stephen.sorriaux@gmail.com> | 2018-11-09 21:47:42 +0100 |
commit | 8c5ce11883a86b15bc6497706cf36abf1b36145f (patch) | |
tree | ad829f7db2a82da4439e7cb67d90d89b4d51e4d1 | |
parent | 7935a3af003684ddc259820900560c99ca477f5a (diff) | |
download | kazoo-8c5ce11883a86b15bc6497706cf36abf1b36145f.tar.gz |
fix(core): ensure timeout argument is positive (#534)
Previously, a gap between calls to `time.time()` could lead to a
situation where the current time was less than `end` during the
`while` condition, but it was greater than `end` when assigning
a value to `timeout_at`.
Add tests to ensure a socket.error is raised instead of passing a
nonpositive value as a timeout to socket.create_connection.
-rw-r--r-- | kazoo/handlers/utils.py | 7 | ||||
-rw-r--r-- | kazoo/tests/test_utils.py | 78 |
2 files changed, 84 insertions, 1 deletions
diff --git a/kazoo/handlers/utils.py b/kazoo/handlers/utils.py index 0d36506..2517390 100644 --- a/kazoo/handlers/utils.py +++ b/kazoo/handlers/utils.py @@ -198,8 +198,13 @@ def create_tcp_connection(module, address, timeout=None, end = time.time() + timeout sock = None - while end is None or time.time() < end: + while True: timeout_at = end if end is None else end - time.time() + # The condition is not '< 0' here because socket.settimeout treats 0 as + # a special case to put the socket in non-blocking mode. + if timeout_at is not None and timeout_at <= 0: + break + if use_ssl: # Disallow use of SSLv2 and V3 (meaning we require TLSv1.0+) context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) diff --git a/kazoo/tests/test_utils.py b/kazoo/tests/test_utils.py new file mode 100644 index 0000000..260d993 --- /dev/null +++ b/kazoo/tests/test_utils.py @@ -0,0 +1,78 @@ +import unittest + +from mock import patch +from nose import SkipTest + +try: + from kazoo.handlers.eventlet import green_socket as socket + EVENTLET_HANDLER_AVAILABLE = True +except ImportError: + EVENTLET_HANDLER_AVAILABLE = False + + +class TestCreateTCPConnection(unittest.TestCase): + def test_timeout_arg(self): + from kazoo.handlers import utils + from kazoo.handlers.utils import create_tcp_connection, socket, time + + with patch.object(socket, 'create_connection') as create_connection: + with patch.object(utils, '_set_default_tcpsock_options'): + # Ensure a gap between calls to time.time() does not result in + # create_connection being called with a negative timeout + # argument. + with patch.object(time, 'time', side_effect=range(10)): + create_tcp_connection(socket, ('127.0.0.1', 2181), + timeout=1.5) + + for call_args in create_connection.call_args_list: + timeout = call_args[0][1] + assert timeout >= 0, 'socket timeout must be nonnegative' + + def test_timeout_arg_eventlet(self): + if not EVENTLET_HANDLER_AVAILABLE: + raise SkipTest('eventlet handler not available.') + + from kazoo.handlers import utils + from kazoo.handlers.utils import create_tcp_connection, time + + with patch.object(socket, 'create_connection') as create_connection: + with patch.object(utils, '_set_default_tcpsock_options'): + # Ensure a gap between calls to time.time() does not result in + # create_connection being called with a negative timeout + # argument. + with patch.object(time, 'time', side_effect=range(10)): + create_tcp_connection(socket, ('127.0.0.1', 2181), + timeout=1.5) + + for call_args in create_connection.call_args_list: + timeout = call_args[0][1] + assert timeout >= 0, 'socket timeout must be nonnegative' + + def test_slow_connect(self): + # Currently, create_tcp_connection will raise a socket timeout if it + # takes longer than the specified "timeout" to create a connection. + # In the future, "timeout" might affect only the created socket and not + # the time it takes to create it. + from kazoo.handlers.utils import create_tcp_connection, socket, time + + # Simulate a second passing between calls to check the current time. + with patch.object(time, 'time', side_effect=range(10)): + with self.assertRaises(socket.error): + create_tcp_connection(socket, ('127.0.0.1', 2181), timeout=0.5) + + def test_negative_timeout(self): + from kazoo.handlers.utils import create_tcp_connection, socket + with self.assertRaises(socket.error): + create_tcp_connection(socket, ('127.0.0.1', 2181), timeout=-1) + + def test_zero_timeout(self): + # Rather than pass '0' through as a timeout to + # socket.create_connection, create_tcp_connection should raise + # socket.error. This is because the socket library treats '0' as an + # indicator to create a non-blocking socket. + from kazoo.handlers.utils import create_tcp_connection, socket, time + + # Simulate no time passing between calls to check the current time. + with patch.object(time, 'time', return_value=time.time()): + with self.assertRaises(socket.error): + create_tcp_connection(socket, ('127.0.0.1', 2181), timeout=0) |