Skip to content

Commit 4d44564

Browse files
cornopaezclaude
authored andcommitted
gh-159875: Keep the SSL protocol of a closed transport after connection loss (GH-160069)
* 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> * gh-159875: Keep the SSL protocol of a closed transport after connection loss _SSLProtocolTransport.close() dropped its _ssl_protocol when called a second time, and the transport is already marked closed once the connection is lost, so an application closing it from connection_lost() left a transport whose write(), set_protocol(), get_write_buffer_size(), ... raise AttributeError. That branch was also what released the SSLContext of a transport the application keeps. Keep the protocol, and release the TLS state from SSLProtocol.connection_lost() instead: one loop iteration later when the application's connection_lost() was scheduled, so that it can still read get_extra_info('ssl_object') and ('sslcontext'). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- (cherry picked from commit 0626b14) Co-authored-by: Mauricio Paez <jose.m.paez@icloud.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9c66941 commit 4d44564

5 files changed

Lines changed: 82 additions & 3 deletions

File tree

‎Lib/asyncio/sslproto.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ def close(self):
114114
if not self._closed:
115115
self._closed = True
116116
self._ssl_protocol._start_shutdown()
117-
else:
118-
self._ssl_protocol = None
119117

120118
def __del__(self, _warnings=warnings):
121119
if not self._closed:
@@ -412,12 +410,14 @@ def connection_lost(self, exc):
412410
# The app protocol gets connection_lost() only if it got
413411
# connection_made(). After start_tls() that is already the case
414412
# while the handshake is in progress.
413+
app_notified = False
415414
if (
416415
self._app_state == AppProtocolState.STATE_CON_MADE or
417416
self._app_state == AppProtocolState.STATE_EOF
418417
):
419418
self._app_state = AppProtocolState.STATE_CON_LOST
420419
self._loop.call_soon(self._app_protocol.connection_lost, exc)
420+
app_notified = True
421421
self._set_state(SSLProtocolState.UNWRAPPED)
422422
self._transport = None
423423
self._app_transport = None
@@ -430,6 +430,21 @@ def connection_lost(self, exc):
430430
if self._handshake_timeout_handle:
431431
self._handshake_timeout_handle.cancel()
432432
self._handshake_timeout_handle = None
433+
if app_notified:
434+
# After the app protocol's connection_lost() (scheduled above),
435+
# so that it can still read the TLS extra info.
436+
self._loop.call_soon(self._release_tls_state)
437+
else:
438+
self._release_tls_state()
439+
440+
def _release_tls_state(self):
441+
# The connection is gone: drop everything that refers to the
442+
# SSLContext, so that an application holding the transport does
443+
# not keep the context alive.
444+
self._sslobj = None
445+
self._sslcontext = None
446+
self._extra.pop('sslcontext', None)
447+
self._extra.pop('ssl_object', None)
433448

434449
def get_buffer(self, n):
435450
want = n

‎Lib/test/test_asyncio/test_sendfile.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,38 @@ def test_sendfile_ssl_close_peer_after_receiving(self):
512512
self.assertEqual(srv_proto.data, self.DATA)
513513
self.assertEqual(self.file.tell(), len(self.DATA))
514514

515+
def test_sendfile_ssl_close_peer_in_the_middle_closes_transport(self):
516+
# The protocol closes its transport when the connection is lost, as
517+
# aiohttp's does. That close() must leave a transport on which the
518+
# fallback can still restore the protocol: sendfile() raises
519+
# ConnectionError, not AttributeError.
520+
srv_proto, cli_proto = self.prepare_sendfile(
521+
is_ssl=True, close_after=1024)
522+
connection_lost = cli_proto.connection_lost
523+
524+
def close_on_lost(exc):
525+
cli_proto.transport.close()
526+
connection_lost(exc)
527+
528+
cli_proto.connection_lost = close_on_lost
529+
# Read the file synchronously, so that the fallback only waits in
530+
# drain(). A read in the executor that completes in the same loop
531+
# iteration as the reset lets the fallback finish before close() is
532+
# called, which hides the bug.
533+
def run_in_executor(executor, func, *args):
534+
fut = self.loop.create_future()
535+
fut.set_result(func(*args))
536+
return fut
537+
538+
with mock.patch.object(self.loop, 'run_in_executor', run_in_executor):
539+
with self.assertRaises(ConnectionError):
540+
self.run_loop(
541+
self.loop.sendfile(cli_proto.transport, self.file))
542+
self.run_loop(srv_proto.done)
543+
self.run_loop(cli_proto.done)
544+
self.assertTrue(1024 <= srv_proto.nbytes < len(self.DATA),
545+
srv_proto.nbytes)
546+
515547
# On Solaris, lowering SO_RCVBUF on a TCP connection after it has been
516548
# established has no effect. Due to its age, this bug affects both Oracle
517549
# 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
@@ -791,16 +791,21 @@ async def client(addr):
791791
await on_eof
792792

793793
tr.close()
794+
return tr
794795

795796
with self.tcp_server(serve, timeout=self.TIMEOUT) as srv:
796-
self.loop.run_until_complete(
797+
tr = self.loop.run_until_complete(
797798
asyncio.wait_for(client(srv.addr),
798799
timeout=support.SHORT_TIMEOUT))
800+
# Let the protocol's connection_lost() run.
801+
self.loop.run_until_complete(asyncio.sleep(0))
799802

800803
# No garbage is left for SSL client from loop.create_connection, even
801804
# if user stores the SSLTransport in corresponding protocol instance
805+
# (and keeps the transport after the connection is lost).
802806
client_context = weakref.ref(client_context)
803807
self.assertIsNone(client_context())
808+
del tr
804809

805810
def test_start_tls_client_buf_proto_1(self):
806811
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
@@ -289,6 +289,26 @@ def test_write_after_closing(self):
289289
# should not raise
290290
self.assertIsNone(transp.write(b'data'))
291291

292+
def test_transport_usable_after_connection_lost_and_close(self):
293+
# connection_lost() marks the app transport closed, so the
294+
# application's own close() afterwards (often made from its
295+
# protocol's connection_lost()) must not be taken as a second
296+
# close() that leaves the transport without its SSL protocol.
297+
ssl_proto = self.ssl_protocol()
298+
self.connection_made(ssl_proto)
299+
transp = ssl_proto._app_transport
300+
ssl_proto.connection_lost(None)
301+
transp.close()
302+
303+
# should not raise, as after closing a live connection
304+
self.assertIsNone(transp.write(b'data'))
305+
self.assertEqual(transp.get_write_buffer_size(), 0)
306+
new_proto = asyncio.Protocol()
307+
transp.set_protocol(new_proto) # as loop.sendfile()'s fallback does
308+
self.assertIs(transp.get_protocol(), new_proto)
309+
transp.abort()
310+
self.assertTrue(transp.is_closing())
311+
292312
def test_pause_reading_if_ssl_replies_are_not_read(self):
293313
# Output which OpenSSL produces on its own while reading (alerts,
294314
# KeyUpdate, ...) must not pile up if the peer does not read it.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fix :exc:`AttributeError` from the methods of an :mod:`asyncio` SSL transport,
2+
such as :meth:`~asyncio.WriteTransport.write` and
3+
:meth:`~asyncio.BaseTransport.set_protocol`, when the transport is closed after
4+
its connection was lost. This also affected the fallback of
5+
:meth:`loop.sendfile() <asyncio.loop.sendfile>` over SSL when the peer went
6+
away mid-transfer. The SSL context is still released once the connection is
7+
lost.

0 commit comments

Comments
 (0)