From 9e771fd05245d4a62e727779d5ed867a561f1952 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Sat, 9 Jul 2016 14:11:04 -0400 Subject: [PATCH 01/12] Provide an API to handle connections that are accepted outside of asyncio. This addresses: https://bugs.python.org/issue27392 --- asyncio/base_events.py | 30 ++++++++++++++++++++ tests/test_events.py | 64 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/asyncio/base_events.py b/asyncio/base_events.py index 172a463e..35c98750 100644 --- a/asyncio/base_events.py +++ b/asyncio/base_events.py @@ -979,6 +979,36 @@ def create_server(self, protocol_factory, host=None, port=None, logger.info("%r is serving", server) return server + @coroutine + def handle_connection(self, protocol_factory, sock, ssl=None): + """Handle an accepted connection + + This is used by servers that accept connections outside of + asyncio but that use asyncio to handle connections. + + This method is a coroutine. When completed, the coroutine + returns a (transport, protocol) pair. + """ + sock.setblocking(False) + + protocol = protocol_factory() + waiter = self.create_future() + if ssl: + sslcontext = None if isinstance(ssl, bool) else ssl + transport = self._make_ssl_transport( + sock, protocol, sslcontext, waiter, + server_side=True) + else: + transport = self._make_socket_transport(sock, protocol, waiter) + + try: + yield from waiter + except: + transport.close() + raise + + return transport, protocol + @coroutine def connect_read_pipe(self, protocol_factory, pipe): protocol = protocol_factory() diff --git a/tests/test_events.py b/tests/test_events.py index d0777758..336f9161 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -568,6 +568,70 @@ def test_create_connection(self): lambda: MyProto(loop=self.loop), *httpd.address) self._basetest_create_connection(conn_fut) + def test_handle_connection(self, server_ssl=None, client_ssl=None): + loop = self.loop + + class MyProto(MyBaseProto): + + def connection_lost(self, exc): + super().connection_lost(exc) + loop.call_soon(loop.stop) + + def data_received(self, data): + super().data_received(data) + self.transport.write(expected_response) + + lsock = socket.socket() + lsock.bind(('127.0.0.1', 0)) + lsock.listen(1) + addr = lsock.getsockname() + + message = b'test data' + reponse = None + expected_response = b'roger' + + def client(): + global response + csock = socket.socket() + if client_ssl is not None: + csock = client_ssl.wrap_socket(csock) + csock.connect(addr) + csock.sendall(message) + response = csock.recv(99) + csock.close() + + thread = threading.Thread(target=client, daemon=True) + thread.start() + + conn, _ = lsock.accept() + proto = MyProto(loop=loop) + proto.loop = loop + f = loop.create_task( + loop.handle_connection((lambda : proto), conn, ssl=server_ssl)) + loop.run_forever() + conn.close() + lsock.close() + + thread.join(1) + self.assertFalse(thread.is_alive()) + self.assertEqual(proto.state, 'CLOSED') + self.assertEqual(proto.nbytes, len(message)) + self.assertEqual(response, expected_response) + + if ssl is not None: + def test_handle_ssl_connection(self): + server_context = ssl.create_default_context( + ssl.Purpose.CLIENT_AUTH) + server_context.load_cert_chain(ONLYCERT, ONLYKEY) + server_context.check_hostname = False + server_context.verify_mode = ssl.CERT_NONE + + client_context = ssl.create_default_context() + client_context.check_hostname = False + client_context.verify_mode = ssl.CERT_NONE + + self.test_handle_connection(server_context, client_context) + @unittest.skipUnless(hasattr(socket, 'AF_UNIX'), 'No UNIX Sockets') def test_create_unix_connection(self): # Issue #20682: On Mac OS X Tiger, getsockname() returns a From e2f7fe04692dcb8738f3f27f503b6cd142d17ca0 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Sat, 9 Jul 2016 14:49:01 -0400 Subject: [PATCH 02/12] use SSLContext not create_default_context because Python 3.3 --- tests/test_events.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_events.py b/tests/test_events.py index 336f9161..643f1664 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -620,12 +620,13 @@ def client(): if ssl is not None: def test_handle_ssl_connection(self): - server_context = ssl.create_default_context( - ssl.Purpose.CLIENT_AUTH) + + server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) server_context.load_cert_chain(ONLYCERT, ONLYKEY) server_context.check_hostname = False server_context.verify_mode = ssl.CERT_NONE + client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) client_context = ssl.create_default_context() client_context.check_hostname = False client_context.verify_mode = ssl.CERT_NONE From 8b8cbd07b8d42f6b17b1c36f47cb22cf30c356bb Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Sat, 9 Jul 2016 14:52:41 -0400 Subject: [PATCH 03/12] more Python 3.3 --- tests/test_events.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_events.py b/tests/test_events.py index 643f1664..d73a93f5 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -623,12 +623,14 @@ def test_handle_ssl_connection(self): server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) server_context.load_cert_chain(ONLYCERT, ONLYKEY) - server_context.check_hostname = False + if sys.version_info > (3, 3): + server_context.check_hostname = False server_context.verify_mode = ssl.CERT_NONE client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) client_context = ssl.create_default_context() - client_context.check_hostname = False + if sys.version_info > (3, 3): + client_context.check_hostname = False client_context.verify_mode = ssl.CERT_NONE self.test_handle_connection(server_context, client_context) From 5d9bf64d74912647b8e194a9cc695e096c535e37 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Sat, 9 Jul 2016 14:57:27 -0400 Subject: [PATCH 04/12] more Python 3.3 --- tests/test_events.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_events.py b/tests/test_events.py index d73a93f5..ee5019d3 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -623,13 +623,12 @@ def test_handle_ssl_connection(self): server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) server_context.load_cert_chain(ONLYCERT, ONLYKEY) - if sys.version_info > (3, 3): + if hasattr(server_context, 'check_hostname'): server_context.check_hostname = False server_context.verify_mode = ssl.CERT_NONE client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - client_context = ssl.create_default_context() - if sys.version_info > (3, 3): + if hasattr(server_context, 'check_hostname'): client_context.check_hostname = False client_context.verify_mode = ssl.CERT_NONE From 27f749c6bf23c018afaf1b2540eac1ada8c67eb0 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Sat, 9 Jul 2016 15:11:18 -0400 Subject: [PATCH 05/12] move test_handle_connection and test_handle_ssl_connection below test_create_connection tests --- tests/test_events.py | 132 +++++++++++++++++++++---------------------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/tests/test_events.py b/tests/test_events.py index ee5019d3..3e5e9e76 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -568,72 +568,6 @@ def test_create_connection(self): lambda: MyProto(loop=self.loop), *httpd.address) self._basetest_create_connection(conn_fut) - def test_handle_connection(self, server_ssl=None, client_ssl=None): - loop = self.loop - - class MyProto(MyBaseProto): - - def connection_lost(self, exc): - super().connection_lost(exc) - loop.call_soon(loop.stop) - - def data_received(self, data): - super().data_received(data) - self.transport.write(expected_response) - - lsock = socket.socket() - lsock.bind(('127.0.0.1', 0)) - lsock.listen(1) - addr = lsock.getsockname() - - message = b'test data' - reponse = None - expected_response = b'roger' - - def client(): - global response - csock = socket.socket() - if client_ssl is not None: - csock = client_ssl.wrap_socket(csock) - csock.connect(addr) - csock.sendall(message) - response = csock.recv(99) - csock.close() - - thread = threading.Thread(target=client, daemon=True) - thread.start() - - conn, _ = lsock.accept() - proto = MyProto(loop=loop) - proto.loop = loop - f = loop.create_task( - loop.handle_connection((lambda : proto), conn, ssl=server_ssl)) - loop.run_forever() - conn.close() - lsock.close() - - thread.join(1) - self.assertFalse(thread.is_alive()) - self.assertEqual(proto.state, 'CLOSED') - self.assertEqual(proto.nbytes, len(message)) - self.assertEqual(response, expected_response) - - if ssl is not None: - def test_handle_ssl_connection(self): - - server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - server_context.load_cert_chain(ONLYCERT, ONLYKEY) - if hasattr(server_context, 'check_hostname'): - server_context.check_hostname = False - server_context.verify_mode = ssl.CERT_NONE - - client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - if hasattr(server_context, 'check_hostname'): - client_context.check_hostname = False - client_context.verify_mode = ssl.CERT_NONE - - self.test_handle_connection(server_context, client_context) - @unittest.skipUnless(hasattr(socket, 'AF_UNIX'), 'No UNIX Sockets') def test_create_unix_connection(self): # Issue #20682: On Mac OS X Tiger, getsockname() returns a @@ -810,6 +744,72 @@ def test_create_connection_local_addr_in_use(self): self.assertEqual(cm.exception.errno, errno.EADDRINUSE) self.assertIn(str(httpd.address), cm.exception.strerror) + def test_handle_connection(self, server_ssl=None, client_ssl=None): + loop = self.loop + + class MyProto(MyBaseProto): + + def connection_lost(self, exc): + super().connection_lost(exc) + loop.call_soon(loop.stop) + + def data_received(self, data): + super().data_received(data) + self.transport.write(expected_response) + + lsock = socket.socket() + lsock.bind(('127.0.0.1', 0)) + lsock.listen(1) + addr = lsock.getsockname() + + message = b'test data' + reponse = None + expected_response = b'roger' + + def client(): + global response + csock = socket.socket() + if client_ssl is not None: + csock = client_ssl.wrap_socket(csock) + csock.connect(addr) + csock.sendall(message) + response = csock.recv(99) + csock.close() + + thread = threading.Thread(target=client, daemon=True) + thread.start() + + conn, _ = lsock.accept() + proto = MyProto(loop=loop) + proto.loop = loop + f = loop.create_task( + loop.handle_connection((lambda : proto), conn, ssl=server_ssl)) + loop.run_forever() + conn.close() + lsock.close() + + thread.join(1) + self.assertFalse(thread.is_alive()) + self.assertEqual(proto.state, 'CLOSED') + self.assertEqual(proto.nbytes, len(message)) + self.assertEqual(response, expected_response) + + if ssl is not None: + def test_handle_ssl_connection(self): + + server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + server_context.load_cert_chain(ONLYCERT, ONLYKEY) + if hasattr(server_context, 'check_hostname'): + server_context.check_hostname = False + server_context.verify_mode = ssl.CERT_NONE + + client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + if hasattr(server_context, 'check_hostname'): + client_context.check_hostname = False + client_context.verify_mode = ssl.CERT_NONE + + self.test_handle_connection(server_context, client_context) + @mock.patch('asyncio.base_events.socket') def create_server_multiple_hosts(self, family, hosts, mock_sock): @asyncio.coroutine From 51a11e262ffe15c0134a1b4b8b61517f8aad5a0b Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Sun, 10 Jul 2016 16:46:25 -0400 Subject: [PATCH 06/12] refactored to share more code between create_connection and handle_connection --- asyncio/base_events.py | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/asyncio/base_events.py b/asyncio/base_events.py index 35c98750..69becb46 100644 --- a/asyncio/base_events.py +++ b/asyncio/base_events.py @@ -707,8 +707,6 @@ def create_connection(self, protocol_factory, host=None, port=None, *, raise ValueError( 'host and port was not specified and no sock specified') - sock.setblocking(False) - transport, protocol = yield from self._create_connection_transport( sock, protocol_factory, ssl, server_hostname) if self._debug: @@ -721,14 +719,17 @@ def create_connection(self, protocol_factory, host=None, port=None, *, @coroutine def _create_connection_transport(self, sock, protocol_factory, ssl, - server_hostname): + server_hostname, server_side=False): + + sock.setblocking(False) + protocol = protocol_factory() waiter = self.create_future() if ssl: sslcontext = None if isinstance(ssl, bool) else ssl transport = self._make_ssl_transport( sock, protocol, sslcontext, waiter, - server_side=False, server_hostname=server_hostname) + server_side=server_side, server_hostname=server_hostname) else: transport = self._make_socket_transport(sock, protocol, waiter) @@ -989,24 +990,13 @@ def handle_connection(self, protocol_factory, sock, ssl=None): This method is a coroutine. When completed, the coroutine returns a (transport, protocol) pair. """ - sock.setblocking(False) - - protocol = protocol_factory() - waiter = self.create_future() - if ssl: - sslcontext = None if isinstance(ssl, bool) else ssl - transport = self._make_ssl_transport( - sock, protocol, sslcontext, waiter, - server_side=True) - else: - transport = self._make_socket_transport(sock, protocol, waiter) - - try: - yield from waiter - except: - transport.close() - raise - + transport, protocol = yield from self._create_connection_transport( + sock, protocol_factory, ssl, '', server_side=True) + if self._debug: + # Get the socket from the transport because SSL transport closes + # the old socket and creates a new SSL socket + sock = transport.get_extra_info('socket') + logger.debug("%r handled: (%r, %r)", sock, transport, protocol) return transport, protocol @coroutine From 4f9cf47f2158c27ab697fdc64fcec007913852e2 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Mon, 11 Jul 2016 11:14:33 -0400 Subject: [PATCH 07/12] rename handle_connection to connect_accepted_socket --- asyncio/base_events.py | 2 +- tests/test_events.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/asyncio/base_events.py b/asyncio/base_events.py index 69becb46..e01c23b5 100644 --- a/asyncio/base_events.py +++ b/asyncio/base_events.py @@ -981,7 +981,7 @@ def create_server(self, protocol_factory, host=None, port=None, return server @coroutine - def handle_connection(self, protocol_factory, sock, ssl=None): + def connect_accepted_socket(self, protocol_factory, sock, ssl=None): """Handle an accepted connection This is used by servers that accept connections outside of diff --git a/tests/test_events.py b/tests/test_events.py index 3e5e9e76..f923a5c6 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -744,7 +744,7 @@ def test_create_connection_local_addr_in_use(self): self.assertEqual(cm.exception.errno, errno.EADDRINUSE) self.assertIn(str(httpd.address), cm.exception.strerror) - def test_handle_connection(self, server_ssl=None, client_ssl=None): + def test_connect_accepted_socket(self, server_ssl=None, client_ssl=None): loop = self.loop class MyProto(MyBaseProto): @@ -783,7 +783,8 @@ def client(): proto = MyProto(loop=loop) proto.loop = loop f = loop.create_task( - loop.handle_connection((lambda : proto), conn, ssl=server_ssl)) + loop.connect_accepted_socket( + (lambda : proto), conn, ssl=server_ssl)) loop.run_forever() conn.close() lsock.close() @@ -795,7 +796,7 @@ def client(): self.assertEqual(response, expected_response) if ssl is not None: - def test_handle_ssl_connection(self): + def test_ssl_connect_accepted_socket(self): server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) server_context.load_cert_chain(ONLYCERT, ONLYKEY) @@ -808,7 +809,7 @@ def test_handle_ssl_connection(self): client_context.check_hostname = False client_context.verify_mode = ssl.CERT_NONE - self.test_handle_connection(server_context, client_context) + self.test_connect_accepted_socket(server_context, client_context) @mock.patch('asyncio.base_events.socket') def create_server_multiple_hosts(self, family, hosts, mock_sock): From f121a75d3949d4c74ce5f85a28ea3282b7371549 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Mon, 11 Jul 2016 11:17:26 -0400 Subject: [PATCH 08/12] Add exception handling in a test client thread. In hopes of seeing wtf failure on windows. --- tests/test_events.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/test_events.py b/tests/test_events.py index f923a5c6..b5a2771e 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -768,13 +768,18 @@ def data_received(self, data): def client(): global response - csock = socket.socket() - if client_ssl is not None: - csock = client_ssl.wrap_socket(csock) - csock.connect(addr) - csock.sendall(message) - response = csock.recv(99) - csock.close() + try: + csock = socket.socket() + if client_ssl is not None: + csock = client_ssl.wrap_socket(csock) + csock.connect(addr) + csock.sendall(message) + response = csock.recv(99) + csock.close() + except Exception as exc: + print( + "Failure in client thread in test_connect_accepted_socket", + exc) thread = threading.Thread(target=client, daemon=True) thread.start() From db42b9dbe2dab6984aa7de4f7e96574bf154ca65 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Mon, 11 Jul 2016 11:58:02 -0400 Subject: [PATCH 09/12] Address reviewer comments. --- asyncio/base_events.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/asyncio/base_events.py b/asyncio/base_events.py index e01c23b5..01743755 100644 --- a/asyncio/base_events.py +++ b/asyncio/base_events.py @@ -981,8 +981,8 @@ def create_server(self, protocol_factory, host=None, port=None, return server @coroutine - def connect_accepted_socket(self, protocol_factory, sock, ssl=None): - """Handle an accepted connection + def connect_accepted_socket(self, protocol_factory, sock, *, ssl=None): + """Handle an accepted connection. This is used by servers that accept connections outside of asyncio but that use asyncio to handle connections. From 905621fe2350c8671816253732001a294063a625 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Mon, 11 Jul 2016 12:03:25 -0400 Subject: [PATCH 10/12] Try to get more output from appveyor, at least temporarily --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 9d7c5280..9b41216c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,4 +8,4 @@ environment: build: false test_script: - - "%PYTHON%\\python.exe runtests.py" + - "%PYTHON%\\python.exe runtests.py -v" From 27d643f43e690a3e0bdb721674bab88712db0b90 Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Mon, 11 Jul 2016 14:06:59 -0400 Subject: [PATCH 11/12] skip test_ssl_connect_accepted_socket where ssl isn't supported --- tests/test_events.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/tests/test_events.py b/tests/test_events.py index b5a2771e..5c186ceb 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -800,21 +800,28 @@ def client(): self.assertEqual(proto.nbytes, len(message)) self.assertEqual(response, expected_response) - if ssl is not None: - def test_ssl_connect_accepted_socket(self): + @unittest.skipIf(ssl is None, 'No ssl module') + def test_ssl_connect_accepted_socket(self): + if (sys.platform == 'win32' and + sys.version_info < (3, 5) and + isinstance(self.loop, proactor_events.BaseProactorEventLoop) + ): + raise unittest.SkipTest( + 'SSL not supported with proactor event loops before Python 3.5' + ) - server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - server_context.load_cert_chain(ONLYCERT, ONLYKEY) - if hasattr(server_context, 'check_hostname'): - server_context.check_hostname = False - server_context.verify_mode = ssl.CERT_NONE + server_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + server_context.load_cert_chain(ONLYCERT, ONLYKEY) + if hasattr(server_context, 'check_hostname'): + server_context.check_hostname = False + server_context.verify_mode = ssl.CERT_NONE - client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - if hasattr(server_context, 'check_hostname'): - client_context.check_hostname = False - client_context.verify_mode = ssl.CERT_NONE + client_context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + if hasattr(server_context, 'check_hostname'): + client_context.check_hostname = False + client_context.verify_mode = ssl.CERT_NONE - self.test_connect_accepted_socket(server_context, client_context) + self.test_connect_accepted_socket(server_context, client_context) @mock.patch('asyncio.base_events.socket') def create_server_multiple_hosts(self, family, hosts, mock_sock): From 2d31d15f61d9e22627c1f61d599914aa457e281b Mon Sep 17 00:00:00 2001 From: Jim Fulton Date: Mon, 11 Jul 2016 14:59:50 -0400 Subject: [PATCH 12/12] revert verbosity --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 9b41216c..9d7c5280 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,4 +8,4 @@ environment: build: false test_script: - - "%PYTHON%\\python.exe runtests.py -v" + - "%PYTHON%\\python.exe runtests.py"