Skip to content

Commit edf2abd

Browse files
[PR #11289/e38220fc backport][3.12] Fix ClientSession.close() hanging with HTTPS proxy connections (#11291)
**This is a backport of PR #11289 as merged into master (e38220f).** --------- Co-authored-by: J. Nick Koston <[email protected]>
1 parent e8d774f commit edf2abd

File tree

4 files changed

+96
-1
lines changed

4 files changed

+96
-1
lines changed

CHANGES/11273.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed :py:meth:`ClientSession.close() <aiohttp.ClientSession.close>` hanging indefinitely when using HTTPS requests through HTTP proxies -- by :user:`bdraco`.

aiohttp/connector.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,26 @@ def closed(self) -> bool:
229229
return self._protocol is None or not self._protocol.is_connected()
230230

231231

232+
class _ConnectTunnelConnection(Connection):
233+
"""Special connection wrapper for CONNECT tunnels that must never be pooled.
234+
235+
This connection wraps the proxy connection that will be upgraded with TLS.
236+
It must never be released to the pool because:
237+
1. Its 'closed' future will never complete, causing session.close() to hang
238+
2. It represents an intermediate state, not a reusable connection
239+
3. The real connection (with TLS) will be created separately
240+
"""
241+
242+
def release(self) -> None:
243+
"""Do nothing - don't pool or close the connection.
244+
245+
These connections are an intermediate state during the CONNECT tunnel
246+
setup and will be cleaned up naturally after the TLS upgrade. If they
247+
were to be pooled, they would never be properly closed, causing
248+
session.close() to wait forever for their 'closed' future.
249+
"""
250+
251+
232252
class _TransportPlaceholder:
233253
"""placeholder for BaseConnector.connect function"""
234254

@@ -1612,7 +1632,7 @@ async def _create_proxy_connection(
16121632
key = req.connection_key._replace(
16131633
proxy=None, proxy_auth=None, proxy_headers_hash=None
16141634
)
1615-
conn = Connection(self, key, proto, self._loop)
1635+
conn = _ConnectTunnelConnection(self, key, proto, self._loop)
16161636
proxy_resp = await proxy_req.send(conn)
16171637
try:
16181638
protocol = conn._protocol

tests/test_connector.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
AddrInfoType,
4141
Connection,
4242
TCPConnector,
43+
_ConnectTunnelConnection,
4344
_DNSCacheTable,
4445
)
4546
from aiohttp.resolver import ResolveResult
@@ -4311,3 +4312,31 @@ async def test_available_connections_no_limits(
43114312
connection1.close()
43124313
assert conn._available_connections(key) == 1
43134314
assert conn._available_connections(other_host_key2) == 1
4315+
4316+
4317+
async def test_connect_tunnel_connection_release(
4318+
loop: asyncio.AbstractEventLoop,
4319+
) -> None:
4320+
"""Test _ConnectTunnelConnection.release() does not pool the connection."""
4321+
connector = mock.create_autospec(
4322+
aiohttp.BaseConnector, spec_set=True, instance=True
4323+
)
4324+
key = mock.create_autospec(ConnectionKey, spec_set=True, instance=True)
4325+
protocol = mock.create_autospec(ResponseHandler, spec_set=True, instance=True)
4326+
4327+
# Create a connect tunnel connection
4328+
conn = _ConnectTunnelConnection(connector, key, protocol, loop)
4329+
4330+
# Verify protocol is set
4331+
assert conn._protocol is protocol
4332+
4333+
# Release should do nothing (not pool the connection)
4334+
conn.release()
4335+
4336+
# Protocol should still be there (not released to pool)
4337+
assert conn._protocol is protocol
4338+
# Connector._release should NOT have been called
4339+
connector._release.assert_not_called()
4340+
4341+
# Clean up to avoid resource warning
4342+
conn.close()

tests/test_proxy_functional.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import platform
55
import ssl
66
import sys
7+
from contextlib import suppress
78
from re import match as match_regex
89
from typing import Awaitable, Callable
910
from unittest import mock
@@ -17,6 +18,7 @@
1718
from aiohttp import ClientResponse, web
1819
from aiohttp.client_exceptions import ClientConnectionError
1920
from aiohttp.helpers import IS_MACOS, IS_WINDOWS
21+
from aiohttp.pytest_plugin import AiohttpServer
2022

2123
ASYNCIO_SUPPORTS_TLS_IN_TLS = sys.version_info >= (3, 11)
2224

@@ -884,3 +886,46 @@ async def test_proxy_auth() -> None:
884886
proxy_auth=("user", "pass"),
885887
):
886888
pass
889+
890+
891+
async def test_https_proxy_connect_tunnel_session_close_no_hang(
892+
aiohttp_server: AiohttpServer,
893+
) -> None:
894+
"""Test that CONNECT tunnel connections are not pooled."""
895+
# Regression test for issue #11273.
896+
897+
# Create a minimal proxy server
898+
# The CONNECT method is handled at the protocol level, not by the handler
899+
proxy_app = web.Application()
900+
proxy_server = await aiohttp_server(proxy_app)
901+
proxy_url = f"http://{proxy_server.host}:{proxy_server.port}"
902+
903+
# Create session and make HTTPS request through proxy
904+
session = aiohttp.ClientSession()
905+
906+
try:
907+
# This will fail during TLS upgrade because proxy doesn't establish tunnel
908+
with suppress(aiohttp.ClientError):
909+
async with session.get("https://example.com/test", proxy=proxy_url) as resp:
910+
await resp.read()
911+
912+
# The critical test: Check if any connections were pooled with proxy=None
913+
# This is the root cause of the hang - CONNECT tunnel connections
914+
# should NOT be pooled
915+
connector = session.connector
916+
assert connector is not None
917+
918+
# Count connections with proxy=None in the pool
919+
proxy_none_keys = [key for key in connector._conns if key.proxy is None]
920+
proxy_none_count = len(proxy_none_keys)
921+
922+
# Before the fix, there would be a connection with proxy=None
923+
# After the fix, CONNECT tunnel connections are not pooled
924+
assert proxy_none_count == 0, (
925+
f"Found {proxy_none_count} connections with proxy=None in pool. "
926+
f"CONNECT tunnel connections should not be pooled - this is bug #11273"
927+
)
928+
929+
finally:
930+
# Clean close
931+
await session.close()

0 commit comments

Comments
 (0)