Skip to content

Commit

Permalink
bpo-37322: Fix ResourceWarning and exception handling in test (GH-25553)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tiran committed Apr 24, 2021
1 parent f05c2ae commit c8666cf
Showing 1 changed file with 38 additions and 38 deletions.
76 changes: 38 additions & 38 deletions Lib/test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit c8666cf

Please sign in to comment.