bpo-37322: Fix ResourceWarning and exception handling in test (GH-25553)
Revert 73ea546, increase logging, and improve stability of test.
Handle all OSErrors in a single block. OSError also takes care of
SSLError and socket's connection errors.
Partly reverts commit fb7e750. The
threaded connection handler must not raise an unhandled exception.
diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index 31e1e73..71260bd 100644
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -2390,7 +2390,10 @@ def wrap_conn(self):
sys.stdout.write(" client cert is " + pprint.pformat(cert) + "\n")
cert_binary = self.sslconn.getpeercert(True)
if support.verbose and self.server.chatty:
- sys.stdout.write(" cert binary is " + str(len(cert_binary)) + " bytes\n")
+ if cert_binary is None:
+ sys.stdout.write(" client did not provide a cert\n")
+ else:
+ sys.stdout.write(f" cert binary is {len(cert_binary)}b\n")
cipher = self.sslconn.cipher()
if support.verbose and self.server.chatty:
sys.stdout.write(" server: connection cipher is now " + str(cipher) + "\n")
@@ -2486,31 +2489,22 @@ def run(self):
sys.stdout.write(" server: read %r (%s), sending back %r (%s)...\n"
% (msg, ctype, msg.lower(), ctype))
self.write(msg.lower())
- except (ConnectionResetError, ConnectionAbortedError):
- # XXX: OpenSSL 1.1.1 sometimes raises ConnectionResetError
- # when connection is not shut down gracefully.
+ except OSError as e:
+ # handles SSLError and socket errors
if self.server.chatty and support.verbose:
- sys.stdout.write(
- " Connection reset by peer: {}\n".format(
- self.addr)
- )
- self.close()
- self.running = False
- except ssl.SSLError as err:
- # On Windows sometimes test_pha_required_nocert receives the
- # PEER_DID_NOT_RETURN_A_CERTIFICATE exception
- # before the 'tlsv13 alert certificate required' exception.
- # If the server is stopped when PEER_DID_NOT_RETURN_A_CERTIFICATE
- # is received test_pha_required_nocert fails with ConnectionResetError
- # because the underlying socket is closed
- if 'PEER_DID_NOT_RETURN_A_CERTIFICATE' == err.reason:
- if self.server.chatty and support.verbose:
- sys.stdout.write(err.args[1])
- # test_pha_required_nocert is expecting this exception
- raise ssl.SSLError('tlsv13 alert certificate required')
- except OSError:
- if self.server.chatty:
- handle_error("Test server failure:\n")
+ if isinstance(e, ConnectionError):
+ # OpenSSL 1.1.1 sometimes raises
+ # ConnectionResetError when connection is not
+ # shut down gracefully.
+ print(
+ f" Connection reset by peer: {self.addr}"
+ )
+ else:
+ handle_error("Test server failure:\n")
+ try:
+ self.write(b"ERROR\n")
+ except OSError:
+ pass
self.close()
self.running = False
@@ -4416,24 +4410,30 @@ def test_pha_required_nocert(self):
server_context.verify_mode = ssl.CERT_REQUIRED
client_context.post_handshake_auth = True
- # Ignore expected SSLError in ConnectionHandler of ThreadedEchoServer
- # (it is only raised sometimes on Windows)
- with threading_helper.catch_threading_exception() as cm:
- server = ThreadedEchoServer(context=server_context, chatty=False)
- with server:
- with client_context.wrap_socket(socket.socket(),
- server_hostname=hostname) as s:
- s.connect((HOST, server.port))
- s.write(b'PHA')
+ def msg_cb(conn, direction, version, content_type, msg_type, data):
+ if support.verbose and content_type == _TLSContentType.ALERT:
+ info = (conn, direction, version, content_type, msg_type, data)
+ sys.stdout.write(f"TLS: {info!r}\n")
+
+ server_context._msg_callback = msg_cb
+ client_context._msg_callback = msg_cb
+
+ server = ThreadedEchoServer(context=server_context, chatty=True)
+ with server:
+ with client_context.wrap_socket(socket.socket(),
+ server_hostname=hostname) as s:
+ s.connect((HOST, server.port))
+ s.write(b'PHA')
+ with self.assertRaisesRegex(
+ ssl.SSLError,
+ 'tlsv13 alert certificate required'
+ ):
# receive CertificateRequest
self.assertEqual(s.recv(1024), b'OK\n')
# send empty Certificate + Finish
s.write(b'HASCERT')
# receive alert
- with self.assertRaisesRegex(
- ssl.SSLError,
- 'tlsv13 alert certificate required'):
- s.recv(1024)
+ s.recv(1024)
def test_pha_optional(self):
if support.verbose: