Skip to content

bpo-29970: Make ssh_handshake_timeout None by default #4939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asvetlov merged 7 commits into from
Dec 20, 2017
Merged

bpo-29970: Make ssh_handshake_timeout None by default #4939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a4b641c
Make ssh_handshake_timeout None by default.
asvetlov Dec 20, 2017
2bf9800
Add tests
asvetlov Dec 20, 2017
b6b28f8
Fix typo
asvetlov Dec 20, 2017
099cf5f
Raise ValueError if ssl_handshake_timeout<=0
asvetlov Dec 20, 2017
2f76ef9
Merge branch 'master' into ssl_handshake_timeout
asvetlov Dec 20, 2017
67a2b9f
Fix resource warning
asvetlov Dec 20, 2017
81daa60
assertRaises -> asserRaisesRegex
asvetlov Dec 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions Doc/library/asyncio-eventloop.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ Tasks
Creating connections
--------------------

.. coroutinemethod:: AbstractEventLoop.create_connection(protocol_factory, host=None, port=None, \*, ssl=None, family=0, proto=0, flags=0, sock=None, local_addr=None, server_hostname=None, ssl_handshake_timeout=10.0)
.. coroutinemethod:: AbstractEventLoop.create_connection(protocol_factory, host=None, port=None, \*, ssl=None, family=0, proto=0, flags=0, sock=None, local_addr=None, server_hostname=None, ssl_handshake_timeout=None)

Create a streaming transport connection to a given Internet *host* and
*port*: socket family :py:data:`~socket.AF_INET` or
Expand Down Expand Up @@ -327,6 +327,7 @@ Creating connections

* *ssl_handshake_timeout* is (for an SSL connection) the time in seconds
to wait for the SSL handshake to complete before aborting the connection.
``10.0`` seconds if ``None`` (default).

.. versionadded:: 3.7

Expand Down Expand Up @@ -393,7 +394,7 @@ Creating connections
:ref:`UDP echo server protocol <asyncio-udp-echo-server-protocol>` examples.


.. coroutinemethod:: AbstractEventLoop.create_unix_connection(protocol_factory, path=None, \*, ssl=None, sock=None, server_hostname=None, ssl_handshake_timeout=10.0)
.. coroutinemethod:: AbstractEventLoop.create_unix_connection(protocol_factory, path=None, \*, ssl=None, sock=None, server_hostname=None, ssl_handshake_timeout=None)

Create UNIX connection: socket family :py:data:`~socket.AF_UNIX`, socket
type :py:data:`~socket.SOCK_STREAM`. The :py:data:`~socket.AF_UNIX` socket
Expand Down Expand Up @@ -423,7 +424,7 @@ Creating connections
Creating listening connections
------------------------------

.. coroutinemethod:: AbstractEventLoop.create_server(protocol_factory, host=None, port=None, \*, family=socket.AF_UNSPEC, flags=socket.AI_PASSIVE, sock=None, backlog=100, ssl=None, reuse_address=None, reuse_port=None, ssl_handshake_timeout=10.0)
.. coroutinemethod:: AbstractEventLoop.create_server(protocol_factory, host=None, port=None, \*, family=socket.AF_UNSPEC, flags=socket.AI_PASSIVE, sock=None, backlog=100, ssl=None, reuse_address=None, reuse_port=None, ssl_handshake_timeout=None)

Create a TCP server (socket type :data:`~socket.SOCK_STREAM`) bound to
*host* and *port*.
Expand Down Expand Up @@ -469,6 +470,7 @@ Creating listening connections

* *ssl_handshake_timeout* is (for an SSL server) the time in seconds to wait
for the SSL handshake to complete before aborting the connection.
``10.0`` seconds if ``None`` (default).

.. versionadded:: 3.7

Expand All @@ -488,7 +490,7 @@ Creating listening connections
The *host* parameter can now be a sequence of strings.


.. coroutinemethod:: AbstractEventLoop.create_unix_server(protocol_factory, path=None, \*, sock=None, backlog=100, ssl=None, ssl_handshake_timeout=10.0)
.. coroutinemethod:: AbstractEventLoop.create_unix_server(protocol_factory, path=None, \*, sock=None, backlog=100, ssl=None, ssl_handshake_timeout=None)

Similar to :meth:`AbstractEventLoop.create_server`, but specific to the
socket family :py:data:`~socket.AF_UNIX`.
Expand All @@ -507,7 +509,7 @@ Creating listening connections

The *path* parameter can now be a :class:`~pathlib.Path` object.

.. coroutinemethod:: BaseEventLoop.connect_accepted_socket(protocol_factory, sock, \*, ssl=None, ssl_handshake_timeout=10.0)
.. coroutinemethod:: BaseEventLoop.connect_accepted_socket(protocol_factory, sock, \*, ssl=None, ssl_handshake_timeout=None)

Handle an accepted connection.

Expand All @@ -524,6 +526,7 @@ Creating listening connections

* *ssl_handshake_timeout* is (for an SSL connection) the time in seconds to
wait for the SSL handshake to complete before aborting the connection.
``10.0`` seconds if ``None`` (default).

When completed it returns a ``(transport, protocol)`` pair.

Expand Down
24 changes: 18 additions & 6 deletions Lib/asyncio/base_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import warnings
import weakref

from . import constants
from . import coroutines
from . import events
from . import futures
Expand Down Expand Up @@ -280,7 +279,7 @@ def _make_ssl_transport(
self, rawsock, protocol, sslcontext, waiter=None,
*, server_side=False, server_hostname=None,
extra=None, server=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
"""Create SSL transport."""
raise NotImplementedError

Expand Down Expand Up @@ -643,7 +642,7 @@ async def create_connection(
*, ssl=None, family=0,
proto=0, flags=0, sock=None,
local_addr=None, server_hostname=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
"""Connect to a TCP server.

Create a streaming transport connection to a given Internet host and
Expand Down Expand Up @@ -674,6 +673,10 @@ async def create_connection(
'when using ssl without a host')
server_hostname = host

if ssl_handshake_timeout is not None and not ssl:
raise ValueError(
'ssl_handshake_timeout is only meaningful with ssl')

if host is not None or port is not None:
if sock is not None:
raise ValueError(
Expand Down Expand Up @@ -769,7 +772,7 @@ async def create_connection(
async def _create_connection_transport(
self, sock, protocol_factory, ssl,
server_hostname, server_side=False,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):

sock.setblocking(False)

Expand Down Expand Up @@ -948,7 +951,7 @@ async def create_server(
ssl=None,
reuse_address=None,
reuse_port=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
"""Create a TCP server.

The host parameter can be a string, in that case the TCP server is
Expand All @@ -966,6 +969,11 @@ async def create_server(
"""
if isinstance(ssl, bool):
raise TypeError('ssl argument must be an SSLContext or None')

if ssl_handshake_timeout is not None and ssl is None:
raise ValueError(
'ssl_handshake_timeout is only meaningful with ssl')

if host is not None or port is not None:
if sock is not None:
raise ValueError(
Expand Down Expand Up @@ -1046,7 +1054,7 @@ async def create_server(
async def connect_accepted_socket(
self, protocol_factory, sock,
*, ssl=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
"""Handle an accepted connection.

This is used by servers that accept connections outside of
Expand All @@ -1058,6 +1066,10 @@ async def connect_accepted_socket(
if sock.type != socket.SOCK_STREAM:
raise ValueError(f'A Stream Socket was expected, got {sock!r}')

if ssl_handshake_timeout is not None and not ssl:
raise ValueError(
'ssl_handshake_timeout is only meaningful with ssl')

transport, protocol = await self._create_connection_transport(
sock, protocol_factory, ssl, '', server_side=True,
ssl_handshake_timeout=ssl_handshake_timeout)
Expand Down
9 changes: 4 additions & 5 deletions Lib/asyncio/events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import sys
import threading

from . import constants
from . import format_helpers


Expand Down Expand Up @@ -255,15 +254,15 @@ async def create_connection(
*, ssl=None, family=0, proto=0,
flags=0, sock=None, local_addr=None,
server_hostname=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
raise NotImplementedError

async def create_server(
self, protocol_factory, host=None, port=None,
*, family=socket.AF_UNSPEC,
flags=socket.AI_PASSIVE, sock=None, backlog=100,
ssl=None, reuse_address=None, reuse_port=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
"""A coroutine which creates a TCP server bound to host and port.

The return value is a Server object which can be used to stop
Expand Down Expand Up @@ -310,13 +309,13 @@ async def create_unix_connection(
self, protocol_factory, path=None, *,
ssl=None, sock=None,
server_hostname=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
raise NotImplementedError

async def create_unix_server(
self, protocol_factory, path=None, *,
sock=None, backlog=100, ssl=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
"""A coroutine which creates a UNIX Domain Socket server.

The return value is a Server object, which can be used to stop
Expand Down
4 changes: 2 additions & 2 deletions Lib/asyncio/proactor_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ def _make_ssl_transport(
self, rawsock, protocol, sslcontext, waiter=None,
*, server_side=False, server_hostname=None,
extra=None, server=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
ssl_protocol = sslproto.SSLProtocol(
self, protocol, sslcontext, waiter,
server_side, server_hostname,
Expand Down Expand Up @@ -491,7 +491,7 @@ def _write_to_self(self):

def _start_serving(self, protocol_factory, sock,
sslcontext=None, server=None, backlog=100,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):

def loop(f=None):
try:
Expand Down
9 changes: 8 additions & 1 deletion Lib/asyncio/sslproto.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,10 +402,17 @@ class SSLProtocol(protocols.Protocol):
def __init__(self, loop, app_protocol, sslcontext, waiter,
server_side=False, server_hostname=None,
call_connection_made=True,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
if ssl is None:
raise RuntimeError('stdlib ssl module not available')

if ssl_handshake_timeout is None:
ssl_handshake_timeout = constants.SSL_HANDSHAKE_TIMEOUT
elif ssl_handshake_timeout <= 0:
raise ValueError(
f"ssl_handshake_timeout should be a positive number, "
f"got {ssl_handshake_timeout}")

if not sslcontext:
sslcontext = _create_transport_context(
server_side, server_hostname)
Expand Down
11 changes: 9 additions & 2 deletions Lib/asyncio/unix_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ async def create_unix_connection(
self, protocol_factory, path=None, *,
ssl=None, sock=None,
server_hostname=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
assert server_hostname is None or isinstance(server_hostname, str)
if ssl:
if server_hostname is None:
Expand All @@ -205,6 +205,9 @@ async def create_unix_connection(
else:
if server_hostname is not None:
raise ValueError('server_hostname is only meaningful with ssl')
if ssl_handshake_timeout is not None:
raise ValueError(
'ssl_handshake_timeout is only meaningful with ssl')

if path is not None:
if sock is not None:
Expand Down Expand Up @@ -237,10 +240,14 @@ async def create_unix_connection(
async def create_unix_server(
self, protocol_factory, path=None, *,
sock=None, backlog=100, ssl=None,
ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT):
ssl_handshake_timeout=None):
if isinstance(ssl, bool):
raise TypeError('ssl argument must be an SSLContext or None')

if ssl_handshake_timeout is not None and not ssl:
raise ValueError(
'ssl_handshake_timeout is only meaningful with ssl')

if path is not None:
if sock is not None:
raise ValueError(
Expand Down
16 changes: 16 additions & 0 deletions Lib/test/test_asyncio/test_base_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,14 @@ def test_create_server_wrong_sock(self):
'A Stream Socket was expected'):
self.loop.run_until_complete(coro)

def test_create_server_ssl_timeout_for_plain_socket(self):
coro = self.loop.create_server(
MyProto, 'example.com', 80, ssl_handshake_timeout=1)
with self.assertRaisesRegex(
ValueError,
'ssl_handshake_timeout is only meaningful with ssl'):
self.loop.run_until_complete(coro)

@unittest.skipUnless(hasattr(socket, 'SOCK_NONBLOCK'),
'no socket.SOCK_NONBLOCK (linux only)')
def test_create_server_stream_bittype(self):
Expand Down Expand Up @@ -1362,6 +1370,14 @@ def test_create_connection_ssl_server_hostname_errors(self):
self.addCleanup(sock.close)
self.assertRaises(ValueError, self.loop.run_until_complete, coro)

def test_create_connection_ssl_timeout_for_plain_socket(self):
coro = self.loop.create_connection(
MyProto, 'example.com', 80, ssl_handshake_timeout=1)
with self.assertRaisesRegex(
ValueError,
'ssl_handshake_timeout is only meaningful with ssl'):
self.loop.run_until_complete(coro)

def test_create_server_empty_host(self):
# if host is empty string use None instead
host = object()
Expand Down
10 changes: 10 additions & 0 deletions Lib/test/test_asyncio/test_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,16 @@ def test_ssl_connect_accepted_socket(self):

self.test_connect_accepted_socket(server_context, client_context)

def test_connect_accepted_socket_ssl_timeout_for_plain_socket(self):
sock = socket.socket()
self.addCleanup(sock.close)
coro = self.loop.connect_accepted_socket(
MyProto, sock, ssl_handshake_timeout=1)
with self.assertRaisesRegex(
ValueError,
'ssl_handshake_timeout is only meaningful with ssl'):
self.loop.run_until_complete(coro)

@mock.patch('asyncio.base_events.socket')
def create_server_multiple_hosts(self, family, hosts, mock_sock):
@asyncio.coroutine
Expand Down
16 changes: 16 additions & 0 deletions Lib/test/test_asyncio/test_sslproto.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,22 @@ def test_handshake_timeout(self):
self.loop.run_until_complete(tasks.sleep(0.2, loop=self.loop))
self.assertTrue(transport.abort.called)

def test_handshake_timeout_zero(self):
sslcontext = test_utils.dummy_ssl_context()
app_proto = mock.Mock()
waiter = mock.Mock()
with self.assertRaisesRegex(ValueError, 'a positive number'):
sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter,
ssl_handshake_timeout=0)

def test_handshake_timeout_negative(self):
sslcontext = test_utils.dummy_ssl_context()
app_proto = mock.Mock()
waiter = mock.Mock()
with self.assertRaisesRegex(ValueError, 'a positive number'):
sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter,
ssl_handshake_timeout=-10)

def test_eof_received_waiter(self):
waiter = asyncio.Future(loop=self.loop)
ssl_proto = self.ssl_protocol(waiter)
Expand Down
17 changes: 17 additions & 0 deletions Lib/test/test_asyncio/test_unix_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,14 @@ def test_create_unix_server_path_stream_bittype(self):
finally:
os.unlink(fn)

def test_create_unix_server_ssl_timeout_with_plain_sock(self):
coro = self.loop.create_unix_server(lambda: None, path='spam',
ssl_handshake_timeout=1)
with self.assertRaisesRegex(
ValueError,
'ssl_handshake_timeout is only meaningful with ssl'):
self.loop.run_until_complete(coro)

def test_create_unix_connection_path_inetsock(self):
sock = socket.socket()
with sock:
Expand Down Expand Up @@ -383,6 +391,15 @@ def test_create_unix_connection_ssl_noserverhost(self):

self.loop.run_until_complete(coro)

def test_create_unix_connection_ssl_timeout_with_plain_sock(self):
coro = self.loop.create_unix_connection(lambda: None, path='spam',
ssl_handshake_timeout=1)
with self.assertRaisesRegex(
ValueError,
'ssl_handshake_timeout is only meaningful with ssl'):
self.loop.run_until_complete(coro)



class UnixReadPipeTransportTests(test_utils.TestCase):

Expand Down