From 25f30573cc01da08f5b962128eebb16b9701ada3 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 27 Sep 2026 19:18:20 +0100 Subject: [PATCH 1/4] Deprecate TCPConnector ssl (#13829) --- CHANGES/13829.deprecation.rst | 5 +++ aiohttp/connector.py | 33 +++++++++++++------- docs/client_advanced.rst | 10 +++--- docs/client_reference.rst | 10 ++++-- examples/fake_server.py | 4 +-- tests/test_client_functional.py | 40 ++++++++++++------------ tests/test_connector.py | 44 ++++++++++++++++++++++++--- tests/test_proxy_functional.py | 5 +-- tests/test_web_sendfile_functional.py | 4 +-- 9 files changed, 105 insertions(+), 50 deletions(-) create mode 100644 CHANGES/13829.deprecation.rst diff --git a/CHANGES/13829.deprecation.rst b/CHANGES/13829.deprecation.rst new file mode 100644 index 00000000000..108ee6e5539 --- /dev/null +++ b/CHANGES/13829.deprecation.rst @@ -0,0 +1,5 @@ +Deprecated the ``ssl`` parameter of :class:`~aiohttp.TCPConnector`, scheduled for +removal in 5.0 -- by :user:`Dreamsorcerer`. + +Pass ``ssl`` to :class:`~aiohttp.ClientSession` for a session-wide default, or to +:meth:`~aiohttp.ClientSession.get` and the other request methods per request. diff --git a/aiohttp/connector.py b/aiohttp/connector.py index cdf6ca5bd57..005e117a5be 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -954,11 +954,11 @@ def _make_ssl_context(verified: bool) -> SSLContext: class TCPConnector(BaseConnector): """TCP connector. - verify_ssl - Set to True to check ssl certifications. - fingerprint - Pass the binary sha256 - digest of the expected certificate in DER format to verify - that the certificate the server presents matches. See also - https://en.wikipedia.org/wiki/HTTP_Public_Key_Pinning + ssl - DEPRECATED. Will be removed in aiohttp 5.0. + SSL validation mode: ``True`` for the default checks, ``False`` to + skip certificate validation, a Fingerprint for certificate pinning + or an ssl.SSLContext for custom validation. Pass ``ssl`` to + ClientSession or to the individual request instead. resolver - Enable DNS lookups and use this resolver use_dns_cache - Use memory cache for DNS lookups. @@ -999,7 +999,7 @@ def __init__( ttl_dns_cache: int | None = 10, dns_cache_max_size: int = 1000, family: socket.AddressFamily = socket.AddressFamily.AF_UNSPEC, - ssl: bool | Fingerprint | SSLContext = True, + ssl: bool | Fingerprint | SSLContext | _SENTINEL = sentinel, local_addr: tuple[str, int] | None = None, resolver: AbstractResolver | None = None, keepalive_timeout: None | float | _SENTINEL = sentinel, @@ -1022,12 +1022,23 @@ def __init__( timeout_ceil_threshold=timeout_ceil_threshold, ) - if not isinstance(ssl, SSL_ALLOWED_TYPES): - raise TypeError( - "ssl should be SSLContext, Fingerprint, or bool, " - f"got {ssl!r} instead." + self._ssl: bool | Fingerprint | SSLContext + if ssl is sentinel: + self._ssl = True + else: + if not isinstance(ssl, SSL_ALLOWED_TYPES): + raise TypeError( + "ssl should be SSLContext, Fingerprint, or bool, " + f"got {ssl!r} instead." + ) + warnings.warn( + "The ssl parameter is deprecated since 4.0 and scheduled for " + "removal in 5.0, pass ssl to ClientSession() or to the " + "individual request instead", + DeprecationWarning, + stacklevel=2, ) - self._ssl = ssl + self._ssl = ssl self._resolver: AbstractResolver if resolver is None: diff --git a/docs/client_advanced.rst b/docs/client_advanced.rst index 55420e47bde..46ca6ebbc4e 100644 --- a/docs/client_advanced.rst +++ b/docs/client_advanced.rst @@ -618,7 +618,7 @@ checks can be relaxed by setting *ssl* to ``False``:: If you need to setup custom ssl parameters (use own certification files for example) you can create a :class:`ssl.SSLContext` instance and pass it into the :meth:`ClientSession.request` methods or set it for the -entire session with ``ClientSession(connector=TCPConnector(ssl=ssl_context))``. +entire session with ``ClientSession(ssl=ssl_context)``. There are explicit errors when ssl verification fails @@ -660,7 +660,7 @@ installed or Python is unable to find them, resulting in a error like One way to work around this problem is to use the `certifi` package:: ssl_context = ssl.create_default_context(cafile=certifi.where()) - async with ClientSession(connector=TCPConnector(ssl=ssl_context)) as sess: + async with ClientSession(ssl=ssl_context) as sess: ... Example: Use self-signed certificate @@ -706,9 +706,9 @@ DER with e.g:: Tip: to convert from a hexadecimal digest to a binary byte-string, you can use :func:`binascii.unhexlify`. - *ssl* parameter could be passed - to :class:`TCPConnector` as default, the value from - :meth:`ClientSession.get` and others override default. + *ssl* parameter could be passed to :class:`ClientSession` as the + session-wide default, the value from :meth:`ClientSession.get` and + others override it. .. _aiohttp-client-proxy-support: diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 9884f5d22fc..799b1ee21a6 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -1231,8 +1231,8 @@ is controlled by *force_close* constructor's parameter). Constructor accepts all parameters suitable for :class:`BaseConnector` plus several TCP-specific ones: - :param ssl: SSL validation mode. ``True`` for default SSL check - (:func:`ssl.create_default_context` is used), + :param ssl: **(DEPRECATED)** SSL validation mode. ``True`` for default + SSL check (:func:`ssl.create_default_context` is used), ``False`` for skip SSL certificate validation, :class:`aiohttp.Fingerprint` for fingerprint validation, :class:`ssl.SSLContext` for custom SSL @@ -1243,6 +1243,12 @@ is controlled by *force_close* constructor's parameter). .. versionadded:: 3.0 + .. deprecated:: 4.0 + + Scheduled for removal in 5.0. Pass *ssl* to + :class:`ClientSession` for a session-wide default, or to + :meth:`ClientSession.get` and others per request. + :param bool verify_ssl: perform SSL certificate validation for *HTTPS* requests (enabled by default). May be disabled to skip validation for sites with invalid certificates. diff --git a/examples/fake_server.py b/examples/fake_server.py index 4b72fa61da1..68285606278 100755 --- a/examples/fake_server.py +++ b/examples/fake_server.py @@ -101,9 +101,9 @@ async def main() -> None: fake_facebook = FakeFacebook() info = await fake_facebook.start() resolver = FakeResolver(info) - connector = TCPConnector(resolver=resolver, ssl=False) + connector = TCPConnector(resolver=resolver) - async with ClientSession(connector=connector) as session: + async with ClientSession(connector=connector, ssl=False) as session: async with session.get( "https://graph.facebook.com/v2.7/me", params={"access_token": token} ) as resp: diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 7e6de06936d..3e639f9b5e0 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -706,15 +706,13 @@ async def test_ssl_client( aiohttp_client: AiohttpClient, client_ssl_ctx: ssl.SSLContext, ) -> None: - connector = aiohttp.TCPConnector(ssl=client_ssl_ctx) - async def handler(request: web.Request) -> web.Response: return web.Response(text="Test message") app = web.Application() app.router.add_route("GET", "/", handler) server = await aiohttp_server(app, ssl=ssl_ctx) - client = await aiohttp_client(server, connector=connector) # type: ignore[var-annotated] + client = await aiohttp_client(server, ssl=client_ssl_ctx) # type: ignore[var-annotated] async with client.get("/") as resp: assert resp.status == 200 @@ -743,8 +741,8 @@ async def handler(request: web.Request) -> web.Response: server = await aiohttp_server(app, ssl=server_ctx) url = server.make_url("/") - connector = aiohttp.TCPConnector(ssl=client_ctx, limit=1, limit_per_host=1) - async with aiohttp.ClientSession(connector=connector) as session: + connector = aiohttp.TCPConnector(limit=1, limit_per_host=1) + async with aiohttp.ClientSession(connector=connector, ssl=client_ctx) as session: async with session.get(url, server_hostname="first.example") as resp: assert resp.status == 200 await resp.read() @@ -767,7 +765,7 @@ async def test_ssl_client_shutdown_timeout( with pytest.warns( DeprecationWarning, match="ssl_shutdown_timeout parameter is deprecated" ): - connector = aiohttp.TCPConnector(ssl=client_ssl_ctx, ssl_shutdown_timeout=0.1) + connector = aiohttp.TCPConnector(ssl_shutdown_timeout=0.1) async def streaming_handler(request: web.Request) -> NoReturn: # Create a streaming response that continuously sends data @@ -784,7 +782,9 @@ async def streaming_handler(request: web.Request) -> NoReturn: app = web.Application() app.router.add_route("GET", "/stream", streaming_handler) server = await aiohttp_server(app, ssl=ssl_ctx) - client = await aiohttp_client(server, connector=connector) # type: ignore[var-annotated] + client = await aiohttp_client( # type: ignore[var-annotated] + server, connector=connector, ssl=client_ssl_ctx + ) # Verify the connector has the correct timeout assert connector._ssl_shutdown_timeout == 0.1 @@ -836,8 +836,7 @@ async def handler(request: web.Request) -> web.Response: ssl_ctx.set_alpn_protocols(("http/1.1",)) server = await aiohttp_server(app, ssl=ssl_ctx) - connector = aiohttp.TCPConnector(ssl=False) - client = await aiohttp_client(server, connector=connector) # type: ignore[var-annotated] + client = await aiohttp_client(server, ssl=False) # type: ignore[var-annotated] async with client.get("/") as resp: assert resp.status == 200 txt = await resp.text() @@ -855,11 +854,13 @@ async def test_tcp_connector_fingerprint_ok( async def handler(request: web.Request) -> web.Response: return web.Response(text="Test message") - connector = aiohttp.TCPConnector(ssl=tls_fingerprint) + connector = aiohttp.TCPConnector() app = web.Application() app.router.add_route("GET", "/", handler) server = await aiohttp_server(app, ssl=ssl_ctx) - client = await aiohttp_client(server, connector=connector) # type: ignore[var-annotated] + client = await aiohttp_client( # type: ignore[var-annotated] + server, connector=connector, ssl=tls_fingerprint + ) async with client.get("/") as resp: assert resp.status == 200 @@ -882,12 +883,12 @@ async def handler(request: web.Request) -> NoReturn: bad_fingerprint = b"\x00" * len(tls_certificate_fingerprint_sha256) - connector = aiohttp.TCPConnector(ssl=Fingerprint(bad_fingerprint)) - app = web.Application() app.router.add_route("GET", "/", handler) server = await aiohttp_server(app, ssl=ssl_ctx) - client = await aiohttp_client(server, connector=connector) # type: ignore[var-annotated] + client = await aiohttp_client( # type: ignore[var-annotated] + server, ssl=Fingerprint(bad_fingerprint) + ) with pytest.raises(ServerFingerprintMismatch) as cm: await client.get("/") @@ -3487,10 +3488,10 @@ async def resolve( async def close(self) -> None: """Dummy""" - connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False) + connector = aiohttp.TCPConnector(resolver=FakeResolver()) async with ( - aiohttp.ClientSession(connector=connector) as client, + aiohttp.ClientSession(connector=connector, ssl=False) as client, client.get( url_from, headers={"Authorization": aiohttp.encode_basic_auth("user", "pass")}, @@ -3597,9 +3598,9 @@ async def resolve( async def close(self) -> None: """Dummy""" - connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False) + connector = aiohttp.TCPConnector(resolver=FakeResolver()) - async with aiohttp.ClientSession(connector=connector) as client: + async with aiohttp.ClientSession(connector=connector, ssl=False) as client: async with client.get( url_from, headers={ @@ -3722,10 +3723,11 @@ async def resolve( async def close(self) -> None: """Dummy""" - connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False) + connector = aiohttp.TCPConnector(resolver=FakeResolver()) async with aiohttp.ClientSession( connector=connector, + ssl=False, headers={"Authorization": "Basic dXNlcjpwYXNz"}, ) as client: async with client.get(url_from) as resp: diff --git a/tests/test_connector.py b/tests/test_connector.py index f36619a668c..85ddf8c75c4 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -2702,12 +2702,45 @@ async def test_invalid_ssl_param() -> None: async def test_tcp_connector_ctor_fingerprint_valid() -> None: valid = aiohttp.Fingerprint(hashlib.sha256(b"foo").digest()) - conn = aiohttp.TCPConnector(ssl=valid) + with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"): + conn = aiohttp.TCPConnector(ssl=valid) assert conn._ssl is valid await conn.close() +async def test_tcp_connector_ssl_deprecated() -> None: + with pytest.warns( + DeprecationWarning, + match="ssl parameter is deprecated since 4.0 and scheduled for removal in 5.0", + ): + conn = aiohttp.TCPConnector(ssl=False) + assert conn._ssl is False + + await conn.close() + + +async def test_tcp_connector_ssl_default_not_deprecated() -> None: + with warnings.catch_warnings(): + warnings.simplefilter("error") + conn = aiohttp.TCPConnector() + assert conn._ssl is True + + await conn.close() + + +async def test_tcp_connector_fingerprint_from_deprecated_ssl_param() -> None: + """The deprecated connector-level ssl is still used when the request has none.""" + fingerprint = aiohttp.Fingerprint(hashlib.sha256(b"foo").digest()) + with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"): + conn = aiohttp.TCPConnector(ssl=fingerprint) + req = mock.Mock() + req.ssl = True + assert conn._get_fingerprint(req) is fingerprint + + await conn.close() + + async def test_insecure_fingerprint_md5() -> None: with pytest.raises(ValueError): aiohttp.TCPConnector(ssl=aiohttp.Fingerprint(hashlib.md5(b"foo").digest())) @@ -2787,7 +2820,8 @@ async def test___get_ssl_context2() -> None: async def test___get_ssl_context3() -> None: ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - conn = aiohttp.TCPConnector(ssl=ctx) + with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"): + conn = aiohttp.TCPConnector(ssl=ctx) req = mock.Mock() req.is_ssl.return_value = True req.ssl = True @@ -2798,7 +2832,8 @@ async def test___get_ssl_context3() -> None: async def test___get_ssl_context4() -> None: ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - conn = aiohttp.TCPConnector(ssl=ctx) + with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"): + conn = aiohttp.TCPConnector(ssl=ctx) req = mock.Mock() req.is_ssl.return_value = True req.ssl = False @@ -2809,7 +2844,8 @@ async def test___get_ssl_context4() -> None: async def test___get_ssl_context5() -> None: ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - conn = aiohttp.TCPConnector(ssl=ctx) + with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"): + conn = aiohttp.TCPConnector(ssl=ctx) req = mock.Mock() req.is_ssl.return_value = True req.ssl = aiohttp.Fingerprint(hashlib.sha256(b"1").digest()) diff --git a/tests/test_proxy_functional.py b/tests/test_proxy_functional.py index 17af853a341..71dc4823107 100644 --- a/tests/test_proxy_functional.py +++ b/tests/test_proxy_functional.py @@ -58,10 +58,7 @@ async def get_request( trust_env: bool = False, **kwargs: Any, ) -> ClientResponse: - connector = aiohttp.TCPConnector(ssl=False) - async with aiohttp.ClientSession( - connector=connector, trust_env=trust_env - ) as client: + async with aiohttp.ClientSession(ssl=False, trust_env=trust_env) as client: async with client.request(method, url, **kwargs) as resp: return resp diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 2ab0153509a..178995d8d1d 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -742,8 +742,7 @@ async def test_static_file_ssl( app = web.Application() app.router.add_static("/static", dirname) server = await aiohttp_server(app, ssl=ssl_ctx) - conn = aiohttp.TCPConnector(ssl=client_ssl_ctx) - client = await aiohttp_client(server, connector=conn) # type: ignore[var-annotated] + client = await aiohttp_client(server, ssl=client_ssl_ctx) # type: ignore[var-annotated] resp = await client.get("/static/" + filename) assert 200 == resp.status @@ -755,7 +754,6 @@ async def test_static_file_ssl( resp.release() await client.close() - await conn.close() async def test_static_file_directory_traversal_attack( From e11d2836203a21bec59095498e578d37801027e7 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 27 Sep 2026 20:36:58 +0100 Subject: [PATCH 2/4] Fix secure shared cookie (#13830) --- CHANGES/13830.bugfix.rst | 1 + aiohttp/cookiejar.py | 3 +++ docs/client_reference.rst | 3 ++- docs/spelling_wordlist.txt | 1 + tests/test_client_functional.py | 9 +++++++-- tests/test_cookiejar.py | 28 ++++++++++++++++++++++++++++ 6 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 CHANGES/13830.bugfix.rst diff --git a/CHANGES/13830.bugfix.rst b/CHANGES/13830.bugfix.rst new file mode 100644 index 00000000000..2a3976924ba --- /dev/null +++ b/CHANGES/13830.bugfix.rst @@ -0,0 +1 @@ +Fixed ``CookieJar.filter_cookies()`` sending shared cookies (cookies without a ``Domain`` attribute) marked ``Secure`` over unencrypted connections -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 913e4a51ec4..f2bfe59efde 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -455,6 +455,9 @@ def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": # Send shared cookie key = ("", "") for c in self._cookies[key].values(): + if is_not_secure and c["secure"]: + continue + # Check cache first if c.key in self._morsel_cache[key]: filtered[c.key] = self._morsel_cache[key][c.key] diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 799b1ee21a6..3f5fb695705 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -2511,7 +2511,8 @@ Utilities :param ~yarl.URL response_url: URL of response, ``None`` for *shared cookies*. Regular cookies are coupled with server's URL and are sent only to this server, shared ones are sent in every - client request. + client request (except that shared cookies marked ``Secure`` + are only sent over encrypted connections). .. method:: filter_cookies(request_url) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index beb650819a4..715a6c9b9a1 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -378,6 +378,7 @@ un unawaited unclosed undercounting +unencrypted unescaped unhandled unicode diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 3e639f9b5e0..6f4f17a514b 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2884,12 +2884,17 @@ async def handler(request: web.Request) -> web.Response: c: http.cookies.Morsel[str] = http.cookies.Morsel() c.set("test3", "456", "456") c["httponly"] = True - c["secure"] = True c["max-age"] = 1000 + # A Secure shared cookie must be withheld entirely: the test server + # is plain http, so it must not reach the handler at all. + c2: http.cookies.Morsel[str] = http.cookies.Morsel() + c2.set("test4", "789", "789") + c2["secure"] = True + app = web.Application() app.router.add_get("/", handler) - client = await aiohttp_client(app, cookies={"test2": c}) + client = await aiohttp_client(app, cookies={"test2": c, "test4": c2}) async with client.get("/") as resp: assert 200 == resp.status diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 72d9b28c1d5..ed5ad78c492 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -1156,6 +1156,34 @@ async def test_treat_as_secure_origin() -> None: assert len(filtered_cookies) == 1 +async def test_treat_as_secure_origin_sends_secure_shared_cookie() -> None: + endpoint = URL("http://127.0.0.1/") + + jar = CookieJar(unsafe=True, treat_as_secure_origin=[endpoint]) + jar.update_cookies(SimpleCookie("shared-secure-cookie=first; Secure;")) + + filtered_cookies = jar.filter_cookies(request_url=endpoint) + assert "shared-secure-cookie" in filtered_cookies + + +async def test_filter_cookies_with_secure_shared_cookie() -> None: + """Secure shared cookies (no Domain attribute) must not be sent over cleartext.""" + jar = CookieJar() + jar.update_cookies( + SimpleCookie("shared-secure-cookie=first; Secure; shared-cookie=second;") + ) + + filtered = jar.filter_cookies(URL("http://example.com/")) + assert set(filtered.keys()) == {"shared-cookie"} + + filtered = jar.filter_cookies(URL("https://example.com/")) + assert set(filtered.keys()) == {"shared-cookie", "shared-secure-cookie"} + + # The morsel cached by the https request must not bypass the Secure check. + filtered = jar.filter_cookies(URL("http://example.com/")) + assert set(filtered.keys()) == {"shared-cookie"} + + async def test_filter_cookies_does_not_leak_memory() -> None: """Test that filter_cookies doesn't create empty cookie entries. From 6bc84990f2a6429667f741d97f5da22884b8a8d3 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 27 Sep 2026 23:15:39 +0100 Subject: [PATCH 3/4] Fix treat_as_secure_origin on request cookies (#13833) --- CHANGES/13833.bugfix.rst | 1 + aiohttp/abc.py | 5 ++++ aiohttp/client.py | 1 + aiohttp/cookiejar.py | 5 ++++ docs/client_reference.rst | 13 +++++++-- tests/test_client_functional.py | 49 +++++++++++++++++++++++++++++++++ tests/test_cookiejar.py | 3 +- 7 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 CHANGES/13833.bugfix.rst diff --git a/CHANGES/13833.bugfix.rst b/CHANGES/13833.bugfix.rst new file mode 100644 index 00000000000..2d0f96ce0b3 --- /dev/null +++ b/CHANGES/13833.bugfix.rst @@ -0,0 +1 @@ +Fixed per-request cookies (the ``cookies`` argument of a request method) marked ``Secure`` not being sent to origins listed in ``CookieJar``'s ``treat_as_secure_origin`` -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/abc.py b/aiohttp/abc.py index b9c7199a20c..d0f1816f2d4 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -164,6 +164,11 @@ def unsafe(self) -> bool: def quote_cookie(self) -> bool: """Return True if cookies should be quoted.""" + @property + def treat_as_secure_origin(self) -> frozenset[URL]: + """Return origins considered secure even over cleartext connections.""" + return frozenset() + @property @abstractmethod def cookies(self) -> MappingProxyType[tuple[str, str], SimpleCookie]: diff --git a/aiohttp/client.py b/aiohttp/client.py index b5f563d3f3f..9e2112791c1 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -650,6 +650,7 @@ async def _request( tmp_cookie_jar = CookieJar( unsafe=self._cookie_jar.unsafe, quote_cookie=self._cookie_jar.quote_cookie, + treat_as_secure_origin=self._cookie_jar.treat_as_secure_origin, ) tmp_cookie_jar.update_cookies(cookies) req_cookies = tmp_cookie_jar.filter_cookies(url) diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index f2bfe59efde..27412940a94 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -122,6 +122,11 @@ def unsafe(self) -> bool: def quote_cookie(self) -> bool: return self._quote_cookie + @property + def treat_as_secure_origin(self) -> frozenset[URL]: + """Return origins considered secure even over cleartext connections.""" + return self._treat_as_secure_origin + @property def cookies(self) -> MappingProxyType[tuple[str, str], SimpleCookie]: """Return the cookies stored in this jar.""" diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 3f5fb695705..4e7e4da5ca2 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -2488,7 +2488,7 @@ Utilities .. versionadded:: 3.7 :param treat_as_secure_origin: (optional) Mark origins as secure - for cookies marked as Secured. Possible types are + for cookies marked as Secured. Possible types are: @@ -2512,7 +2512,8 @@ Utilities cookies*. Regular cookies are coupled with server's URL and are sent only to this server, shared ones are sent in every client request (except that shared cookies marked ``Secure`` - are only sent over encrypted connections). + are only sent over encrypted connections or to origins listed + in *treat_as_secure_origin*). .. method:: filter_cookies(request_url) @@ -2576,6 +2577,14 @@ Utilities per ``(domain, path, name)`` cookie identity so that same-named cookies on other paths cannot affect it. + .. attribute:: treat_as_secure_origin + + A :class:`frozenset` of :class:`~yarl.URL` origins that are + treated as secure even when the connection is not encrypted, as + configured by the *treat_as_secure_origin* parameter. + + .. versionadded:: 3.14.4 + .. class:: DummyCookieJar(*, loop=None) :canonical: aiohttp.cookiejar.DummyCookieJar diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 6f4f17a514b..340bb2b29f3 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2900,6 +2900,55 @@ async def handler(request: web.Request) -> web.Response: assert 200 == resp.status +async def test_request_secure_cookie_treat_as_secure_origin( + aiohttp_server: AiohttpServer, aiohttp_client: AiohttpClient +) -> None: + """Per-request Secure cookies must honor the session jar's trusted origins.""" + + async def handler(request: web.Request) -> web.Response: + assert request.cookies.get("auth") == "token" + return web.Response() + + app = web.Application() + app.router.add_get("/", handler) + server = await aiohttp_server(app) + + jar = aiohttp.CookieJar(unsafe=True, treat_as_secure_origin=[server.make_url("/")]) + client = await aiohttp_client(server, cookie_jar=jar) # type: ignore[var-annotated] + + c: http.cookies.Morsel[str] = http.cookies.Morsel() + c.set("auth", "token", "token") + c["secure"] = True + + async with client.get("/", cookies={"auth": c}) as resp: + assert resp.status == 200 + + +async def test_request_secure_cookie_not_sent_over_http( + aiohttp_client: AiohttpClient, +) -> None: + """A per-request Secure cookie is only sent to the jar's trusted origins.""" + + async def handler(request: web.Request) -> web.Response: + assert request.cookies.keys() == {"plain"} + return web.Response() + + app = web.Application() + app.router.add_get("/", handler) + # The trusted origin never matches the test server, so the Secure + # cookie must still be withheld from this plain-http request. + jar = aiohttp.CookieJar(treat_as_secure_origin=[URL("http://example.com")]) + client = await aiohttp_client(app, cookie_jar=jar) + + c: http.cookies.Morsel[str] = http.cookies.Morsel() + c.set("auth", "token", "token") + c["secure"] = True + + cookies: dict[str, str | http.cookies.Morsel[str]] = {"auth": c, "plain": "ok"} + async with client.get("/", cookies=cookies) as resp: + assert resp.status == 200 + + async def test_set_cookies( aiohttp_client: AiohttpClient, caplog: pytest.LogCaptureFixture ) -> None: diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index ed5ad78c492..b0833156d7b 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -780,6 +780,7 @@ async def test_dummy_cookie_jar() -> None: dummy_jar = DummyCookieJar() assert dummy_jar.unsafe is False assert dummy_jar.quote_cookie is True + assert dummy_jar.treat_as_secure_origin == frozenset() assert len(dummy_jar) == 0 dummy_jar.update_cookies(cookie) assert len(dummy_jar) == 0 @@ -1135,7 +1136,7 @@ async def test_treat_as_secure_origin_init( url: str | URL | list[str] | list[URL], ) -> None: jar = CookieJar(unsafe=True, treat_as_secure_origin=url) - assert jar._treat_as_secure_origin == frozenset({URL("http://127.0.0.1")}) + assert jar.treat_as_secure_origin == frozenset({URL("http://127.0.0.1")}) async def test_treat_as_secure_origin() -> None: From e07684ef9056183bbefba1ae3f717a23f243631e Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 27 Sep 2026 23:45:49 +0100 Subject: [PATCH 4/4] Close idle connections (#13681) --- CHANGES/13681.bugfix.rst | 1 + aiohttp/web_protocol.py | 14 ++- docs/web_reference.rst | 11 ++- tests/test_web_server.py | 119 +++++++++++++++++++++++++ tests/test_web_websocket_functional.py | 30 +++++++ 5 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 CHANGES/13681.bugfix.rst diff --git a/CHANGES/13681.bugfix.rst b/CHANGES/13681.bugfix.rst new file mode 100644 index 00000000000..e39b73a91fa --- /dev/null +++ b/CHANGES/13681.bugfix.rst @@ -0,0 +1 @@ +Fixed idle connections not being closed if no request was received -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 8fd73c84055..fa635848324 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -140,8 +140,7 @@ class RequestHandler(BaseProtocol, Generic[_Request]): status line, bad headers or incomplete payload. If any error occurs, connection gets closed. - keepalive_timeout -- number of seconds before closing - keep-alive connection + keepalive_timeout -- number of seconds before closing an idle connection. tcp_keepalive -- TCP keep-alive is on, default is on @@ -240,7 +239,9 @@ def __init__( super().__init__(loop, parser) self._request_count = 0 - self._keepalive = False + # True from the start so the deadline armed in connection_made() + # closes connections that never deliver a complete first request. + self._keepalive = True self._current_request: _Request | None = None self._manager: Server[_Request] | None = manager self._request_handler: _RequestHandler[_Request] | None = ( @@ -401,6 +402,13 @@ def connection_made(self, transport: asyncio.BaseTransport) -> None: self._manager.connection_made(self, real_transport) loop = self._loop + # Need to enable keepalive timeout at start of connection, as there's no other + # protection against a dead connection that doesn't send a request at all. + if self._keepalive_timeout > 0: + close_time = loop.time() + self._keepalive_timeout + self._next_keepalive_close_time = close_time + self._keepalive_handle = loop.call_at(close_time, self._process_keepalive) + if sys.version_info >= (3, 14): if isinstance(loop, BaseEventLoop): task = asyncio.create_task(self.start(), eager_start=True) diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 1797e74dc03..645a0f4f345 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -3139,7 +3139,7 @@ Utilities .. function:: run_app(app, *, debug=False, host=None, port=None, \ path=None, sock=None, shutdown_timeout=60.0, \ - keepalive_timeout=3630, ssl_context=None, \ + keepalive_timeout=75.0, ssl_context=None, \ print=print, backlog=128, \ access_log_class=aiohttp.helpers.AccessLogger, \ access_log_format=aiohttp.helpers.AccessLogger.LOG_FORMAT, \ @@ -3208,6 +3208,10 @@ Utilities :param float keepalive_timeout: a delay before a TCP connection is closed after a HTTP request. The delay allows for reuse of a TCP connection. + The same delay bounds how long a newly + accepted connection may take to deliver + its first complete request; connections + that stay idle or incomplete are closed. When deployed behind a reverse proxy it's important for this value to be @@ -3217,6 +3221,11 @@ Utilities .. versionadded:: 3.8 + .. versionchanged:: 3.14.4 + + The timeout is now also applied while waiting for the first + request, closing connections that never send a complete request. + :param ssl_context: :class:`ssl.SSLContext` for HTTPS server, ``None`` for HTTP connection. diff --git a/tests/test_web_server.py b/tests/test_web_server.py index b04815539be..06efc6e5c05 100644 --- a/tests/test_web_server.py +++ b/tests/test_web_server.py @@ -419,3 +419,122 @@ async def wait_for_backpressure() -> None: loop.set_exception_handler(original_handler) assert not exc_handler_calls + + +async def test_idle_connection_closed_before_first_request( + aiohttp_raw_server: AiohttpRawServer, +) -> None: + """A connection that never sends a request must not be held open forever.""" + + async def handler(request: web.BaseRequest) -> web.Response: + assert False + + server = await aiohttp_raw_server(handler, keepalive_timeout=0.2) + + reader, writer = await asyncio.open_connection(server.host, server.port) + try: + # The server must close the connection once keepalive_timeout + # expires without a complete request having arrived. + assert await asyncio.wait_for(reader.read(), timeout=5) == b"" + finally: + writer.close() + await writer.wait_closed() + + +# uvloop closes the socket before running connection_lost, the reverse of the +# default loop's ordering, so run under both to cover the disconnect race. +@pytest.mark.asyncio(loop_factories=("uvloop", "selector")) +async def test_trickled_headers_closed_at_first_request_deadline( + aiohttp_raw_server: AiohttpRawServer, monkeypatch: pytest.MonkeyPatch +) -> None: + """Partial header bytes must not extend the first-request deadline.""" + + async def handler(request: web.BaseRequest) -> web.Response: + assert False + + server = await aiohttp_raw_server(handler, keepalive_timeout=0.4) + runner = server.runner + assert runner is not None + server_impl = runner.server + assert server_impl is not None + + disconnected = asyncio.Event() + original_connection_lost = server_impl.connection_lost + + def connection_lost(protocol: Any, exc: BaseException | None = None) -> None: + original_connection_lost(protocol, exc) + disconnected.set() + + monkeypatch.setattr(server_impl, "connection_lost", connection_lost) + + _, writer = await asyncio.open_connection(server.host, server.port) + + async def trickle_until_closed() -> None: + writer.write(b"GET / HTTP/1.1\r\nHost: example.com\r\nX-Slow: ") + # asyncio's drain() raises a ConnectionError once the connection is + # lost; uvloop's write() raises RuntimeError on the closed transport. + with pytest.raises((ConnectionError, RuntimeError)): + while True: + await writer.drain() + await asyncio.sleep(0.05) + writer.write(b"a") + + try: + # The deadline must fire despite the steady trickle of bytes. + await asyncio.wait_for(trickle_until_closed(), timeout=5) + await asyncio.wait_for(disconnected.wait(), timeout=5) + assert not server_impl.connections + assert server_impl.requests_count == 0 + finally: + writer.close() + with suppress(ConnectionError, RuntimeError): + await writer.wait_closed() + + +async def test_keepalive_timeout_zero_no_first_request_deadline( + aiohttp_raw_server: AiohttpRawServer, +) -> None: + """keepalive_timeout=0 must not impose a deadline on the first request.""" + + async def handler(request: web.BaseRequest) -> web.Response: + return web.Response(text="ok") + + server = await aiohttp_raw_server(handler, keepalive_timeout=0) + + reader, writer = await asyncio.open_connection(server.host, server.port) + try: + # A zero timeout must not arm a deadline that fires at accept time: + # the connection has to survive an idle pause and serve the request. + await asyncio.sleep(0.1) + writer.write(b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n") + await writer.drain() + head = await asyncio.wait_for(reader.readuntil(b"\r\n\r\n"), timeout=5) + assert head.startswith(b"HTTP/1.1 200 ") + # Between requests a zero timeout still closes the idle connection + # right after the response, so the body is followed by EOF. + assert await asyncio.wait_for(reader.read(), timeout=5) == b"ok" + finally: + writer.close() + await writer.wait_closed() + + +async def test_handler_slower_than_first_request_deadline( + aiohttp_raw_server: AiohttpRawServer, +) -> None: + """A parsed request being handled is not subject to the idle deadline.""" + + async def handler(request: web.BaseRequest) -> web.Response: + await asyncio.sleep(0.4) # Longer than keepalive_timeout. + return web.Response(text="ok") + + server = await aiohttp_raw_server(handler, keepalive_timeout=0.2) + + reader, writer = await asyncio.open_connection(server.host, server.port) + try: + writer.write(b"GET / HTTP/1.1\r\nHost: example.com\r\n\r\n") + await writer.drain() + head = await asyncio.wait_for(reader.readuntil(b"\r\n\r\n"), timeout=5) + assert head.startswith(b"HTTP/1.1 200 ") + finally: + writer.close() + await writer.wait_closed() diff --git a/tests/test_web_websocket_functional.py b/tests/test_web_websocket_functional.py index 353807d9364..7cbdb55dd68 100644 --- a/tests/test_web_websocket_functional.py +++ b/tests/test_web_websocket_functional.py @@ -1317,6 +1317,36 @@ async def handler(request: web.Request) -> web.StreamResponse: await ws.receive() # Handle close +async def test_websocket_disable_keepalive_no_armed_handle( + aiohttp_server: AiohttpServer, aiohttp_client: AiohttpClient +) -> None: + """keep_alive(False) when no keepalive timer was armed (keepalive_timeout=0).""" + + async def handler(request: web.Request) -> web.StreamResponse: + ws = web.WebSocketResponse() + assert ws.can_prepare(request) + # A zero timeout arms no deadline at connection_made(). + assert request.protocol._keepalive_handle is None + await ws.prepare(request) + assert not request.protocol._keepalive + assert request.protocol._keepalive_handle is None + + await ws.send_str("OK") + await ws.close() + return ws + + app = web.Application() + app.router.add_route("GET", "/", handler) + server = await aiohttp_server(app, keepalive_timeout=0) + client = await aiohttp_client(server) # type: ignore[var-annotated] + + ws = await client.ws_connect("/") + data = await ws.receive_str() + assert data == "OK" + + await ws.receive() # Handle close + + async def test_receive_str_nonstring(aiohttp_client: AiohttpClient) -> None: async def handler(request: web.Request) -> web.WebSocketResponse: ws = web.WebSocketResponse()