From ea5230fe318a304325f9e46220d3286ec0249eea Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 19 May 2022 12:28:06 +0300 Subject: [PATCH 01/83] Add a threaded server factory Instead of threads and flags, employ high-level concurrency primitives like tasks and futures. A thread pool allows to run tests in parallel so it is future-proof. --- Lib/test/support/threading_helper.py | 66 ++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 26cbc6f4d2439c..a468d5f723c5d3 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -1,13 +1,17 @@ import _thread +from concurrent.futures import ThreadPoolExecutor, wait import contextlib import functools import sys +from test.support.socket_helper import bind_port, HOST import threading +from socket import socket import time import unittest from test import support +_thread_pool = ThreadPoolExecutor() #======================================================================= # Threading support to prevent reporting refleaks when running regrtest.py -R @@ -242,3 +246,65 @@ def requires_working_threading(*, module=False): raise unittest.SkipTest(msg) else: return unittest.skipUnless(can_start_thread, msg) + + +class Server: + """A context manager for a blocking server in a thread pool. + + The server is designed: + + - for testing purposes so it serves a single client for its lifetime + - to be one-pass, short-lived, and terminated by in-protocol means so no + stopper flag is used + - to be used where asyncio has no application + + The server listens on an address returned from the ``with`` statement. + """ + + def __init__(self, client_func, *args, results=[], **kwargs): + """Create and run the server. + + The method blocks until a server is ready to accept clients. + + When client_func raises an exception, all server-side sockets are + closed. + + Args: + client_func: a function called in a dedicated thread for each new + connected client. The function receives all argument passed to + the __init__ method excluding client_func. + args: positional arguments passed to client_func. + results: a reference to a list for collecting client_func + return values. Populated after execution leaves a ``with`` + blocks associated with the Server context manager. + kwargs: keyword arguments passed to client_func. + + Throws: + When client_func throws, this method catches the exception, wraps + it into RuntimeError("server-side error") and rethrows. + """ + server_socket = socket() + self._port = bind_port(server_socket) + self._result = _thread_pool.submit(self._thread_func, server_socket, + client_func, args, kwargs) + self._result_out = results + + def _thread_func(self, server_socket, client_func, args, kwargs): + with server_socket: + server_socket.settimeout(1.0) + server_socket.listen(1) + client, peer_address = server_socket.accept() + with client: + return client_func(client, peer_address, *args, **kwargs) + + def __enter__(self): + return HOST, self._port + + def __exit__(self, etype, evalue, traceback): + wait([self._result]) + if etype is ConnectionAbortedError or etype is ConnectionResetError: + if self._result.exception() is not None: + raise RuntimeError("server-side error") from self._result.exception() + return False + self._result_out = self._result.result() + return False From c6e68e961adace42f0c58d2617e0426c3bb10d8f Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 19 May 2022 12:36:36 +0300 Subject: [PATCH 02/83] Add an alternative implementation of ThreadedEchoServer Start delegating all exceptions to a main, tester thread. - thread creation for each new client has no use in single-shot disposable servers that need to be created fresh for every test case anyway - stop-flag has no use either; there is no interactive session as well as waiting for other users. To test server-side connection abruption, instruct a server instance in advance - wrap_conn either modified self.sslconn and returned True, or swallowed exceptions and returned False. Now it either return sslconn directly or lets an exception to bubble up through a future into a main thread. The main thread, in its turn, either expects it with assertRaises() or lets it bubble further so the test case is reported as failed - transparent bubbling up means no explicit collection of error messages in conn_errors. Let unittest package do this job for us - universal read-write-close operations are changed from inner ifs to assignment of specific lambda functions - constant repetition of `if support.verbose and chatty:` // `sys.stdout.write(' server: ...\n')` is moved into log() - chatty and connectionchatty flags are merged to form a single story of communication between a client and a server - the first usage of ThreadedEchoServer is on line 1286 while the server itself was declared starting with line 2557; the new implementation is put before the first usage for sequential narration --- Lib/test/test_ssl.py | 159 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index fed76378726c92..6c3802536e52eb 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -325,6 +325,165 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True): return client_context, server_context, hostname +class Server(threading_helper.Server): + + def __init__(self, *args, **kwargs): + super().__init__(self._on_client, *args, **kwargs) + + def _on_client(self, socket, peer_address, certificate=None, + ssl_version=ssl.PROTOCOL_TLS_SERVER, + certreqs=ssl.CERT_NONE, cacerts=None, + chatty=True, starttls_server=False, + alpn_protocols=None, ciphers=None, context=None): + # A mildly complicated server, because we want it to work both + # with and without the SSL wrapper around the socket connection, so + # that we can test the STARTTLS functionality. + + def log(message): + if support.verbose and chatty: + sys.stdout.write(f' server: {message}\n') + + if context is None: + context = ssl.SSLContext(ssl_version) + context.verify_mode = certreqs + if cacerts: + context.load_verify_locations(cacerts) + if certificate: + context.load_cert_chain(certificate) + if alpn_protocols: + context.set_alpn_protocols(alpn_protocols) + if ciphers: + context.set_ciphers(ciphers) + + # Returned via the future + selected_alpn_protocols = [] + shared_ciphers = [] + + # Functions swithed on wrapping/unwrapping + read = lambda: socket.recv(1024) + write = socket.send + # A caller of on_client will close the socket + close = lambda: None + + def wrap_conn(socket): + # Notes on how to treat exceptions thrown by wrap_socket: + # + # We treat ConnectionResetError as though it were an + # SSLError - OpenSSL on Ubuntu abruptly closes the + # connection when asked to use an unsupported protocol. + # + # BrokenPipeError is raised in TLS 1.3 mode, when OpenSSL + # tries to send session tickets after handshake. + # https://github.com/openssl/openssl/issues/6342 + # + # ConnectionAbortedError is raised in TLS 1.3 mode, when OpenSSL + # tries to send session tickets after handshake when using WinSock. + # + # OSError may occur with wrong protocols, e.g. both + # sides use PROTOCOL_TLS_SERVER. + + try: + sslconn = context.wrap_socket(socket, server_side=True) + nonlocal read, write, close + read = lambda: sslconn.read() + write = sslconn.write + close = sslconn.close + + nonlocal selected_alpn_protocols, shared_ciphers + selected_alpn_protocols = sslconn.selected_alpn_protocol() + shared_ciphers = sslconn.shared_ciphers() + + if context.verify_mode == ssl.CERT_REQUIRED: + cert = sslconn.getpeercert() + log(f"client cert is {pprint.pformat(cert)}") + cert_binary = sslconn.getpeercert(True) + if cert_binary is None: + log("client did not provide a cert") + else: + log(f"cert binary is {len(cert_binary)}b") + + log(f"connection cipher is now {sslconn.cipher()}") + return sslconn + + except (ssl.SSLError, OSError) as e: + # bpo-44229, bpo-43855, bpo-44237, and bpo-33450: + # Ignore spurious EPROTOTYPE returned by write() on macOS. + # See also http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/ + if e.errno != errno.EPROTOTYPE and sys.platform != "darwin": + raise + + log(f'new connection from {peer_address!r}') + sslconn = None if starttls_server else wrap_conn(socket) + + forced_exit = False + while not forced_exit and (msg := read()): + stripped = msg.strip() + + if stripped == b'over': + log("client closed connection") + forced_exit = True + close() + + elif starttls_server and stripped == b'STARTTLS': + log("read STARTTLS from client, sending OK") + write(b"OK\n") + sslconn = wrap_conn(socket) + + elif starttls_server and sslconn and stripped == b'ENDTLS': + log("read ENDTLS from client, sending OK") + write(b"OK\n") + socket = sslconn.unwrap() + sslconn = None + log("connection is now unencrypted") + + elif stripped == b'CB tls-unique': + log("read CB tls-unique from client, sending our CB data") + data = sslconn.get_channel_binding("tls-unique") + write(repr(data).encode("us-ascii") + b"\n") + + elif stripped == b'PHA': + log("initiating post handshake auth") + try: + sslconn.verify_client_post_handshake() + except ssl.SSLError as e: + write(repr(e).encode("us-ascii") + b"\n") + else: + write(b"OK\n") + + elif stripped == b'HASCERT': + if sslconn.getpeercert() is not None: + write(b'TRUE\n') + else: + write(b'FALSE\n') + + elif stripped == b'GETCERT': + cert = sslconn.getpeercert() + write(repr(cert).encode("us-ascii") + b"\n") + + elif stripped == b'VERIFIEDCHAIN': + certs = sslconn._sslobj.get_verified_chain() + write(len(certs).to_bytes(1, "big") + b"\n") + + elif stripped == b'UNVERIFIEDCHAIN': + certs = sslconn._sslobj.get_unverified_chain() + write(len(certs).to_bytes(1, "big") + b"\n") + + else: + ctype = "encrypted" if sslconn else "unencrypted" + log(f"read {msg} ({ctype}), sending back {msg.lower()} ({ctype})") + write(msg.lower()) + + try: + socket = sslconn.unwrap() + except OSError: + # Many tests shut the TCP connection down without an SSL shutdown. + # This causes unwrap() to raise OSError with errno=0. + pass + + close() + return selected_alpn_protocols, shared_ciphers + + class BasicSocketTests(unittest.TestCase): def test_constants(self): From 47ac0532f59801ff06a450b7cdc8f3bc531a652a Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 8 May 2022 16:38:48 +0300 Subject: [PATCH 03/83] Port two simple cases as an example In ThreadedEchoServer chatty=True by default so omit it. --- Lib/test/test_ssl.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 6c3802536e52eb..59536d7dd57308 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1286,11 +1286,10 @@ def test_connect_ex_error(self): def test_read_write_zero(self): # empty reads and writes now work, bpo-42854, bpo-31711 client_context, server_context, hostname = testing_context() - server = ThreadedEchoServer(context=server_context) - with server: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) self.assertEqual(s.recv(0), b"") self.assertEqual(s.send(b""), 0) @@ -3298,11 +3297,10 @@ def test_dual_rsa_ecc(self): server_context.load_cert_chain(SIGNED_CERTFILE) # correct hostname should verify - server = ThreadedEchoServer(context=server_context, chatty=True) - with server: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") cipher = s.cipher()[0].split('-') From 05fd819bcf0d8677db0d6d08ae594a9fc46ed12e Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 18 May 2022 20:19:13 +0300 Subject: [PATCH 04/83] Port multiserver test_check_hostname --- Lib/test/test_ssl.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 59536d7dd57308..936cf9037ecb58 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3210,31 +3210,27 @@ def test_check_hostname(self): client_context, server_context, hostname = testing_context() # correct hostname should verify - server = ThreadedEchoServer(context=server_context, chatty=True) - with server: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") # incorrect hostname should raise an exception - server = ThreadedEchoServer(context=server_context, chatty=True) - with server: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname="invalid") as s: with self.assertRaisesRegex( ssl.CertificateError, "Hostname mismatch, certificate is not valid for 'invalid'."): - s.connect((HOST, server.port)) + s.connect(address) # missing server_hostname arg should cause an exception, too - server = ThreadedEchoServer(context=server_context, chatty=True) - with server: - with socket.socket() as s: - with self.assertRaisesRegex(ValueError, - "check_hostname requires server_hostname"): - client_context.wrap_socket(s) + with socket.socket() as s: + with self.assertRaisesRegex(ValueError, + "check_hostname requires server_hostname"): + client_context.wrap_socket(s) @unittest.skipUnless( ssl.HAS_NEVER_CHECK_COMMON_NAME, "test requires hostname_checks_common_name" From 149d18c4707ecffa8dea5be513c909068f1d5011 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 19 May 2022 12:47:29 +0300 Subject: [PATCH 05/83] Port a case where client connects twice --- Lib/test/support/threading_helper.py | 23 +++++++++++++++-------- Lib/test/test_ssl.py | 11 +++-------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index a468d5f723c5d3..7240037c8ce095 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -253,7 +253,7 @@ class Server: The server is designed: - - for testing purposes so it serves a single client for its lifetime + - for testing purposes so it serves a fixed count of clients, one by one - to be one-pass, short-lived, and terminated by in-protocol means so no stopper flag is used - to be used where asyncio has no application @@ -261,7 +261,7 @@ class Server: The server listens on an address returned from the ``with`` statement. """ - def __init__(self, client_func, *args, results=[], **kwargs): + def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): """Create and run the server. The method blocks until a server is ready to accept clients. @@ -272,8 +272,10 @@ def __init__(self, client_func, *args, results=[], **kwargs): Args: client_func: a function called in a dedicated thread for each new connected client. The function receives all argument passed to - the __init__ method excluding client_func. + the __init__ method excluding client_func and client_count. args: positional arguments passed to client_func. + client_count: count of clients the server processes one by one + before stopping. results: a reference to a list for collecting client_func return values. Populated after execution leaves a ``with`` blocks associated with the Server context manager. @@ -286,16 +288,21 @@ def __init__(self, client_func, *args, results=[], **kwargs): server_socket = socket() self._port = bind_port(server_socket) self._result = _thread_pool.submit(self._thread_func, server_socket, - client_func, args, kwargs) + client_func, client_count, + args, kwargs) self._result_out = results - def _thread_func(self, server_socket, client_func, args, kwargs): + def _thread_func(self, server_socket, client_func, client_count, args, kwargs): with server_socket: server_socket.settimeout(1.0) server_socket.listen(1) - client, peer_address = server_socket.accept() - with client: - return client_func(client, peer_address, *args, **kwargs) + results = [] + for i in range(client_count): + client, peer_address = server_socket.accept() + with client: + results.append(client_func(client, peer_address, + *args, **kwargs)) + return results def __enter__(self): return HOST, self._port diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 936cf9037ecb58..3482206688057d 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4152,21 +4152,16 @@ def test_default_ecdh_curve(self): @unittest.skipUnless("tls-unique" in ssl.CHANNEL_BINDING_TYPES, "'tls-unique' channel binding not available") def test_tls_unique_channel_binding(self): - """Test tls-unique channel binding.""" if support.verbose: sys.stdout.write("\n") client_context, server_context, hostname = testing_context() - server = ThreadedEchoServer(context=server_context, - chatty=True, - connectionchatty=False) - - with server: + with Server(context=server_context, client_count=2) as address: with client_context.wrap_socket( socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) # get the data cb_data = s.get_channel_binding("tls-unique") if support.verbose: @@ -4190,7 +4185,7 @@ def test_tls_unique_channel_binding(self): with client_context.wrap_socket( socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) new_cb_data = s.get_channel_binding("tls-unique") if support.verbose: sys.stdout.write( From e97b07c6cdc6d009dbfb978833ce2349e71b1294 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Mon, 9 May 2022 00:29:06 +0300 Subject: [PATCH 06/83] Port a case where client connects four times --- Lib/test/test_ssl.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 3482206688057d..7628d93d26cda7 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4540,14 +4540,13 @@ def test_session_handling(self): client_context.maximum_version = ssl.TLSVersion.TLSv1_2 client_context2.maximum_version = ssl.TLSVersion.TLSv1_2 - server = ThreadedEchoServer(context=server_context, chatty=False) - with server: + with Server(context=server_context, chatty=False, client_count=4) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: # session is None before handshake self.assertEqual(s.session, None) self.assertEqual(s.session_reused, None) - s.connect((HOST, server.port)) + s.connect(address) session = s.session self.assertTrue(session) with self.assertRaises(TypeError) as e: @@ -4556,7 +4555,7 @@ def test_session_handling(self): with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) # cannot set session after handshake with self.assertRaises(ValueError) as e: s.session = session @@ -4568,7 +4567,7 @@ def test_session_handling(self): # can set session before handshake and before the # connection was established s.session = session - s.connect((HOST, server.port)) + s.connect(address) self.assertEqual(s.session.id, session.id) self.assertEqual(s.session, session) self.assertEqual(s.session_reused, True) @@ -4578,7 +4577,7 @@ def test_session_handling(self): # cannot re-use session with a different SSLContext with self.assertRaises(ValueError) as e: s.session = session - s.connect((HOST, server.port)) + s.connect(address) self.assertEqual(str(e.exception), 'Session refers to a different SSLContext.') From 4fa30f1b3d47d481364e8acca3eeb1a0055c9183 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Mon, 9 May 2022 06:50:40 +0300 Subject: [PATCH 07/83] Fix a race between client connection and server launch Force clients to wait on a server socket while the server worker thread starts up, instead of erroring on unavailable host. For this, open the socket in a main thread. Server socket timeout has no use because the only operation it limits is listen(). To have it permawaiting we need a main thread interrupted without terminating the whole program; that's possible under a debugger only. --- Lib/test/support/threading_helper.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 7240037c8ce095..be7c086fdbfcdd 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -287,6 +287,7 @@ def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): """ server_socket = socket() self._port = bind_port(server_socket) + server_socket.listen(1) self._result = _thread_pool.submit(self._thread_func, server_socket, client_func, client_count, args, kwargs) @@ -294,8 +295,6 @@ def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): def _thread_func(self, server_socket, client_func, client_count, args, kwargs): with server_socket: - server_socket.settimeout(1.0) - server_socket.listen(1) results = [] for i in range(client_count): client, peer_address = server_socket.accept() From d5c476214ed63d8453878a0b1e52ad7718fab740 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 19 May 2022 18:37:53 +0300 Subject: [PATCH 08/83] Add a temporary workaround for zero-byte SSL transfers --- Lib/test/test_ssl.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 7628d93d26cda7..6773adc37b65b8 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1292,6 +1292,13 @@ def test_read_write_zero(self): s.connect(address) self.assertEqual(s.recv(0), b"") self.assertEqual(s.send(b""), 0) + # A temporary workaround for delayed tlsv1.3 session ticket + # eachange; the proper fix will be merges after this PR because + # it hangs test_session_handling. + # + # This problem manufested before as well but was just logged + # andsilenced by a server thread. + s.unwrap().close() class ContextTests(unittest.TestCase): From 289e5635a8d9ffa17f318ba4841a61399ea3d427 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 21 May 2022 19:06:03 +0300 Subject: [PATCH 09/83] Prevent hanging in case of client overexpectation --- Lib/test/support/threading_helper.py | 5 ++++- Lib/test/test_ssl.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index be7c086fdbfcdd..95f03814e855dc 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -30,6 +30,8 @@ def threading_setup(): def threading_cleanup(*original_values): + _thread_pool = None + _MAX_COUNT = 100 for count in range(_MAX_COUNT): @@ -287,13 +289,14 @@ def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): """ server_socket = socket() self._port = bind_port(server_socket) - server_socket.listen(1) + server_socket.listen() self._result = _thread_pool.submit(self._thread_func, server_socket, client_func, client_count, args, kwargs) self._result_out = results def _thread_func(self, server_socket, client_func, client_count, args, kwargs): + server_socket.settimeout(3) with server_socket: results = [] for i in range(client_count): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 6773adc37b65b8..56179434e1f780 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4547,7 +4547,7 @@ def test_session_handling(self): client_context.maximum_version = ssl.TLSVersion.TLSv1_2 client_context2.maximum_version = ssl.TLSVersion.TLSv1_2 - with Server(context=server_context, chatty=False, client_count=4) as address: + with Server(context=server_context, chatty=False, client_count=3) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: # session is None before handshake From 9aa2bef5f548db204984573481d2a80bf8666c28 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 21 May 2022 21:52:16 +0300 Subject: [PATCH 10/83] Fix more sporadic errors --- Lib/test/test_ssl.py | 45 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 56179434e1f780..4e57750ebe20d3 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2168,19 +2168,20 @@ def setUp(self): self.server_addr = (HOST, server.port) def test_connect(self): - with test_wrap_socket(socket.socket(socket.AF_INET), - cert_reqs=ssl.CERT_NONE) as s: - s.connect(self.server_addr) - self.assertEqual({}, s.getpeercert()) - self.assertFalse(s.server_side) + with Server(context=self.server_context, client_count=2) as address: + with test_wrap_socket(socket.socket(socket.AF_INET), + cert_reqs=ssl.CERT_NONE) as s: + s.connect(address) + self.assertEqual({}, s.getpeercert()) + self.assertFalse(s.server_side) - # this should succeed because we specify the root cert - with test_wrap_socket(socket.socket(socket.AF_INET), - cert_reqs=ssl.CERT_REQUIRED, - ca_certs=SIGNING_CA) as s: - s.connect(self.server_addr) - self.assertTrue(s.getpeercert()) - self.assertFalse(s.server_side) + # this should succeed because we specify the root cert + with test_wrap_socket(socket.socket(socket.AF_INET), + cert_reqs=ssl.CERT_REQUIRED, + ca_certs=SIGNING_CA) as s: + s.connect(address) + self.assertTrue(s.getpeercert()) + self.assertFalse(s.server_side) def test_connect_fail(self): # This should fail because we have no verification certs. Connection @@ -3225,13 +3226,14 @@ def test_check_hostname(self): self.assertTrue(cert, "Can't get peer certificate.") # incorrect hostname should raise an exception - with Server(context=server_context) as address: - with client_context.wrap_socket(socket.socket(), - server_hostname="invalid") as s: - with self.assertRaisesRegex( - ssl.CertificateError, - "Hostname mismatch, certificate is not valid for 'invalid'."): - s.connect(address) + with self.assertRaisesRegex(ssl.SSLError, 'SSL: SSLV3_ALERT_BAD_CERTIFICATE'): + with Server(context=server_context) as address: + with client_context.wrap_socket(socket.socket(), + server_hostname="invalid") as s: + with self.assertRaisesRegex( + ssl.CertificateError, + "Hostname mismatch, certificate is not valid for 'invalid'."): + s.connect(address) # missing server_hostname arg should cause an exception, too with socket.socket() as s: @@ -3274,11 +3276,10 @@ def test_ecc_cert(self): server_context.load_cert_chain(SIGNED_CERTFILE_ECC) # correct hostname should verify - server = ThreadedEchoServer(context=server_context, chatty=True) - with server: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") cipher = s.cipher()[0].split('-') From d39fba9dc338583b17ac6aaf906fd0880c099424 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 08:38:54 +0300 Subject: [PATCH 11/83] Automatic thread management not only needs no manual leak detector, it breaks it with a long-live thread pool --- Lib/test/support/threading_helper.py | 4 +--- Lib/test/test_ssl.py | 3 --- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 034a48da777b50..3b474c9caa4cad 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -30,8 +30,6 @@ def threading_setup(): def threading_cleanup(*original_values): - _thread_pool = None - _MAX_COUNT = 100 for count in range(_MAX_COUNT): @@ -41,7 +39,7 @@ def threading_cleanup(*original_values): if not count: # Display a warning at the first iteration - support.environment_altered = True + support.environment_altered = True ### ---- именно этот участок выбрасывает ошибку - мы не завершвем работу сервера dangling_threads = values[1] support.print_warning(f"threading_cleanup() failed to cleanup " f"{values[0] - original_values[0]} threads " diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 713e4af387579e..0979f68ec93611 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4962,9 +4962,6 @@ def setUpModule(): if not os.path.exists(filename): raise support.TestFailed("Can't read certificate file %r" % filename) - thread_info = threading_helper.threading_setup() - unittest.addModuleCleanup(threading_helper.threading_cleanup, *thread_info) - if __name__ == "__main__": unittest.main() From a2f0b07ca79755c69a4661649c3c6c02f5a4d0a9 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 11:49:57 +0300 Subject: [PATCH 12/83] Use a named constant for a timeout --- Lib/test/support/threading_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 3b474c9caa4cad..ea3ebb33af6a6f 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -292,7 +292,7 @@ def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): self._result_out = results def _thread_func(self, server_socket, client_func, client_count, args, kwargs): - server_socket.settimeout(3) + server_socket.settimeout(support.LOOPBACK_TIMEOUT) with server_socket: results = [] for i in range(client_count): From bbce6679ce051cd0aafba6d956aa2bef170b475a Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 15:00:56 +0300 Subject: [PATCH 13/83] Restore an unstable test --- Lib/test/test_ssl.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 0979f68ec93611..49feebade0d2be 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1963,20 +1963,19 @@ def setUp(self): self.server_addr = (HOST, server.port) def test_connect(self): - with Server(context=self.server_context, client_count=2) as address: - with test_wrap_socket(socket.socket(socket.AF_INET), - cert_reqs=ssl.CERT_NONE) as s: - s.connect(address) - self.assertEqual({}, s.getpeercert()) - self.assertFalse(s.server_side) + with test_wrap_socket(socket.socket(socket.AF_INET), + cert_reqs=ssl.CERT_NONE) as s: + s.connect(self.server_addr) + self.assertEqual({}, s.getpeercert()) + self.assertFalse(s.server_side) - # this should succeed because we specify the root cert - with test_wrap_socket(socket.socket(socket.AF_INET), - cert_reqs=ssl.CERT_REQUIRED, - ca_certs=SIGNING_CA) as s: - s.connect(address) - self.assertTrue(s.getpeercert()) - self.assertFalse(s.server_side) + # this should succeed because we specify the root cert + with test_wrap_socket(socket.socket(socket.AF_INET), + cert_reqs=ssl.CERT_REQUIRED, + ca_certs=SIGNING_CA) as s: + s.connect(self.server_addr) + self.assertTrue(s.getpeercert()) + self.assertFalse(s.server_side) def test_connect_fail(self): # This should fail because we have no verification certs. Connection From d10cc726a69555239551367e2dc1100275961de8 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 15:14:33 +0300 Subject: [PATCH 14/83] Restore the second unstable test --- Lib/test/test_ssl.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 49feebade0d2be..6204d27b428584 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1080,19 +1080,13 @@ def test_connect_ex_error(self): def test_read_write_zero(self): # empty reads and writes now work, bpo-42854, bpo-31711 client_context, server_context, hostname = testing_context() - with Server(context=server_context) as address: + server = ThreadedEchoServer(context=server_context) + with server: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect(address) + s.connect((HOST, server.port)) self.assertEqual(s.recv(0), b"") self.assertEqual(s.send(b""), 0) - # A temporary workaround for delayed tlsv1.3 session ticket - # eachange; the proper fix will be merges after this PR because - # it hangs test_session_handling. - # - # This problem manufested before as well but was just logged - # andsilenced by a server thread. - s.unwrap().close() class ContextTests(unittest.TestCase): From 18b3c154844f31da7e810d42726fe5c897eb4125 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 17:19:26 +0300 Subject: [PATCH 15/83] Remove accidentally included comment fragment --- Lib/test/support/threading_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index ea3ebb33af6a6f..d0a72a096ae793 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -39,7 +39,7 @@ def threading_cleanup(*original_values): if not count: # Display a warning at the first iteration - support.environment_altered = True ### ---- именно этот участок выбрасывает ошибку - мы не завершвем работу сервера + support.environment_altered = True dangling_threads = values[1] support.print_warning(f"threading_cleanup() failed to cleanup " f"{values[0] - original_values[0]} threads " From 1c8a6f1e875a8cb564d5c7867e27b79558385816 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 19:35:31 +0300 Subject: [PATCH 16/83] Add a NEWS entry --- .../next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst diff --git a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst new file mode 100644 index 00000000000000..1343a6666476a2 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst @@ -0,0 +1,2 @@ +Added a unified client-side context for managing a TCP server in its own thread +(:class:`test.support.threading_helper.Server`). Patch by Oleg Iarygin. From bbb7c6f44b77fcc18bbca0a9b6cf5ea6f39ca4be Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 19:17:59 +0300 Subject: [PATCH 17/83] Fix a potential method name conflict between base and derived classes --- Lib/test/test_ssl.py | 264 +++++++++++++++++++++---------------------- 1 file changed, 131 insertions(+), 133 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 6204d27b428584..4ead974a75d369 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -320,163 +320,161 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True): return client_context, server_context, hostname -class Server(threading_helper.Server): - - def __init__(self, *args, **kwargs): - super().__init__(self._on_client, *args, **kwargs) - - def _on_client(self, socket, peer_address, certificate=None, +def _on_ssl_client(socket, peer_address, certificate=None, ssl_version=ssl.PROTOCOL_TLS_SERVER, certreqs=ssl.CERT_NONE, cacerts=None, chatty=True, starttls_server=False, alpn_protocols=None, ciphers=None, context=None): - # A mildly complicated server, because we want it to work both - # with and without the SSL wrapper around the socket connection, so - # that we can test the STARTTLS functionality. + # A mildly complicated server, because we want it to work both + # with and without the SSL wrapper around the socket connection, so + # that we can test the STARTTLS functionality. + + def log(message): + if support.verbose and chatty: + sys.stdout.write(f' server: {message}\n') + + if context is None: + context = ssl.SSLContext(ssl_version) + context.verify_mode = certreqs + if cacerts: + context.load_verify_locations(cacerts) + if certificate: + context.load_cert_chain(certificate) + if alpn_protocols: + context.set_alpn_protocols(alpn_protocols) + if ciphers: + context.set_ciphers(ciphers) + + # Returned via the future + selected_alpn_protocols = [] + shared_ciphers = [] + + # Functions swithed on wrapping/unwrapping + read = lambda: socket.recv(1024) + write = socket.send + # A caller of on_client will close the socket + close = lambda: None + + def wrap_conn(socket): + # Notes on how to treat exceptions thrown by wrap_socket: + # + # We treat ConnectionResetError as though it were an + # SSLError - OpenSSL on Ubuntu abruptly closes the + # connection when asked to use an unsupported protocol. + # + # BrokenPipeError is raised in TLS 1.3 mode, when OpenSSL + # tries to send session tickets after handshake. + # https://github.com/openssl/openssl/issues/6342 + # + # ConnectionAbortedError is raised in TLS 1.3 mode, when OpenSSL + # tries to send session tickets after handshake when using WinSock. + # + # OSError may occur with wrong protocols, e.g. both + # sides use PROTOCOL_TLS_SERVER. - def log(message): - if support.verbose and chatty: - sys.stdout.write(f' server: {message}\n') + try: + sslconn = context.wrap_socket(socket, server_side=True) + nonlocal read, write, close + read = lambda: sslconn.read() + write = sslconn.write + close = sslconn.close - if context is None: - context = ssl.SSLContext(ssl_version) - context.verify_mode = certreqs - if cacerts: - context.load_verify_locations(cacerts) - if certificate: - context.load_cert_chain(certificate) - if alpn_protocols: - context.set_alpn_protocols(alpn_protocols) - if ciphers: - context.set_ciphers(ciphers) - - # Returned via the future - selected_alpn_protocols = [] - shared_ciphers = [] - - # Functions swithed on wrapping/unwrapping - read = lambda: socket.recv(1024) - write = socket.send - # A caller of on_client will close the socket - close = lambda: None - - def wrap_conn(socket): - # Notes on how to treat exceptions thrown by wrap_socket: - # - # We treat ConnectionResetError as though it were an - # SSLError - OpenSSL on Ubuntu abruptly closes the - # connection when asked to use an unsupported protocol. - # - # BrokenPipeError is raised in TLS 1.3 mode, when OpenSSL - # tries to send session tickets after handshake. - # https://github.com/openssl/openssl/issues/6342 - # - # ConnectionAbortedError is raised in TLS 1.3 mode, when OpenSSL - # tries to send session tickets after handshake when using WinSock. - # - # OSError may occur with wrong protocols, e.g. both - # sides use PROTOCOL_TLS_SERVER. + nonlocal selected_alpn_protocols, shared_ciphers + selected_alpn_protocols = sslconn.selected_alpn_protocol() + shared_ciphers = sslconn.shared_ciphers() - try: - sslconn = context.wrap_socket(socket, server_side=True) - nonlocal read, write, close - read = lambda: sslconn.read() - write = sslconn.write - close = sslconn.close - - nonlocal selected_alpn_protocols, shared_ciphers - selected_alpn_protocols = sslconn.selected_alpn_protocol() - shared_ciphers = sslconn.shared_ciphers() - - if context.verify_mode == ssl.CERT_REQUIRED: - cert = sslconn.getpeercert() - log(f"client cert is {pprint.pformat(cert)}") - cert_binary = sslconn.getpeercert(True) - if cert_binary is None: - log("client did not provide a cert") - else: - log(f"cert binary is {len(cert_binary)}b") + if context.verify_mode == ssl.CERT_REQUIRED: + cert = sslconn.getpeercert() + log(f"client cert is {pprint.pformat(cert)}") + cert_binary = sslconn.getpeercert(True) + if cert_binary is None: + log("client did not provide a cert") + else: + log(f"cert binary is {len(cert_binary)}b") - log(f"connection cipher is now {sslconn.cipher()}") - return sslconn + log(f"connection cipher is now {sslconn.cipher()}") + return sslconn - except (ssl.SSLError, OSError) as e: - # bpo-44229, bpo-43855, bpo-44237, and bpo-33450: - # Ignore spurious EPROTOTYPE returned by write() on macOS. - # See also http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/ - if e.errno != errno.EPROTOTYPE and sys.platform != "darwin": - raise + except (ssl.SSLError, OSError) as e: + # bpo-44229, bpo-43855, bpo-44237, and bpo-33450: + # Ignore spurious EPROTOTYPE returned by write() on macOS. + # See also http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/ + if e.errno != errno.EPROTOTYPE and sys.platform != "darwin": + raise - log(f'new connection from {peer_address!r}') - sslconn = None if starttls_server else wrap_conn(socket) + log(f'new connection from {peer_address!r}') + sslconn = None if starttls_server else wrap_conn(socket) - forced_exit = False - while not forced_exit and (msg := read()): - stripped = msg.strip() + forced_exit = False + while not forced_exit and (msg := read()): + stripped = msg.strip() - if stripped == b'over': - log("client closed connection") - forced_exit = True - close() + if stripped == b'over': + log("client closed connection") + forced_exit = True + close() - elif starttls_server and stripped == b'STARTTLS': - log("read STARTTLS from client, sending OK") - write(b"OK\n") - sslconn = wrap_conn(socket) + elif starttls_server and stripped == b'STARTTLS': + log("read STARTTLS from client, sending OK") + write(b"OK\n") + sslconn = wrap_conn(socket) - elif starttls_server and sslconn and stripped == b'ENDTLS': - log("read ENDTLS from client, sending OK") + elif starttls_server and sslconn and stripped == b'ENDTLS': + log("read ENDTLS from client, sending OK") + write(b"OK\n") + socket = sslconn.unwrap() + sslconn = None + log("connection is now unencrypted") + + elif stripped == b'CB tls-unique': + log("read CB tls-unique from client, sending our CB data") + data = sslconn.get_channel_binding("tls-unique") + write(repr(data).encode("us-ascii") + b"\n") + + elif stripped == b'PHA': + log("initiating post handshake auth") + try: + sslconn.verify_client_post_handshake() + except ssl.SSLError as e: + write(repr(e).encode("us-ascii") + b"\n") + else: write(b"OK\n") - socket = sslconn.unwrap() - sslconn = None - log("connection is now unencrypted") - elif stripped == b'CB tls-unique': - log("read CB tls-unique from client, sending our CB data") - data = sslconn.get_channel_binding("tls-unique") - write(repr(data).encode("us-ascii") + b"\n") + elif stripped == b'HASCERT': + if sslconn.getpeercert() is not None: + write(b'TRUE\n') + else: + write(b'FALSE\n') - elif stripped == b'PHA': - log("initiating post handshake auth") - try: - sslconn.verify_client_post_handshake() - except ssl.SSLError as e: - write(repr(e).encode("us-ascii") + b"\n") - else: - write(b"OK\n") + elif stripped == b'GETCERT': + cert = sslconn.getpeercert() + write(repr(cert).encode("us-ascii") + b"\n") - elif stripped == b'HASCERT': - if sslconn.getpeercert() is not None: - write(b'TRUE\n') - else: - write(b'FALSE\n') + elif stripped == b'VERIFIEDCHAIN': + certs = sslconn._sslobj.get_verified_chain() + write(len(certs).to_bytes(1, "big") + b"\n") - elif stripped == b'GETCERT': - cert = sslconn.getpeercert() - write(repr(cert).encode("us-ascii") + b"\n") + elif stripped == b'UNVERIFIEDCHAIN': + certs = sslconn._sslobj.get_unverified_chain() + write(len(certs).to_bytes(1, "big") + b"\n") - elif stripped == b'VERIFIEDCHAIN': - certs = sslconn._sslobj.get_verified_chain() - write(len(certs).to_bytes(1, "big") + b"\n") + else: + ctype = "encrypted" if sslconn else "unencrypted" + log(f"read {msg} ({ctype}), sending back {msg.lower()} ({ctype})") + write(msg.lower()) - elif stripped == b'UNVERIFIEDCHAIN': - certs = sslconn._sslobj.get_unverified_chain() - write(len(certs).to_bytes(1, "big") + b"\n") + try: + socket = sslconn.unwrap() + except OSError: + # Many tests shut the TCP connection down without an SSL shutdown. + # This causes unwrap() to raise OSError with errno=0. + pass - else: - ctype = "encrypted" if sslconn else "unencrypted" - log(f"read {msg} ({ctype}), sending back {msg.lower()} ({ctype})") - write(msg.lower()) + close() + return selected_alpn_protocols, shared_ciphers - try: - socket = sslconn.unwrap() - except OSError: - # Many tests shut the TCP connection down without an SSL shutdown. - # This causes unwrap() to raise OSError with errno=0. - pass - close() - return selected_alpn_protocols, shared_ciphers +Server = functools.partial(threading_helper.Server, _on_ssl_client) class BasicSocketTests(unittest.TestCase): From beb1650236961b3648cb2a0e907c3a1883916b81 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 19:38:19 +0300 Subject: [PATCH 18/83] Remove a redundant parameter --- Lib/test/test_ssl.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 4ead974a75d369..fb1bbb0cd405c3 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -321,7 +321,6 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True): def _on_ssl_client(socket, peer_address, certificate=None, - ssl_version=ssl.PROTOCOL_TLS_SERVER, certreqs=ssl.CERT_NONE, cacerts=None, chatty=True, starttls_server=False, alpn_protocols=None, ciphers=None, context=None): @@ -334,7 +333,7 @@ def log(message): sys.stdout.write(f' server: {message}\n') if context is None: - context = ssl.SSLContext(ssl_version) + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) context.verify_mode = certreqs if cacerts: context.load_verify_locations(cacerts) From 4b4b4e550bdbc84f998bbb9ae63736f5fa406c15 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 20:04:32 +0300 Subject: [PATCH 19/83] Revert "Restore the second unstable test" This reverts commit d10cc726a69555239551367e2dc1100275961de8. --- Lib/test/test_ssl.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index fb1bbb0cd405c3..4cf87eaab8dec4 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1077,13 +1077,19 @@ def test_connect_ex_error(self): def test_read_write_zero(self): # empty reads and writes now work, bpo-42854, bpo-31711 client_context, server_context, hostname = testing_context() - server = ThreadedEchoServer(context=server_context) - with server: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: - s.connect((HOST, server.port)) + s.connect(address) self.assertEqual(s.recv(0), b"") self.assertEqual(s.send(b""), 0) + # A temporary workaround for delayed tlsv1.3 session ticket + # eachange; the proper fix will be merges after this PR because + # it hangs test_session_handling. + # + # This problem manufested before as well but was just logged + # andsilenced by a server thread. + s.unwrap().close() class ContextTests(unittest.TestCase): From 0f3663bf49a2e776d22838f4d1f8c91c65da623f Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 21:55:11 +0300 Subject: [PATCH 20/83] Forcefully release a thread after a module is over --- Lib/test/support/threading_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index d0a72a096ae793..5140e645d9cb4f 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -12,6 +12,7 @@ from test import support _thread_pool = ThreadPoolExecutor() +unittest.addModuleCleanup(lambda: _thread_pool = None) #======================================================================= # Threading support to prevent reporting refleaks when running regrtest.py -R From a999eaba8a23bd019a1e20c7307a41e52e2e6907 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 6 Jul 2022 22:04:34 +0300 Subject: [PATCH 21/83] Account for a fact that assignments cannot be inside lambdas --- Lib/test/support/threading_helper.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 5140e645d9cb4f..f61c522a92ae4a 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -11,8 +11,12 @@ from test import support +def release_thread_pool(): + global _thread_pool + _thread_pool = None + _thread_pool = ThreadPoolExecutor() -unittest.addModuleCleanup(lambda: _thread_pool = None) +unittest.addModuleCleanup(release_thread_pool) #======================================================================= # Threading support to prevent reporting refleaks when running regrtest.py -R From 5e570f215a0f1b3e40c7eb5955b4e3fc95f8fcee Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 00:35:24 +0300 Subject: [PATCH 22/83] Split test_check_hostname to discover which one fails --- Lib/test/test_ssl.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 4cf87eaab8dec4..f3d8e001358e7d 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2999,7 +2999,7 @@ def test_crl_check(self): cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") - def test_check_hostname(self): + def test_check_hostname_correct(self): if support.verbose: sys.stdout.write("\n") @@ -3013,7 +3013,12 @@ def test_check_hostname(self): cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") - # incorrect hostname should raise an exception + def test_check_hostname_incorrect(self): + if support.verbose: + sys.stdout.write("\n") + + client_context, server_context, hostname = testing_context() + with self.assertRaisesRegex(ssl.SSLError, 'SSL: SSLV3_ALERT_BAD_CERTIFICATE'): with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), @@ -3023,7 +3028,12 @@ def test_check_hostname(self): "Hostname mismatch, certificate is not valid for 'invalid'."): s.connect(address) - # missing server_hostname arg should cause an exception, too + def test_check_hostname_missing(self): + if support.verbose: + sys.stdout.write("\n") + + client_context, server_context, hostname = testing_context() + with socket.socket() as s: with self.assertRaisesRegex(ValueError, "check_hostname requires server_hostname"): From 2ea2217ee410e19b9fec468869db68977f29a0f4 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 08:02:53 +0300 Subject: [PATCH 23/83] Teach the server to expect client errors on demand --- Lib/test/support/threading_helper.py | 19 ++++++++++++++----- Lib/test/test_ssl.py | 2 +- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index f61c522a92ae4a..4445951ad3ab22 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -264,7 +264,8 @@ class Server: The server listens on an address returned from the ``with`` statement. """ - def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): + def __init__(self, client_func, *args, client_count=1, results=[], + client_fails=False, **kwargs): """Create and run the server. The method blocks until a server is ready to accept clients. @@ -282,6 +283,10 @@ def __init__(self, client_func, *args, client_count=1, results=[], **kwargs): results: a reference to a list for collecting client_func return values. Populated after execution leaves a ``with`` blocks associated with the Server context manager. + client_fails: if True, a client is expected to cause + connection-related exceptions by reasons asserted on its side. + As a result, a server should swallow these exceptions and + proceed to the next client instead. kwargs: keyword arguments passed to client_func. Throws: @@ -301,10 +306,14 @@ def _thread_func(self, server_socket, client_func, client_count, args, kwargs): with server_socket: results = [] for i in range(client_count): - client, peer_address = server_socket.accept() - with client: - results.append(client_func(client, peer_address, - *args, **kwargs)) + try: + client, peer_address = server_socket.accept() + with client: + results.append(client_func(client, peer_address, + *args, **kwargs)) + except (ConnectionAbortedError, ConnectionResetError): + if not client_fails: + raise return results def __enter__(self): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index f3d8e001358e7d..d3c92cfbf156ec 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3020,7 +3020,7 @@ def test_check_hostname_incorrect(self): client_context, server_context, hostname = testing_context() with self.assertRaisesRegex(ssl.SSLError, 'SSL: SSLV3_ALERT_BAD_CERTIFICATE'): - with Server(context=server_context) as address: + with Server(context=server_context, client_fails=True) as address: with client_context.wrap_socket(socket.socket(), server_hostname="invalid") as s: with self.assertRaisesRegex( From 8c78bb9c7a8cb9f536000269c326b8b73ff355cd Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 08:12:35 +0300 Subject: [PATCH 24/83] Make sure that a heavy context object with captures does not pass thread border --- Lib/test/support/threading_helper.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 4445951ad3ab22..04f6b939b339e1 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -298,10 +298,11 @@ def __init__(self, client_func, *args, client_count=1, results=[], server_socket.listen() self._result = _thread_pool.submit(self._thread_func, server_socket, client_func, client_count, - args, kwargs) + client_fails, args, kwargs) self._result_out = results - def _thread_func(self, server_socket, client_func, client_count, args, kwargs): + def _thread_func(self, server_socket, client_func, client_count, + client_fails, args, kwargs): server_socket.settimeout(support.LOOPBACK_TIMEOUT) with server_socket: results = [] From 5dcea72a4703fbd1f96a3a7a57473323638da091 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 08:39:31 +0300 Subject: [PATCH 25/83] Teach the server to handle expectedly closed sockets --- Lib/test/support/threading_helper.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 04f6b939b339e1..ded2930ef22b5c 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -312,7 +312,9 @@ def _thread_func(self, server_socket, client_func, client_count, with client: results.append(client_func(client, peer_address, *args, **kwargs)) - except (ConnectionAbortedError, ConnectionResetError): + # OSError is caused by read()/write() on a socket unexpectedly + # closed by a client. + except (ConnectionAbortedError, ConnectionResetError, OSError): if not client_fails: raise return results From 535c54c941f40dbe1556197136a82fddbf52e564 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 08:40:37 +0300 Subject: [PATCH 26/83] Fit threading_helper into 79 columns --- Lib/test/support/threading_helper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index ded2930ef22b5c..c38970716eaae1 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -326,7 +326,8 @@ def __exit__(self, etype, evalue, traceback): wait([self._result]) if etype is ConnectionAbortedError or etype is ConnectionResetError: if self._result.exception() is not None: - raise RuntimeError("server-side error") from self._result.exception() + generic = RuntimeError('server-side error') + raise generic from self._result.exception() return False self._result_out = self._result.result() return False From a39981fe768a304246126454454231df5131b2a4 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 08:43:33 +0300 Subject: [PATCH 27/83] Use single quotation because there won't be any chance to fix it later --- Lib/test/test_ssl.py | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index d3c92cfbf156ec..86590da9b81bcb 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -384,21 +384,21 @@ def wrap_conn(socket): if context.verify_mode == ssl.CERT_REQUIRED: cert = sslconn.getpeercert() - log(f"client cert is {pprint.pformat(cert)}") + log(f'client cert is {pprint.pformat(cert)}') cert_binary = sslconn.getpeercert(True) if cert_binary is None: - log("client did not provide a cert") + log('client did not provide a cert') else: - log(f"cert binary is {len(cert_binary)}b") + log(f'cert binary is {len(cert_binary)}b') - log(f"connection cipher is now {sslconn.cipher()}") + log(f'connection cipher is now {sslconn.cipher()}') return sslconn except (ssl.SSLError, OSError) as e: # bpo-44229, bpo-43855, bpo-44237, and bpo-33450: # Ignore spurious EPROTOTYPE returned by write() on macOS. # See also http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/ - if e.errno != errno.EPROTOTYPE and sys.platform != "darwin": + if e.errno != errno.EPROTOTYPE and sys.platform != 'darwin': raise log(f'new connection from {peer_address!r}') @@ -409,35 +409,35 @@ def wrap_conn(socket): stripped = msg.strip() if stripped == b'over': - log("client closed connection") + log('client closed connection') forced_exit = True close() elif starttls_server and stripped == b'STARTTLS': - log("read STARTTLS from client, sending OK") - write(b"OK\n") + log('read STARTTLS from client, sending OK') + write(b'OK\n') sslconn = wrap_conn(socket) elif starttls_server and sslconn and stripped == b'ENDTLS': - log("read ENDTLS from client, sending OK") - write(b"OK\n") + log('read ENDTLS from client, sending OK') + write(b'OK\n') socket = sslconn.unwrap() sslconn = None - log("connection is now unencrypted") + log('connection is now unencrypted') elif stripped == b'CB tls-unique': - log("read CB tls-unique from client, sending our CB data") - data = sslconn.get_channel_binding("tls-unique") - write(repr(data).encode("us-ascii") + b"\n") + log('read CB tls-unique from client, sending our CB data') + data = sslconn.get_channel_binding('tls-unique') + write(repr(data).encode('us-ascii') + b'\n') elif stripped == b'PHA': - log("initiating post handshake auth") + log('initiating post handshake auth') try: sslconn.verify_client_post_handshake() except ssl.SSLError as e: - write(repr(e).encode("us-ascii") + b"\n") + write(repr(e).encode('us-ascii') + b'\n') else: - write(b"OK\n") + write(b'OK\n') elif stripped == b'HASCERT': if sslconn.getpeercert() is not None: @@ -447,19 +447,19 @@ def wrap_conn(socket): elif stripped == b'GETCERT': cert = sslconn.getpeercert() - write(repr(cert).encode("us-ascii") + b"\n") + write(repr(cert).encode('us-ascii') + b'\n') elif stripped == b'VERIFIEDCHAIN': certs = sslconn._sslobj.get_verified_chain() - write(len(certs).to_bytes(1, "big") + b"\n") + write(len(certs).to_bytes(1, 'big') + b'\n') elif stripped == b'UNVERIFIEDCHAIN': certs = sslconn._sslobj.get_unverified_chain() - write(len(certs).to_bytes(1, "big") + b"\n") + write(len(certs).to_bytes(1, 'big') + b'\n') else: - ctype = "encrypted" if sslconn else "unencrypted" - log(f"read {msg} ({ctype}), sending back {msg.lower()} ({ctype})") + ctype = 'encrypted' if sslconn else 'unencrypted' + log(f'read {msg} ({ctype}), sending back {msg.lower()} ({ctype})') write(msg.lower()) try: From 2d7d843483d3bad123c90fd8dc694907df0c0f6e Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 09:35:30 +0300 Subject: [PATCH 28/83] Account that SSLError subclasses OSError --- Lib/test/support/threading_helper.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index c38970716eaae1..015dab771a5535 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -312,11 +312,16 @@ def _thread_func(self, server_socket, client_func, client_count, with client: results.append(client_func(client, peer_address, *args, **kwargs)) - # OSError is caused by read()/write() on a socket unexpectedly - # closed by a client. - except (ConnectionAbortedError, ConnectionResetError, OSError): + except (ConnectionAbortedError, ConnectionResetError): if not client_fails: raise + # OSError is caused by read()/write() on a socket unexpectedly + # closed by a client. However, important exceprions like + # ssl.SSLError subclass OSError so we need a separate logic + # to split them away. Fortunately, they always set errno to 0. + except OSError as e: + if not client_fails or e.errno != 0: + raise return results def __enter__(self): From 1418cc348b6df4177f478ef800b28362ac1de209 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 10:16:52 +0300 Subject: [PATCH 29/83] Debug output of whatever happens with SSLError --- Lib/test/support/threading_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 015dab771a5535..a33753e074f246 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -320,6 +320,7 @@ def _thread_func(self, server_socket, client_func, client_count, # ssl.SSLError subclass OSError so we need a separate logic # to split them away. Fortunately, they always set errno to 0. except OSError as e: + print('*************************************** {e:r}') if not client_fails or e.errno != 0: raise return results From 74bd397e5d07b831c43fa86fa668fba60701488b Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 10:42:22 +0300 Subject: [PATCH 30/83] (once again) --- Lib/test/support/threading_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index a33753e074f246..c1e140e73267c0 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -320,7 +320,7 @@ def _thread_func(self, server_socket, client_func, client_count, # ssl.SSLError subclass OSError so we need a separate logic # to split them away. Fortunately, they always set errno to 0. except OSError as e: - print('*************************************** {e:r}') + print(f'*************************************** {e:r}') if not client_fails or e.errno != 0: raise return results From 2b3cabf49a3d9563eb6e057686355f3e2821c3b0 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 14:16:50 +0300 Subject: [PATCH 31/83] (and again) --- Lib/test/support/threading_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index c1e140e73267c0..d3e9f4cd35113a 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -320,7 +320,7 @@ def _thread_func(self, server_socket, client_func, client_count, # ssl.SSLError subclass OSError so we need a separate logic # to split them away. Fortunately, they always set errno to 0. except OSError as e: - print(f'*************************************** {e:r}') + print('***************************************', e) if not client_fails or e.errno != 0: raise return results From 555bde6743fca744cae623b2bdfa15e7a14779d8 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 15:25:51 +0300 Subject: [PATCH 32/83] SSLError seems to have nonzero errno - print it --- Lib/test/support/threading_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index d3e9f4cd35113a..f54a092519cebf 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -320,7 +320,7 @@ def _thread_func(self, server_socket, client_func, client_count, # ssl.SSLError subclass OSError so we need a separate logic # to split them away. Fortunately, they always set errno to 0. except OSError as e: - print('***************************************', e) + print('***************************************', e.errno, e) if not client_fails or e.errno != 0: raise return results From 8578b98b1c67f3aca9112ce9e1f96a5755f2501a Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 16:10:15 +0300 Subject: [PATCH 33/83] Properly separate exceptions --- Lib/test/support/threading_helper.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index f54a092519cebf..c4da8bb336eab5 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -318,10 +318,9 @@ def _thread_func(self, server_socket, client_func, client_count, # OSError is caused by read()/write() on a socket unexpectedly # closed by a client. However, important exceprions like # ssl.SSLError subclass OSError so we need a separate logic - # to split them away. Fortunately, they always set errno to 0. + # to split them away. except OSError as e: - print('***************************************', e.errno, e) - if not client_fails or e.errno != 0: + if not client_fails or type(e) is not OSError: raise return results From 8858099730c799cb6c5b202fcc5c1b42e2f115ab Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 17:09:38 +0300 Subject: [PATCH 34/83] After all fixes, try to rely on natural garbage collection of thread pools --- Lib/test/support/threading_helper.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index c4da8bb336eab5..5e865533cd25cf 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -11,12 +11,7 @@ from test import support -def release_thread_pool(): - global _thread_pool - _thread_pool = None - _thread_pool = ThreadPoolExecutor() -unittest.addModuleCleanup(release_thread_pool) #======================================================================= # Threading support to prevent reporting refleaks when running regrtest.py -R From 59773f879953709a464132da0e1e9517ee9392dc Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 17:36:53 +0300 Subject: [PATCH 35/83] Another attempt at threadpool cleanup --- Lib/test/support/threading_helper.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 5e865533cd25cf..78c49573cce73a 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -11,7 +11,14 @@ from test import support -_thread_pool = ThreadPoolExecutor() +def setUpModule(): + global _thread_pool + _thread_pool = ThreadPoolExecutor() + + +def tearDownModule(): + global _thread_pool + _thread_pool = None #======================================================================= # Threading support to prevent reporting refleaks when running regrtest.py -R From 29e6a6de493409cac1e442ab7543d84b541cc892 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 17:47:15 +0300 Subject: [PATCH 36/83] Make global threadpool explicit --- Lib/test/support/threading_helper.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 78c49573cce73a..bbe7cccbc5098b 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -11,15 +11,17 @@ from test import support +_thread_pool = None + + def setUpModule(): - global _thread_pool _thread_pool = ThreadPoolExecutor() def tearDownModule(): - global _thread_pool _thread_pool = None + #======================================================================= # Threading support to prevent reporting refleaks when running regrtest.py -R From c35630ee88ad7656129bb619a5b57336d6253f43 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 17:57:31 +0300 Subject: [PATCH 37/83] Another attempt --- Lib/test/support/threading_helper.py | 9 +++++---- Lib/test/test_ssl.py | 4 ++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index bbe7cccbc5098b..0cbe1f1242b15e 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -14,12 +14,13 @@ _thread_pool = None -def setUpModule(): - _thread_pool = ThreadPoolExecutor() +def _release(): + _thread_pool = None -def tearDownModule(): - _thread_pool = None +def init(): + _thread_pool = ThreadPoolExecutor() + unittest.addModuleCleanup(_release) #======================================================================= diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 86590da9b81bcb..316d9a7d8169c5 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4969,5 +4969,9 @@ def setUpModule(): raise support.TestFailed("Can't read certificate file %r" % filename) +def setUpModule(): + threading_helper.init() + + if __name__ == "__main__": unittest.main() From 0a671997799231ffcf868f25c3f16b63c6463ce7 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 17:59:39 +0300 Subject: [PATCH 38/83] Merge two setUpModule copies --- Lib/test/test_ssl.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 316d9a7d8169c5..85b15cc6b5f4b7 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4968,8 +4968,6 @@ def setUpModule(): if not os.path.exists(filename): raise support.TestFailed("Can't read certificate file %r" % filename) - -def setUpModule(): threading_helper.init() From 02a1bc884b5ec74e8959fd3a02e1e42ca2e0c03e Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 19:16:00 +0300 Subject: [PATCH 39/83] Explicit waiting on futures seem to destabilize a program --- Lib/test/support/threading_helper.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 0cbe1f1242b15e..391fb07e0a85d3 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -1,5 +1,5 @@ import _thread -from concurrent.futures import ThreadPoolExecutor, wait +from concurrent.futures import ThreadPoolExecutor import contextlib import functools import sys @@ -15,10 +15,12 @@ def _release(): + global _thread_pool _thread_pool = None def init(): + global _thread_pool _thread_pool = ThreadPoolExecutor() unittest.addModuleCleanup(_release) @@ -333,7 +335,6 @@ def __enter__(self): return HOST, self._port def __exit__(self, etype, evalue, traceback): - wait([self._result]) if etype is ConnectionAbortedError or etype is ConnectionResetError: if self._result.exception() is not None: generic = RuntimeError('server-side error') From 5c15509959ec7270d24b6affbc05ec5fc696256b Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 19:19:27 +0300 Subject: [PATCH 40/83] Fix incorrect pass of normal results from a server to a client --- Lib/test/support/threading_helper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 391fb07e0a85d3..aa2702f43ef6f2 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -271,7 +271,7 @@ class Server: The server listens on an address returned from the ``with`` statement. """ - def __init__(self, client_func, *args, client_count=1, results=[], + def __init__(self, client_func, *args, client_count=1, results=None, client_fails=False, **kwargs): """Create and run the server. @@ -306,7 +306,7 @@ def __init__(self, client_func, *args, client_count=1, results=[], self._result = _thread_pool.submit(self._thread_func, server_socket, client_func, client_count, client_fails, args, kwargs) - self._result_out = results + self._result_out = results if results else [] def _thread_func(self, server_socket, client_func, client_count, client_fails, args, kwargs): @@ -340,5 +340,5 @@ def __exit__(self, etype, evalue, traceback): generic = RuntimeError('server-side error') raise generic from self._result.exception() return False - self._result_out = self._result.result() + self._result_out.append(self._result.result()) return False From 404d1204ca3a8d0952287e97dd16fc894875b26f Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 20:27:36 +0300 Subject: [PATCH 41/83] Simplify expected exception regex --- Lib/test/test_ssl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 85b15cc6b5f4b7..f317e7b2ad1023 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3019,7 +3019,7 @@ def test_check_hostname_incorrect(self): client_context, server_context, hostname = testing_context() - with self.assertRaisesRegex(ssl.SSLError, 'SSL: SSLV3_ALERT_BAD_CERTIFICATE'): + with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): with Server(context=server_context, client_fails=True) as address: with client_context.wrap_socket(socket.socket(), server_hostname="invalid") as s: From a2a526ea0921118da52384a79df2683b886520f1 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 20:33:29 +0300 Subject: [PATCH 42/83] Show what really is thrown on macOS runner --- Lib/test/test_ssl.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index f317e7b2ad1023..79ff55bad98bba 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3020,13 +3020,18 @@ def test_check_hostname_incorrect(self): client_context, server_context, hostname = testing_context() with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): - with Server(context=server_context, client_fails=True) as address: - with client_context.wrap_socket(socket.socket(), - server_hostname="invalid") as s: - with self.assertRaisesRegex( - ssl.CertificateError, - "Hostname mismatch, certificate is not valid for 'invalid'."): - s.connect(address) + try: + with Server(context=server_context, client_fails=True) as address: + with client_context.wrap_socket(socket.socket(), + server_hostname="invalid") as s: + with self.assertRaisesRegex( + ssl.CertificateError, + "Hostname mismatch, certificate is not valid for 'invalid'."): + s.connect(address) + except Excaption as e: + print('**************************************************') + print(e) + raise def test_check_hostname_missing(self): if support.verbose: From c7a3f133f7d1e7ca8ea3a224cfa2666768d1483b Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 20:38:03 +0300 Subject: [PATCH 43/83] typo --- Lib/test/test_ssl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 79ff55bad98bba..32e3741eb8676a 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3028,7 +3028,7 @@ def test_check_hostname_incorrect(self): ssl.CertificateError, "Hostname mismatch, certificate is not valid for 'invalid'."): s.connect(address) - except Excaption as e: + except Exception as e: print('**************************************************') print(e) raise From 2caabcbae7edc037d65de983240f952863ea574c Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 20:57:21 +0300 Subject: [PATCH 44/83] Context managers are not terminated after a `with` section --- Lib/test/support/threading_helper.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index aa2702f43ef6f2..2c17316f608d69 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -271,7 +271,7 @@ class Server: The server listens on an address returned from the ``with`` statement. """ - def __init__(self, client_func, *args, client_count=1, results=None, + def __init__(self, client_func, *args, client_count=1, client_fails=False, **kwargs): """Create and run the server. @@ -280,6 +280,10 @@ def __init__(self, client_func, *args, client_count=1, results=None, When client_func raises an exception, all server-side sockets are closed. + If a client_func returns a value, it got stored as a ``Server.result`` + field. Since ``with ... as`` section keeps its parameter alive, the + field can be accessed outside of the section. + Args: client_func: a function called in a dedicated thread for each new connected client. The function receives all argument passed to @@ -293,7 +297,7 @@ def __init__(self, client_func, *args, client_count=1, results=None, client_fails: if True, a client is expected to cause connection-related exceptions by reasons asserted on its side. As a result, a server should swallow these exceptions and - proceed to the next client instead. + proceed to the next client instead. kwargs: keyword arguments passed to client_func. Throws: @@ -306,7 +310,6 @@ def __init__(self, client_func, *args, client_count=1, results=None, self._result = _thread_pool.submit(self._thread_func, server_socket, client_func, client_count, client_fails, args, kwargs) - self._result_out = results if results else [] def _thread_func(self, server_socket, client_func, client_count, client_fails, args, kwargs): @@ -340,5 +343,5 @@ def __exit__(self, etype, evalue, traceback): generic = RuntimeError('server-side error') raise generic from self._result.exception() return False - self._result_out.append(self._result.result()) + self.result = self._result.result() return False From 70f8f9023b4a92b49988b1be8c32595235e15b2f Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Thu, 7 Jul 2022 21:20:00 +0300 Subject: [PATCH 45/83] Temporarily disable expected exception handling --- Lib/test/test_ssl.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 32e3741eb8676a..a457d62014014a 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3019,19 +3019,19 @@ def test_check_hostname_incorrect(self): client_context, server_context, hostname = testing_context() - with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): - try: - with Server(context=server_context, client_fails=True) as address: - with client_context.wrap_socket(socket.socket(), - server_hostname="invalid") as s: - with self.assertRaisesRegex( - ssl.CertificateError, - "Hostname mismatch, certificate is not valid for 'invalid'."): - s.connect(address) - except Exception as e: - print('**************************************************') - print(e) - raise + #with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): + #try: + with Server(context=server_context, client_fails=True) as address: + with client_context.wrap_socket(socket.socket(), + server_hostname="invalid") as s: + with self.assertRaisesRegex( + ssl.CertificateError, + "Hostname mismatch, certificate is not valid for 'invalid'."): + s.connect(address) + #except Exception as e: + # print('**************************************************') + # print(e) + # raise def test_check_hostname_missing(self): if support.verbose: From 1f1da1ee406709124837be449229ee6aea8a13f2 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 8 Jul 2022 07:40:05 +0300 Subject: [PATCH 46/83] Sort out discrepancy between macOS and others --- Lib/test/test_ssl.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index a457d62014014a..2cb19d76f82ced 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3013,25 +3013,26 @@ def test_check_hostname_correct(self): cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") - def test_check_hostname_incorrect(self): + def _do_test_check_hostname_incorrect(self): if support.verbose: sys.stdout.write("\n") client_context, server_context, hostname = testing_context() - #with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): - #try: with Server(context=server_context, client_fails=True) as address: - with client_context.wrap_socket(socket.socket(), - server_hostname="invalid") as s: + with client_context.wrap_socket(socket.socket(), + server_hostname="invalid") as s: with self.assertRaisesRegex( ssl.CertificateError, "Hostname mismatch, certificate is not valid for 'invalid'."): s.connect(address) - #except Exception as e: - # print('**************************************************') - # print(e) - # raise + + def test_check_hostname_incorrect(self): + if sys.platform == 'darwin': + self._do_test_check_hostname_incorrect() + else: + with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): + self._do_test_check_hostname_incorrect() def test_check_hostname_missing(self): if support.verbose: From 23cf45d2511793fd7cd88fb3dbdf93b1853c2bfe Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 8 Jul 2022 22:34:47 +0300 Subject: [PATCH 47/83] Final touch-ups --- Lib/test/support/threading_helper.py | 23 +++++++++++++++-------- Lib/test/test_ssl.py | 4 ++-- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 2c17316f608d69..de0864fc958593 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -15,12 +15,12 @@ def _release(): - global _thread_pool + nonlocal _thread_pool _thread_pool = None def init(): - global _thread_pool + nonlocal _thread_pool _thread_pool = ThreadPoolExecutor() unittest.addModuleCleanup(_release) @@ -277,12 +277,19 @@ def __init__(self, client_func, *args, client_count=1, The method blocks until a server is ready to accept clients. - When client_func raises an exception, all server-side sockets are - closed. + The server: - If a client_func returns a value, it got stored as a ``Server.result`` - field. Since ``with ... as`` section keeps its parameter alive, the - field can be accessed outside of the section. + 1. consequently waits for client_count clients + 2. Calls client_func for each of them + 3. Closes client connection when the function returns + 4. Collects returned values into Server.result list + 5. Terminates a server + 6. Allows a context manager to exit + 7. Since ``with ... as`` section keeps its parameter alive, the field + can be accessed outside of the section. + + If client_func raises an exception, the server is stopped, all pending + clients are discarded and the context manager raises an exception. Args: client_func: a function called in a dedicated thread for each new @@ -302,7 +309,7 @@ def __init__(self, client_func, *args, client_count=1, Throws: When client_func throws, this method catches the exception, wraps - it into RuntimeError("server-side error") and rethrows. + it into RuntimeError('server-side error') and rethrows. """ server_socket = socket() self._port = bind_port(server_socket) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 2cb19d76f82ced..4576150fa49f61 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3020,8 +3020,8 @@ def _do_test_check_hostname_incorrect(self): client_context, server_context, hostname = testing_context() with Server(context=server_context, client_fails=True) as address: - with client_context.wrap_socket(socket.socket(), - server_hostname="invalid") as s: + with client_context.wrap_socket(socket.socket(), + server_hostname="invalid") as s: with self.assertRaisesRegex( ssl.CertificateError, "Hostname mismatch, certificate is not valid for 'invalid'."): From 2ee7b5ad76d1d2a7a2b218784b55c17b3ee9fffe Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 8 Jul 2022 22:39:28 +0300 Subject: [PATCH 48/83] Fix `SyntaxError: no binding for nonlocal '_thread_pool' found` --- Lib/test/support/threading_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index de0864fc958593..9c2fb1676aa129 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -15,12 +15,12 @@ def _release(): - nonlocal _thread_pool + global _thread_pool _thread_pool = None def init(): - nonlocal _thread_pool + global _thread_pool _thread_pool = ThreadPoolExecutor() unittest.addModuleCleanup(_release) From a77d6e48ced4d9dbe8e2e4b57ff448733bea1de9 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 8 Jul 2022 23:33:21 +0300 Subject: [PATCH 49/83] Stabilize tests by forcing TLS handshake with empty data packets --- Lib/test/support/threading_helper.py | 2 +- Lib/test/test_ssl.py | 26 ++++++++++++++++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 9c2fb1676aa129..1016e52b87da8e 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -279,7 +279,7 @@ def __init__(self, client_func, *args, client_count=1, The server: - 1. consequently waits for client_count clients + 1. Consequently waits for client_count clients 2. Calls client_func for each of them 3. Closes client connection when the function returns 4. Collects returned values into Server.result list diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 36fdb3d3a92a14..2d020c37c9e264 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1053,12 +1053,8 @@ def test_read_write_zero(self): s.connect(address) self.assertEqual(s.recv(0), b"") self.assertEqual(s.send(b""), 0) - # A temporary workaround for delayed tlsv1.3 session ticket - # eachange; the proper fix will be merges after this PR because - # it hangs test_session_handling. - # - # This problem manufested before as well but was just logged - # andsilenced by a server thread. + # The same motivalion as behind self.wait_connection but + # without sending any data. s.unwrap().close() @@ -2849,6 +2845,16 @@ def try_protocol_combo(server_protocol, client_protocol, expect_success, class ThreadedTests(unittest.TestCase): + def wait_connection(self, socket): + """Force a socket to immediately initiate and process a TLS handshake. + + TLS 1.3 delays session ticket exchange until some data are sent. So we + need to dend some data to avoid ConnectionAbortedError ("An established + connection was aborted by the software in your host machine") caused + by closing half-open TLS connection. + """ + socket.write(b'') + def test_echo(self): """Basic test of an SSL client connecting to a server""" if support.verbose: @@ -2980,6 +2986,7 @@ def test_check_hostname_correct(self): with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: s.connect(address) + self.wait_connection(s) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") @@ -2996,6 +3003,7 @@ def _do_test_check_hostname_incorrect(self): ssl.CertificateError, "Hostname mismatch, certificate is not valid for 'invalid'."): s.connect(address) + self.wait_connection(s) def test_check_hostname_incorrect(self): if sys.platform == 'darwin': @@ -3054,6 +3062,7 @@ def test_ecc_cert(self): with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: s.connect(address) + self.wait_connection(s) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") cipher = s.cipher()[0].split('-') @@ -3079,6 +3088,7 @@ def test_dual_rsa_ecc(self): with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: s.connect(address) + self.wait_connection(s) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") cipher = s.cipher()[0].split('-') @@ -3911,6 +3921,7 @@ def test_tls_unique_channel_binding(self): socket.socket(), server_hostname=hostname) as s: s.connect(address) + self.wait_connection(s) # get the data cb_data = s.get_channel_binding("tls-unique") if support.verbose: @@ -4296,6 +4307,7 @@ def test_session_handling(self): self.assertEqual(s.session, None) self.assertEqual(s.session_reused, None) s.connect(address) + self.wait_connection(s) session = s.session self.assertTrue(session) with self.assertRaises(TypeError) as e: @@ -4305,6 +4317,7 @@ def test_session_handling(self): with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: s.connect(address) + self.wait_connection(s) # cannot set session after handshake with self.assertRaises(ValueError) as e: s.session = session @@ -4317,6 +4330,7 @@ def test_session_handling(self): # connection was established s.session = session s.connect(address) + self.wait_connection(s) self.assertEqual(s.session.id, session.id) self.assertEqual(s.session, session) self.assertEqual(s.session_reused, True) From 7e85ec88cc9104a31721c773ac54dd54264945ce Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Fri, 8 Jul 2022 23:35:39 +0300 Subject: [PATCH 50/83] Remove unnecessary wait_connection invocations --- Lib/test/test_ssl.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 2d020c37c9e264..1f598421b3749e 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3003,7 +3003,6 @@ def _do_test_check_hostname_incorrect(self): ssl.CertificateError, "Hostname mismatch, certificate is not valid for 'invalid'."): s.connect(address) - self.wait_connection(s) def test_check_hostname_incorrect(self): if sys.platform == 'darwin': From d167f1c3ae7150c0be69f3ec88f95d3ebc829401 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 00:08:37 +0300 Subject: [PATCH 51/83] Temporarily show exact exception --- Lib/test/support/threading_helper.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 1016e52b87da8e..20af4a45288655 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -347,6 +347,8 @@ def __enter__(self): def __exit__(self, etype, evalue, traceback): if etype is ConnectionAbortedError or etype is ConnectionResetError: if self._result.exception() is not None: + print('*******************************************') + print(etype, evalue) generic = RuntimeError('server-side error') raise generic from self._result.exception() return False From 910083bfe73e65e080d0410babcb6c36f653f0f3 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 00:32:16 +0300 Subject: [PATCH 52/83] Revert "Temporarily show exact exception" This reverts commit d167f1c3ae7150c0be69f3ec88f95d3ebc829401. --- Lib/test/support/threading_helper.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 20af4a45288655..1016e52b87da8e 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -347,8 +347,6 @@ def __enter__(self): def __exit__(self, etype, evalue, traceback): if etype is ConnectionAbortedError or etype is ConnectionResetError: if self._result.exception() is not None: - print('*******************************************') - print(etype, evalue) generic = RuntimeError('server-side error') raise generic from self._result.exception() return False From 458a0a686c333d6be83d5f36a059c9400e40c910 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 08:10:28 +0300 Subject: [PATCH 53/83] An attempt to fix in-handshake connection abruption --- Lib/test/support/threading_helper.py | 4 ++-- Lib/test/test_ssl.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 1016e52b87da8e..055f26512d3d4a 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -327,8 +327,8 @@ def _thread_func(self, server_socket, client_func, client_count, try: client, peer_address = server_socket.accept() with client: - results.append(client_func(client, peer_address, - *args, **kwargs)) + r = client_func(client, peer_address, *args, **kwargs) + results.append(r) except (ConnectionAbortedError, ConnectionResetError): if not client_fails: raise diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 1f598421b3749e..b091116a6f6e6f 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -374,7 +374,7 @@ def wrap_conn(socket): try: sslconn = context.wrap_socket(socket, server_side=True) nonlocal read, write, close - read = lambda: sslconn.read() + read = lambda: sslconn.recv(1024) write = sslconn.write close = sslconn.close @@ -2849,11 +2849,15 @@ def wait_connection(self, socket): """Force a socket to immediately initiate and process a TLS handshake. TLS 1.3 delays session ticket exchange until some data are sent. So we - need to dend some data to avoid ConnectionAbortedError ("An established - connection was aborted by the software in your host machine") caused - by closing half-open TLS connection. + need to send some data to avoid server-side ConnectionAbortedError + ("An established connection was aborted by the software in your host + machine") caused by closing half-open TLS connection. + + For more details on how TLS 1.3 changed the handshake, see + . """ - socket.write(b'') + socket.write(b'ECHO hi') + socket.read(100) def test_echo(self): """Basic test of an SSL client connecting to a server""" From 1fa3f0c38a850ca3e64ff7dcc4799ca0f58cf2ca Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 10:18:59 +0300 Subject: [PATCH 54/83] Reorder imports --- Lib/test/support/threading_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 055f26512d3d4a..48d7749e27dc73 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -1,13 +1,13 @@ import _thread -from concurrent.futures import ThreadPoolExecutor import contextlib import functools import sys -from test.support.socket_helper import bind_port, HOST import threading from socket import socket import time import unittest +from concurrent.futures import ThreadPoolExecutor +from test.support.socket_helper import bind_port, HOST from test import support From 17c56dc3ab3382932fe851ea07a28cb368f58d57 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 10:48:18 +0300 Subject: [PATCH 55/83] Fix potential infinite waiting on handshake enforcement --- Lib/test/test_ssl.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index b091116a6f6e6f..57170912a18398 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2856,8 +2856,9 @@ def wait_connection(self, socket): For more details on how TLS 1.3 changed the handshake, see . """ - socket.write(b'ECHO hi') - socket.read(100) + echo_message = b'hi' + socket.write(echo_message) + socket.read(len(echo_message)) def test_echo(self): """Basic test of an SSL client connecting to a server""" From 37e3470875309d3ba6a9c0b4176fd23ab218092b Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 11:18:53 +0300 Subject: [PATCH 56/83] Debug why runners hang --- Lib/test/test_ssl.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 57170912a18398..3d113fb34c475c 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2857,8 +2857,11 @@ def wait_connection(self, socket): . """ echo_message = b'hi' + print('******************* forcing the handshake') socket.write(echo_message) + print('******************* sent') socket.read(len(echo_message)) + print('******************* received') def test_echo(self): """Basic test of an SSL client connecting to a server""" From 6b196f92ad2be38b9060d5a11ba22a29df6b0593 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 17:10:10 +0300 Subject: [PATCH 57/83] Try to remove server response waiting --- Lib/test/test_ssl.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 3d113fb34c475c..8f809e5b7eaeff 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2857,11 +2857,7 @@ def wait_connection(self, socket): . """ echo_message = b'hi' - print('******************* forcing the handshake') socket.write(echo_message) - print('******************* sent') - socket.read(len(echo_message)) - print('******************* received') def test_echo(self): """Basic test of an SSL client connecting to a server""" From 2152234bc42e4591f2a8065b3e62fa378c4e3010 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 19:14:34 +0300 Subject: [PATCH 58/83] Discriminate when we need to wait server response --- Lib/test/test_ssl.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 8f809e5b7eaeff..e40e3c93269b57 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2845,7 +2845,7 @@ def try_protocol_combo(server_protocol, client_protocol, expect_success, class ThreadedTests(unittest.TestCase): - def wait_connection(self, socket): + def wait_connection(self, socket, *, wait_response=True): """Force a socket to immediately initiate and process a TLS handshake. TLS 1.3 delays session ticket exchange until some data are sent. So we @@ -2855,9 +2855,14 @@ def wait_connection(self, socket): For more details on how TLS 1.3 changed the handshake, see . + + In some testing sequences the function can block in indefinite waiting + of server response. In such a case, set wait_response argument to False. """ echo_message = b'hi' socket.write(echo_message) + if wait_response: + socket.read(len(echo_message)) def test_echo(self): """Basic test of an SSL client connecting to a server""" @@ -2985,12 +2990,11 @@ def test_check_hostname_correct(self): client_context, server_context, hostname = testing_context() - # correct hostname should verify with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: s.connect(address) - self.wait_connection(s) + self.wait_connection(s, wait_response=False) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") From f4406cf49d8dc27bdbf0cac6ddf430d3fb420032 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 20:10:09 +0300 Subject: [PATCH 59/83] TEMPORARILY force full-fledged logging --- Lib/test/test_ssl.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index e40e3c93269b57..eb6be0fc77d0fa 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -329,8 +329,7 @@ def _on_ssl_client(socket, peer_address, certificate=None, # that we can test the STARTTLS functionality. def log(message): - if support.verbose and chatty: - sys.stdout.write(f' server: {message}\n') + sys.stdout.write(f' server: {message}\n') if context is None: context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) From 9f6e8266a2cb01d45441e16cd347d2b3eb6cd91a Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 22:42:14 +0300 Subject: [PATCH 60/83] Add a socket timeout to not hang the whole test --- Lib/test/test_ssl.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index eb6be0fc77d0fa..b3498b4ebb2175 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2858,6 +2858,8 @@ def wait_connection(self, socket, *, wait_response=True): In some testing sequences the function can block in indefinite waiting of server response. In such a case, set wait_response argument to False. """ + socket.settimeout(support.LOOPBACK_TIMEOUT) + echo_message = b'hi' socket.write(echo_message) if wait_response: From 19486c97e0a14c31d00ac73518d639e30991b088 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 23:43:11 +0300 Subject: [PATCH 61/83] Fix server-side echo --- Lib/test/test_ssl.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index b3498b4ebb2175..3da7ebe9bcacad 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -329,7 +329,8 @@ def _on_ssl_client(socket, peer_address, certificate=None, # that we can test the STARTTLS functionality. def log(message): - sys.stdout.write(f' server: {message}\n') + if support.verbose and chatty: + sys.stdout.write(f' server: {message}\n') if context is None: context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) @@ -458,8 +459,10 @@ def wrap_conn(socket): else: ctype = 'encrypted' if sslconn else 'unencrypted' - log(f'read {msg} ({ctype}), sending back {msg.lower()} ({ctype})') - write(msg.lower()) + in_str = msg.decode() + out_str = in_str.lower() + log(f'read {in_str} ({ctype}), sending back {out_str} ({ctype})') + write(out_str.encode()) try: socket = sslconn.unwrap() @@ -2844,7 +2847,7 @@ def try_protocol_combo(server_protocol, client_protocol, expect_success, class ThreadedTests(unittest.TestCase): - def wait_connection(self, socket, *, wait_response=True): + def wait_connection(self, socket): """Force a socket to immediately initiate and process a TLS handshake. TLS 1.3 delays session ticket exchange until some data are sent. So we @@ -2854,16 +2857,15 @@ def wait_connection(self, socket, *, wait_response=True): For more details on how TLS 1.3 changed the handshake, see . - - In some testing sequences the function can block in indefinite waiting - of server response. In such a case, set wait_response argument to False. """ + + # If server throws, it may continue to keep a living client connection. + # So we need to disconnect client on timeout. socket.settimeout(support.LOOPBACK_TIMEOUT) echo_message = b'hi' socket.write(echo_message) - if wait_response: - socket.read(len(echo_message)) + socket.read(len(echo_message)) def test_echo(self): """Basic test of an SSL client connecting to a server""" @@ -2995,7 +2997,7 @@ def test_check_hostname_correct(self): with client_context.wrap_socket(socket.socket(), server_hostname=hostname) as s: s.connect(address) - self.wait_connection(s, wait_response=False) + self.wait_connection(s) cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") From cad138b11408395d5a258f3a8fb31ade22176804 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 9 Jul 2022 23:58:33 +0300 Subject: [PATCH 62/83] Fix THE REAL REASON: sockets do not implement context managers hence do not autoclose --- Lib/test/support/threading_helper.py | 4 ++-- Lib/test/test_ssl.py | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 48d7749e27dc73..b41d1c3ce5ef74 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -321,12 +321,12 @@ def __init__(self, client_func, *args, client_count=1, def _thread_func(self, server_socket, client_func, client_count, client_fails, args, kwargs): server_socket.settimeout(support.LOOPBACK_TIMEOUT) - with server_socket: + with contextlib.closing(server_socket): results = [] for i in range(client_count): try: client, peer_address = server_socket.accept() - with client: + with contextlib.closing(client): r = client_func(client, peer_address, *args, **kwargs) results.append(r) except (ConnectionAbortedError, ConnectionResetError): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 3da7ebe9bcacad..86b9d66bb46075 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2858,11 +2858,6 @@ def wait_connection(self, socket): For more details on how TLS 1.3 changed the handshake, see . """ - - # If server throws, it may continue to keep a living client connection. - # So we need to disconnect client on timeout. - socket.settimeout(support.LOOPBACK_TIMEOUT) - echo_message = b'hi' socket.write(echo_message) socket.read(len(echo_message)) From 4ced1f83107097c73e2b737c73e75f20ec10d563 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 07:09:01 +0300 Subject: [PATCH 63/83] Run patchcheck.py --- Lib/test/test_ssl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 86b9d66bb46075..670a22f6fc158f 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -459,7 +459,7 @@ def wrap_conn(socket): else: ctype = 'encrypted' if sslconn else 'unencrypted' - in_str = msg.decode() + in_str = msg.decode() out_str = in_str.lower() log(f'read {in_str} ({ctype}), sending back {out_str} ({ctype})') write(out_str.encode()) From 78225c0d856025d1dba61490fc65bacea6986aa9 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 07:25:57 +0300 Subject: [PATCH 64/83] Reword a NEWS entry --- .../next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst index 1343a6666476a2..b62b8f731559c7 100644 --- a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst +++ b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst @@ -1,2 +1,2 @@ -Added a unified client-side context for managing a TCP server in its own thread -(:class:`test.support.threading_helper.Server`). Patch by Oleg Iarygin. +Added a unified :class:`test.support.threading_helper.Server` context for +management of a blocking TCP server. Patch by Oleg Iarygin. From eba78fafee18ce51b0717f1953509ec3b5516c67 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 07:32:44 +0300 Subject: [PATCH 65/83] Add socket timeout to all server-related tests --- Lib/test/support/threading_helper.py | 1 - Lib/test/test_ssl.py | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index b41d1c3ce5ef74..518655bcdc4cf4 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -320,7 +320,6 @@ def __init__(self, client_func, *args, client_count=1, def _thread_func(self, server_socket, client_func, client_count, client_fails, args, kwargs): - server_socket.settimeout(support.LOOPBACK_TIMEOUT) with contextlib.closing(server_socket): results = [] for i in range(client_count): diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 670a22f6fc158f..0340665d4a72f1 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1921,12 +1921,18 @@ class SimpleBackgroundTests(unittest.TestCase): """Tests that connect to a simple server running in the background""" def setUp(self): + self._old_socket_timeout = socket.getdefaulttimeout() + socket.setdefaulttimeout(support.LOOPBACK_TIMEOUT) + self.server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) self.server_context.load_cert_chain(SIGNED_CERTFILE) server = ThreadedEchoServer(context=self.server_context) self.enterContext(server) self.server_addr = (HOST, server.port) + def tearDown(self): + socket.setdefaulttimeout(self._old_socket_timeout) + def test_connect(self): with test_wrap_socket(socket.socket(socket.AF_INET), cert_reqs=ssl.CERT_NONE) as s: From 5287287cd547dfc4c8f8602c2cf56da8630b8424 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 07:47:44 +0300 Subject: [PATCH 66/83] Restore code removed while debugging --- Lib/test/support/threading_helper.py | 26 +++++++++++-------- Lib/test/test_ssl.py | 1 + ...2-07-06-18-46-00.gh-issue-94609.eeM1cu.rst | 5 ++-- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 518655bcdc4cf4..54096600f593de 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -269,24 +269,32 @@ class Server: - to be used where asyncio has no application The server listens on an address returned from the ``with`` statement. + + For each client connected, the server calls a user-supplied function and + preserves whatever the function returns or throws to pass it to a client + later. + + When a client attempt to exit the context manager, it blocks until a server + stops processing all clients and exits. """ def __init__(self, client_func, *args, client_count=1, client_fails=False, **kwargs): """Create and run the server. - The method blocks until a server is ready to accept clients. + The method blocks until the server is ready to accept clients. - The server: + After this constructor returns, the server: 1. Consequently waits for client_count clients - 2. Calls client_func for each of them - 3. Closes client connection when the function returns - 4. Collects returned values into Server.result list + 1. For each client: + a. Calls client_func for each of them + b. Closes client connection when the function returns + c. Collects returned values into Server.result list 5. Terminates a server 6. Allows a context manager to exit - 7. Since ``with ... as`` section keeps its parameter alive, the field - can be accessed outside of the section. + 7. Since ``with ... as`` section keeps its parameter alive, + Server.result field can be accessed outside of the section. If client_func raises an exception, the server is stopped, all pending clients are discarded and the context manager raises an exception. @@ -306,10 +314,6 @@ def __init__(self, client_func, *args, client_count=1, As a result, a server should swallow these exceptions and proceed to the next client instead. kwargs: keyword arguments passed to client_func. - - Throws: - When client_func throws, this method catches the exception, wraps - it into RuntimeError('server-side error') and rethrows. """ server_socket = socket() self._port = bind_port(server_socket) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 0340665d4a72f1..0e1460a4fb4170 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4969,6 +4969,7 @@ def setUpModule(): if not os.path.exists(filename): raise support.TestFailed("Can't read certificate file %r" % filename) + thread_info = threading_helper.threading_setup() threading_helper.init() diff --git a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst index b62b8f731559c7..0b739f3cf5b3a9 100644 --- a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst +++ b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst @@ -1,2 +1,3 @@ -Added a unified :class:`test.support.threading_helper.Server` context for -management of a blocking TCP server. Patch by Oleg Iarygin. +Added a unified :class:`test.support.threading_helper.Server` context manager +to create, wait, destroy and communicate with a blocking TCP server both +in-bound and out-of-bound. Patch by Oleg Iarygin. From 71197b92e0d4d11720ede5371ef43f8db9cd2bb1 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 07:53:45 +0300 Subject: [PATCH 67/83] Restore more code --- Lib/test/test_ssl.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 0e1460a4fb4170..f5a5b3e45cfff2 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4970,6 +4970,7 @@ def setUpModule(): raise support.TestFailed("Can't read certificate file %r" % filename) thread_info = threading_helper.threading_setup() + unittest.addModuleCleanup(threading_helper.threading_cleanup, *thread_info) threading_helper.init() From feb2b206ee4615fd595bc5641321aec761a3e883 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 08:01:50 +0300 Subject: [PATCH 68/83] Try to replace server-side error swallow with client-side ignorance --- Lib/test/support/threading_helper.py | 35 +++++++++------------------- Lib/test/test_ssl.py | 2 +- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 54096600f593de..0e7be4210dcac8 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -278,8 +278,7 @@ class Server: stops processing all clients and exits. """ - def __init__(self, client_func, *args, client_count=1, - client_fails=False, **kwargs): + def __init__(self, client_func, *args, client_count=1, **kwargs): """Create and run the server. The method blocks until the server is ready to accept clients. @@ -309,10 +308,6 @@ def __init__(self, client_func, *args, client_count=1, results: a reference to a list for collecting client_func return values. Populated after execution leaves a ``with`` blocks associated with the Server context manager. - client_fails: if True, a client is expected to cause - connection-related exceptions by reasons asserted on its side. - As a result, a server should swallow these exceptions and - proceed to the next client instead. kwargs: keyword arguments passed to client_func. """ server_socket = socket() @@ -320,35 +315,27 @@ def __init__(self, client_func, *args, client_count=1, server_socket.listen() self._result = _thread_pool.submit(self._thread_func, server_socket, client_func, client_count, - client_fails, args, kwargs) + args, kwargs) def _thread_func(self, server_socket, client_func, client_count, - client_fails, args, kwargs): + args, kwargs): with contextlib.closing(server_socket): results = [] for i in range(client_count): - try: - client, peer_address = server_socket.accept() - with contextlib.closing(client): - r = client_func(client, peer_address, *args, **kwargs) - results.append(r) - except (ConnectionAbortedError, ConnectionResetError): - if not client_fails: - raise - # OSError is caused by read()/write() on a socket unexpectedly - # closed by a client. However, important exceprions like - # ssl.SSLError subclass OSError so we need a separate logic - # to split them away. - except OSError as e: - if not client_fails or type(e) is not OSError: - raise + client, peer_address = server_socket.accept() + with contextlib.closing(client): + r = client_func(client, peer_address, *args, **kwargs) + results.append(r) return results def __enter__(self): return HOST, self._port def __exit__(self, etype, evalue, traceback): - if etype is ConnectionAbortedError or etype is ConnectionResetError: + peer_willingly_closed = isinstance(etype, ConnectionError) + # We find our client disappeared when our socket read() fails. + peer_disappeared = etype is OSError + if peer_willingly_closed or peer_disappeared: if self._result.exception() is not None: generic = RuntimeError('server-side error') raise generic from self._result.exception() diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index f5a5b3e45cfff2..656aa6840c047e 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3008,7 +3008,7 @@ def _do_test_check_hostname_incorrect(self): client_context, server_context, hostname = testing_context() - with Server(context=server_context, client_fails=True) as address: + with Server(context=server_context) as address: with client_context.wrap_socket(socket.socket(), server_hostname="invalid") as s: with self.assertRaisesRegex( From e4c7fca30e15c4828e3073d6ade47d4efb58d992 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 08:41:20 +0300 Subject: [PATCH 69/83] Clarify a purpose of wait_connection() --- Lib/test/test_ssl.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 656aa6840c047e..ea1a3f7dfd9816 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1055,8 +1055,8 @@ def test_read_write_zero(self): s.connect(address) self.assertEqual(s.recv(0), b"") self.assertEqual(s.send(b""), 0) - # The same motivalion as behind self.wait_connection but - # without sending any data. + # OpenSSL postpones the handshake until some data are sent so + # force it by queuing explicit shutdown. s.unwrap().close() @@ -2856,13 +2856,12 @@ class ThreadedTests(unittest.TestCase): def wait_connection(self, socket): """Force a socket to immediately initiate and process a TLS handshake. - TLS 1.3 delays session ticket exchange until some data are sent. So we - need to send some data to avoid server-side ConnectionAbortedError - ("An established connection was aborted by the software in your host - machine") caused by closing half-open TLS connection. - - For more details on how TLS 1.3 changed the handshake, see - . + OpenSSL delays TLS 1.3 session ticket exchange until a socket user + attempts to send some data. As a result, we need to write al least + an empty message to force the handshake and avoid server-side + ConnectionAbortedError ("An established connection was aborted by the + software in your host machine") when some test has nothing to send so + closes a half-open TLS connection. """ echo_message = b'hi' socket.write(echo_message) From ed98010130093b47c0cf4b1e3b259ef7c8141eda Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 08:43:14 +0300 Subject: [PATCH 70/83] A typo --- Lib/test/test_ssl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index ea1a3f7dfd9816..90ca1a010b831b 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2857,8 +2857,8 @@ def wait_connection(self, socket): """Force a socket to immediately initiate and process a TLS handshake. OpenSSL delays TLS 1.3 session ticket exchange until a socket user - attempts to send some data. As a result, we need to write al least - an empty message to force the handshake and avoid server-side + attempts to send some data. As a result, we need to write some + non-empty string to force the handshake and avoid server-side ConnectionAbortedError ("An established connection was aborted by the software in your host machine") when some test has nothing to send so closes a half-open TLS connection. From cc1b400cb9ec3a5a13a0012040588589ebc015f9 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 09:07:40 +0300 Subject: [PATCH 71/83] Restore explicit server timeout; getdefaulttimeout does not propagate between threads --- Lib/test/support/threading_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 0e7be4210dcac8..6c8fc8a7d9427d 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -319,6 +319,7 @@ def __init__(self, client_func, *args, client_count=1, **kwargs): def _thread_func(self, server_socket, client_func, client_count, args, kwargs): + server_socket.settimeout(support.LOOPBACK_TIMEOUT) with contextlib.closing(server_socket): results = [] for i in range(client_count): From fd0468ef3c946f0bcd4beeff71effb166a9fdc2d Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 09:28:11 +0300 Subject: [PATCH 72/83] Try to fix the ENV CHANGED error probably caused by cleanup misorder --- Lib/test/test_ssl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 90ca1a010b831b..0e3faa1507f3a4 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4968,9 +4968,9 @@ def setUpModule(): if not os.path.exists(filename): raise support.TestFailed("Can't read certificate file %r" % filename) + threading_helper.init() thread_info = threading_helper.threading_setup() unittest.addModuleCleanup(threading_helper.threading_cleanup, *thread_info) - threading_helper.init() if __name__ == "__main__": From de27807b3251a680325d71714b62238c46c2e7a9 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 09:52:12 +0300 Subject: [PATCH 73/83] Add some debug scaffolding to track down a leaked thread --- Lib/test/support/threading_helper.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 6c8fc8a7d9427d..df05ec68bf3603 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -258,6 +258,8 @@ def requires_working_threading(*, module=False): return unittest.skipUnless(can_start_thread, msg) +global_thread_count = 0 + class Server: """A context manager for a blocking server in a thread pool. @@ -319,15 +321,22 @@ def __init__(self, client_func, *args, client_count=1, **kwargs): def _thread_func(self, server_socket, client_func, client_count, args, kwargs): - server_socket.settimeout(support.LOOPBACK_TIMEOUT) - with contextlib.closing(server_socket): - results = [] - for i in range(client_count): - client, peer_address = server_socket.accept() - with contextlib.closing(client): - r = client_func(client, peer_address, *args, **kwargs) - results.append(r) - return results + global global_thread_count + thread_pass = global_thread_count + global_thread_count = global_thread_count + 1 + sys.stdout.write(f'thread {thread_pass} started') + try: + server_socket.settimeout(support.LOOPBACK_TIMEOUT) + with contextlib.closing(server_socket): + results = [] + for i in range(client_count): + client, peer_address = server_socket.accept() + with contextlib.closing(client): + r = client_func(client, peer_address, *args, **kwargs) + results.append(r) + return results + finally: + sys.stdout.write(f'thread {thread_pass} exited') def __enter__(self): return HOST, self._port From b410384adc3603f38b18e467ddb36612f6baa953 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 11:36:10 +0300 Subject: [PATCH 74/83] Avoid thread leaks by shutting down the pool --- Lib/test/support/threading_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index df05ec68bf3603..c98aab5b6f1a4d 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -16,6 +16,7 @@ def _release(): global _thread_pool + _thread_pool.shutdown(cancel_futures=True) _thread_pool = None From 2a44b2a5d698686b4f0ee1414fd01637dcc5addd Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 16:22:59 +0300 Subject: [PATCH 75/83] Revert "Avoid thread leaks by shutting down the pool" This reverts commit b410384adc3603f38b18e467ddb36612f6baa953. --- Lib/test/support/threading_helper.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index c98aab5b6f1a4d..df05ec68bf3603 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -16,7 +16,6 @@ def _release(): global _thread_pool - _thread_pool.shutdown(cancel_futures=True) _thread_pool = None From 1f6378a6a9e3bf1ca52b90491c5931bb5ccdbc7a Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 15:34:38 +0300 Subject: [PATCH 76/83] Remove threading control (a thread pool interferes with it) --- Lib/test/test_ssl.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 0e3faa1507f3a4..3d84c296101276 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -4969,8 +4969,6 @@ def setUpModule(): raise support.TestFailed("Can't read certificate file %r" % filename) threading_helper.init() - thread_info = threading_helper.threading_setup() - unittest.addModuleCleanup(threading_helper.threading_cleanup, *thread_info) if __name__ == "__main__": From 4f7b0e915866073fe80ef39d510d4c44f1f7317b Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 16:25:03 +0300 Subject: [PATCH 77/83] Revert "Add some debug scaffolding to track down a leaked thread" --- Lib/test/support/threading_helper.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index df05ec68bf3603..0e7be4210dcac8 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -258,8 +258,6 @@ def requires_working_threading(*, module=False): return unittest.skipUnless(can_start_thread, msg) -global_thread_count = 0 - class Server: """A context manager for a blocking server in a thread pool. @@ -321,22 +319,14 @@ def __init__(self, client_func, *args, client_count=1, **kwargs): def _thread_func(self, server_socket, client_func, client_count, args, kwargs): - global global_thread_count - thread_pass = global_thread_count - global_thread_count = global_thread_count + 1 - sys.stdout.write(f'thread {thread_pass} started') - try: - server_socket.settimeout(support.LOOPBACK_TIMEOUT) - with contextlib.closing(server_socket): - results = [] - for i in range(client_count): - client, peer_address = server_socket.accept() - with contextlib.closing(client): - r = client_func(client, peer_address, *args, **kwargs) - results.append(r) - return results - finally: - sys.stdout.write(f'thread {thread_pass} exited') + with contextlib.closing(server_socket): + results = [] + for i in range(client_count): + client, peer_address = server_socket.accept() + with contextlib.closing(client): + r = client_func(client, peer_address, *args, **kwargs) + results.append(r) + return results def __enter__(self): return HOST, self._port From f0e7c8d9232f98a8f92f90b83d908fd5583fe6c5 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 16:33:01 +0300 Subject: [PATCH 78/83] On MacOS server that lost a client still raises an exception --- Lib/test/test_ssl.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 3d84c296101276..4c0c8166768a99 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -3001,26 +3001,27 @@ def test_check_hostname_correct(self): cert = s.getpeercert() self.assertTrue(cert, "Can't get peer certificate.") - def _do_test_check_hostname_incorrect(self): + def test_check_hostname_incorrect(self): if support.verbose: sys.stdout.write("\n") - client_context, server_context, hostname = testing_context() - - with Server(context=server_context) as address: - with client_context.wrap_socket(socket.socket(), - server_hostname="invalid") as s: - with self.assertRaisesRegex( - ssl.CertificateError, - "Hostname mismatch, certificate is not valid for 'invalid'."): - s.connect(address) - - def test_check_hostname_incorrect(self): if sys.platform == 'darwin': - self._do_test_check_hostname_incorrect() + server_exc_type = OSError + server_exc_msg = '[Errno 9]' else: - with self.assertRaisesRegex(ssl.SSLError, 'SSLV3_ALERT_BAD_CERTIFICATE'): - self._do_test_check_hostname_incorrect() + server_exc_type = ssl.SSLError + server_exc_msg = 'SSLV3_ALERT_BAD_CERTIFICATE' + + with self.assertRaisesRegex(server_exc_type, server_exc_msg): + client_context, server_context, hostname = testing_context() + + with Server(context=server_context) as address: + with client_context.wrap_socket(socket.socket(), + server_hostname="invalid") as s: + with self.assertRaisesRegex( + ssl.CertificateError, + "Hostname mismatch, certificate is not valid for 'invalid'."): + s.connect(address) def test_check_hostname_missing(self): if support.verbose: From c67bb1a12ac77221c654699253e7c1ea7749acee Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 17:29:45 +0300 Subject: [PATCH 79/83] Clarify terms in NEWS --- .../next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst index 0b739f3cf5b3a9..a4ccb9ed009c78 100644 --- a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst +++ b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst @@ -1,3 +1,3 @@ Added a unified :class:`test.support.threading_helper.Server` context manager to create, wait, destroy and communicate with a blocking TCP server both -in-bound and out-of-bound. Patch by Oleg Iarygin. +in-band and out-of-band. Patch by Oleg Iarygin. From cdb9137ea8b10afe127eecb4bb6d680d1757a7e9 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 17:34:11 +0300 Subject: [PATCH 80/83] Streamline comments --- Lib/test/support/threading_helper.py | 2 +- Lib/test/test_ssl.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 0e7be4210dcac8..926119e9125f2e 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -306,7 +306,7 @@ def __init__(self, client_func, *args, client_count=1, **kwargs): client_count: count of clients the server processes one by one before stopping. results: a reference to a list for collecting client_func - return values. Populated after execution leaves a ``with`` + return values. Populated after the execution leaves a ``with`` blocks associated with the Server context manager. kwargs: keyword arguments passed to client_func. """ diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 4c0c8166768a99..6d68a91b635a4b 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -2860,7 +2860,7 @@ def wait_connection(self, socket): attempts to send some data. As a result, we need to write some non-empty string to force the handshake and avoid server-side ConnectionAbortedError ("An established connection was aborted by the - software in your host machine") when some test has nothing to send so + software in your host machine") when some test has nothing to send and closes a half-open TLS connection. """ echo_message = b'hi' From fbacdc8f5748951d0bf03a104ef04944d8370f20 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 17:52:09 +0300 Subject: [PATCH 81/83] Set a default socket timeout globally --- Lib/test/test_ssl.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 6d68a91b635a4b..a008d2beb1608d 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1921,18 +1921,12 @@ class SimpleBackgroundTests(unittest.TestCase): """Tests that connect to a simple server running in the background""" def setUp(self): - self._old_socket_timeout = socket.getdefaulttimeout() - socket.setdefaulttimeout(support.LOOPBACK_TIMEOUT) - self.server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) self.server_context.load_cert_chain(SIGNED_CERTFILE) server = ThreadedEchoServer(context=self.server_context) self.enterContext(server) self.server_addr = (HOST, server.port) - def tearDown(self): - socket.setdefaulttimeout(self._old_socket_timeout) - def test_connect(self): with test_wrap_socket(socket.socket(socket.AF_INET), cert_reqs=ssl.CERT_NONE) as s: @@ -4970,6 +4964,7 @@ def setUpModule(): raise support.TestFailed("Can't read certificate file %r" % filename) threading_helper.init() + socket.setdefaulttimeout(support.LOOPBACK_TIMEOUT) if __name__ == "__main__": From e526af4730f5a6164a5792fdc4223dd2534959c3 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 10 Jul 2022 21:36:43 +0300 Subject: [PATCH 82/83] Use sockets directly, without contextlib.closing --- Lib/test/support/threading_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 926119e9125f2e..1cd2a020ced0d6 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -319,11 +319,11 @@ def __init__(self, client_func, *args, client_count=1, **kwargs): def _thread_func(self, server_socket, client_func, client_count, args, kwargs): - with contextlib.closing(server_socket): + with server_socket: results = [] for i in range(client_count): client, peer_address = server_socket.accept() - with contextlib.closing(client): + with client: r = client_func(client, peer_address, *args, **kwargs) results.append(r) return results From b8174aeb05885a72e92a067fe66f45809405e6aa Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sun, 30 Oct 2022 18:57:32 +0300 Subject: [PATCH 83/83] Put the news entry into a present tense --- .../next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst index a4ccb9ed009c78..3e4192c058b158 100644 --- a/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst +++ b/Misc/NEWS.d/next/Tests/2022-07-06-18-46-00.gh-issue-94609.eeM1cu.rst @@ -1,3 +1,3 @@ -Added a unified :class:`test.support.threading_helper.Server` context manager +Add a unified :class:`test.support.threading_helper.Server` context manager to create, wait, destroy and communicate with a blocking TCP server both in-band and out-of-band. Patch by Oleg Iarygin.