Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit efb46e5

Browse filesBrowse files
ambvambv
authored
[3.8] gh-108342: Make ssl TestPreHandshakeClose more reliable (GH-108370) (#108408)
* In preauth tests of test_ssl, explicitly break reference cycles invoving SingleConnectionTestServerThread to make sure that the thread is deleted. Otherwise, the test marks the environment as altered because the threading module sees a "dangling thread" (SingleConnectionTestServerThread). This test leak was introduced by the test added for the fix of issue gh-108310. * Use support.SHORT_TIMEOUT instead of hardcoded 1.0 or 2.0 seconds timeout. * SingleConnectionTestServerThread.run() catchs TimeoutError * Fix a race condition (missing synchronization) in test_preauth_data_to_tls_client(): the server now waits until the client connect() completed in call_after_accept(). * test_https_client_non_tls_response_ignored() calls server.join() explicitly. * Replace "localhost" with server.listener.getsockname()[0]. (cherry picked from commit 592bacb)
1 parent 6f2b64f commit efb46e5
Copy full SHA for efb46e5

File tree

Expand file treeCollapse file tree

2 files changed

+79
-31
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+79
-31
lines changed

‎.gitignore

Copy file name to clipboardExpand all lines: .gitignore
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,11 @@ Tools/ssl/win32
120120
# Ignore ./python binary on Unix but still look into ./Python/ directory.
121121
/python
122122
!/Python/
123+
124+
# Artifacts generated by 3.11 lying around when switching branches:
125+
/_bootstrap_python
126+
/Programs/_freeze_module
127+
/Modules/Setup.bootstrap
128+
/Modules/Setup.stdlib
129+
/Python/deepfreeze/
130+
/Python/frozen_modules/

‎Lib/test/test_ssl.py

Copy file name to clipboardExpand all lines: Lib/test/test_ssl.py
+71-31Lines changed: 71 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4828,12 +4828,16 @@ class TestPreHandshakeClose(unittest.TestCase):
48284828

48294829
class SingleConnectionTestServerThread(threading.Thread):
48304830

4831-
def __init__(self, *, name, call_after_accept):
4831+
def __init__(self, *, name, call_after_accept, timeout=None):
48324832
self.call_after_accept = call_after_accept
48334833
self.received_data = b'' # set by .run()
48344834
self.wrap_error = None # set by .run()
48354835
self.listener = None # set by .start()
48364836
self.port = None # set by .start()
4837+
if timeout is None:
4838+
self.timeout = support.SHORT_TIMEOUT
4839+
else:
4840+
self.timeout = timeout
48374841
super().__init__(name=name)
48384842

48394843
def __enter__(self):
@@ -4856,13 +4860,19 @@ def start(self):
48564860
self.ssl_ctx.load_cert_chain(certfile=ONLYCERT, keyfile=ONLYKEY)
48574861
self.listener = socket.socket()
48584862
self.port = support.bind_port(self.listener)
4859-
self.listener.settimeout(2.0)
4863+
self.listener.settimeout(self.timeout)
48604864
self.listener.listen(1)
48614865
super().start()
48624866

48634867
def run(self):
4864-
conn, address = self.listener.accept()
4865-
self.listener.close()
4868+
try:
4869+
conn, address = self.listener.accept()
4870+
except TimeoutError:
4871+
# on timeout, just close the listener
4872+
return
4873+
finally:
4874+
self.listener.close()
4875+
48664876
with conn:
48674877
if self.call_after_accept(conn):
48684878
return
@@ -4890,8 +4900,13 @@ def non_linux_skip_if_other_okay_error(self, err):
48904900
# we're specifically trying to test. The way this test is written
48914901
# is known to work on Linux. We'll skip it anywhere else that it
48924902
# does not present as doing so.
4893-
self.skipTest(f"Could not recreate conditions on {sys.platform}:"
4894-
f" {err=}")
4903+
try:
4904+
self.skipTest(f"Could not recreate conditions on {sys.platform}:"
4905+
f" {err=}")
4906+
finally:
4907+
# gh-108342: Explicitly break the reference cycle
4908+
err = None
4909+
48954910
# If maintaining this conditional winds up being a problem.
48964911
# just turn this into an unconditional skip anything but Linux.
48974912
# The important thing is that our CI has the logic covered.
@@ -4902,7 +4917,7 @@ def test_preauth_data_to_tls_server(self):
49024917

49034918
def call_after_accept(unused):
49044919
server_accept_called.set()
4905-
if not ready_for_server_wrap_socket.wait(2.0):
4920+
if not ready_for_server_wrap_socket.wait(support.SHORT_TIMEOUT):
49064921
raise RuntimeError("wrap_socket event never set, test may fail.")
49074922
return False # Tell the server thread to continue.
49084923

@@ -4924,20 +4939,31 @@ def call_after_accept(unused):
49244939

49254940
ready_for_server_wrap_socket.set()
49264941
server.join()
4942+
49274943
wrap_error = server.wrap_error
4928-
self.assertEqual(b"", server.received_data)
4929-
self.assertIsInstance(wrap_error, OSError) # All platforms.
4930-
self.non_linux_skip_if_other_okay_error(wrap_error)
4931-
self.assertIsInstance(wrap_error, ssl.SSLError)
4932-
self.assertIn("before TLS handshake with data", wrap_error.args[1])
4933-
self.assertIn("before TLS handshake with data", wrap_error.reason)
4934-
self.assertNotEqual(0, wrap_error.args[0])
4935-
self.assertIsNone(wrap_error.library, msg="attr must exist")
4944+
server.wrap_error = None
4945+
try:
4946+
self.assertEqual(b"", server.received_data)
4947+
self.assertIsInstance(wrap_error, OSError) # All platforms.
4948+
self.non_linux_skip_if_other_okay_error(wrap_error)
4949+
self.assertIsInstance(wrap_error, ssl.SSLError)
4950+
self.assertIn("before TLS handshake with data", wrap_error.args[1])
4951+
self.assertIn("before TLS handshake with data", wrap_error.reason)
4952+
self.assertNotEqual(0, wrap_error.args[0])
4953+
self.assertIsNone(wrap_error.library, msg="attr must exist")
4954+
finally:
4955+
# gh-108342: Explicitly break the reference cycle
4956+
wrap_error = None
4957+
server = None
49364958

49374959
def test_preauth_data_to_tls_client(self):
4960+
server_can_continue_with_wrap_socket = threading.Event()
49384961
client_can_continue_with_wrap_socket = threading.Event()
49394962

49404963
def call_after_accept(conn_to_client):
4964+
if not server_can_continue_with_wrap_socket.wait(support.SHORT_TIMEOUT):
4965+
print("ERROR: test client took too long")
4966+
49414967
# This forces an immediate connection close via RST on .close().
49424968
set_socket_so_linger_on_with_zero_timeout(conn_to_client)
49434969
conn_to_client.send(
@@ -4959,8 +4985,10 @@ def call_after_accept(conn_to_client):
49594985

49604986
with socket.socket() as client:
49614987
client.connect(server.listener.getsockname())
4962-
if not client_can_continue_with_wrap_socket.wait(2.0):
4963-
self.fail("test server took too long.")
4988+
server_can_continue_with_wrap_socket.set()
4989+
4990+
if not client_can_continue_with_wrap_socket.wait(support.SHORT_TIMEOUT):
4991+
self.fail("test server took too long")
49644992
ssl_ctx = ssl.create_default_context()
49654993
try:
49664994
tls_client = ssl_ctx.wrap_socket(
@@ -4974,24 +5002,31 @@ def call_after_accept(conn_to_client):
49745002
tls_client.close()
49755003

49765004
server.join()
4977-
self.assertEqual(b"", received_data)
4978-
self.assertIsInstance(wrap_error, OSError) # All platforms.
4979-
self.non_linux_skip_if_other_okay_error(wrap_error)
4980-
self.assertIsInstance(wrap_error, ssl.SSLError)
4981-
self.assertIn("before TLS handshake with data", wrap_error.args[1])
4982-
self.assertIn("before TLS handshake with data", wrap_error.reason)
4983-
self.assertNotEqual(0, wrap_error.args[0])
4984-
self.assertIsNone(wrap_error.library, msg="attr must exist")
5005+
try:
5006+
self.assertEqual(b"", received_data)
5007+
self.assertIsInstance(wrap_error, OSError) # All platforms.
5008+
self.non_linux_skip_if_other_okay_error(wrap_error)
5009+
self.assertIsInstance(wrap_error, ssl.SSLError)
5010+
self.assertIn("before TLS handshake with data", wrap_error.args[1])
5011+
self.assertIn("before TLS handshake with data", wrap_error.reason)
5012+
self.assertNotEqual(0, wrap_error.args[0])
5013+
self.assertIsNone(wrap_error.library, msg="attr must exist")
5014+
finally:
5015+
# gh-108342: Explicitly break the reference cycle
5016+
wrap_error = None
5017+
server = None
49855018

49865019
def test_https_client_non_tls_response_ignored(self):
4987-
49885020
server_responding = threading.Event()
49895021

49905022
class SynchronizedHTTPSConnection(http.client.HTTPSConnection):
49915023
def connect(self):
5024+
# Call clear text HTTP connect(), not the encrypted HTTPS (TLS)
5025+
# connect(): wrap_socket() is called manually below.
49925026
http.client.HTTPConnection.connect(self)
5027+
49935028
# Wait for our fault injection server to have done its thing.
4994-
if not server_responding.wait(1.0) and support.verbose:
5029+
if not server_responding.wait(support.SHORT_TIMEOUT) and support.verbose:
49955030
sys.stdout.write("server_responding event never set.")
49965031
self.sock = self._context.wrap_socket(
49975032
self.sock, server_hostname=self.host)
@@ -5006,29 +5041,34 @@ def call_after_accept(conn_to_client):
50065041
server_responding.set()
50075042
return True # Tell the server to stop.
50085043

5044+
timeout = 2.0
50095045
server = self.SingleConnectionTestServerThread(
50105046
call_after_accept=call_after_accept,
5011-
name="non_tls_http_RST_responder")
5047+
name="non_tls_http_RST_responder",
5048+
timeout=timeout)
50125049
server.__enter__() # starts it
50135050
self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it.
50145051
# Redundant; call_after_accept sets SO_LINGER on the accepted conn.
50155052
set_socket_so_linger_on_with_zero_timeout(server.listener)
50165053

50175054
connection = SynchronizedHTTPSConnection(
5018-
f"localhost",
5055+
server.listener.getsockname()[0],
50195056
port=server.port,
50205057
context=ssl.create_default_context(),
5021-
timeout=2.0,
5058+
timeout=timeout,
50225059
)
5060+
50235061
# There are lots of reasons this raises as desired, long before this
50245062
# test was added. Sending the request requires a successful TLS wrapped
50255063
# socket; that fails if the connection is broken. It may seem pointless
50265064
# to test this. It serves as an illustration of something that we never
50275065
# want to happen... properly not happening.
5028-
with self.assertRaises(OSError) as err_ctx:
5066+
with self.assertRaises(OSError):
50295067
connection.request("HEAD", "/test", headers={"Host": "localhost"})
50305068
response = connection.getresponse()
50315069

5070+
server.join()
5071+
50325072

50335073
def test_main(verbose=False):
50345074
if support.verbose:

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.