Skip to content

Commit 455f39e

Browse files
cornopaezclaude
andcommitted
gh-159875: Add tests for an SSL transport closed after connection loss
An application that closes its transport from connection_lost(), as aiohttp does, must be left with a transport whose write(), get_write_buffer_size() and set_protocol() behave as after closing a live connection, and loop.sendfile()'s fallback must then raise ConnectionError rather than AttributeError from set_protocol(). The sendfile test reads the file synchronously, so that the fallback is waiting in drain() when the connection is lost; a read in the executor that completes in the same loop iteration as the reset lets the fallback finish first and hides the bug. test_create_connection_memory_leak now keeps the transport past its check, as its comment says it covers, so that it would see an SSLContext kept alive by a transport the application still holds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 0d35e3b commit 455f39e

3 files changed

Lines changed: 58 additions & 1 deletion

File tree

‎Lib/test/test_asyncio/test_sendfile.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,38 @@ def test_sendfile_ssl_close_peer_after_receiving(self):
553553
self.assertEqual(srv_proto.data, self.DATA)
554554
self.assertEqual(self.file.tell(), len(self.DATA))
555555

556+
def test_sendfile_ssl_close_peer_in_the_middle_closes_transport(self):
557+
# The protocol closes its transport when the connection is lost, as
558+
# aiohttp's does. That close() must leave a transport on which the
559+
# fallback can still restore the protocol: sendfile() raises
560+
# ConnectionError, not AttributeError.
561+
srv_proto, cli_proto = self.prepare_sendfile(
562+
is_ssl=True, close_after=1024)
563+
connection_lost = cli_proto.connection_lost
564+
565+
def close_on_lost(exc):
566+
cli_proto.transport.close()
567+
connection_lost(exc)
568+
569+
cli_proto.connection_lost = close_on_lost
570+
# Read the file synchronously, so that the fallback only waits in
571+
# drain(). A read in the executor that completes in the same loop
572+
# iteration as the reset lets the fallback finish before close() is
573+
# called, which hides the bug.
574+
def run_in_executor(executor, func, *args):
575+
fut = self.loop.create_future()
576+
fut.set_result(func(*args))
577+
return fut
578+
579+
with mock.patch.object(self.loop, 'run_in_executor', run_in_executor):
580+
with self.assertRaises(ConnectionError):
581+
self.run_loop(
582+
self.loop.sendfile(cli_proto.transport, self.file))
583+
self.run_loop(srv_proto.done)
584+
self.run_loop(cli_proto.done)
585+
self.assertTrue(1024 <= srv_proto.nbytes < len(self.DATA),
586+
srv_proto.nbytes)
587+
556588
# On Solaris, lowering SO_RCVBUF on a TCP connection after it has been
557589
# established has no effect. Due to its age, this bug affects both Oracle
558590
# Solaris as well as all other OpenSolaris forks (unless they fixed it

‎Lib/test/test_asyncio/test_ssl.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,16 +788,21 @@ async def client(addr):
788788
await on_eof
789789

790790
tr.close()
791+
return tr
791792

792793
with self.tcp_server(serve, timeout=self.TIMEOUT) as srv:
793-
self.loop.run_until_complete(
794+
tr = self.loop.run_until_complete(
794795
asyncio.wait_for(client(srv.addr),
795796
timeout=support.SHORT_TIMEOUT))
797+
# Let the protocol's connection_lost() run.
798+
self.loop.run_until_complete(asyncio.sleep(0))
796799

797800
# No garbage is left for SSL client from loop.create_connection, even
798801
# if user stores the SSLTransport in corresponding protocol instance
802+
# (and keeps the transport after the connection is lost).
799803
client_context = weakref.ref(client_context)
800804
self.assertIsNone(client_context())
805+
del tr
801806

802807
def test_start_tls_client_buf_proto_1(self):
803808
HELLO_MSG = b'1' * self.PAYLOAD_SIZE

‎Lib/test/test_asyncio/test_sslproto.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,26 @@ def test_write_after_closing(self):
265265
# should not raise
266266
self.assertIsNone(transp.write(b'data'))
267267

268+
def test_transport_usable_after_connection_lost_and_close(self):
269+
# connection_lost() marks the app transport closed, so the
270+
# application's own close() afterwards (often made from its
271+
# protocol's connection_lost()) must not be taken as a second
272+
# close() that leaves the transport without its SSL protocol.
273+
ssl_proto = self.ssl_protocol()
274+
self.connection_made(ssl_proto)
275+
transp = ssl_proto._app_transport
276+
ssl_proto.connection_lost(None)
277+
transp.close()
278+
279+
# should not raise, as after closing a live connection
280+
self.assertIsNone(transp.write(b'data'))
281+
self.assertEqual(transp.get_write_buffer_size(), 0)
282+
new_proto = asyncio.Protocol()
283+
transp.set_protocol(new_proto) # as loop.sendfile()'s fallback does
284+
self.assertIs(transp.get_protocol(), new_proto)
285+
transp.abort()
286+
self.assertTrue(transp.is_closing())
287+
268288

269289
##############################################################################
270290
# Start TLS Tests

0 commit comments

Comments
 (0)