From 1710f059dd6c63fcac1efbfd041b1e1cdda9f474 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 31 May 2025 01:00:46 -0500 Subject: [PATCH 01/11] Increment version to 3.12.7.dev0 (#11099) --- aiohttp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index 6a0b167be83..78f22b4051f 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = "3.12.6" +__version__ = "3.12.7.dev0" from typing import TYPE_CHECKING, Tuple From 1f02d7db7e138f7b7b6b97204c7bc267dedb5546 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Jun 2025 09:43:01 -0500 Subject: [PATCH 02/11] [PR #11106/cfb9931 backport][3.13] Fix cookies with duplicate names being lost when updating cookie jar (#11109) --- CHANGES/11105.bugfix.rst | 10 ++ CHANGES/11106.bugfix.rst | 1 + CHANGES/4486.bugfix.rst | 1 + aiohttp/abc.py | 29 ++++- aiohttp/client.py | 7 +- aiohttp/client_reqrep.py | 29 +++-- docs/client_reference.rst | 13 ++ tests/test_client_functional.py | 146 +++++++++++++++++++++ tests/test_client_response.py | 144 ++++++++++++++++++++- tests/test_client_session.py | 63 ++++++++-- tests/test_cookiejar.py | 217 ++++++++++++++++++++++++++++++++ 11 files changed, 635 insertions(+), 25 deletions(-) create mode 100644 CHANGES/11105.bugfix.rst create mode 120000 CHANGES/11106.bugfix.rst create mode 120000 CHANGES/4486.bugfix.rst diff --git a/CHANGES/11105.bugfix.rst b/CHANGES/11105.bugfix.rst new file mode 100644 index 00000000000..33578aa7a95 --- /dev/null +++ b/CHANGES/11105.bugfix.rst @@ -0,0 +1,10 @@ +Fixed an issue where cookies with duplicate names but different domains or paths +were lost when updating the cookie jar. The :class:`~aiohttp.ClientSession` +cookie jar now correctly stores all cookies even if they have the same name but +different domain or path, following the :rfc:`6265#section-5.3` storage model -- by :user:`bdraco`. + +Note that :attr:`ClientResponse.cookies ` returns +a :class:`~http.cookies.SimpleCookie` which uses the cookie name as a key, so +only the last cookie with each name is accessible via this interface. All cookies +can be accessed via :meth:`ClientResponse.headers.getall('Set-Cookie') +` if needed. diff --git a/CHANGES/11106.bugfix.rst b/CHANGES/11106.bugfix.rst new file mode 120000 index 00000000000..3e5efb0f3f3 --- /dev/null +++ b/CHANGES/11106.bugfix.rst @@ -0,0 +1 @@ +11105.bugfix.rst \ No newline at end of file diff --git a/CHANGES/4486.bugfix.rst b/CHANGES/4486.bugfix.rst new file mode 120000 index 00000000000..3e5efb0f3f3 --- /dev/null +++ b/CHANGES/4486.bugfix.rst @@ -0,0 +1 @@ +11105.bugfix.rst \ No newline at end of file diff --git a/aiohttp/abc.py b/aiohttp/abc.py index a5e00a952d0..96839b4f378 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -3,7 +3,7 @@ import socket from abc import ABC, abstractmethod from collections.abc import Sized -from http.cookies import BaseCookie, Morsel +from http.cookies import BaseCookie, CookieError, Morsel, SimpleCookie from typing import ( TYPE_CHECKING, Any, @@ -14,6 +14,7 @@ Iterable, List, Optional, + Sequence, Tuple, TypedDict, Union, @@ -22,6 +23,7 @@ from multidict import CIMultiDict from yarl import URL +from .log import client_logger from .typedefs import LooseCookies if TYPE_CHECKING: @@ -192,6 +194,31 @@ def clear_domain(self, domain: str) -> None: def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> None: """Update cookies.""" + def update_cookies_from_headers( + self, headers: Sequence[str], response_url: URL + ) -> None: + """ + Update cookies from raw Set-Cookie headers. + + Default implementation parses each header separately to preserve + cookies with same name but different domain/path. + """ + # Default implementation for backward compatibility + cookies_to_update: List[Tuple[str, Morsel[str]]] = [] + for cookie_header in headers: + tmp_cookie = SimpleCookie() + try: + tmp_cookie.load(cookie_header) + # Collect all cookies as tuples (name, morsel) + for name, morsel in tmp_cookie.items(): + cookies_to_update.append((name, morsel)) + except CookieError as exc: + client_logger.warning("Can not load response cookies: %s", exc) + + # Update all cookies at once for efficiency + if cookies_to_update: + self.update_cookies(cookies_to_update, response_url) + @abstractmethod def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": """Return the jar's cookies filtered by their attributes.""" diff --git a/aiohttp/client.py b/aiohttp/client.py index 6457248d5ea..576a965ba5d 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -779,8 +779,11 @@ async def _connect_and_send_request( raise raise ClientOSError(*exc.args) from exc - if cookies := resp._cookies: - self._cookie_jar.update_cookies(cookies, resp.url) + # Update cookies from raw headers to preserve duplicates + if resp._raw_cookie_headers: + self._cookie_jar.update_cookies_from_headers( + resp._raw_cookie_headers, resp.url + ) # redirects if resp.status in (301, 302, 303, 307, 308) and allow_redirects: diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index e437ef67aff..01835260cc5 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -291,6 +291,7 @@ class ClientResponse(HeadersMixin): _connection: Optional["Connection"] = None # current connection _cookies: Optional[SimpleCookie] = None + _raw_cookie_headers: Optional[Tuple[str, ...]] = None _continue: Optional["asyncio.Future[bool]"] = None _source_traceback: Optional[traceback.StackSummary] = None _session: Optional["ClientSession"] = None @@ -372,12 +373,29 @@ def _writer(self, writer: Optional["asyncio.Task[None]"]) -> None: @property def cookies(self) -> SimpleCookie: if self._cookies is None: - self._cookies = SimpleCookie() + if self._raw_cookie_headers is not None: + # Parse cookies for response.cookies (SimpleCookie for backward compatibility) + cookies = SimpleCookie() + for hdr in self._raw_cookie_headers: + try: + cookies.load(hdr) + except CookieError as exc: + client_logger.warning("Can not load response cookies: %s", exc) + self._cookies = cookies + else: + self._cookies = SimpleCookie() return self._cookies @cookies.setter def cookies(self, cookies: SimpleCookie) -> None: self._cookies = cookies + # Generate raw cookie headers from the SimpleCookie + if cookies: + self._raw_cookie_headers = tuple( + morsel.OutputString() for morsel in cookies.values() + ) + else: + self._raw_cookie_headers = None @reify def url(self) -> URL: @@ -543,13 +561,8 @@ async def start(self, connection: "Connection") -> "ClientResponse": # cookies if cookie_hdrs := self.headers.getall(hdrs.SET_COOKIE, ()): - cookies = SimpleCookie() - for hdr in cookie_hdrs: - try: - cookies.load(hdr) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) - self._cookies = cookies + # Store raw cookie headers for CookieJar + self._raw_cookie_headers = tuple(cookie_hdrs) return self def _response_eof(self) -> None: diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 07839686039..8a721f514cd 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -1499,6 +1499,19 @@ Response object HTTP cookies of response (*Set-Cookie* HTTP header, :class:`~http.cookies.SimpleCookie`). + .. note:: + + Since :class:`~http.cookies.SimpleCookie` uses cookie name as the + key, cookies with the same name but different domains or paths will + be overwritten. Only the last cookie with a given name will be + accessible via this attribute. + + To access all cookies, including duplicates with the same name, + use :meth:`response.headers.getall('Set-Cookie') `. + + The session's cookie jar will correctly store all cookies, even if + they are not accessible via this attribute. + .. attribute:: headers A case-insensitive multidict proxy with HTTP headers of diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 1d91956c4a3..ca1a7dd1d6b 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2712,6 +2712,7 @@ async def handler(request): async with client.get("/") as resp: assert 200 == resp.status cookie_names = {c.key for c in client.session.cookie_jar} + _ = resp.cookies assert cookie_names == {"c1", "c2"} m_log.warning.assert_called_with("Can not load response cookies: %s", mock.ANY) @@ -5111,3 +5112,148 @@ async def redirect_handler(request: web.Request) -> web.Response: assert ( payload.close_called ), "Payload.close() was not called when InvalidUrlRedirectClientError (invalid origin) was raised" + + +async def test_amazon_like_cookie_scenario(aiohttp_client: AiohttpClient) -> None: + """Test real-world cookie scenario similar to Amazon.""" + + class FakeResolver(AbstractResolver): + def __init__(self, port: int): + self._port = port + + async def resolve( + self, host: str, port: int = 0, family: int = 0 + ) -> List[ResolveResult]: + if host in ("amazon.it", "www.amazon.it"): + return [ + { + "hostname": host, + "host": "127.0.0.1", + "port": self._port, + "family": socket.AF_INET, + "proto": 0, + "flags": 0, + } + ] + assert False, f"Unexpected host: {host}" + + async def close(self) -> None: + """Close the resolver if needed.""" + + async def handler(request: web.Request) -> web.Response: + response = web.Response(text="Login successful") + + # Simulate Amazon-like cookies from the issue + cookies = [ + "session-id=146-7423990-7621939; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; " + "Secure; HttpOnly", + "session-id=147-8529641-8642103; Domain=.www.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; HttpOnly", + "session-id-time=2082758401l; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; Secure", + "session-id-time=2082758402l; Domain=.www.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/", + "ubid-acbit=257-7531983-5395266; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; Secure", + 'x-acbit="KdvJzu8W@Fx6Jj3EuNFLuP0N7OtkuCfs"; Version=1; ' + "Domain=.amazon.it; Path=/; Secure; HttpOnly", + "at-acbit=Atza|IwEBIM-gLr8; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; " + "Secure; HttpOnly", + 'sess-at-acbit="4+6VzSJPHIFD/OqO264hFxIng8Y="; ' + "Domain=.amazon.it; Expires=Mon, 31-May-2027 10:00:00 GMT; " + "Path=/; Secure; HttpOnly", + "lc-acbit=it_IT; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/", + "i18n-prefs=EUR; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/", + "av-profile=null; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; Secure", + 'user-pref-token="Am81ywsJ69xObBnuJ2FbilVH0mg="; ' + "Domain=.amazon.it; Path=/; Secure", + ] + + for cookie in cookies: + response.headers.add("Set-Cookie", cookie) + + return response + + app = web.Application() + app.router.add_get("/", handler) + + # Get the test server + server = await aiohttp_client(app) + port = server.port + + # Create a new client session with our fake resolver + resolver = FakeResolver(port) + + async with ( + aiohttp.TCPConnector(resolver=resolver, force_close=True) as connector, + aiohttp.ClientSession(connector=connector) as session, + ): + # Make request to www.amazon.it which will resolve to + # 127.0.0.1:port. This allows cookies for both .amazon.it + # and .www.amazon.it domains + resp = await session.get(f"http://www.amazon.it:{port}/") + + # Check headers + cookie_headers = resp.headers.getall("Set-Cookie") + assert ( + len(cookie_headers) == 12 + ), f"Expected 12 headers, got {len(cookie_headers)}" + + # Check parsed cookies - SimpleCookie only keeps the last + # cookie with each name. So we expect 10 unique cookie names + # (not 12) + expected_cookie_names = { + "session-id", # Will only have one + "session-id-time", # Will only have one + "ubid-acbit", + "x-acbit", + "at-acbit", + "sess-at-acbit", + "lc-acbit", + "i18n-prefs", + "av-profile", + "user-pref-token", + } + assert set(resp.cookies.keys()) == expected_cookie_names + assert ( + len(resp.cookies) == 10 + ), f"Expected 10 cookies in SimpleCookie, got {len(resp.cookies)}" + + # The important part: verify the session's cookie jar has + # all cookies. The cookie jar should have all 12 cookies, + # not just 10 + jar_cookies = list(session.cookie_jar) + assert ( + len(jar_cookies) == 12 + ), f"Expected 12 cookies in jar, got {len(jar_cookies)}" + + # Verify we have both session-id cookies with different domains + session_ids = [c for c in jar_cookies if c.key == "session-id"] + assert ( + len(session_ids) == 2 + ), f"Expected 2 session-id cookies, got {len(session_ids)}" + + # Verify the domains are different + session_id_domains = {c["domain"] for c in session_ids} + assert session_id_domains == { + "amazon.it", + "www.amazon.it", + }, f"Got domains: {session_id_domains}" + + # Verify we have both session-id-time cookies with different + # domains + session_id_times = [c for c in jar_cookies if c.key == "session-id-time"] + assert ( + len(session_id_times) == 2 + ), f"Expected 2 session-id-time cookies, got {len(session_id_times)}" + + # Now test that the raw headers were properly preserved + assert resp._raw_cookie_headers is not None + assert ( + len(resp._raw_cookie_headers) == 12 + ), "All raw headers should be preserved" diff --git a/tests/test_client_response.py b/tests/test_client_response.py index 4a8000962d1..2d70feaf06d 100644 --- a/tests/test_client_response.py +++ b/tests/test_client_response.py @@ -3,6 +3,7 @@ import asyncio import gc import sys +from http.cookies import SimpleCookie from typing import Callable from unittest import mock @@ -11,7 +12,7 @@ from yarl import URL import aiohttp -from aiohttp import ClientSession, http +from aiohttp import ClientSession, hdrs, http from aiohttp.client_reqrep import ClientResponse, RequestInfo from aiohttp.helpers import TimerNoop @@ -1333,3 +1334,144 @@ def test_response_not_closed_after_get_ok(mocker) -> None: assert not response.ok assert not response.closed assert spy.call_count == 0 + + +def test_response_duplicate_cookie_names( + loop: asyncio.AbstractEventLoop, session: ClientSession +) -> None: + """ + Test that response.cookies handles duplicate cookie names correctly. + + Note: This behavior (losing cookies with same name but different domains/paths) + is arguably undesirable, but we promise to return a SimpleCookie object, and + SimpleCookie uses cookie name as the key. This is documented behavior. + + To access all cookies including duplicates, users should use: + - response.headers.getall('Set-Cookie') for raw headers + - The session's cookie jar correctly stores all cookies + """ + response = ClientResponse( + "get", + URL("http://example.com"), + request_info=mock.Mock(), + writer=WriterMock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + + # Set headers with duplicate cookie names but different domains + headers = CIMultiDict( + [ + ( + "Set-Cookie", + "session-id=123-4567890; Domain=.example.com; Path=/; Secure", + ), + ("Set-Cookie", "session-id=098-7654321; Domain=.www.example.com; Path=/"), + ("Set-Cookie", "user-pref=dark; Domain=.example.com; Path=/"), + ("Set-Cookie", "user-pref=light; Domain=api.example.com; Path=/"), + ] + ) + response._headers = CIMultiDictProxy(headers) + # Set raw cookie headers as done in ClientResponse.start() + response._raw_cookie_headers = tuple(headers.getall("Set-Cookie", [])) + + # SimpleCookie only keeps the last cookie with each name + # This is expected behavior since SimpleCookie uses name as the key + assert len(response.cookies) == 2 # Only 'session-id' and 'user-pref' + assert response.cookies["session-id"].value == "098-7654321" # Last one wins + assert response.cookies["user-pref"].value == "light" # Last one wins + + +def test_response_raw_cookie_headers_preserved( + loop: asyncio.AbstractEventLoop, session: ClientSession +) -> None: + """Test that raw Set-Cookie headers are preserved in _raw_cookie_headers.""" + response = ClientResponse( + "get", + URL("http://example.com"), + request_info=mock.Mock(), + writer=WriterMock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + + # Set headers with multiple cookies + cookie_headers = [ + "session-id=123; Domain=.example.com; Path=/; Secure", + "session-id=456; Domain=.www.example.com; Path=/", + "tracking=xyz; Domain=.example.com; Path=/; HttpOnly", + ] + + headers: CIMultiDict[str] = CIMultiDict() + for cookie_hdr in cookie_headers: + headers.add("Set-Cookie", cookie_hdr) + + response._headers = CIMultiDictProxy(headers) + + # Set raw cookie headers as done in ClientResponse.start() + response._raw_cookie_headers = tuple(response.headers.getall(hdrs.SET_COOKIE, [])) + + # Verify raw headers are preserved + assert response._raw_cookie_headers == tuple(cookie_headers) + assert len(response._raw_cookie_headers) == 3 + + # But SimpleCookie only has unique names + assert len(response.cookies) == 2 # 'session-id' and 'tracking' + + +def test_response_cookies_setter_updates_raw_headers( + loop: asyncio.AbstractEventLoop, session: ClientSession +) -> None: + """Test that setting cookies property updates _raw_cookie_headers.""" + response = ClientResponse( + "get", + URL("http://example.com"), + request_info=mock.Mock(), + writer=WriterMock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + + # Create a SimpleCookie with some cookies + cookies = SimpleCookie() + cookies["session-id"] = "123456" + cookies["session-id"]["domain"] = ".example.com" + cookies["session-id"]["path"] = "/" + cookies["session-id"]["secure"] = True + + cookies["tracking"] = "xyz789" + cookies["tracking"]["domain"] = ".example.com" + cookies["tracking"]["httponly"] = True + + # Set the cookies property + response.cookies = cookies + + # Verify _raw_cookie_headers was updated + assert response._raw_cookie_headers is not None + assert len(response._raw_cookie_headers) == 2 + assert isinstance(response._raw_cookie_headers, tuple) + + # Check the raw headers contain the expected cookie strings + raw_headers = list(response._raw_cookie_headers) + assert any("session-id=123456" in h for h in raw_headers) + assert any("tracking=xyz789" in h for h in raw_headers) + assert any("Secure" in h for h in raw_headers) + assert any("HttpOnly" in h for h in raw_headers) + + # Verify cookies property returns the same object + assert response.cookies is cookies + + # Test setting empty cookies + empty_cookies = SimpleCookie() + response.cookies = empty_cookies + # Should not set _raw_cookie_headers for empty cookies + assert response._raw_cookie_headers is None diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 0fdfaee6761..2702350f132 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -4,8 +4,8 @@ import io import json from collections import deque -from http.cookies import SimpleCookie -from typing import Any, Awaitable, Callable, List +from http.cookies import BaseCookie, SimpleCookie +from typing import Any, Awaitable, Callable, Iterator, List, Optional, cast from unittest import mock from uuid import uuid4 @@ -15,7 +15,7 @@ from yarl import URL import aiohttp -from aiohttp import CookieJar, client, hdrs, web +from aiohttp import CookieJar, abc, client, hdrs, web from aiohttp.client import ClientSession from aiohttp.client_proto import ResponseHandler from aiohttp.client_reqrep import ClientRequest @@ -639,8 +639,43 @@ async def create_connection( async def test_cookie_jar_usage(loop: Any, aiohttp_client: Any) -> None: req_url = None - jar = mock.Mock() - jar.filter_cookies.return_value = None + class MockCookieJar(abc.AbstractCookieJar): + def __init__(self) -> None: + self._update_cookies_mock = mock.Mock() + self._filter_cookies_mock = mock.Mock(return_value=BaseCookie()) + self._clear_mock = mock.Mock() + self._clear_domain_mock = mock.Mock() + self._items: List[Any] = [] + + @property + def quote_cookie(self) -> bool: + return True + + def clear(self, predicate: Optional[abc.ClearCookiePredicate] = None) -> None: + self._clear_mock(predicate) + + def clear_domain(self, domain: str) -> None: + self._clear_domain_mock(domain) + + def update_cookies(self, cookies: Any, response_url: URL = URL()) -> None: + self._update_cookies_mock(cookies, response_url) + + def filter_cookies(self, request_url: URL) -> BaseCookie[str]: + return cast(BaseCookie[str], self._filter_cookies_mock(request_url)) + + def __len__(self) -> int: + return len(self._items) + + def __iter__(self) -> Iterator[Any]: + return iter(self._items) + + jar = MockCookieJar() + + assert jar.quote_cookie is True + assert len(jar) == 0 + assert list(jar) == [] + jar.clear() + jar.clear_domain("example.com") async def handler(request): nonlocal req_url @@ -657,22 +692,24 @@ async def handler(request): ) # Updating the cookie jar with initial user defined cookies - jar.update_cookies.assert_called_with({"request": "req_value"}) + jar._update_cookies_mock.assert_called_with({"request": "req_value"}, URL()) - jar.update_cookies.reset_mock() + jar._update_cookies_mock.reset_mock() resp = await session.get("/") await resp.release() # Filtering the cookie jar before sending the request, # getting the request URL as only parameter - jar.filter_cookies.assert_called_with(URL(req_url)) + jar._filter_cookies_mock.assert_called_with(URL(req_url)) # Updating the cookie jar with the response cookies - assert jar.update_cookies.called - resp_cookies = jar.update_cookies.call_args[0][0] - assert isinstance(resp_cookies, SimpleCookie) - assert "response" in resp_cookies - assert resp_cookies["response"].value == "resp_value" + assert jar._update_cookies_mock.called + resp_cookies = jar._update_cookies_mock.call_args[0][0] + # Now update_cookies is called with a list of tuples + assert isinstance(resp_cookies, list) + assert len(resp_cookies) == 1 + assert resp_cookies[0][0] == "response" + assert resp_cookies[0][1].value == "resp_value" async def test_cookies_with_not_quoted_cookie_jar( diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 26efaa30d04..e1b6e351e3d 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -2,11 +2,13 @@ import datetime import heapq import itertools +import logging import pathlib import pickle import unittest from http.cookies import BaseCookie, Morsel, SimpleCookie from operator import not_ +from typing import List, Set from unittest import mock import pytest @@ -1181,3 +1183,218 @@ async def test_filter_cookies_does_not_leak_memory() -> None: for key, morsels in jar._morsel_cache.items(): assert key in jar._cookies, f"Orphaned morsel cache entry for {key}" assert len(morsels) > 0, f"Empty morsel cache entry found for {key}" + + +async def test_update_cookies_from_headers() -> None: + """Test update_cookies_from_headers method.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/path") + + # Test with simple cookies + headers = [ + "session-id=123456; Path=/", + "user-pref=dark-mode; Domain=.example.com", + "tracking=xyz789; Secure; HttpOnly", + ] + + jar.update_cookies_from_headers(headers, url) + + # Verify all cookies were added to the jar + assert len(jar) == 3 + + # Check cookies available for HTTP URL (secure cookie should be filtered out) + filtered_http: BaseCookie[str] = jar.filter_cookies(url) + assert len(filtered_http) == 2 + assert "session-id" in filtered_http + assert filtered_http["session-id"].value == "123456" + assert "user-pref" in filtered_http + assert filtered_http["user-pref"].value == "dark-mode" + assert "tracking" not in filtered_http # Secure cookie not available on HTTP + + # Check cookies available for HTTPS URL (all cookies should be available) + url_https: URL = URL("https://example.com/path") + filtered_https: BaseCookie[str] = jar.filter_cookies(url_https) + assert len(filtered_https) == 3 + assert "tracking" in filtered_https + assert filtered_https["tracking"].value == "xyz789" + + +async def test_update_cookies_from_headers_duplicate_names() -> None: + """Test that duplicate cookie names with different domains are preserved.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://www.example.com/") + + # Headers with duplicate names but different domains + headers: List[str] = [ + "session-id=123456; Domain=.example.com; Path=/", + "session-id=789012; Domain=.www.example.com; Path=/", + "user-pref=light; Domain=.example.com", + "user-pref=dark; Domain=sub.example.com", + ] + + jar.update_cookies_from_headers(headers, url) + + # Should have 3 cookies (user-pref=dark for sub.example.com is rejected) + assert len(jar) == 3 + + # Verify we have both session-id cookies + all_cookies: List[Morsel[str]] = list(jar) + session_ids: List[Morsel[str]] = [c for c in all_cookies if c.key == "session-id"] + assert len(session_ids) == 2 + + # Check their domains are different + domains: Set[str] = {c["domain"] for c in session_ids} + assert domains == {"example.com", "www.example.com"} + + +async def test_update_cookies_from_headers_invalid_cookies( + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that invalid cookies are logged and skipped.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Mix of valid and invalid cookies + headers: List[str] = [ + "valid-cookie=value123", + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", # This cookie with curly braces causes CookieError + "another-valid=value456", + ] + + # Enable logging for the client logger + with caplog.at_level(logging.WARNING, logger="aiohttp.client"): + jar.update_cookies_from_headers(headers, url) + + # Check that we logged warnings for invalid cookies + assert "Can not load response cookies" in caplog.text + + # Valid cookies should still be added + assert len(jar) >= 2 # At least the two clearly valid cookies + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "valid-cookie" in filtered + assert "another-valid" in filtered + + +async def test_update_cookies_from_headers_empty_list() -> None: + """Test that empty header list is handled gracefully.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Should not raise any errors + jar.update_cookies_from_headers([], url) + + assert len(jar) == 0 + + +async def test_update_cookies_from_headers_with_attributes() -> None: + """Test cookies with various attributes are handled correctly.""" + jar: CookieJar = CookieJar() + url: URL = URL("https://secure.example.com/app/page") + + headers: List[str] = [ + "secure-cookie=value1; Secure; HttpOnly; SameSite=Strict", + "expiring-cookie=value2; Max-Age=3600; Path=/app", + "domain-cookie=value3; Domain=.example.com; Path=/", + "dated-cookie=value4; Expires=Wed, 09 Jun 2030 10:18:14 GMT", + ] + + jar.update_cookies_from_headers(headers, url) + + # All cookies should be stored + assert len(jar) == 4 + + # Verify secure cookie (should work on HTTPS subdomain) + # Note: cookies without explicit path get path from URL (/app) + filtered_https_root: BaseCookie[str] = jar.filter_cookies( + URL("https://secure.example.com/") + ) + assert len(filtered_https_root) == 1 # Only domain-cookie has Path=/ + assert "domain-cookie" in filtered_https_root + + # Check app path + filtered_https_app: BaseCookie[str] = jar.filter_cookies( + URL("https://secure.example.com/app/") + ) + assert len(filtered_https_app) == 4 # All cookies match + assert "secure-cookie" in filtered_https_app + assert "expiring-cookie" in filtered_https_app + assert "domain-cookie" in filtered_https_app + assert "dated-cookie" in filtered_https_app + + # Secure cookie should not be available on HTTP + filtered_http_app: BaseCookie[str] = jar.filter_cookies( + URL("http://secure.example.com/app/") + ) + assert "secure-cookie" not in filtered_http_app + assert "expiring-cookie" in filtered_http_app # Non-secure cookies still available + assert "domain-cookie" in filtered_http_app + assert "dated-cookie" in filtered_http_app + + +async def test_update_cookies_from_headers_preserves_existing() -> None: + """Test that update_cookies_from_headers preserves existing cookies.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Add some initial cookies + jar.update_cookies( + { + "existing1": "value1", + "existing2": "value2", + }, + url, + ) + + # Add more cookies via headers + headers: List[str] = [ + "new-cookie1=value3", + "new-cookie2=value4", + ] + + jar.update_cookies_from_headers(headers, url) + + # Should have all 4 cookies + assert len(jar) == 4 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "existing1" in filtered + assert "existing2" in filtered + assert "new-cookie1" in filtered + assert "new-cookie2" in filtered + + +async def test_update_cookies_from_headers_overwrites_same_cookie() -> None: + """Test that cookies with same name/domain/path are overwritten.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Add initial cookie + jar.update_cookies({"session": "old-value"}, url) + + # Update with new value via headers + headers: List[str] = ["session=new-value"] + jar.update_cookies_from_headers(headers, url) + + # Should still have just 1 cookie with updated value + assert len(jar) == 1 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert filtered["session"].value == "new-value" + + +async def test_dummy_cookie_jar_update_cookies_from_headers() -> None: + """Test that DummyCookieJar ignores update_cookies_from_headers.""" + jar: DummyCookieJar = DummyCookieJar() + url: URL = URL("http://example.com/") + + headers: List[str] = [ + "cookie1=value1", + "cookie2=value2", + ] + + # Should not raise and should not store anything + jar.update_cookies_from_headers(headers, url) + + assert len(jar) == 0 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert len(filtered) == 0 From b1aa238220d0bd73bd6a91f065961e67cc1fb0b8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Jun 2025 09:48:49 -0500 Subject: [PATCH 03/11] [PR #11106/cfb9931 backport][3.12] Fix cookies with duplicate names being lost when updating cookie jar (#11108) --- CHANGES/11105.bugfix.rst | 10 ++ CHANGES/11106.bugfix.rst | 1 + CHANGES/4486.bugfix.rst | 1 + aiohttp/abc.py | 29 ++++- aiohttp/client.py | 7 +- aiohttp/client_reqrep.py | 29 +++-- docs/client_reference.rst | 13 ++ tests/test_client_functional.py | 146 +++++++++++++++++++++ tests/test_client_response.py | 144 ++++++++++++++++++++- tests/test_client_session.py | 63 ++++++++-- tests/test_cookiejar.py | 217 ++++++++++++++++++++++++++++++++ 11 files changed, 635 insertions(+), 25 deletions(-) create mode 100644 CHANGES/11105.bugfix.rst create mode 120000 CHANGES/11106.bugfix.rst create mode 120000 CHANGES/4486.bugfix.rst diff --git a/CHANGES/11105.bugfix.rst b/CHANGES/11105.bugfix.rst new file mode 100644 index 00000000000..33578aa7a95 --- /dev/null +++ b/CHANGES/11105.bugfix.rst @@ -0,0 +1,10 @@ +Fixed an issue where cookies with duplicate names but different domains or paths +were lost when updating the cookie jar. The :class:`~aiohttp.ClientSession` +cookie jar now correctly stores all cookies even if they have the same name but +different domain or path, following the :rfc:`6265#section-5.3` storage model -- by :user:`bdraco`. + +Note that :attr:`ClientResponse.cookies ` returns +a :class:`~http.cookies.SimpleCookie` which uses the cookie name as a key, so +only the last cookie with each name is accessible via this interface. All cookies +can be accessed via :meth:`ClientResponse.headers.getall('Set-Cookie') +` if needed. diff --git a/CHANGES/11106.bugfix.rst b/CHANGES/11106.bugfix.rst new file mode 120000 index 00000000000..3e5efb0f3f3 --- /dev/null +++ b/CHANGES/11106.bugfix.rst @@ -0,0 +1 @@ +11105.bugfix.rst \ No newline at end of file diff --git a/CHANGES/4486.bugfix.rst b/CHANGES/4486.bugfix.rst new file mode 120000 index 00000000000..3e5efb0f3f3 --- /dev/null +++ b/CHANGES/4486.bugfix.rst @@ -0,0 +1 @@ +11105.bugfix.rst \ No newline at end of file diff --git a/aiohttp/abc.py b/aiohttp/abc.py index c1bf5032d0d..353c18be266 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -3,7 +3,7 @@ import socket from abc import ABC, abstractmethod from collections.abc import Sized -from http.cookies import BaseCookie, Morsel +from http.cookies import BaseCookie, CookieError, Morsel, SimpleCookie from typing import ( TYPE_CHECKING, Any, @@ -14,6 +14,7 @@ Iterable, List, Optional, + Sequence, Tuple, TypedDict, Union, @@ -22,6 +23,7 @@ from multidict import CIMultiDict from yarl import URL +from .log import client_logger from .typedefs import LooseCookies if TYPE_CHECKING: @@ -192,6 +194,31 @@ def clear_domain(self, domain: str) -> None: def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> None: """Update cookies.""" + def update_cookies_from_headers( + self, headers: Sequence[str], response_url: URL + ) -> None: + """ + Update cookies from raw Set-Cookie headers. + + Default implementation parses each header separately to preserve + cookies with same name but different domain/path. + """ + # Default implementation for backward compatibility + cookies_to_update: List[Tuple[str, Morsel[str]]] = [] + for cookie_header in headers: + tmp_cookie = SimpleCookie() + try: + tmp_cookie.load(cookie_header) + # Collect all cookies as tuples (name, morsel) + for name, morsel in tmp_cookie.items(): + cookies_to_update.append((name, morsel)) + except CookieError as exc: + client_logger.warning("Can not load response cookies: %s", exc) + + # Update all cookies at once for efficiency + if cookies_to_update: + self.update_cookies(cookies_to_update, response_url) + @abstractmethod def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": """Return the jar's cookies filtered by their attributes.""" diff --git a/aiohttp/client.py b/aiohttp/client.py index 6457248d5ea..576a965ba5d 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -779,8 +779,11 @@ async def _connect_and_send_request( raise raise ClientOSError(*exc.args) from exc - if cookies := resp._cookies: - self._cookie_jar.update_cookies(cookies, resp.url) + # Update cookies from raw headers to preserve duplicates + if resp._raw_cookie_headers: + self._cookie_jar.update_cookies_from_headers( + resp._raw_cookie_headers, resp.url + ) # redirects if resp.status in (301, 302, 303, 307, 308) and allow_redirects: diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index e437ef67aff..01835260cc5 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -291,6 +291,7 @@ class ClientResponse(HeadersMixin): _connection: Optional["Connection"] = None # current connection _cookies: Optional[SimpleCookie] = None + _raw_cookie_headers: Optional[Tuple[str, ...]] = None _continue: Optional["asyncio.Future[bool]"] = None _source_traceback: Optional[traceback.StackSummary] = None _session: Optional["ClientSession"] = None @@ -372,12 +373,29 @@ def _writer(self, writer: Optional["asyncio.Task[None]"]) -> None: @property def cookies(self) -> SimpleCookie: if self._cookies is None: - self._cookies = SimpleCookie() + if self._raw_cookie_headers is not None: + # Parse cookies for response.cookies (SimpleCookie for backward compatibility) + cookies = SimpleCookie() + for hdr in self._raw_cookie_headers: + try: + cookies.load(hdr) + except CookieError as exc: + client_logger.warning("Can not load response cookies: %s", exc) + self._cookies = cookies + else: + self._cookies = SimpleCookie() return self._cookies @cookies.setter def cookies(self, cookies: SimpleCookie) -> None: self._cookies = cookies + # Generate raw cookie headers from the SimpleCookie + if cookies: + self._raw_cookie_headers = tuple( + morsel.OutputString() for morsel in cookies.values() + ) + else: + self._raw_cookie_headers = None @reify def url(self) -> URL: @@ -543,13 +561,8 @@ async def start(self, connection: "Connection") -> "ClientResponse": # cookies if cookie_hdrs := self.headers.getall(hdrs.SET_COOKIE, ()): - cookies = SimpleCookie() - for hdr in cookie_hdrs: - try: - cookies.load(hdr) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) - self._cookies = cookies + # Store raw cookie headers for CookieJar + self._raw_cookie_headers = tuple(cookie_hdrs) return self def _response_eof(self) -> None: diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 07839686039..8a721f514cd 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -1499,6 +1499,19 @@ Response object HTTP cookies of response (*Set-Cookie* HTTP header, :class:`~http.cookies.SimpleCookie`). + .. note:: + + Since :class:`~http.cookies.SimpleCookie` uses cookie name as the + key, cookies with the same name but different domains or paths will + be overwritten. Only the last cookie with a given name will be + accessible via this attribute. + + To access all cookies, including duplicates with the same name, + use :meth:`response.headers.getall('Set-Cookie') `. + + The session's cookie jar will correctly store all cookies, even if + they are not accessible via this attribute. + .. attribute:: headers A case-insensitive multidict proxy with HTTP headers of diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 1d91956c4a3..ca1a7dd1d6b 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2712,6 +2712,7 @@ async def handler(request): async with client.get("/") as resp: assert 200 == resp.status cookie_names = {c.key for c in client.session.cookie_jar} + _ = resp.cookies assert cookie_names == {"c1", "c2"} m_log.warning.assert_called_with("Can not load response cookies: %s", mock.ANY) @@ -5111,3 +5112,148 @@ async def redirect_handler(request: web.Request) -> web.Response: assert ( payload.close_called ), "Payload.close() was not called when InvalidUrlRedirectClientError (invalid origin) was raised" + + +async def test_amazon_like_cookie_scenario(aiohttp_client: AiohttpClient) -> None: + """Test real-world cookie scenario similar to Amazon.""" + + class FakeResolver(AbstractResolver): + def __init__(self, port: int): + self._port = port + + async def resolve( + self, host: str, port: int = 0, family: int = 0 + ) -> List[ResolveResult]: + if host in ("amazon.it", "www.amazon.it"): + return [ + { + "hostname": host, + "host": "127.0.0.1", + "port": self._port, + "family": socket.AF_INET, + "proto": 0, + "flags": 0, + } + ] + assert False, f"Unexpected host: {host}" + + async def close(self) -> None: + """Close the resolver if needed.""" + + async def handler(request: web.Request) -> web.Response: + response = web.Response(text="Login successful") + + # Simulate Amazon-like cookies from the issue + cookies = [ + "session-id=146-7423990-7621939; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; " + "Secure; HttpOnly", + "session-id=147-8529641-8642103; Domain=.www.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; HttpOnly", + "session-id-time=2082758401l; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; Secure", + "session-id-time=2082758402l; Domain=.www.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/", + "ubid-acbit=257-7531983-5395266; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; Secure", + 'x-acbit="KdvJzu8W@Fx6Jj3EuNFLuP0N7OtkuCfs"; Version=1; ' + "Domain=.amazon.it; Path=/; Secure; HttpOnly", + "at-acbit=Atza|IwEBIM-gLr8; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; " + "Secure; HttpOnly", + 'sess-at-acbit="4+6VzSJPHIFD/OqO264hFxIng8Y="; ' + "Domain=.amazon.it; Expires=Mon, 31-May-2027 10:00:00 GMT; " + "Path=/; Secure; HttpOnly", + "lc-acbit=it_IT; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/", + "i18n-prefs=EUR; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/", + "av-profile=null; Domain=.amazon.it; " + "Expires=Mon, 31-May-2027 10:00:00 GMT; Path=/; Secure", + 'user-pref-token="Am81ywsJ69xObBnuJ2FbilVH0mg="; ' + "Domain=.amazon.it; Path=/; Secure", + ] + + for cookie in cookies: + response.headers.add("Set-Cookie", cookie) + + return response + + app = web.Application() + app.router.add_get("/", handler) + + # Get the test server + server = await aiohttp_client(app) + port = server.port + + # Create a new client session with our fake resolver + resolver = FakeResolver(port) + + async with ( + aiohttp.TCPConnector(resolver=resolver, force_close=True) as connector, + aiohttp.ClientSession(connector=connector) as session, + ): + # Make request to www.amazon.it which will resolve to + # 127.0.0.1:port. This allows cookies for both .amazon.it + # and .www.amazon.it domains + resp = await session.get(f"http://www.amazon.it:{port}/") + + # Check headers + cookie_headers = resp.headers.getall("Set-Cookie") + assert ( + len(cookie_headers) == 12 + ), f"Expected 12 headers, got {len(cookie_headers)}" + + # Check parsed cookies - SimpleCookie only keeps the last + # cookie with each name. So we expect 10 unique cookie names + # (not 12) + expected_cookie_names = { + "session-id", # Will only have one + "session-id-time", # Will only have one + "ubid-acbit", + "x-acbit", + "at-acbit", + "sess-at-acbit", + "lc-acbit", + "i18n-prefs", + "av-profile", + "user-pref-token", + } + assert set(resp.cookies.keys()) == expected_cookie_names + assert ( + len(resp.cookies) == 10 + ), f"Expected 10 cookies in SimpleCookie, got {len(resp.cookies)}" + + # The important part: verify the session's cookie jar has + # all cookies. The cookie jar should have all 12 cookies, + # not just 10 + jar_cookies = list(session.cookie_jar) + assert ( + len(jar_cookies) == 12 + ), f"Expected 12 cookies in jar, got {len(jar_cookies)}" + + # Verify we have both session-id cookies with different domains + session_ids = [c for c in jar_cookies if c.key == "session-id"] + assert ( + len(session_ids) == 2 + ), f"Expected 2 session-id cookies, got {len(session_ids)}" + + # Verify the domains are different + session_id_domains = {c["domain"] for c in session_ids} + assert session_id_domains == { + "amazon.it", + "www.amazon.it", + }, f"Got domains: {session_id_domains}" + + # Verify we have both session-id-time cookies with different + # domains + session_id_times = [c for c in jar_cookies if c.key == "session-id-time"] + assert ( + len(session_id_times) == 2 + ), f"Expected 2 session-id-time cookies, got {len(session_id_times)}" + + # Now test that the raw headers were properly preserved + assert resp._raw_cookie_headers is not None + assert ( + len(resp._raw_cookie_headers) == 12 + ), "All raw headers should be preserved" diff --git a/tests/test_client_response.py b/tests/test_client_response.py index 4a8000962d1..2d70feaf06d 100644 --- a/tests/test_client_response.py +++ b/tests/test_client_response.py @@ -3,6 +3,7 @@ import asyncio import gc import sys +from http.cookies import SimpleCookie from typing import Callable from unittest import mock @@ -11,7 +12,7 @@ from yarl import URL import aiohttp -from aiohttp import ClientSession, http +from aiohttp import ClientSession, hdrs, http from aiohttp.client_reqrep import ClientResponse, RequestInfo from aiohttp.helpers import TimerNoop @@ -1333,3 +1334,144 @@ def test_response_not_closed_after_get_ok(mocker) -> None: assert not response.ok assert not response.closed assert spy.call_count == 0 + + +def test_response_duplicate_cookie_names( + loop: asyncio.AbstractEventLoop, session: ClientSession +) -> None: + """ + Test that response.cookies handles duplicate cookie names correctly. + + Note: This behavior (losing cookies with same name but different domains/paths) + is arguably undesirable, but we promise to return a SimpleCookie object, and + SimpleCookie uses cookie name as the key. This is documented behavior. + + To access all cookies including duplicates, users should use: + - response.headers.getall('Set-Cookie') for raw headers + - The session's cookie jar correctly stores all cookies + """ + response = ClientResponse( + "get", + URL("http://example.com"), + request_info=mock.Mock(), + writer=WriterMock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + + # Set headers with duplicate cookie names but different domains + headers = CIMultiDict( + [ + ( + "Set-Cookie", + "session-id=123-4567890; Domain=.example.com; Path=/; Secure", + ), + ("Set-Cookie", "session-id=098-7654321; Domain=.www.example.com; Path=/"), + ("Set-Cookie", "user-pref=dark; Domain=.example.com; Path=/"), + ("Set-Cookie", "user-pref=light; Domain=api.example.com; Path=/"), + ] + ) + response._headers = CIMultiDictProxy(headers) + # Set raw cookie headers as done in ClientResponse.start() + response._raw_cookie_headers = tuple(headers.getall("Set-Cookie", [])) + + # SimpleCookie only keeps the last cookie with each name + # This is expected behavior since SimpleCookie uses name as the key + assert len(response.cookies) == 2 # Only 'session-id' and 'user-pref' + assert response.cookies["session-id"].value == "098-7654321" # Last one wins + assert response.cookies["user-pref"].value == "light" # Last one wins + + +def test_response_raw_cookie_headers_preserved( + loop: asyncio.AbstractEventLoop, session: ClientSession +) -> None: + """Test that raw Set-Cookie headers are preserved in _raw_cookie_headers.""" + response = ClientResponse( + "get", + URL("http://example.com"), + request_info=mock.Mock(), + writer=WriterMock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + + # Set headers with multiple cookies + cookie_headers = [ + "session-id=123; Domain=.example.com; Path=/; Secure", + "session-id=456; Domain=.www.example.com; Path=/", + "tracking=xyz; Domain=.example.com; Path=/; HttpOnly", + ] + + headers: CIMultiDict[str] = CIMultiDict() + for cookie_hdr in cookie_headers: + headers.add("Set-Cookie", cookie_hdr) + + response._headers = CIMultiDictProxy(headers) + + # Set raw cookie headers as done in ClientResponse.start() + response._raw_cookie_headers = tuple(response.headers.getall(hdrs.SET_COOKIE, [])) + + # Verify raw headers are preserved + assert response._raw_cookie_headers == tuple(cookie_headers) + assert len(response._raw_cookie_headers) == 3 + + # But SimpleCookie only has unique names + assert len(response.cookies) == 2 # 'session-id' and 'tracking' + + +def test_response_cookies_setter_updates_raw_headers( + loop: asyncio.AbstractEventLoop, session: ClientSession +) -> None: + """Test that setting cookies property updates _raw_cookie_headers.""" + response = ClientResponse( + "get", + URL("http://example.com"), + request_info=mock.Mock(), + writer=WriterMock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + + # Create a SimpleCookie with some cookies + cookies = SimpleCookie() + cookies["session-id"] = "123456" + cookies["session-id"]["domain"] = ".example.com" + cookies["session-id"]["path"] = "/" + cookies["session-id"]["secure"] = True + + cookies["tracking"] = "xyz789" + cookies["tracking"]["domain"] = ".example.com" + cookies["tracking"]["httponly"] = True + + # Set the cookies property + response.cookies = cookies + + # Verify _raw_cookie_headers was updated + assert response._raw_cookie_headers is not None + assert len(response._raw_cookie_headers) == 2 + assert isinstance(response._raw_cookie_headers, tuple) + + # Check the raw headers contain the expected cookie strings + raw_headers = list(response._raw_cookie_headers) + assert any("session-id=123456" in h for h in raw_headers) + assert any("tracking=xyz789" in h for h in raw_headers) + assert any("Secure" in h for h in raw_headers) + assert any("HttpOnly" in h for h in raw_headers) + + # Verify cookies property returns the same object + assert response.cookies is cookies + + # Test setting empty cookies + empty_cookies = SimpleCookie() + response.cookies = empty_cookies + # Should not set _raw_cookie_headers for empty cookies + assert response._raw_cookie_headers is None diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 0fdfaee6761..2702350f132 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -4,8 +4,8 @@ import io import json from collections import deque -from http.cookies import SimpleCookie -from typing import Any, Awaitable, Callable, List +from http.cookies import BaseCookie, SimpleCookie +from typing import Any, Awaitable, Callable, Iterator, List, Optional, cast from unittest import mock from uuid import uuid4 @@ -15,7 +15,7 @@ from yarl import URL import aiohttp -from aiohttp import CookieJar, client, hdrs, web +from aiohttp import CookieJar, abc, client, hdrs, web from aiohttp.client import ClientSession from aiohttp.client_proto import ResponseHandler from aiohttp.client_reqrep import ClientRequest @@ -639,8 +639,43 @@ async def create_connection( async def test_cookie_jar_usage(loop: Any, aiohttp_client: Any) -> None: req_url = None - jar = mock.Mock() - jar.filter_cookies.return_value = None + class MockCookieJar(abc.AbstractCookieJar): + def __init__(self) -> None: + self._update_cookies_mock = mock.Mock() + self._filter_cookies_mock = mock.Mock(return_value=BaseCookie()) + self._clear_mock = mock.Mock() + self._clear_domain_mock = mock.Mock() + self._items: List[Any] = [] + + @property + def quote_cookie(self) -> bool: + return True + + def clear(self, predicate: Optional[abc.ClearCookiePredicate] = None) -> None: + self._clear_mock(predicate) + + def clear_domain(self, domain: str) -> None: + self._clear_domain_mock(domain) + + def update_cookies(self, cookies: Any, response_url: URL = URL()) -> None: + self._update_cookies_mock(cookies, response_url) + + def filter_cookies(self, request_url: URL) -> BaseCookie[str]: + return cast(BaseCookie[str], self._filter_cookies_mock(request_url)) + + def __len__(self) -> int: + return len(self._items) + + def __iter__(self) -> Iterator[Any]: + return iter(self._items) + + jar = MockCookieJar() + + assert jar.quote_cookie is True + assert len(jar) == 0 + assert list(jar) == [] + jar.clear() + jar.clear_domain("example.com") async def handler(request): nonlocal req_url @@ -657,22 +692,24 @@ async def handler(request): ) # Updating the cookie jar with initial user defined cookies - jar.update_cookies.assert_called_with({"request": "req_value"}) + jar._update_cookies_mock.assert_called_with({"request": "req_value"}, URL()) - jar.update_cookies.reset_mock() + jar._update_cookies_mock.reset_mock() resp = await session.get("/") await resp.release() # Filtering the cookie jar before sending the request, # getting the request URL as only parameter - jar.filter_cookies.assert_called_with(URL(req_url)) + jar._filter_cookies_mock.assert_called_with(URL(req_url)) # Updating the cookie jar with the response cookies - assert jar.update_cookies.called - resp_cookies = jar.update_cookies.call_args[0][0] - assert isinstance(resp_cookies, SimpleCookie) - assert "response" in resp_cookies - assert resp_cookies["response"].value == "resp_value" + assert jar._update_cookies_mock.called + resp_cookies = jar._update_cookies_mock.call_args[0][0] + # Now update_cookies is called with a list of tuples + assert isinstance(resp_cookies, list) + assert len(resp_cookies) == 1 + assert resp_cookies[0][0] == "response" + assert resp_cookies[0][1].value == "resp_value" async def test_cookies_with_not_quoted_cookie_jar( diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 26efaa30d04..e1b6e351e3d 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -2,11 +2,13 @@ import datetime import heapq import itertools +import logging import pathlib import pickle import unittest from http.cookies import BaseCookie, Morsel, SimpleCookie from operator import not_ +from typing import List, Set from unittest import mock import pytest @@ -1181,3 +1183,218 @@ async def test_filter_cookies_does_not_leak_memory() -> None: for key, morsels in jar._morsel_cache.items(): assert key in jar._cookies, f"Orphaned morsel cache entry for {key}" assert len(morsels) > 0, f"Empty morsel cache entry found for {key}" + + +async def test_update_cookies_from_headers() -> None: + """Test update_cookies_from_headers method.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/path") + + # Test with simple cookies + headers = [ + "session-id=123456; Path=/", + "user-pref=dark-mode; Domain=.example.com", + "tracking=xyz789; Secure; HttpOnly", + ] + + jar.update_cookies_from_headers(headers, url) + + # Verify all cookies were added to the jar + assert len(jar) == 3 + + # Check cookies available for HTTP URL (secure cookie should be filtered out) + filtered_http: BaseCookie[str] = jar.filter_cookies(url) + assert len(filtered_http) == 2 + assert "session-id" in filtered_http + assert filtered_http["session-id"].value == "123456" + assert "user-pref" in filtered_http + assert filtered_http["user-pref"].value == "dark-mode" + assert "tracking" not in filtered_http # Secure cookie not available on HTTP + + # Check cookies available for HTTPS URL (all cookies should be available) + url_https: URL = URL("https://example.com/path") + filtered_https: BaseCookie[str] = jar.filter_cookies(url_https) + assert len(filtered_https) == 3 + assert "tracking" in filtered_https + assert filtered_https["tracking"].value == "xyz789" + + +async def test_update_cookies_from_headers_duplicate_names() -> None: + """Test that duplicate cookie names with different domains are preserved.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://www.example.com/") + + # Headers with duplicate names but different domains + headers: List[str] = [ + "session-id=123456; Domain=.example.com; Path=/", + "session-id=789012; Domain=.www.example.com; Path=/", + "user-pref=light; Domain=.example.com", + "user-pref=dark; Domain=sub.example.com", + ] + + jar.update_cookies_from_headers(headers, url) + + # Should have 3 cookies (user-pref=dark for sub.example.com is rejected) + assert len(jar) == 3 + + # Verify we have both session-id cookies + all_cookies: List[Morsel[str]] = list(jar) + session_ids: List[Morsel[str]] = [c for c in all_cookies if c.key == "session-id"] + assert len(session_ids) == 2 + + # Check their domains are different + domains: Set[str] = {c["domain"] for c in session_ids} + assert domains == {"example.com", "www.example.com"} + + +async def test_update_cookies_from_headers_invalid_cookies( + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that invalid cookies are logged and skipped.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Mix of valid and invalid cookies + headers: List[str] = [ + "valid-cookie=value123", + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", # This cookie with curly braces causes CookieError + "another-valid=value456", + ] + + # Enable logging for the client logger + with caplog.at_level(logging.WARNING, logger="aiohttp.client"): + jar.update_cookies_from_headers(headers, url) + + # Check that we logged warnings for invalid cookies + assert "Can not load response cookies" in caplog.text + + # Valid cookies should still be added + assert len(jar) >= 2 # At least the two clearly valid cookies + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "valid-cookie" in filtered + assert "another-valid" in filtered + + +async def test_update_cookies_from_headers_empty_list() -> None: + """Test that empty header list is handled gracefully.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Should not raise any errors + jar.update_cookies_from_headers([], url) + + assert len(jar) == 0 + + +async def test_update_cookies_from_headers_with_attributes() -> None: + """Test cookies with various attributes are handled correctly.""" + jar: CookieJar = CookieJar() + url: URL = URL("https://secure.example.com/app/page") + + headers: List[str] = [ + "secure-cookie=value1; Secure; HttpOnly; SameSite=Strict", + "expiring-cookie=value2; Max-Age=3600; Path=/app", + "domain-cookie=value3; Domain=.example.com; Path=/", + "dated-cookie=value4; Expires=Wed, 09 Jun 2030 10:18:14 GMT", + ] + + jar.update_cookies_from_headers(headers, url) + + # All cookies should be stored + assert len(jar) == 4 + + # Verify secure cookie (should work on HTTPS subdomain) + # Note: cookies without explicit path get path from URL (/app) + filtered_https_root: BaseCookie[str] = jar.filter_cookies( + URL("https://secure.example.com/") + ) + assert len(filtered_https_root) == 1 # Only domain-cookie has Path=/ + assert "domain-cookie" in filtered_https_root + + # Check app path + filtered_https_app: BaseCookie[str] = jar.filter_cookies( + URL("https://secure.example.com/app/") + ) + assert len(filtered_https_app) == 4 # All cookies match + assert "secure-cookie" in filtered_https_app + assert "expiring-cookie" in filtered_https_app + assert "domain-cookie" in filtered_https_app + assert "dated-cookie" in filtered_https_app + + # Secure cookie should not be available on HTTP + filtered_http_app: BaseCookie[str] = jar.filter_cookies( + URL("http://secure.example.com/app/") + ) + assert "secure-cookie" not in filtered_http_app + assert "expiring-cookie" in filtered_http_app # Non-secure cookies still available + assert "domain-cookie" in filtered_http_app + assert "dated-cookie" in filtered_http_app + + +async def test_update_cookies_from_headers_preserves_existing() -> None: + """Test that update_cookies_from_headers preserves existing cookies.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Add some initial cookies + jar.update_cookies( + { + "existing1": "value1", + "existing2": "value2", + }, + url, + ) + + # Add more cookies via headers + headers: List[str] = [ + "new-cookie1=value3", + "new-cookie2=value4", + ] + + jar.update_cookies_from_headers(headers, url) + + # Should have all 4 cookies + assert len(jar) == 4 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "existing1" in filtered + assert "existing2" in filtered + assert "new-cookie1" in filtered + assert "new-cookie2" in filtered + + +async def test_update_cookies_from_headers_overwrites_same_cookie() -> None: + """Test that cookies with same name/domain/path are overwritten.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Add initial cookie + jar.update_cookies({"session": "old-value"}, url) + + # Update with new value via headers + headers: List[str] = ["session=new-value"] + jar.update_cookies_from_headers(headers, url) + + # Should still have just 1 cookie with updated value + assert len(jar) == 1 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert filtered["session"].value == "new-value" + + +async def test_dummy_cookie_jar_update_cookies_from_headers() -> None: + """Test that DummyCookieJar ignores update_cookies_from_headers.""" + jar: DummyCookieJar = DummyCookieJar() + url: URL = URL("http://example.com/") + + headers: List[str] = [ + "cookie1=value1", + "cookie2=value2", + ] + + # Should not raise and should not store anything + jar.update_cookies_from_headers(headers, url) + + assert len(jar) == 0 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert len(filtered) == 0 From fa82b1ed1986c349dcabfc988f0e7ed611d7ba57 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Jun 2025 12:04:57 -0500 Subject: [PATCH 04/11] [PR #11107/21d640d backport][3.13] Avoid creating closed futures that will never be awaited (#11111) --- CHANGES/11107.misc.rst | 1 + aiohttp/client_proto.py | 59 ++++++++++++++++++++++++++------------ aiohttp/connector.py | 6 ++-- tests/test_client_proto.py | 41 ++++++++++++++++++++++++-- tests/test_connector.py | 22 ++++++++++++++ 5 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 CHANGES/11107.misc.rst diff --git a/CHANGES/11107.misc.rst b/CHANGES/11107.misc.rst new file mode 100644 index 00000000000..37ac4622bd9 --- /dev/null +++ b/CHANGES/11107.misc.rst @@ -0,0 +1 @@ +Avoided creating closed futures in ``ResponseHandler`` that will never be awaited -- by :user:`bdraco`. diff --git a/aiohttp/client_proto.py b/aiohttp/client_proto.py index 2d8c2e578c4..7d00b366a79 100644 --- a/aiohttp/client_proto.py +++ b/aiohttp/client_proto.py @@ -1,6 +1,6 @@ import asyncio from contextlib import suppress -from typing import Any, Optional, Tuple +from typing import Any, Optional, Tuple, Union from .base_protocol import BaseProtocol from .client_exceptions import ( @@ -45,7 +45,27 @@ def __init__(self, loop: asyncio.AbstractEventLoop) -> None: self._read_timeout_handle: Optional[asyncio.TimerHandle] = None self._timeout_ceil_threshold: Optional[float] = 5 - self.closed: asyncio.Future[None] = self._loop.create_future() + + self._closed: Union[None, asyncio.Future[None]] = None + self._connection_lost_called = False + + @property + def closed(self) -> Union[None, asyncio.Future[None]]: + """Future that is set when the connection is closed. + + This property returns a Future that will be completed when the connection + is closed. The Future is created lazily on first access to avoid creating + futures that will never be awaited. + + Returns: + - A Future[None] if the connection is still open or was closed after + this property was accessed + - None if connection_lost() was already called before this property + was ever accessed (indicating no one is waiting for the closure) + """ + if self._closed is None and not self._connection_lost_called: + self._closed = self._loop.create_future() + return self._closed @property def upgraded(self) -> bool: @@ -79,6 +99,7 @@ def is_connected(self) -> bool: return self.transport is not None and not self.transport.is_closing() def connection_lost(self, exc: Optional[BaseException]) -> None: + self._connection_lost_called = True self._drop_timeout() original_connection_error = exc @@ -86,23 +107,23 @@ def connection_lost(self, exc: Optional[BaseException]) -> None: connection_closed_cleanly = original_connection_error is None - if connection_closed_cleanly: - set_result(self.closed, None) - else: - assert original_connection_error is not None - set_exception( - self.closed, - ClientConnectionError( - f"Connection lost: {original_connection_error !s}", - ), - original_connection_error, - ) - # Mark the exception as retrieved to prevent - # "Future exception was never retrieved" warnings - # The exception is always passed on through - # other means, so this is safe - with suppress(Exception): - self.closed.exception() + if self._closed is not None: + # If someone is waiting for the closed future, + # we should set it to None or an exception. If + # self._closed is None, it means that + # connection_lost() was called already + # or nobody is waiting for it. + if connection_closed_cleanly: + set_result(self._closed, None) + else: + assert original_connection_error is not None + set_exception( + self._closed, + ClientConnectionError( + f"Connection lost: {original_connection_error !s}", + ), + original_connection_error, + ) if self._payload_parser is not None: with suppress(Exception): # FIXME: log this somehow? diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 6fa75d31a98..11bd36c487e 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -507,11 +507,13 @@ def _close(self) -> List[Awaitable[object]]: for data in self._conns.values(): for proto, _ in data: proto.close() - waiters.append(proto.closed) + if closed := proto.closed: + waiters.append(closed) for proto in self._acquired: proto.close() - waiters.append(proto.closed) + if closed := proto.closed: + waiters.append(closed) for transport in self._cleanup_closed_transports: if transport is not None: diff --git a/tests/test_client_proto.py b/tests/test_client_proto.py index c7fb79a5f44..2a42996950f 100644 --- a/tests/test_client_proto.py +++ b/tests/test_client_proto.py @@ -256,13 +256,50 @@ async def test_connection_lost_exception_is_marked_retrieved( proto = ResponseHandler(loop=loop) proto.connection_made(mock.Mock()) + # Access closed property before connection_lost to ensure future is created + closed_future = proto.closed + assert closed_future is not None + # Simulate an SSL shutdown timeout error ssl_error = TimeoutError("SSL shutdown timed out") proto.connection_lost(ssl_error) # Verify the exception was set on the closed future - assert proto.closed.done() - exc = proto.closed.exception() + assert closed_future.done() + exc = closed_future.exception() assert exc is not None assert "Connection lost: SSL shutdown timed out" in str(exc) assert exc.__cause__ is ssl_error + + +async def test_closed_property_lazy_creation( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that closed future is created lazily.""" + proto = ResponseHandler(loop=loop) + + # Initially, the closed future should not be created + assert proto._closed is None + + # Accessing the property should create the future + closed_future = proto.closed + assert closed_future is not None + assert isinstance(closed_future, asyncio.Future) + assert not closed_future.done() + + # Subsequent access should return the same future + assert proto.closed is closed_future + + +async def test_closed_property_after_connection_lost( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that closed property returns None after connection_lost if never accessed.""" + proto = ResponseHandler(loop=loop) + proto.connection_made(mock.Mock()) + + # Don't access proto.closed before connection_lost + proto.connection_lost(None) + + # After connection_lost, closed should return None if it was never accessed + assert proto.closed is None diff --git a/tests/test_connector.py b/tests/test_connector.py index 3b2d28ea46c..6fad9e4ccff 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -331,6 +331,28 @@ async def test_close_with_exception_during_closing( assert "Error while closing connector" in caplog.records[0].message assert "RuntimeError('Connection close failed')" in caplog.records[0].message + +async def test_close_with_proto_closed_none(key: ConnectionKey) -> None: + """Test close when protocol.closed is None.""" + # Create protocols where closed property returns None + proto1 = mock.create_autospec(ResponseHandler, instance=True) + proto1.closed = None + proto1.close = mock.Mock() + + proto2 = mock.create_autospec(ResponseHandler, instance=True) + proto2.closed = None + proto2.close = mock.Mock() + + conn = aiohttp.BaseConnector() + conn._conns[key] = deque([(proto1, 0)]) + conn._acquired.add(proto2) + + # Close the connector - this should handle the case where proto.closed is None + await conn.close() + + # Verify close was called on both protocols + assert proto1.close.called + assert proto2.close.called assert conn.closed From 06887a918ee88395082cb5ea53adccaf3e30aa00 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Jun 2025 12:05:10 -0500 Subject: [PATCH 05/11] [PR #11107/21d640d backport][3.12] Avoid creating closed futures that will never be awaited (#11110) --- CHANGES/11107.misc.rst | 1 + aiohttp/client_proto.py | 59 ++++++++++++++++++++++++++------------ aiohttp/connector.py | 6 ++-- tests/test_client_proto.py | 41 ++++++++++++++++++++++++-- tests/test_connector.py | 22 ++++++++++++++ 5 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 CHANGES/11107.misc.rst diff --git a/CHANGES/11107.misc.rst b/CHANGES/11107.misc.rst new file mode 100644 index 00000000000..37ac4622bd9 --- /dev/null +++ b/CHANGES/11107.misc.rst @@ -0,0 +1 @@ +Avoided creating closed futures in ``ResponseHandler`` that will never be awaited -- by :user:`bdraco`. diff --git a/aiohttp/client_proto.py b/aiohttp/client_proto.py index 2d8c2e578c4..7d00b366a79 100644 --- a/aiohttp/client_proto.py +++ b/aiohttp/client_proto.py @@ -1,6 +1,6 @@ import asyncio from contextlib import suppress -from typing import Any, Optional, Tuple +from typing import Any, Optional, Tuple, Union from .base_protocol import BaseProtocol from .client_exceptions import ( @@ -45,7 +45,27 @@ def __init__(self, loop: asyncio.AbstractEventLoop) -> None: self._read_timeout_handle: Optional[asyncio.TimerHandle] = None self._timeout_ceil_threshold: Optional[float] = 5 - self.closed: asyncio.Future[None] = self._loop.create_future() + + self._closed: Union[None, asyncio.Future[None]] = None + self._connection_lost_called = False + + @property + def closed(self) -> Union[None, asyncio.Future[None]]: + """Future that is set when the connection is closed. + + This property returns a Future that will be completed when the connection + is closed. The Future is created lazily on first access to avoid creating + futures that will never be awaited. + + Returns: + - A Future[None] if the connection is still open or was closed after + this property was accessed + - None if connection_lost() was already called before this property + was ever accessed (indicating no one is waiting for the closure) + """ + if self._closed is None and not self._connection_lost_called: + self._closed = self._loop.create_future() + return self._closed @property def upgraded(self) -> bool: @@ -79,6 +99,7 @@ def is_connected(self) -> bool: return self.transport is not None and not self.transport.is_closing() def connection_lost(self, exc: Optional[BaseException]) -> None: + self._connection_lost_called = True self._drop_timeout() original_connection_error = exc @@ -86,23 +107,23 @@ def connection_lost(self, exc: Optional[BaseException]) -> None: connection_closed_cleanly = original_connection_error is None - if connection_closed_cleanly: - set_result(self.closed, None) - else: - assert original_connection_error is not None - set_exception( - self.closed, - ClientConnectionError( - f"Connection lost: {original_connection_error !s}", - ), - original_connection_error, - ) - # Mark the exception as retrieved to prevent - # "Future exception was never retrieved" warnings - # The exception is always passed on through - # other means, so this is safe - with suppress(Exception): - self.closed.exception() + if self._closed is not None: + # If someone is waiting for the closed future, + # we should set it to None or an exception. If + # self._closed is None, it means that + # connection_lost() was called already + # or nobody is waiting for it. + if connection_closed_cleanly: + set_result(self._closed, None) + else: + assert original_connection_error is not None + set_exception( + self._closed, + ClientConnectionError( + f"Connection lost: {original_connection_error !s}", + ), + original_connection_error, + ) if self._payload_parser is not None: with suppress(Exception): # FIXME: log this somehow? diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 6fa75d31a98..11bd36c487e 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -507,11 +507,13 @@ def _close(self) -> List[Awaitable[object]]: for data in self._conns.values(): for proto, _ in data: proto.close() - waiters.append(proto.closed) + if closed := proto.closed: + waiters.append(closed) for proto in self._acquired: proto.close() - waiters.append(proto.closed) + if closed := proto.closed: + waiters.append(closed) for transport in self._cleanup_closed_transports: if transport is not None: diff --git a/tests/test_client_proto.py b/tests/test_client_proto.py index c7fb79a5f44..2a42996950f 100644 --- a/tests/test_client_proto.py +++ b/tests/test_client_proto.py @@ -256,13 +256,50 @@ async def test_connection_lost_exception_is_marked_retrieved( proto = ResponseHandler(loop=loop) proto.connection_made(mock.Mock()) + # Access closed property before connection_lost to ensure future is created + closed_future = proto.closed + assert closed_future is not None + # Simulate an SSL shutdown timeout error ssl_error = TimeoutError("SSL shutdown timed out") proto.connection_lost(ssl_error) # Verify the exception was set on the closed future - assert proto.closed.done() - exc = proto.closed.exception() + assert closed_future.done() + exc = closed_future.exception() assert exc is not None assert "Connection lost: SSL shutdown timed out" in str(exc) assert exc.__cause__ is ssl_error + + +async def test_closed_property_lazy_creation( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that closed future is created lazily.""" + proto = ResponseHandler(loop=loop) + + # Initially, the closed future should not be created + assert proto._closed is None + + # Accessing the property should create the future + closed_future = proto.closed + assert closed_future is not None + assert isinstance(closed_future, asyncio.Future) + assert not closed_future.done() + + # Subsequent access should return the same future + assert proto.closed is closed_future + + +async def test_closed_property_after_connection_lost( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that closed property returns None after connection_lost if never accessed.""" + proto = ResponseHandler(loop=loop) + proto.connection_made(mock.Mock()) + + # Don't access proto.closed before connection_lost + proto.connection_lost(None) + + # After connection_lost, closed should return None if it was never accessed + assert proto.closed is None diff --git a/tests/test_connector.py b/tests/test_connector.py index 3b2d28ea46c..6fad9e4ccff 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -331,6 +331,28 @@ async def test_close_with_exception_during_closing( assert "Error while closing connector" in caplog.records[0].message assert "RuntimeError('Connection close failed')" in caplog.records[0].message + +async def test_close_with_proto_closed_none(key: ConnectionKey) -> None: + """Test close when protocol.closed is None.""" + # Create protocols where closed property returns None + proto1 = mock.create_autospec(ResponseHandler, instance=True) + proto1.closed = None + proto1.close = mock.Mock() + + proto2 = mock.create_autospec(ResponseHandler, instance=True) + proto2.closed = None + proto2.close = mock.Mock() + + conn = aiohttp.BaseConnector() + conn._conns[key] = deque([(proto1, 0)]) + conn._acquired.add(proto2) + + # Close the connector - this should handle the case where proto.closed is None + await conn.close() + + # Verify close was called on both protocols + assert proto1.close.called + assert proto2.close.called assert conn.closed From f42b744229c0a963a78b90c07c4e43031c21776d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 03:46:30 -0500 Subject: [PATCH 06/11] [PR #11114/758738e backport][3.13] Downgrade connector close error to debug (#11116) --- CHANGES/11114.misc.rst | 1 + aiohttp/connector.py | 4 ++-- tests/test_connector.py | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 CHANGES/11114.misc.rst diff --git a/CHANGES/11114.misc.rst b/CHANGES/11114.misc.rst new file mode 100644 index 00000000000..2fcb1468c67 --- /dev/null +++ b/CHANGES/11114.misc.rst @@ -0,0 +1 @@ +Downgraded the logging level for connector close errors from ERROR to DEBUG, as these are expected behavior with TLS 1.3 connections -- by :user:`bdraco`. diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 11bd36c487e..62b418a4bed 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -1,6 +1,5 @@ import asyncio import functools -import logging import random import socket import sys @@ -60,6 +59,7 @@ set_exception, set_result, ) +from .log import client_logger from .resolver import DefaultResolver if sys.version_info >= (3, 12): @@ -137,7 +137,7 @@ async def _wait_for_close(waiters: List[Awaitable[object]]) -> None: results = await asyncio.gather(*waiters, return_exceptions=True) for res in results: if isinstance(res, Exception): - logging.error("Error while closing connector: %r", res) + client_logger.debug("Error while closing connector: %r", res) class Connection: diff --git a/tests/test_connector.py b/tests/test_connector.py index 6fad9e4ccff..54da8743ed7 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -2,6 +2,7 @@ import asyncio import gc import hashlib +import logging import platform import socket import ssl @@ -309,6 +310,7 @@ async def test_close_with_exception_during_closing( loop: asyncio.AbstractEventLoop, caplog: pytest.LogCaptureFixture ) -> None: """Test that exceptions during connection closing are logged.""" + caplog.set_level(logging.DEBUG) proto = create_mocked_conn() # Make the closed future raise an exception when awaited @@ -327,7 +329,7 @@ async def test_close_with_exception_during_closing( # Check that the error was logged assert len(caplog.records) == 1 - assert caplog.records[0].levelname == "ERROR" + assert caplog.records[0].levelname == "DEBUG" assert "Error while closing connector" in caplog.records[0].message assert "RuntimeError('Connection close failed')" in caplog.records[0].message From a57ff76e933434b6142c0bcc6770b7d695fa109e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 03:48:43 -0500 Subject: [PATCH 07/11] [PR #11114/758738e backport][3.12] Downgrade connector close error to debug (#11115) --- CHANGES/11114.misc.rst | 1 + aiohttp/connector.py | 4 ++-- tests/test_connector.py | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 CHANGES/11114.misc.rst diff --git a/CHANGES/11114.misc.rst b/CHANGES/11114.misc.rst new file mode 100644 index 00000000000..2fcb1468c67 --- /dev/null +++ b/CHANGES/11114.misc.rst @@ -0,0 +1 @@ +Downgraded the logging level for connector close errors from ERROR to DEBUG, as these are expected behavior with TLS 1.3 connections -- by :user:`bdraco`. diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 11bd36c487e..62b418a4bed 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -1,6 +1,5 @@ import asyncio import functools -import logging import random import socket import sys @@ -60,6 +59,7 @@ set_exception, set_result, ) +from .log import client_logger from .resolver import DefaultResolver if sys.version_info >= (3, 12): @@ -137,7 +137,7 @@ async def _wait_for_close(waiters: List[Awaitable[object]]) -> None: results = await asyncio.gather(*waiters, return_exceptions=True) for res in results: if isinstance(res, Exception): - logging.error("Error while closing connector: %r", res) + client_logger.debug("Error while closing connector: %r", res) class Connection: diff --git a/tests/test_connector.py b/tests/test_connector.py index 6fad9e4ccff..54da8743ed7 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -2,6 +2,7 @@ import asyncio import gc import hashlib +import logging import platform import socket import ssl @@ -309,6 +310,7 @@ async def test_close_with_exception_during_closing( loop: asyncio.AbstractEventLoop, caplog: pytest.LogCaptureFixture ) -> None: """Test that exceptions during connection closing are logged.""" + caplog.set_level(logging.DEBUG) proto = create_mocked_conn() # Make the closed future raise an exception when awaited @@ -327,7 +329,7 @@ async def test_close_with_exception_during_closing( # Check that the error was logged assert len(caplog.records) == 1 - assert caplog.records[0].levelname == "ERROR" + assert caplog.records[0].levelname == "DEBUG" assert "Error while closing connector" in caplog.records[0].message assert "RuntimeError('Connection close failed')" in caplog.records[0].message From 24e030b7125d84d016e2e2ad05803102973c7dbf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 03:53:48 -0500 Subject: [PATCH 08/11] PR #11112/8edec63 backport][3.13] Fix cookie parsing issues (#11118) --- CHANGES/11112.bugfix.rst | 8 + CHANGES/2683.bugfix.rst | 1 + CHANGES/5397.bugfix.rst | 1 + CHANGES/7993.bugfix.rst | 1 + aiohttp/_cookie_helpers.py | 221 +++++++ aiohttp/abc.py | 26 +- aiohttp/client_reqrep.py | 21 +- aiohttp/cookiejar.py | 45 +- aiohttp/web_request.py | 9 +- docs/spelling_wordlist.txt | 1 + pyproject.toml | 2 +- tests/test_client_functional.py | 38 +- tests/test_client_request.py | 53 ++ tests/test_cookie_helpers.py | 1031 +++++++++++++++++++++++++++++++ tests/test_cookiejar.py | 225 ++++++- tests/test_web_request.py | 151 +++++ 16 files changed, 1769 insertions(+), 65 deletions(-) create mode 100644 CHANGES/11112.bugfix.rst create mode 120000 CHANGES/2683.bugfix.rst create mode 120000 CHANGES/5397.bugfix.rst create mode 120000 CHANGES/7993.bugfix.rst create mode 100644 aiohttp/_cookie_helpers.py create mode 100644 tests/test_cookie_helpers.py diff --git a/CHANGES/11112.bugfix.rst b/CHANGES/11112.bugfix.rst new file mode 100644 index 00000000000..6edea1c9b23 --- /dev/null +++ b/CHANGES/11112.bugfix.rst @@ -0,0 +1,8 @@ +Fixed cookie parsing to be more lenient when handling cookies with special characters +in names or values. Cookies with characters like ``{``, ``}``, and ``/`` in names are now +accepted instead of causing a :exc:`~http.cookies.CookieError` and 500 errors. Additionally, +cookies with mismatched quotes in values are now parsed correctly, and quoted cookie +values are now handled consistently whether or not they include special attributes +like ``Domain``. Also fixed :class:`~aiohttp.CookieJar` to ensure shared cookies (domain="", path="") +respect the ``quote_cookie`` parameter, making cookie quoting behavior consistent for +all cookies -- by :user:`bdraco`. diff --git a/CHANGES/2683.bugfix.rst b/CHANGES/2683.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/2683.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/5397.bugfix.rst b/CHANGES/5397.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/5397.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/7993.bugfix.rst b/CHANGES/7993.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/7993.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/aiohttp/_cookie_helpers.py b/aiohttp/_cookie_helpers.py new file mode 100644 index 00000000000..8184cc9bdc1 --- /dev/null +++ b/aiohttp/_cookie_helpers.py @@ -0,0 +1,221 @@ +""" +Internal cookie handling helpers. + +This module contains internal utilities for cookie parsing and manipulation. +These are not part of the public API and may change without notice. +""" + +import re +import sys +from http.cookies import Morsel +from typing import List, Optional, Sequence, Tuple, cast + +from .log import internal_logger + +__all__ = ("parse_cookie_headers", "preserve_morsel_with_coded_value") + +# Cookie parsing constants +# Allow more characters in cookie names to handle real-world cookies +# that don't strictly follow RFC standards (fixes #2683) +# RFC 6265 defines cookie-name token as per RFC 2616 Section 2.2, +# but many servers send cookies with characters like {} [] () etc. +# This makes the cookie parser more tolerant of real-world cookies +# while still providing some validation to catch obviously malformed names. +_COOKIE_NAME_RE = re.compile(r"^[!#$%&\'()*+\-./0-9:<=>?@A-Z\[\]^_`a-z{|}~]+$") +_COOKIE_KNOWN_ATTRS = frozenset( # AKA Morsel._reserved + ( + "path", + "domain", + "max-age", + "expires", + "secure", + "httponly", + "samesite", + "partitioned", + "version", + "comment", + ) +) +_COOKIE_BOOL_ATTRS = frozenset( # AKA Morsel._flags + ("secure", "httponly", "partitioned") +) + +# SimpleCookie's pattern for parsing cookies with relaxed validation +# Based on http.cookies pattern but extended to allow more characters in cookie names +# to handle real-world cookies (fixes #2683) +_COOKIE_PATTERN = re.compile( + r""" + \s* # Optional whitespace at start of cookie + (?P # Start of group 'key' + # aiohttp has extended to include [] for compatibility with real-world cookies + [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]+? # Any word of at least one letter + ) # End of group 'key' + ( # Optional group: there may not be a value. + \s*=\s* # Equal Sign + (?P # Start of group 'val' + "(?:[^\\"]|\\.)*" # Any double-quoted string (properly closed) + | # or + "[^";]* # Unmatched opening quote (differs from SimpleCookie - issue #7993) + | # or + # Special case for "expires" attr - RFC 822, RFC 850, RFC 1036, RFC 1123 + (\w{3,6}day|\w{3}),\s # Day of the week or abbreviated day (with comma) + [\w\d\s-]{9,11}\s[\d:]{8}\s # Date and time in specific format + (GMT|[+-]\d{4}) # Timezone: GMT or RFC 2822 offset like -0000, +0100 + # NOTE: RFC 2822 timezone support is an aiohttp extension + # for issue #4493 - SimpleCookie does NOT support this + | # or + # ANSI C asctime() format: "Wed Jun 9 10:18:14 2021" + # NOTE: This is an aiohttp extension for issue #4327 - SimpleCookie does NOT support this format + \w{3}\s+\w{3}\s+[\s\d]\d\s+\d{2}:\d{2}:\d{2}\s+\d{4} + | # or + [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]* # Any word or empty string + ) # End of group 'val' + )? # End of optional value group + \s* # Any number of spaces. + (\s+|;|$) # Ending either at space, semicolon, or EOS. + """, + re.VERBOSE | re.ASCII, +) + + +def preserve_morsel_with_coded_value(cookie: Morsel[str]) -> Morsel[str]: + """ + Preserve a Morsel's coded_value exactly as received from the server. + + This function ensures that cookie encoding is preserved exactly as sent by + the server, which is critical for compatibility with old servers that have + strict requirements about cookie formats. + + This addresses the issue described in https://github.com/aio-libs/aiohttp/pull/1453 + where Python's SimpleCookie would re-encode cookies, breaking authentication + with certain servers. + + Args: + cookie: A Morsel object from SimpleCookie + + Returns: + A Morsel object with preserved coded_value + + """ + mrsl_val = cast("Morsel[str]", cookie.get(cookie.key, Morsel())) + # We use __setstate__ instead of the public set() API because it allows us to + # bypass validation and set already validated state. This is more stable than + # setting protected attributes directly and unlikely to change since it would + # break pickling. + mrsl_val.__setstate__( # type: ignore[attr-defined] + {"key": cookie.key, "value": cookie.value, "coded_value": cookie.coded_value} + ) + return mrsl_val + + +def _unquote(text: str) -> str: + """ + Unquote a cookie value. + + Vendored from http.cookies._unquote to ensure compatibility. + """ + # If there are no quotes, return as-is + if len(text) < 2 or text[0] != '"' or text[-1] != '"': + return text + # Remove quotes and handle escaped characters + text = text[1:-1] + # Replace escaped quotes and backslashes + text = text.replace('\\"', '"').replace("\\\\", "\\") + return text + + +def parse_cookie_headers(headers: Sequence[str]) -> List[Tuple[str, Morsel[str]]]: + """ + Parse cookie headers using a vendored version of SimpleCookie parsing. + + This implementation is based on SimpleCookie.__parse_string to ensure + compatibility with how SimpleCookie parses cookies, including handling + of malformed cookies with missing semicolons. + + This function is used for both Cookie and Set-Cookie headers in order to be + forgiving. Ideally we would have followed RFC 6265 Section 5.2 (for Cookie + headers) and RFC 6265 Section 4.2.1 (for Set-Cookie headers), but the + real world data makes it impossible since we need to be a bit more forgiving. + + NOTE: This implementation differs from SimpleCookie in handling unmatched quotes. + SimpleCookie will stop parsing when it encounters a cookie value with an unmatched + quote (e.g., 'cookie="value'), causing subsequent cookies to be silently dropped. + This implementation handles unmatched quotes more gracefully to prevent cookie loss. + See https://github.com/aio-libs/aiohttp/issues/7993 + """ + parsed_cookies: List[Tuple[str, Morsel[str]]] = [] + + for header in headers: + if not header: + continue + + # Parse cookie string using SimpleCookie's algorithm + i = 0 + n = len(header) + current_morsel: Optional[Morsel[str]] = None + morsel_seen = False + + while 0 <= i < n: + # Start looking for a cookie + match = _COOKIE_PATTERN.match(header, i) + if not match: + # No more cookies + break + + key, value = match.group("key"), match.group("val") + i = match.end(0) + lower_key = key.lower() + + if key[0] == "$": + if not morsel_seen: + # We ignore attributes which pertain to the cookie + # mechanism as a whole, such as "$Version". + continue + # Process as attribute + if current_morsel is not None: + attr_lower_key = lower_key[1:] + if attr_lower_key in _COOKIE_KNOWN_ATTRS: + current_morsel[attr_lower_key] = value or "" + elif lower_key in _COOKIE_KNOWN_ATTRS: + if not morsel_seen: + # Invalid cookie string - attribute before cookie + break + if lower_key in _COOKIE_BOOL_ATTRS: + # Boolean attribute with any value should be True + if current_morsel is not None: + if lower_key == "partitioned" and sys.version_info < (3, 14): + dict.__setitem__(current_morsel, lower_key, True) + else: + current_morsel[lower_key] = True + elif value is None: + # Invalid cookie string - non-boolean attribute without value + break + elif current_morsel is not None: + # Regular attribute with value + current_morsel[lower_key] = _unquote(value) + elif value is not None: + # This is a cookie name=value pair + # Validate the name + if key in _COOKIE_KNOWN_ATTRS or not _COOKIE_NAME_RE.match(key): + internal_logger.warning( + "Can not load cookies: Illegal cookie name %r", key + ) + current_morsel = None + else: + # Create new morsel + current_morsel = Morsel() + # Preserve the original value as coded_value (with quotes if present) + # We use __setstate__ instead of the public set() API because it allows us to + # bypass validation and set already validated state. This is more stable than + # setting protected attributes directly and unlikely to change since it would + # break pickling. + current_morsel.__setstate__( # type: ignore[attr-defined] + {"key": key, "value": _unquote(value), "coded_value": value} + ) + parsed_cookies.append((key, current_morsel)) + morsel_seen = True + else: + # Invalid cookie string - no value for non-attribute + break + + return parsed_cookies diff --git a/aiohttp/abc.py b/aiohttp/abc.py index 96839b4f378..de11bfee4da 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -3,7 +3,7 @@ import socket from abc import ABC, abstractmethod from collections.abc import Sized -from http.cookies import BaseCookie, CookieError, Morsel, SimpleCookie +from http.cookies import BaseCookie, Morsel from typing import ( TYPE_CHECKING, Any, @@ -23,7 +23,7 @@ from multidict import CIMultiDict from yarl import URL -from .log import client_logger +from ._cookie_helpers import parse_cookie_headers from .typedefs import LooseCookies if TYPE_CHECKING: @@ -197,26 +197,8 @@ def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> No def update_cookies_from_headers( self, headers: Sequence[str], response_url: URL ) -> None: - """ - Update cookies from raw Set-Cookie headers. - - Default implementation parses each header separately to preserve - cookies with same name but different domain/path. - """ - # Default implementation for backward compatibility - cookies_to_update: List[Tuple[str, Morsel[str]]] = [] - for cookie_header in headers: - tmp_cookie = SimpleCookie() - try: - tmp_cookie.load(cookie_header) - # Collect all cookies as tuples (name, morsel) - for name, morsel in tmp_cookie.items(): - cookies_to_update.append((name, morsel)) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) - - # Update all cookies at once for efficiency - if cookies_to_update: + """Update cookies from raw Set-Cookie headers.""" + if headers and (cookies_to_update := parse_cookie_headers(headers)): self.update_cookies(cookies_to_update, response_url) @abstractmethod diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 01835260cc5..793864b95a5 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -9,7 +9,7 @@ import warnings from collections.abc import Mapping from hashlib import md5, sha1, sha256 -from http.cookies import CookieError, Morsel, SimpleCookie +from http.cookies import Morsel, SimpleCookie from types import MappingProxyType, TracebackType from typing import ( TYPE_CHECKING, @@ -31,6 +31,7 @@ from yarl import URL from . import hdrs, helpers, http, multipart, payload +from ._cookie_helpers import parse_cookie_headers, preserve_morsel_with_coded_value from .abc import AbstractStreamWriter from .client_exceptions import ( ClientConnectionError, @@ -62,7 +63,6 @@ HttpVersion11, StreamWriter, ) -from .log import client_logger from .streams import StreamReader from .typedefs import ( DEFAULT_JSON_DECODER, @@ -376,11 +376,9 @@ def cookies(self) -> SimpleCookie: if self._raw_cookie_headers is not None: # Parse cookies for response.cookies (SimpleCookie for backward compatibility) cookies = SimpleCookie() - for hdr in self._raw_cookie_headers: - try: - cookies.load(hdr) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) + # Use parse_cookie_headers for more lenient parsing that handles + # malformed cookies better than SimpleCookie.load + cookies.update(parse_cookie_headers(self._raw_cookie_headers)) self._cookies = cookies else: self._cookies = SimpleCookie() @@ -1095,7 +1093,8 @@ def update_cookies(self, cookies: Optional[LooseCookies]) -> None: c = SimpleCookie() if hdrs.COOKIE in self.headers: - c.load(self.headers.get(hdrs.COOKIE, "")) + # parse_cookie_headers already preserves coded values + c.update(parse_cookie_headers((self.headers.get(hdrs.COOKIE, ""),))) del self.headers[hdrs.COOKIE] if isinstance(cookies, Mapping): @@ -1104,10 +1103,8 @@ def update_cookies(self, cookies: Optional[LooseCookies]) -> None: iter_cookies = cookies # type: ignore[assignment] for name, value in iter_cookies: if isinstance(value, Morsel): - # Preserve coded_value - mrsl_val = value.get(value.key, Morsel()) - mrsl_val.set(value.key, value.value, value.coded_value) - c[name] = mrsl_val + # Use helper to preserve coded_value exactly as sent by server + c[name] = preserve_morsel_with_coded_value(value) else: c[name] = value # type: ignore[assignment] diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index a755a893409..193648d4309 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -23,11 +23,11 @@ Set, Tuple, Union, - cast, ) from yarl import URL +from ._cookie_helpers import preserve_morsel_with_coded_value from .abc import AbstractCookieJar, ClearCookiePredicate from .helpers import is_ip_address from .typedefs import LooseCookies, PathLike, StrOrURL @@ -45,6 +45,7 @@ # the expiration heap. This is a performance optimization to avoid cleaning up the # heap too often when there are only a few scheduled expirations. _MIN_SCHEDULED_COOKIE_EXPIRATION = 100 +_SIMPLE_COOKIE = SimpleCookie() class CookieJar(AbstractCookieJar): @@ -304,9 +305,10 @@ def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> No def filter_cookies(self, request_url: URL = URL()) -> "BaseCookie[str]": """Returns this jar's cookies filtered by their attributes.""" - filtered: Union[SimpleCookie, "BaseCookie[str]"] = ( - SimpleCookie() if self._quote_cookie else BaseCookie() - ) + # We always use BaseCookie now since all + # cookies set on on filtered are fully constructed + # Morsels, not just names and values. + filtered: BaseCookie[str] = BaseCookie() if not self._cookies: # Skip do_expiration() if there are no cookies. return filtered @@ -332,8 +334,17 @@ def filter_cookies(self, request_url: URL = URL()) -> "BaseCookie[str]": is_not_secure = request_origin not in self._treat_as_secure_origin # Send shared cookie - for c in self._cookies[("", "")].values(): - filtered[c.key] = c.value + key = ("", "") + for c in self._cookies[key].values(): + # Check cache first + if c.key in self._morsel_cache[key]: + filtered[c.key] = self._morsel_cache[key][c.key] + continue + + # Build and cache the morsel + mrsl_val = self._build_morsel(c) + self._morsel_cache[key][c.key] = mrsl_val + filtered[c.key] = mrsl_val if is_ip_address(hostname): if not self._unsafe: @@ -373,15 +384,29 @@ def filter_cookies(self, request_url: URL = URL()) -> "BaseCookie[str]": filtered[name] = self._morsel_cache[p][name] continue - # It's critical we use the Morsel so the coded_value - # (based on cookie version) is preserved - mrsl_val = cast("Morsel[str]", cookie.get(cookie.key, Morsel())) - mrsl_val.set(cookie.key, cookie.value, cookie.coded_value) + # Build and cache the morsel + mrsl_val = self._build_morsel(cookie) self._morsel_cache[p][name] = mrsl_val filtered[name] = mrsl_val return filtered + def _build_morsel(self, cookie: Morsel[str]) -> Morsel[str]: + """Build a morsel for sending, respecting quote_cookie setting.""" + if self._quote_cookie and cookie.coded_value and cookie.coded_value[0] == '"': + return preserve_morsel_with_coded_value(cookie) + morsel: Morsel[str] = Morsel() + if self._quote_cookie: + value, coded_value = _SIMPLE_COOKIE.value_encode(cookie.value) + else: + coded_value = value = cookie.value + # We use __setstate__ instead of the public set() API because it allows us to + # bypass validation and set already validated state. This is more stable than + # setting protected attributes directly and unlikely to change since it would + # break pickling. + morsel.__setstate__({"key": cookie.key, "value": value, "coded_value": coded_value}) # type: ignore[attr-defined] + return morsel + @staticmethod def _is_domain_match(domain: str, hostname: str) -> bool: """Implements domain matching adhering to RFC 6265.""" diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 6bf5a9dea74..0c5576823f1 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -7,7 +7,6 @@ import tempfile import types import warnings -from http.cookies import SimpleCookie from types import MappingProxyType from typing import ( TYPE_CHECKING, @@ -36,6 +35,7 @@ from yarl import URL from . import hdrs +from ._cookie_helpers import parse_cookie_headers from .abc import AbstractStreamWriter from .helpers import ( _SENTINEL, @@ -589,9 +589,10 @@ def cookies(self) -> Mapping[str, str]: A read-only dictionary-like object. """ - raw = self.headers.get(hdrs.COOKIE, "") - parsed = SimpleCookie(raw) - return MappingProxyType({key: val.value for key, val in parsed.items()}) + # Use parse_cookie_headers for more lenient parsing that accepts + # special characters in cookie names (fixes #2683) + parsed = parse_cookie_headers((self.headers.get(hdrs.COOKIE, ""),)) + return MappingProxyType({name: morsel.value for name, morsel in parsed}) @reify def http_range(self) -> slice: diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 8b389cc11f6..b495a07cb6f 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -336,6 +336,7 @@ toplevel towncrier tp tuples +ue UI un unawaited diff --git a/pyproject.toml b/pyproject.toml index 3ef37b5978b..df8b8465348 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,7 +82,7 @@ skip = "pp*" [tool.codespell] skip = '.git,*.pdf,*.svg,Makefile,CONTRIBUTORS.txt,venvs,_build' -ignore-words-list = 'te' +ignore-words-list = 'te,ue' [tool.slotscheck] # TODO(3.13): Remove aiohttp.helpers once https://github.com/python/cpython/pull/106771 diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index ca1a7dd1d6b..5c18178b714 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -5,6 +5,7 @@ import http.cookies import io import json +import logging import pathlib import socket import ssl @@ -2691,15 +2692,16 @@ async def handler(request): assert 200 == resp.status -async def test_set_cookies(aiohttp_client) -> None: - async def handler(request): +async def test_set_cookies( + aiohttp_client: AiohttpClient, caplog: pytest.LogCaptureFixture +) -> None: + async def handler(request: web.Request) -> web.Response: ret = web.Response() ret.set_cookie("c1", "cookie1") ret.set_cookie("c2", "cookie2") ret.headers.add( "Set-Cookie", - "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" - "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "invalid,cookie=value; " # Comma character is not allowed "HttpOnly; Path=/", ) return ret @@ -2708,14 +2710,38 @@ async def handler(request): app.router.add_get("/", handler) client = await aiohttp_client(app) - with mock.patch("aiohttp.client_reqrep.client_logger") as m_log: + with caplog.at_level(logging.WARNING): async with client.get("/") as resp: assert 200 == resp.status cookie_names = {c.key for c in client.session.cookie_jar} _ = resp.cookies assert cookie_names == {"c1", "c2"} - m_log.warning.assert_called_with("Can not load response cookies: %s", mock.ANY) + assert "Can not load cookies: Illegal cookie name 'invalid,cookie'" in caplog.text + + +async def test_set_cookies_with_curly_braces(aiohttp_client: AiohttpClient) -> None: + """Test that cookies with curly braces in names are now accepted (#2683).""" + + async def handler(request: web.Request) -> web.Response: + ret = web.Response() + ret.set_cookie("c1", "cookie1") + ret.headers.add( + "Set-Cookie", + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", + ) + return ret + + app = web.Application() + app.router.add_get("/", handler) + client = await aiohttp_client(app) + + async with client.get("/") as resp: + assert 200 == resp.status + cookie_names = {c.key for c in client.session.cookie_jar} + assert cookie_names == {"c1", "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}"} async def test_set_cookies_expired(aiohttp_client) -> None: diff --git a/tests/test_client_request.py b/tests/test_client_request.py index b3eb55d921b..2af540599f8 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -612,6 +612,59 @@ def test_gen_netloc_no_port(make_request) -> None: ) +def test_cookie_coded_value_preserved(loop: asyncio.AbstractEventLoop) -> None: + """Verify the coded value of a cookie is preserved.""" + # https://github.com/aio-libs/aiohttp/pull/1453 + req = ClientRequest("get", URL("http://python.org"), loop=loop) + req.update_cookies(cookies=SimpleCookie('ip-cookie="second"; Domain=127.0.0.1;')) + assert req.headers["COOKIE"] == 'ip-cookie="second"' + + +def test_update_cookies_with_special_chars_in_existing_header( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that update_cookies handles existing cookies with special characters.""" + # Create request with a cookie that has special characters (real-world example) + req = ClientRequest( + "get", + URL("http://python.org"), + headers={"Cookie": "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1"}, + loop=loop, + ) + + # Update with another cookie + req.update_cookies(cookies={"normal_cookie": "value2"}) + + # Both cookies should be preserved in the exact order + assert ( + req.headers["COOKIE"] + == "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1; normal_cookie=value2" + ) + + +def test_update_cookies_with_quoted_existing_header( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that update_cookies handles existing cookies with quoted values.""" + # Create request with cookies that have quoted values + req = ClientRequest( + "get", + URL("http://python.org"), + headers={"Cookie": 'session="value;with;semicolon"; token=abc123'}, + loop=loop, + ) + + # Update with another cookie + req.update_cookies(cookies={"new_cookie": "new_value"}) + + # All cookies should be preserved with their original coded values + # The quoted value should be preserved as-is + assert ( + req.headers["COOKIE"] + == 'new_cookie=new_value; session="value;with;semicolon"; token=abc123' + ) + + async def test_connection_header( loop: asyncio.AbstractEventLoop, conn: mock.Mock ) -> None: diff --git a/tests/test_cookie_helpers.py b/tests/test_cookie_helpers.py new file mode 100644 index 00000000000..7a2ac7493ee --- /dev/null +++ b/tests/test_cookie_helpers.py @@ -0,0 +1,1031 @@ +"""Tests for internal cookie helper functions.""" + +from http.cookies import CookieError, Morsel, SimpleCookie + +import pytest + +from aiohttp import _cookie_helpers as helpers +from aiohttp._cookie_helpers import ( + parse_cookie_headers, + preserve_morsel_with_coded_value, +) + + +def test_known_attrs_is_superset_of_morsel_reserved() -> None: + """Test that _COOKIE_KNOWN_ATTRS contains all Morsel._reserved attributes.""" + # Get Morsel._reserved attributes (lowercase) + morsel_reserved = {attr.lower() for attr in Morsel._reserved} # type: ignore[attr-defined] + + # _COOKIE_KNOWN_ATTRS should be a superset of morsel_reserved + assert ( + helpers._COOKIE_KNOWN_ATTRS >= morsel_reserved + ), f"_COOKIE_KNOWN_ATTRS is missing: {morsel_reserved - helpers._COOKIE_KNOWN_ATTRS}" + + +def test_bool_attrs_is_superset_of_morsel_flags() -> None: + """Test that _COOKIE_BOOL_ATTRS contains all Morsel._flags attributes.""" + # Get Morsel._flags attributes (lowercase) + morsel_flags = {attr.lower() for attr in Morsel._flags} # type: ignore[attr-defined] + + # _COOKIE_BOOL_ATTRS should be a superset of morsel_flags + assert ( + helpers._COOKIE_BOOL_ATTRS >= morsel_flags + ), f"_COOKIE_BOOL_ATTRS is missing: {morsel_flags - helpers._COOKIE_BOOL_ATTRS}" + + +def test_preserve_morsel_with_coded_value() -> None: + """Test preserve_morsel_with_coded_value preserves coded_value exactly.""" + # Create a cookie with a coded_value different from value + cookie: Morsel[str] = Morsel() + cookie.set("test_cookie", "decoded value", "encoded%20value") + + # Preserve the coded_value + result = preserve_morsel_with_coded_value(cookie) + + # Check that all values are preserved + assert result.key == "test_cookie" + assert result.value == "decoded value" + assert result.coded_value == "encoded%20value" + + # Should be a different Morsel instance + assert result is not cookie + + +def test_preserve_morsel_with_coded_value_no_coded_value() -> None: + """Test preserve_morsel_with_coded_value when coded_value is same as value.""" + cookie: Morsel[str] = Morsel() + cookie.set("test_cookie", "simple_value", "simple_value") + + result = preserve_morsel_with_coded_value(cookie) + + assert result.key == "test_cookie" + assert result.value == "simple_value" + assert result.coded_value == "simple_value" + + +def test_parse_cookie_headers_simple() -> None: + """Test parse_cookie_headers with simple cookies.""" + headers = ["name=value", "session=abc123"] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + assert result[0][0] == "name" + assert result[0][1].key == "name" + assert result[0][1].value == "value" + assert result[1][0] == "session" + assert result[1][1].key == "session" + assert result[1][1].value == "abc123" + + +def test_parse_cookie_headers_with_attributes() -> None: + """Test parse_cookie_headers with cookie attributes.""" + headers = [ + "sessionid=value123; Path=/; HttpOnly; Secure", + "user=john; Domain=.example.com; Max-Age=3600", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + name1, morsel1 = result[0] + assert name1 == "sessionid" + assert morsel1.value == "value123" + assert morsel1["path"] == "/" + assert morsel1["httponly"] is True + assert morsel1["secure"] is True + + # Second cookie + name2, morsel2 = result[1] + assert name2 == "user" + assert morsel2.value == "john" + assert morsel2["domain"] == ".example.com" + assert morsel2["max-age"] == "3600" + + +def test_parse_cookie_headers_special_chars_in_names() -> None: + """Test parse_cookie_headers accepts special characters in names (#2683).""" + # These should be accepted with relaxed validation + headers = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1", + "cookie[index]=value2", + "cookie(param)=value3", + "cookie:name=value4", + "cookie@domain=value5", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 5 + expected_names = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}", + "cookie[index]", + "cookie(param)", + "cookie:name", + "cookie@domain", + ] + + for i, (name, morsel) in enumerate(result): + assert name == expected_names[i] + assert morsel.key == expected_names[i] + assert morsel.value == f"value{i+1}" + + +def test_parse_cookie_headers_invalid_names() -> None: + """Test parse_cookie_headers rejects truly invalid cookie names.""" + # These should be rejected even with relaxed validation + headers = [ + "invalid\tcookie=value", # Tab character + "invalid\ncookie=value", # Newline + "invalid\rcookie=value", # Carriage return + "\x00badname=value", # Null character + "name with spaces=value", # Spaces in name + ] + + result = parse_cookie_headers(headers) + + # All should be skipped + assert len(result) == 0 + + +def test_parse_cookie_headers_empty_and_invalid() -> None: + """Test parse_cookie_headers handles empty and invalid formats.""" + headers = [ + "", # Empty header + " ", # Whitespace only + "=value", # No name + "name=", # Empty value (should be accepted) + "justname", # No value (should be skipped) + "path=/", # Reserved attribute as name (should be skipped) + "Domain=.com", # Reserved attribute as name (should be skipped) + ] + + result = parse_cookie_headers(headers) + + # Only "name=" should be accepted + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1].value == "" + + +def test_parse_cookie_headers_quoted_values() -> None: + """Test parse_cookie_headers handles quoted values correctly.""" + headers = [ + 'name="quoted value"', + 'session="with;semicolon"', + 'data="with\\"escaped\\""', + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + assert result[0][1].value == "quoted value" + assert result[1][1].value == "with;semicolon" + assert result[2][1].value == 'with"escaped"' + + +@pytest.mark.parametrize( + "header", + [ + 'session="abc;xyz"; token=123', + 'data="value;with;multiple;semicolons"; next=cookie', + 'complex="a=b;c=d"; simple=value', + ], +) +def test_parse_cookie_headers_semicolon_in_quoted_values(header: str) -> None: + """ + Test that semicolons inside properly quoted values are handled correctly. + + Cookie values can contain semicolons when properly quoted. This test ensures + that our parser handles these cases correctly, matching SimpleCookie behavior. + """ + # Test with SimpleCookie + sc = SimpleCookie() + sc.load(header) + + # Test with our parser + result = parse_cookie_headers([header]) + + # Should parse the same number of cookies + assert len(result) == len(sc) + + # Verify each cookie matches SimpleCookie + for (name, morsel), (sc_name, sc_morsel) in zip(result, sc.items()): + assert name == sc_name + assert morsel.value == sc_morsel.value + + +def test_parse_cookie_headers_multiple_cookies_same_header() -> None: + """Test parse_cookie_headers with multiple cookies in one header.""" + # Note: SimpleCookie includes the comma as part of the first cookie's value + headers = ["cookie1=value1, cookie2=value2"] + + result = parse_cookie_headers(headers) + + # Should parse as two separate cookies + assert len(result) == 2 + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1," # Comma is included in the value + assert result[1][0] == "cookie2" + assert result[1][1].value == "value2" + + +@pytest.mark.parametrize( + "header", + [ + # Standard cookies + "session=abc123", + "user=john; Path=/", + "token=xyz; Secure; HttpOnly", + # Empty values + "empty=", + # Quoted values + 'quoted="value with spaces"', + # Multiple attributes + "complex=value; Domain=.example.com; Path=/app; Max-Age=3600", + ], +) +def test_parse_cookie_headers_compatibility_with_simple_cookie(header: str) -> None: + """Test parse_cookie_headers is bug-for-bug compatible with SimpleCookie.load.""" + # Parse with SimpleCookie + sc = SimpleCookie() + sc.load(header) + + # Parse with our function + result = parse_cookie_headers([header]) + + # Should have same number of cookies + assert len(result) == len(sc) + + # Compare each cookie + for name, morsel in result: + assert name in sc + sc_morsel = sc[name] + + # Compare values + assert morsel.value == sc_morsel.value + assert morsel.key == sc_morsel.key + + # Compare attributes (only those that SimpleCookie would set) + for attr in ["path", "domain", "max-age"]: + assert morsel.get(attr) == sc_morsel.get(attr) + + # Boolean attributes are handled differently + # SimpleCookie sets them to empty string when not present, True when present + for bool_attr in ["secure", "httponly"]: + # Only check if SimpleCookie has the attribute set to True + if sc_morsel.get(bool_attr) is True: + assert morsel.get(bool_attr) is True + + +def test_parse_cookie_headers_relaxed_validation_differences() -> None: + """Test where parse_cookie_headers differs from SimpleCookie (relaxed validation).""" + # Test cookies that SimpleCookie rejects with CookieError + rejected_by_simplecookie = [ + ("cookie{with}braces=value1", "cookie{with}braces", "value1"), + ("cookie(with)parens=value3", "cookie(with)parens", "value3"), + ("cookie@with@at=value5", "cookie@with@at", "value5"), + ] + + for header, expected_name, expected_value in rejected_by_simplecookie: + # SimpleCookie should reject these with CookieError + sc = SimpleCookie() + with pytest.raises(CookieError): + sc.load(header) + + # Our parser should accept them + result = parse_cookie_headers([header]) + assert len(result) == 1 # We accept + assert result[0][0] == expected_name + assert result[0][1].value == expected_value + + # Test cookies that SimpleCookie accepts (but we handle more consistently) + accepted_by_simplecookie = [ + ("cookie[with]brackets=value2", "cookie[with]brackets", "value2"), + ("cookie:with:colons=value4", "cookie:with:colons", "value4"), + ] + + for header, expected_name, expected_value in accepted_by_simplecookie: + # SimpleCookie accepts these + sc = SimpleCookie() + sc.load(header) + # May or may not parse correctly in SimpleCookie + + # Our parser should accept them consistently + result = parse_cookie_headers([header]) + assert len(result) == 1 + assert result[0][0] == expected_name + assert result[0][1].value == expected_value + + +def test_parse_cookie_headers_case_insensitive_attrs() -> None: + """Test that known attributes are handled case-insensitively.""" + headers = [ + "cookie1=value1; PATH=/test; DOMAIN=example.com", + "cookie2=value2; Secure; HTTPONLY; max-AGE=60", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie - attributes should be recognized despite case + assert result[0][1]["path"] == "/test" + assert result[0][1]["domain"] == "example.com" + + # Second cookie + assert result[1][1]["secure"] is True + assert result[1][1]["httponly"] is True + assert result[1][1]["max-age"] == "60" + + +def test_parse_cookie_headers_unknown_attrs_ignored() -> None: + """Test that unknown attributes are treated as new cookies (same as SimpleCookie).""" + headers = [ + "cookie=value; Path=/; unknownattr=ignored; HttpOnly", + ] + + result = parse_cookie_headers(headers) + + # SimpleCookie treats unknown attributes with values as new cookies + assert len(result) == 2 + + # First cookie + assert result[0][0] == "cookie" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] == "" # Not set on first cookie + + # Second cookie (the unknown attribute) + assert result[1][0] == "unknownattr" + assert result[1][1].value == "ignored" + assert result[1][1]["httponly"] is True # HttpOnly applies to this cookie + + +def test_parse_cookie_headers_complex_real_world() -> None: + """Test parse_cookie_headers with complex real-world examples.""" + headers = [ + # AWS ELB cookie + "AWSELB=ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890; Path=/", + # Google Analytics + "_ga=GA1.2.1234567890.1234567890; Domain=.example.com; Path=/; Expires=Thu, 31-Dec-2025 23:59:59 GMT", + # Session with all attributes + "session_id=s%3AabcXYZ123.signature123; Path=/; Secure; HttpOnly; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + + # Check each cookie parsed correctly + assert result[0][0] == "AWSELB" + assert result[1][0] == "_ga" + assert result[2][0] == "session_id" + + # Session cookie should have all attributes + session_morsel = result[2][1] + assert session_morsel["secure"] is True + assert session_morsel["httponly"] is True + assert session_morsel.get("samesite") == "Strict" + + +def test_parse_cookie_headers_boolean_attrs() -> None: + """Test that boolean attributes (secure, httponly) work correctly.""" + # Test secure attribute variations + headers = [ + "cookie1=value1; Secure", + "cookie2=value2; Secure=", + "cookie3=value3; Secure=true", # Non-standard but might occur + ] + + result = parse_cookie_headers(headers) + assert len(result) == 3 + + # All should have secure=True + for name, morsel in result: + assert morsel.get("secure") is True, f"{name} should have secure=True" + + # Test httponly attribute variations + headers = [ + "cookie4=value4; HttpOnly", + "cookie5=value5; HttpOnly=", + ] + + result = parse_cookie_headers(headers) + assert len(result) == 2 + + # All should have httponly=True + for name, morsel in result: + assert morsel.get("httponly") is True, f"{name} should have httponly=True" + + +def test_parse_cookie_headers_boolean_attrs_with_partitioned() -> None: + """Test that boolean attributes including partitioned work correctly.""" + # Test secure attribute variations + secure_headers = [ + "cookie1=value1; Secure", + "cookie2=value2; Secure=", + "cookie3=value3; Secure=true", # Non-standard but might occur + ] + + result = parse_cookie_headers(secure_headers) + assert len(result) == 3 + for name, morsel in result: + assert morsel.get("secure") is True, f"{name} should have secure=True" + + # Test httponly attribute variations + httponly_headers = [ + "cookie4=value4; HttpOnly", + "cookie5=value5; HttpOnly=", + ] + + result = parse_cookie_headers(httponly_headers) + assert len(result) == 2 + for name, morsel in result: + assert morsel.get("httponly") is True, f"{name} should have httponly=True" + + # Test partitioned attribute variations + partitioned_headers = [ + "cookie6=value6; Partitioned", + "cookie7=value7; Partitioned=", + "cookie8=value8; Partitioned=yes", # Non-standard but might occur + ] + + result = parse_cookie_headers(partitioned_headers) + assert len(result) == 3 + for name, morsel in result: + assert morsel.get("partitioned") is True, f"{name} should have partitioned=True" + + +def test_parse_cookie_headers_encoded_values() -> None: + """Test that parse_cookie_headers preserves encoded values.""" + headers = [ + "encoded=hello%20world", + "url=https%3A%2F%2Fexample.com%2Fpath", + "special=%21%40%23%24%25%5E%26*%28%29", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + # Values should be preserved as-is (not decoded) + assert result[0][1].value == "hello%20world" + assert result[1][1].value == "https%3A%2F%2Fexample.com%2Fpath" + assert result[2][1].value == "%21%40%23%24%25%5E%26*%28%29" + + +def test_parse_cookie_headers_partitioned() -> None: + """ + Test that parse_cookie_headers handles partitioned attribute correctly. + + This tests the fix for issue #10380 - partitioned cookies support. + The partitioned attribute is a boolean flag like secure and httponly. + """ + headers = [ + "cookie1=value1; Partitioned", + "cookie2=value2; Partitioned=", + "cookie3=value3; Partitioned=true", # Non-standard but might occur + "cookie4=value4; Secure; Partitioned; HttpOnly", + "cookie5=value5; Domain=.example.com; Path=/; Partitioned", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 5 + + # All cookies should have partitioned=True + for i, (name, morsel) in enumerate(result): + assert ( + morsel.get("partitioned") is True + ), f"Cookie {i+1} should have partitioned=True" + assert name == f"cookie{i+1}" + assert morsel.value == f"value{i+1}" + + # Cookie 4 should also have secure and httponly + assert result[3][1].get("secure") is True + assert result[3][1].get("httponly") is True + + # Cookie 5 should also have domain and path + assert result[4][1].get("domain") == ".example.com" + assert result[4][1].get("path") == "/" + + +def test_parse_cookie_headers_partitioned_case_insensitive() -> None: + """Test that partitioned attribute is recognized case-insensitively.""" + headers = [ + "cookie1=value1; partitioned", # lowercase + "cookie2=value2; PARTITIONED", # uppercase + "cookie3=value3; Partitioned", # title case + "cookie4=value4; PaRtItIoNeD", # mixed case + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 4 + + # All should be recognized as partitioned + for i, (_, morsel) in enumerate(result): + assert ( + morsel.get("partitioned") is True + ), f"Cookie {i+1} should have partitioned=True" + + +def test_parse_cookie_headers_partitioned_not_set() -> None: + """Test that cookies without partitioned attribute don't have it set.""" + headers = [ + "normal=value; Secure; HttpOnly", + "regular=cookie; Path=/", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # Check that partitioned is not set (empty string is the default for flags in Morsel) + assert result[0][1].get("partitioned", "") == "" + assert result[1][1].get("partitioned", "") == "" + + +# Tests that don't require partitioned support in SimpleCookie +def test_parse_cookie_headers_partitioned_with_other_attrs_manual() -> None: + """ + Test parsing logic for partitioned cookies combined with all other attributes. + + This test verifies our parsing logic handles partitioned correctly as a boolean + attribute regardless of SimpleCookie support. + """ + # Test that our parser recognizes partitioned in _COOKIE_KNOWN_ATTRS and _COOKIE_BOOL_ATTRS + assert "partitioned" in helpers._COOKIE_KNOWN_ATTRS + assert "partitioned" in helpers._COOKIE_BOOL_ATTRS + + # Test a simple case that won't trigger SimpleCookie errors + headers = ["session=abc123; Secure; HttpOnly"] + result = parse_cookie_headers(headers) + + assert len(result) == 1 + assert result[0][0] == "session" + assert result[0][1]["secure"] is True + assert result[0][1]["httponly"] is True + + +def test_cookie_helpers_constants_include_partitioned() -> None: + """Test that cookie helper constants include partitioned attribute.""" + # Test our constants include partitioned + assert "partitioned" in helpers._COOKIE_KNOWN_ATTRS + assert "partitioned" in helpers._COOKIE_BOOL_ATTRS + + +@pytest.mark.parametrize( + "test_string", + [ + " Partitioned ", + " partitioned ", + " PARTITIONED ", + " Partitioned; ", + " Partitioned= ", + " Partitioned=true ", + ], +) +def test_cookie_pattern_matches_partitioned_attribute(test_string: str) -> None: + """Test that the cookie pattern regex matches various partitioned attribute formats.""" + pattern = helpers._COOKIE_PATTERN + match = pattern.match(test_string) + assert match is not None, f"Pattern should match '{test_string}'" + assert match.group("key").lower() == "partitioned" + + +def test_parse_cookie_headers_issue_7993_double_quotes() -> None: + """ + Test that cookies with unmatched opening quotes don't break parsing of subsequent cookies. + + This reproduces issue #7993 where a cookie containing an unmatched opening double quote + causes subsequent cookies to be silently dropped. + NOTE: This only fixes the specific case where a value starts with a quote but doesn't + end with one (e.g., 'cookie="value'). Other malformed quote cases still behave like + SimpleCookie for compatibility. + """ + # Test case from the issue + headers = ['foo=bar; baz="qux; foo2=bar2'] + + result = parse_cookie_headers(headers) + + # Should parse all cookies correctly + assert len(result) == 3 + assert result[0][0] == "foo" + assert result[0][1].value == "bar" + assert result[1][0] == "baz" + assert result[1][1].value == '"qux' # Unmatched quote included + assert result[2][0] == "foo2" + assert result[2][1].value == "bar2" + + +def test_parse_cookie_headers_empty_headers() -> None: + """Test handling of empty headers in the sequence.""" + # Empty header should be skipped + result = parse_cookie_headers(["", "name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1].value == "value" + + # Multiple empty headers + result = parse_cookie_headers(["", "", ""]) + assert result == [] + + # Empty headers mixed with valid cookies + result = parse_cookie_headers(["", "a=1", "", "b=2", ""]) + assert len(result) == 2 + assert result[0][0] == "a" + assert result[1][0] == "b" + + +def test_parse_cookie_headers_invalid_cookie_syntax() -> None: + """Test handling of invalid cookie syntax.""" + # No valid cookie pattern + result = parse_cookie_headers(["@#$%^&*()"]) + assert result == [] + + # Cookie name without value + result = parse_cookie_headers(["name"]) + assert result == [] + + # Multiple invalid patterns + result = parse_cookie_headers(["!!!!", "????", "name", "@@@"]) + assert result == [] + + +def test_parse_cookie_headers_illegal_cookie_names( + caplog: pytest.LogCaptureFixture, +) -> None: + """ + Test that illegal cookie names are rejected. + + Note: When a known attribute name is used as a cookie name at the start, + parsing stops early (before any warning can be logged). Warnings are only + logged when illegal names appear after a valid cookie. + """ + # Cookie name that is a known attribute (illegal) - parsing stops early + result = parse_cookie_headers(["path=value; domain=test"]) + assert result == [] + + # Cookie name that doesn't match the pattern + result = parse_cookie_headers(["=value"]) + assert result == [] + + # Valid cookie after illegal one - parsing stops at illegal + result = parse_cookie_headers(["domain=bad; good=value"]) + assert result == [] + + # Illegal cookie name that appears after a valid cookie triggers warning + result = parse_cookie_headers(["good=value; Path=/; invalid,cookie=value;"]) + assert len(result) == 1 + assert result[0][0] == "good" + assert "Illegal cookie name 'invalid,cookie'" in caplog.text + + +def test_parse_cookie_headers_attributes_before_cookie() -> None: + """Test that attributes before any cookie are invalid.""" + # Path attribute before cookie + result = parse_cookie_headers(["Path=/; name=value"]) + assert result == [] + + # Domain attribute before cookie + result = parse_cookie_headers(["Domain=.example.com; name=value"]) + assert result == [] + + # Multiple attributes before cookie + result = parse_cookie_headers(["Path=/; Domain=.example.com; Secure; name=value"]) + assert result == [] + + +def test_parse_cookie_headers_attributes_without_values() -> None: + """Test handling of attributes with missing values.""" + # Boolean attribute without value (valid) + result = parse_cookie_headers(["name=value; Secure"]) + assert len(result) == 1 + assert result[0][1]["secure"] is True + + # Non-boolean attribute without value (invalid, stops parsing) + result = parse_cookie_headers(["name=value; Path"]) + assert len(result) == 1 + # Path without value stops further attribute parsing + + # Multiple cookies, invalid attribute in middle + result = parse_cookie_headers(["name=value; Path; Secure"]) + assert len(result) == 1 + # Secure is not parsed because Path without value stops parsing + + +def test_parse_cookie_headers_dollar_prefixed_names() -> None: + """Test handling of cookie names starting with $.""" + # $Version without preceding cookie (ignored) + result = parse_cookie_headers(["$Version=1; name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + + # Multiple $ prefixed without cookie (all ignored) + result = parse_cookie_headers(["$Version=1; $Path=/; $Domain=.com; name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + + # $ prefix at start is ignored, cookie follows + result = parse_cookie_headers(["$Unknown=123; valid=cookie"]) + assert len(result) == 1 + assert result[0][0] == "valid" + + +def test_parse_cookie_headers_dollar_attributes() -> None: + """Test handling of $ prefixed attributes after cookies.""" + # Test multiple $ attributes with cookie (case-insensitive like SimpleCookie) + result = parse_cookie_headers(["name=value; $Path=/test; $Domain=.example.com"]) + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1]["path"] == "/test" + assert result[0][1]["domain"] == ".example.com" + + # Test unknown $ attribute (should be ignored) + result = parse_cookie_headers(["name=value; $Unknown=test"]) + assert len(result) == 1 + assert result[0][0] == "name" + # $Unknown should not be set + + # Test $ attribute with empty value + result = parse_cookie_headers(["name=value; $Path="]) + assert len(result) == 1 + assert result[0][1]["path"] == "" + + # Test case sensitivity compatibility with SimpleCookie + result = parse_cookie_headers(["test=value; $path=/lower; $PATH=/upper"]) + assert len(result) == 1 + # Last one wins, and it's case-insensitive + assert result[0][1]["path"] == "/upper" + + +def test_parse_cookie_headers_attributes_after_illegal_cookie() -> None: + """ + Test that attributes after an illegal cookie name are handled correctly. + + This covers the branches where current_morsel is None because an illegal + cookie name was encountered. + """ + # Illegal cookie followed by $ attribute + result = parse_cookie_headers(["good=value; invalid,cookie=bad; $Path=/test"]) + assert len(result) == 1 + assert result[0][0] == "good" + # $Path should be ignored since current_morsel is None after illegal cookie + + # Illegal cookie followed by boolean attribute + result = parse_cookie_headers(["good=value; invalid,cookie=bad; HttpOnly"]) + assert len(result) == 1 + assert result[0][0] == "good" + # HttpOnly should be ignored since current_morsel is None + + # Illegal cookie followed by regular attribute with value + result = parse_cookie_headers(["good=value; invalid,cookie=bad; Max-Age=3600"]) + assert len(result) == 1 + assert result[0][0] == "good" + # Max-Age should be ignored since current_morsel is None + + # Multiple attributes after illegal cookie + result = parse_cookie_headers( + ["good=value; invalid,cookie=bad; $Path=/; HttpOnly; Max-Age=60; Domain=.com"] + ) + assert len(result) == 1 + assert result[0][0] == "good" + # All attributes should be ignored after illegal cookie + + +def test_parse_cookie_headers_unmatched_quotes_compatibility() -> None: + """ + Test that most unmatched quote scenarios behave like SimpleCookie. + + For compatibility, we only handle the specific case of unmatched opening quotes + (e.g., 'cookie="value'). Other cases behave the same as SimpleCookie. + """ + # Cases that SimpleCookie and our parser both fail to parse completely + incompatible_cases = [ + 'cookie1=val"ue; cookie2=value2', # codespell:ignore + 'cookie1=value"; cookie2=value2', + 'cookie1=va"l"ue"; cookie2=value2', # codespell:ignore + 'cookie1=value1; cookie2=val"ue; cookie3=value3', # codespell:ignore + ] + + for header in incompatible_cases: + # Test SimpleCookie behavior + sc = SimpleCookie() + sc.load(header) + sc_cookies = list(sc.items()) + + # Test our parser behavior + result = parse_cookie_headers([header]) + + # Both should parse the same cookies (partial parsing) + assert len(result) == len(sc_cookies), ( + f"Header: {header}\n" + f"SimpleCookie parsed: {len(sc_cookies)} cookies\n" + f"Our parser parsed: {len(result)} cookies" + ) + + # The case we specifically fix (unmatched opening quote) + fixed_case = 'cookie1=value1; cookie2="unmatched; cookie3=value3' + + # SimpleCookie fails to parse cookie3 + sc = SimpleCookie() + sc.load(fixed_case) + assert len(sc) == 1 # Only cookie1 + + # Our parser handles it better + result = parse_cookie_headers([fixed_case]) + assert len(result) == 3 # All three cookies + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1" + assert result[1][0] == "cookie2" + assert result[1][1].value == '"unmatched' + assert result[2][0] == "cookie3" + assert result[2][1].value == "value3" + + +def test_parse_cookie_headers_expires_attribute() -> None: + """Test parse_cookie_headers handles expires attribute with date formats.""" + headers = [ + "session=abc; Expires=Wed, 09 Jun 2021 10:18:14 GMT", + "user=xyz; expires=Wednesday, 09-Jun-21 10:18:14 GMT", + "token=123; EXPIRES=Wed, 09 Jun 2021 10:18:14 GMT", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + for _, morsel in result: + assert "expires" in morsel + assert "GMT" in morsel["expires"] + + +def test_parse_cookie_headers_edge_cases() -> None: + """Test various edge cases.""" + # Very long cookie values + long_value = "x" * 4096 + result = parse_cookie_headers([f"name={long_value}"]) + assert len(result) == 1 + assert result[0][1].value == long_value + + +def test_parse_cookie_headers_various_date_formats_issue_4327() -> None: + """ + Test that parse_cookie_headers handles various date formats per RFC 6265. + + This tests the fix for issue #4327 - support for RFC 822, RFC 850, + and ANSI C asctime() date formats in cookie expiration. + """ + # Test various date formats + headers = [ + # RFC 822 format (preferred format) + "cookie1=value1; Expires=Wed, 09 Jun 2021 10:18:14 GMT", + # RFC 850 format (obsolete but still used) + "cookie2=value2; Expires=Wednesday, 09-Jun-21 10:18:14 GMT", + # RFC 822 with dashes + "cookie3=value3; Expires=Wed, 09-Jun-2021 10:18:14 GMT", + # ANSI C asctime() format (aiohttp extension - not supported by SimpleCookie) + "cookie4=value4; Expires=Wed Jun 9 10:18:14 2021", + # Various other formats seen in the wild + "cookie5=value5; Expires=Thu, 01 Jan 2030 00:00:00 GMT", + "cookie6=value6; Expires=Mon, 31-Dec-99 23:59:59 GMT", + "cookie7=value7; Expires=Tue, 01-Jan-30 00:00:00 GMT", + ] + + result = parse_cookie_headers(headers) + + # All cookies should be parsed + assert len(result) == 7 + + # Check each cookie was parsed with its expires attribute + expected_cookies = [ + ("cookie1", "value1", "Wed, 09 Jun 2021 10:18:14 GMT"), + ("cookie2", "value2", "Wednesday, 09-Jun-21 10:18:14 GMT"), + ("cookie3", "value3", "Wed, 09-Jun-2021 10:18:14 GMT"), + ("cookie4", "value4", "Wed Jun 9 10:18:14 2021"), + ("cookie5", "value5", "Thu, 01 Jan 2030 00:00:00 GMT"), + ("cookie6", "value6", "Mon, 31-Dec-99 23:59:59 GMT"), + ("cookie7", "value7", "Tue, 01-Jan-30 00:00:00 GMT"), + ] + + for (name, morsel), (exp_name, exp_value, exp_expires) in zip( + result, expected_cookies + ): + assert name == exp_name + assert morsel.value == exp_value + assert morsel.get("expires") == exp_expires + + +def test_parse_cookie_headers_ansi_c_asctime_format() -> None: + """ + Test parsing of ANSI C asctime() format. + + This tests support for ANSI C asctime() format (e.g., "Wed Jun 9 10:18:14 2021"). + NOTE: This is an aiohttp extension - SimpleCookie does NOT support this format. + """ + headers = ["cookie1=value1; Expires=Wed Jun 9 10:18:14 2021"] + + result = parse_cookie_headers(headers) + + # Should parse correctly with the expires attribute preserved + assert len(result) == 1 + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1" + assert result[0][1]["expires"] == "Wed Jun 9 10:18:14 2021" + + +def test_parse_cookie_headers_rfc2822_timezone_issue_4493() -> None: + """ + Test that parse_cookie_headers handles RFC 2822 timezone formats. + + This tests the fix for issue #4493 - support for RFC 2822-compliant dates + with timezone offsets like -0000, +0100, etc. + NOTE: This is an aiohttp extension - SimpleCookie does NOT support this format. + """ + headers = [ + # RFC 2822 with -0000 timezone (common in some APIs) + "hello=world; expires=Wed, 15 Jan 2020 09:45:07 -0000", + # RFC 2822 with positive offset + "session=abc123; expires=Thu, 01 Feb 2024 14:30:00 +0100", + # RFC 2822 with negative offset + "token=xyz789; expires=Fri, 02 Mar 2025 08:15:30 -0500", + # Standard GMT for comparison + "classic=cookie; expires=Sat, 03 Apr 2026 12:00:00 GMT", + ] + + result = parse_cookie_headers(headers) + + # All cookies should be parsed + assert len(result) == 4 + + # Check each cookie was parsed with its expires attribute + assert result[0][0] == "hello" + assert result[0][1].value == "world" + assert result[0][1]["expires"] == "Wed, 15 Jan 2020 09:45:07 -0000" + + assert result[1][0] == "session" + assert result[1][1].value == "abc123" + assert result[1][1]["expires"] == "Thu, 01 Feb 2024 14:30:00 +0100" + + assert result[2][0] == "token" + assert result[2][1].value == "xyz789" + assert result[2][1]["expires"] == "Fri, 02 Mar 2025 08:15:30 -0500" + + assert result[3][0] == "classic" + assert result[3][1].value == "cookie" + assert result[3][1]["expires"] == "Sat, 03 Apr 2026 12:00:00 GMT" + + +def test_parse_cookie_headers_rfc2822_with_attributes() -> None: + """Test that RFC 2822 dates work correctly with other cookie attributes.""" + headers = [ + "session=abc123; expires=Wed, 15 Jan 2020 09:45:07 -0000; Path=/; HttpOnly; Secure", + "token=xyz789; expires=Thu, 01 Feb 2024 14:30:00 +0100; Domain=.example.com; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + assert result[0][0] == "session" + assert result[0][1].value == "abc123" + assert result[0][1]["expires"] == "Wed, 15 Jan 2020 09:45:07 -0000" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] is True + assert result[0][1]["secure"] is True + + # Second cookie + assert result[1][0] == "token" + assert result[1][1].value == "xyz789" + assert result[1][1]["expires"] == "Thu, 01 Feb 2024 14:30:00 +0100" + assert result[1][1]["domain"] == ".example.com" + assert result[1][1]["samesite"] == "Strict" + + +def test_parse_cookie_headers_date_formats_with_attributes() -> None: + """Test that date formats work correctly with other cookie attributes.""" + headers = [ + "session=abc123; Expires=Wed, 09 Jun 2030 10:18:14 GMT; Path=/; HttpOnly; Secure", + "token=xyz789; Expires=Wednesday, 09-Jun-30 10:18:14 GMT; Domain=.example.com; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + assert result[0][0] == "session" + assert result[0][1].value == "abc123" + assert result[0][1]["expires"] == "Wed, 09 Jun 2030 10:18:14 GMT" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] is True + assert result[0][1]["secure"] is True + + # Second cookie + assert result[1][0] == "token" + assert result[1][1].value == "xyz789" + assert result[1][1]["expires"] == "Wednesday, 09-Jun-30 10:18:14 GMT" + assert result[1][1]["domain"] == ".example.com" + assert result[1][1]["samesite"] == "Strict" diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index e1b6e351e3d..15557085b4e 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -371,6 +371,8 @@ async def test_domain_filter_ip_cookie_receive(cookies_to_receive) -> None: ("custom-cookie=value/one;", 'Cookie: custom-cookie="value/one"', True), ("custom-cookie=value1;", "Cookie: custom-cookie=value1", True), ("custom-cookie=value/one;", "Cookie: custom-cookie=value/one", False), + ('foo="quoted_value"', 'Cookie: foo="quoted_value"', True), + ('foo="quoted_value"; domain=127.0.0.1', 'Cookie: foo="quoted_value"', True), ], ids=( "IP domain preserved", @@ -378,6 +380,8 @@ async def test_domain_filter_ip_cookie_receive(cookies_to_receive) -> None: "quoted cookie with special char", "quoted cookie w/o special char", "unquoted cookie with special char", + "pre-quoted cookie", + "pre-quoted cookie with domain", ), ) async def test_quotes_correctly_based_on_input( @@ -1225,7 +1229,7 @@ async def test_update_cookies_from_headers_duplicate_names() -> None: url: URL = URL("http://www.example.com/") # Headers with duplicate names but different domains - headers: List[str] = [ + headers = [ "session-id=123456; Domain=.example.com; Path=/", "session-id=789012; Domain=.www.example.com; Path=/", "user-pref=light; Domain=.example.com", @@ -1255,11 +1259,10 @@ async def test_update_cookies_from_headers_invalid_cookies( url: URL = URL("http://example.com/") # Mix of valid and invalid cookies - headers: List[str] = [ + headers = [ "valid-cookie=value123", - "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" - "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " - "HttpOnly; Path=/", # This cookie with curly braces causes CookieError + "invalid,cookie=value; " # Comma character is not allowed + "HttpOnly; Path=/", "another-valid=value456", ] @@ -1268,7 +1271,7 @@ async def test_update_cookies_from_headers_invalid_cookies( jar.update_cookies_from_headers(headers, url) # Check that we logged warnings for invalid cookies - assert "Can not load response cookies" in caplog.text + assert "Can not load cookies" in caplog.text # Valid cookies should still be added assert len(jar) >= 2 # At least the two clearly valid cookies @@ -1277,6 +1280,52 @@ async def test_update_cookies_from_headers_invalid_cookies( assert "another-valid" in filtered +async def test_update_cookies_from_headers_with_curly_braces() -> None: + """Test that cookies with curly braces in names are now accepted (#2683).""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Cookie names with curly braces should now be accepted + headers = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", + "regular-cookie=value123", + ] + + jar.update_cookies_from_headers(headers, url) + + # Both cookies should be added + assert len(jar) == 2 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}" in filtered + assert "regular-cookie" in filtered + + +async def test_update_cookies_from_headers_with_special_chars() -> None: + """Test that cookies with various special characters are accepted.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Various special characters that should now be accepted + headers = [ + "cookie_with_parens=(value)=test123", + "cookie-with-brackets[index]=value456", + "cookie@with@at=value789", + "cookie:with:colons=value000", + ] + + jar.update_cookies_from_headers(headers, url) + + # All cookies should be added + assert len(jar) == 4 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "cookie_with_parens" in filtered + assert "cookie-with-brackets[index]" in filtered + assert "cookie@with@at" in filtered + assert "cookie:with:colons" in filtered + + async def test_update_cookies_from_headers_empty_list() -> None: """Test that empty header list is handled gracefully.""" jar: CookieJar = CookieJar() @@ -1293,7 +1342,7 @@ async def test_update_cookies_from_headers_with_attributes() -> None: jar: CookieJar = CookieJar() url: URL = URL("https://secure.example.com/app/page") - headers: List[str] = [ + headers = [ "secure-cookie=value1; Secure; HttpOnly; SameSite=Strict", "expiring-cookie=value2; Max-Age=3600; Path=/app", "domain-cookie=value3; Domain=.example.com; Path=/", @@ -1348,7 +1397,7 @@ async def test_update_cookies_from_headers_preserves_existing() -> None: ) # Add more cookies via headers - headers: List[str] = [ + headers = [ "new-cookie1=value3", "new-cookie2=value4", ] @@ -1373,7 +1422,7 @@ async def test_update_cookies_from_headers_overwrites_same_cookie() -> None: jar.update_cookies({"session": "old-value"}, url) # Update with new value via headers - headers: List[str] = ["session=new-value"] + headers = ["session=new-value"] jar.update_cookies_from_headers(headers, url) # Should still have just 1 cookie with updated value @@ -1387,7 +1436,7 @@ async def test_dummy_cookie_jar_update_cookies_from_headers() -> None: jar: DummyCookieJar = DummyCookieJar() url: URL = URL("http://example.com/") - headers: List[str] = [ + headers = [ "cookie1=value1", "cookie2=value2", ] @@ -1398,3 +1447,159 @@ async def test_dummy_cookie_jar_update_cookies_from_headers() -> None: assert len(jar) == 0 filtered: BaseCookie[str] = jar.filter_cookies(url) assert len(filtered) == 0 + + +async def test_shared_cookie_cache_population() -> None: + """Test that shared cookies are cached correctly.""" + jar = CookieJar(unsafe=True) + + # Create a shared cookie (no domain/path restrictions) + sc = SimpleCookie() + sc["shared"] = "value" + sc["shared"]["path"] = "/" # Will be stripped to "" + + # Update with empty URL to avoid domain being set + jar.update_cookies(sc, URL()) + + # Verify cookie is stored at shared key + assert ("", "") in jar._cookies + assert "shared" in jar._cookies[("", "")] + + # Filter cookies to populate cache + filtered = jar.filter_cookies(URL("http://example.com/")) + assert "shared" in filtered + assert filtered["shared"].value == "value" + + # Verify cache was populated + assert ("", "") in jar._morsel_cache + assert "shared" in jar._morsel_cache[("", "")] + + # Verify the cached morsel is the same one returned + cached_morsel = jar._morsel_cache[("", "")]["shared"] + assert cached_morsel is filtered["shared"] + + +async def test_shared_cookie_cache_clearing_on_update() -> None: + """Test that shared cookie cache is cleared when cookie is updated.""" + jar = CookieJar(unsafe=True) + + # Create initial shared cookie + sc = SimpleCookie() + sc["shared"] = "value1" + sc["shared"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Filter to populate cache + filtered1 = jar.filter_cookies(URL("http://example.com/")) + assert filtered1["shared"].value == "value1" + assert "shared" in jar._morsel_cache[("", "")] + + # Update the cookie with new value + sc2 = SimpleCookie() + sc2["shared"] = "value2" + sc2["shared"]["path"] = "/" + jar.update_cookies(sc2, URL()) + + # Verify cache was cleared + assert "shared" not in jar._morsel_cache[("", "")] + + # Filter again to verify new value + filtered2 = jar.filter_cookies(URL("http://example.com/")) + assert filtered2["shared"].value == "value2" + + # Verify cache was repopulated with new value + assert "shared" in jar._morsel_cache[("", "")] + + +async def test_shared_cookie_cache_clearing_on_delete() -> None: + """Test that shared cookie cache is cleared when cookies are deleted.""" + jar = CookieJar(unsafe=True) + + # Create multiple shared cookies + sc = SimpleCookie() + sc["shared1"] = "value1" + sc["shared1"]["path"] = "/" + sc["shared2"] = "value2" + sc["shared2"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Filter to populate cache + jar.filter_cookies(URL("http://example.com/")) + assert "shared1" in jar._morsel_cache[("", "")] + assert "shared2" in jar._morsel_cache[("", "")] + + # Delete one cookie using internal method + jar._delete_cookies([("", "", "shared1")]) + + # Verify cookie and its cache entry were removed + assert "shared1" not in jar._cookies[("", "")] + assert "shared1" not in jar._morsel_cache[("", "")] + + # Verify other cookie remains + assert "shared2" in jar._cookies[("", "")] + assert "shared2" in jar._morsel_cache[("", "")] + + +async def test_shared_cookie_cache_clearing_on_clear() -> None: + """Test that shared cookie cache is cleared when jar is cleared.""" + jar = CookieJar(unsafe=True) + + # Create shared and domain-specific cookies + # Shared cookie + sc1 = SimpleCookie() + sc1["shared"] = "shared_value" + sc1["shared"]["path"] = "/" + jar.update_cookies(sc1, URL()) + + # Domain-specific cookie + sc2 = SimpleCookie() + sc2["domain_cookie"] = "domain_value" + jar.update_cookies(sc2, URL("http://example.com/")) + + # Filter to populate caches + jar.filter_cookies(URL("http://example.com/")) + + # Verify caches are populated + assert ("", "") in jar._morsel_cache + assert "shared" in jar._morsel_cache[("", "")] + assert ("example.com", "") in jar._morsel_cache + assert "domain_cookie" in jar._morsel_cache[("example.com", "")] + + # Clear all cookies + jar.clear() + + # Verify all caches are cleared + assert len(jar._morsel_cache) == 0 + assert len(jar._cookies) == 0 + + # Verify filtering returns no cookies + filtered = jar.filter_cookies(URL("http://example.com/")) + assert len(filtered) == 0 + + +async def test_shared_cookie_with_multiple_domains() -> None: + """Test that shared cookies work across different domains.""" + jar = CookieJar(unsafe=True) + + # Create a truly shared cookie + sc = SimpleCookie() + sc["universal"] = "everywhere" + sc["universal"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Test filtering for different domains + domains = [ + "http://example.com/", + "http://test.org/", + "http://localhost/", + "http://192.168.1.1/", # IP address (requires unsafe=True) + ] + + for domain_url in domains: + filtered = jar.filter_cookies(URL(domain_url)) + assert "universal" in filtered + assert filtered["universal"].value == "everywhere" + + # Verify cache is reused efficiently + assert ("", "") in jar._morsel_cache + assert "universal" in jar._morsel_cache[("", "")] diff --git a/tests/test_web_request.py b/tests/test_web_request.py index 6c9e3826d73..758b8b1f98a 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -303,6 +303,157 @@ def test_request_cookie__set_item() -> None: req.cookies["my"] = "value" +def test_request_cookies_with_special_characters() -> None: + """Test that cookies with special characters in names are accepted. + + This tests the fix for issue #2683 where cookies with special characters + like {, }, / in their names would cause a 500 error. The fix makes the + cookie parser more tolerant to handle real-world cookies. + """ + # Test cookie names with curly braces (e.g., ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}) + headers = CIMultiDict(COOKIE="{test}=value1; normal=value2") + req = make_mocked_request("GET", "/", headers=headers) + # Both cookies should be parsed successfully + assert req.cookies == {"{test}": "value1", "normal": "value2"} + + # Test cookie names with forward slash + headers = CIMultiDict(COOKIE="test/name=value1; valid=value2") + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test/name": "value1", "valid": "value2"} + + # Test cookie names with various special characters + headers = CIMultiDict( + COOKIE="test{foo}bar=value1; test/path=value2; normal_cookie=value3" + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "test{foo}bar": "value1", + "test/path": "value2", + "normal_cookie": "value3", + } + + +def test_request_cookies_real_world_examples() -> None: + """Test handling of real-world cookie examples from issue #2683.""" + # Example from the issue: ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E} + headers = CIMultiDict( + COOKIE="ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}=val1; normal_cookie=val2" + ) + req = make_mocked_request("GET", "/", headers=headers) + # All cookies should be parsed successfully + assert req.cookies == { + "ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}": "val1", + "normal_cookie": "val2", + } + + # Multiple cookies with special characters + headers = CIMultiDict( + COOKIE="{cookie1}=val1; cookie/2=val2; cookie[3]=val3; cookie(4)=val4" + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "{cookie1}": "val1", + "cookie/2": "val2", + "cookie[3]": "val3", + "cookie(4)": "val4", + } + + +def test_request_cookies_edge_cases() -> None: + """Test edge cases for cookie parsing.""" + # Empty cookie value + headers = CIMultiDict(COOKIE="test=; normal=value") + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test": "", "normal": "value"} + + # Cookie with quoted value + headers = CIMultiDict(COOKIE='test="quoted value"; normal=unquoted') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test": "quoted value", "normal": "unquoted"} + + +def test_request_cookies_no_500_error() -> None: + """Test that cookies with special characters don't cause 500 errors. + + This specifically tests that issue #2683 is fixed - previously cookies + with characters like { } would cause CookieError and 500 responses. + """ + # This cookie format previously caused 500 errors + headers = CIMultiDict(COOKIE="ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}=test") + + # Should not raise any exception when accessing cookies + req = make_mocked_request("GET", "/", headers=headers) + cookies = req.cookies # This used to raise CookieError + + # Verify the cookie was parsed successfully + assert "ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}" in cookies + assert cookies["ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}"] == "test" + + +def test_request_cookies_quoted_values() -> None: + """Test that quoted cookie values are handled consistently. + + This tests the fix for issue #5397 where quoted cookie values were + handled inconsistently based on whether domain attributes were present. + The new parser should always unquote cookie values consistently. + """ + # Test simple quoted cookie value + headers = CIMultiDict(COOKIE='sess="quoted_value"') + req = make_mocked_request("GET", "/", headers=headers) + # Quotes should be removed consistently + assert req.cookies == {"sess": "quoted_value"} + + # Test quoted cookie with semicolon in value + headers = CIMultiDict(COOKIE='data="value;with;semicolons"') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"data": "value;with;semicolons"} + + # Test mixed quoted and unquoted cookies + headers = CIMultiDict( + COOKIE='quoted="value1"; unquoted=value2; also_quoted="value3"' + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "quoted": "value1", + "unquoted": "value2", + "also_quoted": "value3", + } + + # Test escaped quotes in cookie value + headers = CIMultiDict(COOKIE=r'escaped="value with \" quote"') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"escaped": 'value with " quote'} + + # Test empty quoted value + headers = CIMultiDict(COOKIE='empty=""') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"empty": ""} + + +def test_request_cookies_with_attributes() -> None: + """Test that cookie attributes don't affect value parsing. + + Related to issue #5397 - ensures that the presence of domain or other + attributes doesn't change how cookie values are parsed. + """ + # Cookie with domain attribute - quotes should still be removed + headers = CIMultiDict(COOKIE='sess="quoted_value"; Domain=.example.com') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"sess": "quoted_value"} + + # Cookie with multiple attributes + headers = CIMultiDict(COOKIE='token="abc123"; Path=/; Secure; HttpOnly') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"token": "abc123"} + + # Multiple cookies with different attributes + headers = CIMultiDict( + COOKIE='c1="v1"; Domain=.example.com; c2="v2"; Path=/api; c3=v3; Secure' + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"c1": "v1", "c2": "v2", "c3": "v3"} + + def test_match_info() -> None: req = make_mocked_request("GET", "/") assert req._match_info is req.match_info From 741cb61dfa3e70af0609599c272ec80a2f9eed7d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 03:58:01 -0500 Subject: [PATCH 09/11] PR #11112/8edec63 backport][3.12] Fix cookie parsing issues (#11117) --- CHANGES/11112.bugfix.rst | 8 + CHANGES/2683.bugfix.rst | 1 + CHANGES/5397.bugfix.rst | 1 + CHANGES/7993.bugfix.rst | 1 + aiohttp/_cookie_helpers.py | 221 +++++++ aiohttp/abc.py | 26 +- aiohttp/client_reqrep.py | 21 +- aiohttp/cookiejar.py | 45 +- aiohttp/web_request.py | 9 +- docs/spelling_wordlist.txt | 1 + pyproject.toml | 2 +- tests/test_client_functional.py | 38 +- tests/test_client_request.py | 53 ++ tests/test_cookie_helpers.py | 1031 +++++++++++++++++++++++++++++++ tests/test_cookiejar.py | 225 ++++++- tests/test_web_request.py | 151 +++++ 16 files changed, 1769 insertions(+), 65 deletions(-) create mode 100644 CHANGES/11112.bugfix.rst create mode 120000 CHANGES/2683.bugfix.rst create mode 120000 CHANGES/5397.bugfix.rst create mode 120000 CHANGES/7993.bugfix.rst create mode 100644 aiohttp/_cookie_helpers.py create mode 100644 tests/test_cookie_helpers.py diff --git a/CHANGES/11112.bugfix.rst b/CHANGES/11112.bugfix.rst new file mode 100644 index 00000000000..6edea1c9b23 --- /dev/null +++ b/CHANGES/11112.bugfix.rst @@ -0,0 +1,8 @@ +Fixed cookie parsing to be more lenient when handling cookies with special characters +in names or values. Cookies with characters like ``{``, ``}``, and ``/`` in names are now +accepted instead of causing a :exc:`~http.cookies.CookieError` and 500 errors. Additionally, +cookies with mismatched quotes in values are now parsed correctly, and quoted cookie +values are now handled consistently whether or not they include special attributes +like ``Domain``. Also fixed :class:`~aiohttp.CookieJar` to ensure shared cookies (domain="", path="") +respect the ``quote_cookie`` parameter, making cookie quoting behavior consistent for +all cookies -- by :user:`bdraco`. diff --git a/CHANGES/2683.bugfix.rst b/CHANGES/2683.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/2683.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/5397.bugfix.rst b/CHANGES/5397.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/5397.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/7993.bugfix.rst b/CHANGES/7993.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/7993.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/aiohttp/_cookie_helpers.py b/aiohttp/_cookie_helpers.py new file mode 100644 index 00000000000..8184cc9bdc1 --- /dev/null +++ b/aiohttp/_cookie_helpers.py @@ -0,0 +1,221 @@ +""" +Internal cookie handling helpers. + +This module contains internal utilities for cookie parsing and manipulation. +These are not part of the public API and may change without notice. +""" + +import re +import sys +from http.cookies import Morsel +from typing import List, Optional, Sequence, Tuple, cast + +from .log import internal_logger + +__all__ = ("parse_cookie_headers", "preserve_morsel_with_coded_value") + +# Cookie parsing constants +# Allow more characters in cookie names to handle real-world cookies +# that don't strictly follow RFC standards (fixes #2683) +# RFC 6265 defines cookie-name token as per RFC 2616 Section 2.2, +# but many servers send cookies with characters like {} [] () etc. +# This makes the cookie parser more tolerant of real-world cookies +# while still providing some validation to catch obviously malformed names. +_COOKIE_NAME_RE = re.compile(r"^[!#$%&\'()*+\-./0-9:<=>?@A-Z\[\]^_`a-z{|}~]+$") +_COOKIE_KNOWN_ATTRS = frozenset( # AKA Morsel._reserved + ( + "path", + "domain", + "max-age", + "expires", + "secure", + "httponly", + "samesite", + "partitioned", + "version", + "comment", + ) +) +_COOKIE_BOOL_ATTRS = frozenset( # AKA Morsel._flags + ("secure", "httponly", "partitioned") +) + +# SimpleCookie's pattern for parsing cookies with relaxed validation +# Based on http.cookies pattern but extended to allow more characters in cookie names +# to handle real-world cookies (fixes #2683) +_COOKIE_PATTERN = re.compile( + r""" + \s* # Optional whitespace at start of cookie + (?P # Start of group 'key' + # aiohttp has extended to include [] for compatibility with real-world cookies + [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]+? # Any word of at least one letter + ) # End of group 'key' + ( # Optional group: there may not be a value. + \s*=\s* # Equal Sign + (?P # Start of group 'val' + "(?:[^\\"]|\\.)*" # Any double-quoted string (properly closed) + | # or + "[^";]* # Unmatched opening quote (differs from SimpleCookie - issue #7993) + | # or + # Special case for "expires" attr - RFC 822, RFC 850, RFC 1036, RFC 1123 + (\w{3,6}day|\w{3}),\s # Day of the week or abbreviated day (with comma) + [\w\d\s-]{9,11}\s[\d:]{8}\s # Date and time in specific format + (GMT|[+-]\d{4}) # Timezone: GMT or RFC 2822 offset like -0000, +0100 + # NOTE: RFC 2822 timezone support is an aiohttp extension + # for issue #4493 - SimpleCookie does NOT support this + | # or + # ANSI C asctime() format: "Wed Jun 9 10:18:14 2021" + # NOTE: This is an aiohttp extension for issue #4327 - SimpleCookie does NOT support this format + \w{3}\s+\w{3}\s+[\s\d]\d\s+\d{2}:\d{2}:\d{2}\s+\d{4} + | # or + [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]* # Any word or empty string + ) # End of group 'val' + )? # End of optional value group + \s* # Any number of spaces. + (\s+|;|$) # Ending either at space, semicolon, or EOS. + """, + re.VERBOSE | re.ASCII, +) + + +def preserve_morsel_with_coded_value(cookie: Morsel[str]) -> Morsel[str]: + """ + Preserve a Morsel's coded_value exactly as received from the server. + + This function ensures that cookie encoding is preserved exactly as sent by + the server, which is critical for compatibility with old servers that have + strict requirements about cookie formats. + + This addresses the issue described in https://github.com/aio-libs/aiohttp/pull/1453 + where Python's SimpleCookie would re-encode cookies, breaking authentication + with certain servers. + + Args: + cookie: A Morsel object from SimpleCookie + + Returns: + A Morsel object with preserved coded_value + + """ + mrsl_val = cast("Morsel[str]", cookie.get(cookie.key, Morsel())) + # We use __setstate__ instead of the public set() API because it allows us to + # bypass validation and set already validated state. This is more stable than + # setting protected attributes directly and unlikely to change since it would + # break pickling. + mrsl_val.__setstate__( # type: ignore[attr-defined] + {"key": cookie.key, "value": cookie.value, "coded_value": cookie.coded_value} + ) + return mrsl_val + + +def _unquote(text: str) -> str: + """ + Unquote a cookie value. + + Vendored from http.cookies._unquote to ensure compatibility. + """ + # If there are no quotes, return as-is + if len(text) < 2 or text[0] != '"' or text[-1] != '"': + return text + # Remove quotes and handle escaped characters + text = text[1:-1] + # Replace escaped quotes and backslashes + text = text.replace('\\"', '"').replace("\\\\", "\\") + return text + + +def parse_cookie_headers(headers: Sequence[str]) -> List[Tuple[str, Morsel[str]]]: + """ + Parse cookie headers using a vendored version of SimpleCookie parsing. + + This implementation is based on SimpleCookie.__parse_string to ensure + compatibility with how SimpleCookie parses cookies, including handling + of malformed cookies with missing semicolons. + + This function is used for both Cookie and Set-Cookie headers in order to be + forgiving. Ideally we would have followed RFC 6265 Section 5.2 (for Cookie + headers) and RFC 6265 Section 4.2.1 (for Set-Cookie headers), but the + real world data makes it impossible since we need to be a bit more forgiving. + + NOTE: This implementation differs from SimpleCookie in handling unmatched quotes. + SimpleCookie will stop parsing when it encounters a cookie value with an unmatched + quote (e.g., 'cookie="value'), causing subsequent cookies to be silently dropped. + This implementation handles unmatched quotes more gracefully to prevent cookie loss. + See https://github.com/aio-libs/aiohttp/issues/7993 + """ + parsed_cookies: List[Tuple[str, Morsel[str]]] = [] + + for header in headers: + if not header: + continue + + # Parse cookie string using SimpleCookie's algorithm + i = 0 + n = len(header) + current_morsel: Optional[Morsel[str]] = None + morsel_seen = False + + while 0 <= i < n: + # Start looking for a cookie + match = _COOKIE_PATTERN.match(header, i) + if not match: + # No more cookies + break + + key, value = match.group("key"), match.group("val") + i = match.end(0) + lower_key = key.lower() + + if key[0] == "$": + if not morsel_seen: + # We ignore attributes which pertain to the cookie + # mechanism as a whole, such as "$Version". + continue + # Process as attribute + if current_morsel is not None: + attr_lower_key = lower_key[1:] + if attr_lower_key in _COOKIE_KNOWN_ATTRS: + current_morsel[attr_lower_key] = value or "" + elif lower_key in _COOKIE_KNOWN_ATTRS: + if not morsel_seen: + # Invalid cookie string - attribute before cookie + break + if lower_key in _COOKIE_BOOL_ATTRS: + # Boolean attribute with any value should be True + if current_morsel is not None: + if lower_key == "partitioned" and sys.version_info < (3, 14): + dict.__setitem__(current_morsel, lower_key, True) + else: + current_morsel[lower_key] = True + elif value is None: + # Invalid cookie string - non-boolean attribute without value + break + elif current_morsel is not None: + # Regular attribute with value + current_morsel[lower_key] = _unquote(value) + elif value is not None: + # This is a cookie name=value pair + # Validate the name + if key in _COOKIE_KNOWN_ATTRS or not _COOKIE_NAME_RE.match(key): + internal_logger.warning( + "Can not load cookies: Illegal cookie name %r", key + ) + current_morsel = None + else: + # Create new morsel + current_morsel = Morsel() + # Preserve the original value as coded_value (with quotes if present) + # We use __setstate__ instead of the public set() API because it allows us to + # bypass validation and set already validated state. This is more stable than + # setting protected attributes directly and unlikely to change since it would + # break pickling. + current_morsel.__setstate__( # type: ignore[attr-defined] + {"key": key, "value": _unquote(value), "coded_value": value} + ) + parsed_cookies.append((key, current_morsel)) + morsel_seen = True + else: + # Invalid cookie string - no value for non-attribute + break + + return parsed_cookies diff --git a/aiohttp/abc.py b/aiohttp/abc.py index 353c18be266..ba371c61b01 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -3,7 +3,7 @@ import socket from abc import ABC, abstractmethod from collections.abc import Sized -from http.cookies import BaseCookie, CookieError, Morsel, SimpleCookie +from http.cookies import BaseCookie, Morsel from typing import ( TYPE_CHECKING, Any, @@ -23,7 +23,7 @@ from multidict import CIMultiDict from yarl import URL -from .log import client_logger +from ._cookie_helpers import parse_cookie_headers from .typedefs import LooseCookies if TYPE_CHECKING: @@ -197,26 +197,8 @@ def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> No def update_cookies_from_headers( self, headers: Sequence[str], response_url: URL ) -> None: - """ - Update cookies from raw Set-Cookie headers. - - Default implementation parses each header separately to preserve - cookies with same name but different domain/path. - """ - # Default implementation for backward compatibility - cookies_to_update: List[Tuple[str, Morsel[str]]] = [] - for cookie_header in headers: - tmp_cookie = SimpleCookie() - try: - tmp_cookie.load(cookie_header) - # Collect all cookies as tuples (name, morsel) - for name, morsel in tmp_cookie.items(): - cookies_to_update.append((name, morsel)) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) - - # Update all cookies at once for efficiency - if cookies_to_update: + """Update cookies from raw Set-Cookie headers.""" + if headers and (cookies_to_update := parse_cookie_headers(headers)): self.update_cookies(cookies_to_update, response_url) @abstractmethod diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 01835260cc5..793864b95a5 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -9,7 +9,7 @@ import warnings from collections.abc import Mapping from hashlib import md5, sha1, sha256 -from http.cookies import CookieError, Morsel, SimpleCookie +from http.cookies import Morsel, SimpleCookie from types import MappingProxyType, TracebackType from typing import ( TYPE_CHECKING, @@ -31,6 +31,7 @@ from yarl import URL from . import hdrs, helpers, http, multipart, payload +from ._cookie_helpers import parse_cookie_headers, preserve_morsel_with_coded_value from .abc import AbstractStreamWriter from .client_exceptions import ( ClientConnectionError, @@ -62,7 +63,6 @@ HttpVersion11, StreamWriter, ) -from .log import client_logger from .streams import StreamReader from .typedefs import ( DEFAULT_JSON_DECODER, @@ -376,11 +376,9 @@ def cookies(self) -> SimpleCookie: if self._raw_cookie_headers is not None: # Parse cookies for response.cookies (SimpleCookie for backward compatibility) cookies = SimpleCookie() - for hdr in self._raw_cookie_headers: - try: - cookies.load(hdr) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) + # Use parse_cookie_headers for more lenient parsing that handles + # malformed cookies better than SimpleCookie.load + cookies.update(parse_cookie_headers(self._raw_cookie_headers)) self._cookies = cookies else: self._cookies = SimpleCookie() @@ -1095,7 +1093,8 @@ def update_cookies(self, cookies: Optional[LooseCookies]) -> None: c = SimpleCookie() if hdrs.COOKIE in self.headers: - c.load(self.headers.get(hdrs.COOKIE, "")) + # parse_cookie_headers already preserves coded values + c.update(parse_cookie_headers((self.headers.get(hdrs.COOKIE, ""),))) del self.headers[hdrs.COOKIE] if isinstance(cookies, Mapping): @@ -1104,10 +1103,8 @@ def update_cookies(self, cookies: Optional[LooseCookies]) -> None: iter_cookies = cookies # type: ignore[assignment] for name, value in iter_cookies: if isinstance(value, Morsel): - # Preserve coded_value - mrsl_val = value.get(value.key, Morsel()) - mrsl_val.set(value.key, value.value, value.coded_value) - c[name] = mrsl_val + # Use helper to preserve coded_value exactly as sent by server + c[name] = preserve_morsel_with_coded_value(value) else: c[name] = value # type: ignore[assignment] diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index a755a893409..193648d4309 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -23,11 +23,11 @@ Set, Tuple, Union, - cast, ) from yarl import URL +from ._cookie_helpers import preserve_morsel_with_coded_value from .abc import AbstractCookieJar, ClearCookiePredicate from .helpers import is_ip_address from .typedefs import LooseCookies, PathLike, StrOrURL @@ -45,6 +45,7 @@ # the expiration heap. This is a performance optimization to avoid cleaning up the # heap too often when there are only a few scheduled expirations. _MIN_SCHEDULED_COOKIE_EXPIRATION = 100 +_SIMPLE_COOKIE = SimpleCookie() class CookieJar(AbstractCookieJar): @@ -304,9 +305,10 @@ def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> No def filter_cookies(self, request_url: URL = URL()) -> "BaseCookie[str]": """Returns this jar's cookies filtered by their attributes.""" - filtered: Union[SimpleCookie, "BaseCookie[str]"] = ( - SimpleCookie() if self._quote_cookie else BaseCookie() - ) + # We always use BaseCookie now since all + # cookies set on on filtered are fully constructed + # Morsels, not just names and values. + filtered: BaseCookie[str] = BaseCookie() if not self._cookies: # Skip do_expiration() if there are no cookies. return filtered @@ -332,8 +334,17 @@ def filter_cookies(self, request_url: URL = URL()) -> "BaseCookie[str]": is_not_secure = request_origin not in self._treat_as_secure_origin # Send shared cookie - for c in self._cookies[("", "")].values(): - filtered[c.key] = c.value + key = ("", "") + for c in self._cookies[key].values(): + # Check cache first + if c.key in self._morsel_cache[key]: + filtered[c.key] = self._morsel_cache[key][c.key] + continue + + # Build and cache the morsel + mrsl_val = self._build_morsel(c) + self._morsel_cache[key][c.key] = mrsl_val + filtered[c.key] = mrsl_val if is_ip_address(hostname): if not self._unsafe: @@ -373,15 +384,29 @@ def filter_cookies(self, request_url: URL = URL()) -> "BaseCookie[str]": filtered[name] = self._morsel_cache[p][name] continue - # It's critical we use the Morsel so the coded_value - # (based on cookie version) is preserved - mrsl_val = cast("Morsel[str]", cookie.get(cookie.key, Morsel())) - mrsl_val.set(cookie.key, cookie.value, cookie.coded_value) + # Build and cache the morsel + mrsl_val = self._build_morsel(cookie) self._morsel_cache[p][name] = mrsl_val filtered[name] = mrsl_val return filtered + def _build_morsel(self, cookie: Morsel[str]) -> Morsel[str]: + """Build a morsel for sending, respecting quote_cookie setting.""" + if self._quote_cookie and cookie.coded_value and cookie.coded_value[0] == '"': + return preserve_morsel_with_coded_value(cookie) + morsel: Morsel[str] = Morsel() + if self._quote_cookie: + value, coded_value = _SIMPLE_COOKIE.value_encode(cookie.value) + else: + coded_value = value = cookie.value + # We use __setstate__ instead of the public set() API because it allows us to + # bypass validation and set already validated state. This is more stable than + # setting protected attributes directly and unlikely to change since it would + # break pickling. + morsel.__setstate__({"key": cookie.key, "value": value, "coded_value": coded_value}) # type: ignore[attr-defined] + return morsel + @staticmethod def _is_domain_match(domain: str, hostname: str) -> bool: """Implements domain matching adhering to RFC 6265.""" diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 6bf5a9dea74..0c5576823f1 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -7,7 +7,6 @@ import tempfile import types import warnings -from http.cookies import SimpleCookie from types import MappingProxyType from typing import ( TYPE_CHECKING, @@ -36,6 +35,7 @@ from yarl import URL from . import hdrs +from ._cookie_helpers import parse_cookie_headers from .abc import AbstractStreamWriter from .helpers import ( _SENTINEL, @@ -589,9 +589,10 @@ def cookies(self) -> Mapping[str, str]: A read-only dictionary-like object. """ - raw = self.headers.get(hdrs.COOKIE, "") - parsed = SimpleCookie(raw) - return MappingProxyType({key: val.value for key, val in parsed.items()}) + # Use parse_cookie_headers for more lenient parsing that accepts + # special characters in cookie names (fixes #2683) + parsed = parse_cookie_headers((self.headers.get(hdrs.COOKIE, ""),)) + return MappingProxyType({name: morsel.value for name, morsel in parsed}) @reify def http_range(self) -> slice: diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 8b389cc11f6..b495a07cb6f 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -336,6 +336,7 @@ toplevel towncrier tp tuples +ue UI un unawaited diff --git a/pyproject.toml b/pyproject.toml index 3ef37b5978b..df8b8465348 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,7 +82,7 @@ skip = "pp*" [tool.codespell] skip = '.git,*.pdf,*.svg,Makefile,CONTRIBUTORS.txt,venvs,_build' -ignore-words-list = 'te' +ignore-words-list = 'te,ue' [tool.slotscheck] # TODO(3.13): Remove aiohttp.helpers once https://github.com/python/cpython/pull/106771 diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index ca1a7dd1d6b..5c18178b714 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -5,6 +5,7 @@ import http.cookies import io import json +import logging import pathlib import socket import ssl @@ -2691,15 +2692,16 @@ async def handler(request): assert 200 == resp.status -async def test_set_cookies(aiohttp_client) -> None: - async def handler(request): +async def test_set_cookies( + aiohttp_client: AiohttpClient, caplog: pytest.LogCaptureFixture +) -> None: + async def handler(request: web.Request) -> web.Response: ret = web.Response() ret.set_cookie("c1", "cookie1") ret.set_cookie("c2", "cookie2") ret.headers.add( "Set-Cookie", - "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" - "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "invalid,cookie=value; " # Comma character is not allowed "HttpOnly; Path=/", ) return ret @@ -2708,14 +2710,38 @@ async def handler(request): app.router.add_get("/", handler) client = await aiohttp_client(app) - with mock.patch("aiohttp.client_reqrep.client_logger") as m_log: + with caplog.at_level(logging.WARNING): async with client.get("/") as resp: assert 200 == resp.status cookie_names = {c.key for c in client.session.cookie_jar} _ = resp.cookies assert cookie_names == {"c1", "c2"} - m_log.warning.assert_called_with("Can not load response cookies: %s", mock.ANY) + assert "Can not load cookies: Illegal cookie name 'invalid,cookie'" in caplog.text + + +async def test_set_cookies_with_curly_braces(aiohttp_client: AiohttpClient) -> None: + """Test that cookies with curly braces in names are now accepted (#2683).""" + + async def handler(request: web.Request) -> web.Response: + ret = web.Response() + ret.set_cookie("c1", "cookie1") + ret.headers.add( + "Set-Cookie", + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", + ) + return ret + + app = web.Application() + app.router.add_get("/", handler) + client = await aiohttp_client(app) + + async with client.get("/") as resp: + assert 200 == resp.status + cookie_names = {c.key for c in client.session.cookie_jar} + assert cookie_names == {"c1", "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}"} async def test_set_cookies_expired(aiohttp_client) -> None: diff --git a/tests/test_client_request.py b/tests/test_client_request.py index b3eb55d921b..2af540599f8 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -612,6 +612,59 @@ def test_gen_netloc_no_port(make_request) -> None: ) +def test_cookie_coded_value_preserved(loop: asyncio.AbstractEventLoop) -> None: + """Verify the coded value of a cookie is preserved.""" + # https://github.com/aio-libs/aiohttp/pull/1453 + req = ClientRequest("get", URL("http://python.org"), loop=loop) + req.update_cookies(cookies=SimpleCookie('ip-cookie="second"; Domain=127.0.0.1;')) + assert req.headers["COOKIE"] == 'ip-cookie="second"' + + +def test_update_cookies_with_special_chars_in_existing_header( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that update_cookies handles existing cookies with special characters.""" + # Create request with a cookie that has special characters (real-world example) + req = ClientRequest( + "get", + URL("http://python.org"), + headers={"Cookie": "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1"}, + loop=loop, + ) + + # Update with another cookie + req.update_cookies(cookies={"normal_cookie": "value2"}) + + # Both cookies should be preserved in the exact order + assert ( + req.headers["COOKIE"] + == "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1; normal_cookie=value2" + ) + + +def test_update_cookies_with_quoted_existing_header( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that update_cookies handles existing cookies with quoted values.""" + # Create request with cookies that have quoted values + req = ClientRequest( + "get", + URL("http://python.org"), + headers={"Cookie": 'session="value;with;semicolon"; token=abc123'}, + loop=loop, + ) + + # Update with another cookie + req.update_cookies(cookies={"new_cookie": "new_value"}) + + # All cookies should be preserved with their original coded values + # The quoted value should be preserved as-is + assert ( + req.headers["COOKIE"] + == 'new_cookie=new_value; session="value;with;semicolon"; token=abc123' + ) + + async def test_connection_header( loop: asyncio.AbstractEventLoop, conn: mock.Mock ) -> None: diff --git a/tests/test_cookie_helpers.py b/tests/test_cookie_helpers.py new file mode 100644 index 00000000000..7a2ac7493ee --- /dev/null +++ b/tests/test_cookie_helpers.py @@ -0,0 +1,1031 @@ +"""Tests for internal cookie helper functions.""" + +from http.cookies import CookieError, Morsel, SimpleCookie + +import pytest + +from aiohttp import _cookie_helpers as helpers +from aiohttp._cookie_helpers import ( + parse_cookie_headers, + preserve_morsel_with_coded_value, +) + + +def test_known_attrs_is_superset_of_morsel_reserved() -> None: + """Test that _COOKIE_KNOWN_ATTRS contains all Morsel._reserved attributes.""" + # Get Morsel._reserved attributes (lowercase) + morsel_reserved = {attr.lower() for attr in Morsel._reserved} # type: ignore[attr-defined] + + # _COOKIE_KNOWN_ATTRS should be a superset of morsel_reserved + assert ( + helpers._COOKIE_KNOWN_ATTRS >= morsel_reserved + ), f"_COOKIE_KNOWN_ATTRS is missing: {morsel_reserved - helpers._COOKIE_KNOWN_ATTRS}" + + +def test_bool_attrs_is_superset_of_morsel_flags() -> None: + """Test that _COOKIE_BOOL_ATTRS contains all Morsel._flags attributes.""" + # Get Morsel._flags attributes (lowercase) + morsel_flags = {attr.lower() for attr in Morsel._flags} # type: ignore[attr-defined] + + # _COOKIE_BOOL_ATTRS should be a superset of morsel_flags + assert ( + helpers._COOKIE_BOOL_ATTRS >= morsel_flags + ), f"_COOKIE_BOOL_ATTRS is missing: {morsel_flags - helpers._COOKIE_BOOL_ATTRS}" + + +def test_preserve_morsel_with_coded_value() -> None: + """Test preserve_morsel_with_coded_value preserves coded_value exactly.""" + # Create a cookie with a coded_value different from value + cookie: Morsel[str] = Morsel() + cookie.set("test_cookie", "decoded value", "encoded%20value") + + # Preserve the coded_value + result = preserve_morsel_with_coded_value(cookie) + + # Check that all values are preserved + assert result.key == "test_cookie" + assert result.value == "decoded value" + assert result.coded_value == "encoded%20value" + + # Should be a different Morsel instance + assert result is not cookie + + +def test_preserve_morsel_with_coded_value_no_coded_value() -> None: + """Test preserve_morsel_with_coded_value when coded_value is same as value.""" + cookie: Morsel[str] = Morsel() + cookie.set("test_cookie", "simple_value", "simple_value") + + result = preserve_morsel_with_coded_value(cookie) + + assert result.key == "test_cookie" + assert result.value == "simple_value" + assert result.coded_value == "simple_value" + + +def test_parse_cookie_headers_simple() -> None: + """Test parse_cookie_headers with simple cookies.""" + headers = ["name=value", "session=abc123"] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + assert result[0][0] == "name" + assert result[0][1].key == "name" + assert result[0][1].value == "value" + assert result[1][0] == "session" + assert result[1][1].key == "session" + assert result[1][1].value == "abc123" + + +def test_parse_cookie_headers_with_attributes() -> None: + """Test parse_cookie_headers with cookie attributes.""" + headers = [ + "sessionid=value123; Path=/; HttpOnly; Secure", + "user=john; Domain=.example.com; Max-Age=3600", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + name1, morsel1 = result[0] + assert name1 == "sessionid" + assert morsel1.value == "value123" + assert morsel1["path"] == "/" + assert morsel1["httponly"] is True + assert morsel1["secure"] is True + + # Second cookie + name2, morsel2 = result[1] + assert name2 == "user" + assert morsel2.value == "john" + assert morsel2["domain"] == ".example.com" + assert morsel2["max-age"] == "3600" + + +def test_parse_cookie_headers_special_chars_in_names() -> None: + """Test parse_cookie_headers accepts special characters in names (#2683).""" + # These should be accepted with relaxed validation + headers = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1", + "cookie[index]=value2", + "cookie(param)=value3", + "cookie:name=value4", + "cookie@domain=value5", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 5 + expected_names = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}", + "cookie[index]", + "cookie(param)", + "cookie:name", + "cookie@domain", + ] + + for i, (name, morsel) in enumerate(result): + assert name == expected_names[i] + assert morsel.key == expected_names[i] + assert morsel.value == f"value{i+1}" + + +def test_parse_cookie_headers_invalid_names() -> None: + """Test parse_cookie_headers rejects truly invalid cookie names.""" + # These should be rejected even with relaxed validation + headers = [ + "invalid\tcookie=value", # Tab character + "invalid\ncookie=value", # Newline + "invalid\rcookie=value", # Carriage return + "\x00badname=value", # Null character + "name with spaces=value", # Spaces in name + ] + + result = parse_cookie_headers(headers) + + # All should be skipped + assert len(result) == 0 + + +def test_parse_cookie_headers_empty_and_invalid() -> None: + """Test parse_cookie_headers handles empty and invalid formats.""" + headers = [ + "", # Empty header + " ", # Whitespace only + "=value", # No name + "name=", # Empty value (should be accepted) + "justname", # No value (should be skipped) + "path=/", # Reserved attribute as name (should be skipped) + "Domain=.com", # Reserved attribute as name (should be skipped) + ] + + result = parse_cookie_headers(headers) + + # Only "name=" should be accepted + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1].value == "" + + +def test_parse_cookie_headers_quoted_values() -> None: + """Test parse_cookie_headers handles quoted values correctly.""" + headers = [ + 'name="quoted value"', + 'session="with;semicolon"', + 'data="with\\"escaped\\""', + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + assert result[0][1].value == "quoted value" + assert result[1][1].value == "with;semicolon" + assert result[2][1].value == 'with"escaped"' + + +@pytest.mark.parametrize( + "header", + [ + 'session="abc;xyz"; token=123', + 'data="value;with;multiple;semicolons"; next=cookie', + 'complex="a=b;c=d"; simple=value', + ], +) +def test_parse_cookie_headers_semicolon_in_quoted_values(header: str) -> None: + """ + Test that semicolons inside properly quoted values are handled correctly. + + Cookie values can contain semicolons when properly quoted. This test ensures + that our parser handles these cases correctly, matching SimpleCookie behavior. + """ + # Test with SimpleCookie + sc = SimpleCookie() + sc.load(header) + + # Test with our parser + result = parse_cookie_headers([header]) + + # Should parse the same number of cookies + assert len(result) == len(sc) + + # Verify each cookie matches SimpleCookie + for (name, morsel), (sc_name, sc_morsel) in zip(result, sc.items()): + assert name == sc_name + assert morsel.value == sc_morsel.value + + +def test_parse_cookie_headers_multiple_cookies_same_header() -> None: + """Test parse_cookie_headers with multiple cookies in one header.""" + # Note: SimpleCookie includes the comma as part of the first cookie's value + headers = ["cookie1=value1, cookie2=value2"] + + result = parse_cookie_headers(headers) + + # Should parse as two separate cookies + assert len(result) == 2 + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1," # Comma is included in the value + assert result[1][0] == "cookie2" + assert result[1][1].value == "value2" + + +@pytest.mark.parametrize( + "header", + [ + # Standard cookies + "session=abc123", + "user=john; Path=/", + "token=xyz; Secure; HttpOnly", + # Empty values + "empty=", + # Quoted values + 'quoted="value with spaces"', + # Multiple attributes + "complex=value; Domain=.example.com; Path=/app; Max-Age=3600", + ], +) +def test_parse_cookie_headers_compatibility_with_simple_cookie(header: str) -> None: + """Test parse_cookie_headers is bug-for-bug compatible with SimpleCookie.load.""" + # Parse with SimpleCookie + sc = SimpleCookie() + sc.load(header) + + # Parse with our function + result = parse_cookie_headers([header]) + + # Should have same number of cookies + assert len(result) == len(sc) + + # Compare each cookie + for name, morsel in result: + assert name in sc + sc_morsel = sc[name] + + # Compare values + assert morsel.value == sc_morsel.value + assert morsel.key == sc_morsel.key + + # Compare attributes (only those that SimpleCookie would set) + for attr in ["path", "domain", "max-age"]: + assert morsel.get(attr) == sc_morsel.get(attr) + + # Boolean attributes are handled differently + # SimpleCookie sets them to empty string when not present, True when present + for bool_attr in ["secure", "httponly"]: + # Only check if SimpleCookie has the attribute set to True + if sc_morsel.get(bool_attr) is True: + assert morsel.get(bool_attr) is True + + +def test_parse_cookie_headers_relaxed_validation_differences() -> None: + """Test where parse_cookie_headers differs from SimpleCookie (relaxed validation).""" + # Test cookies that SimpleCookie rejects with CookieError + rejected_by_simplecookie = [ + ("cookie{with}braces=value1", "cookie{with}braces", "value1"), + ("cookie(with)parens=value3", "cookie(with)parens", "value3"), + ("cookie@with@at=value5", "cookie@with@at", "value5"), + ] + + for header, expected_name, expected_value in rejected_by_simplecookie: + # SimpleCookie should reject these with CookieError + sc = SimpleCookie() + with pytest.raises(CookieError): + sc.load(header) + + # Our parser should accept them + result = parse_cookie_headers([header]) + assert len(result) == 1 # We accept + assert result[0][0] == expected_name + assert result[0][1].value == expected_value + + # Test cookies that SimpleCookie accepts (but we handle more consistently) + accepted_by_simplecookie = [ + ("cookie[with]brackets=value2", "cookie[with]brackets", "value2"), + ("cookie:with:colons=value4", "cookie:with:colons", "value4"), + ] + + for header, expected_name, expected_value in accepted_by_simplecookie: + # SimpleCookie accepts these + sc = SimpleCookie() + sc.load(header) + # May or may not parse correctly in SimpleCookie + + # Our parser should accept them consistently + result = parse_cookie_headers([header]) + assert len(result) == 1 + assert result[0][0] == expected_name + assert result[0][1].value == expected_value + + +def test_parse_cookie_headers_case_insensitive_attrs() -> None: + """Test that known attributes are handled case-insensitively.""" + headers = [ + "cookie1=value1; PATH=/test; DOMAIN=example.com", + "cookie2=value2; Secure; HTTPONLY; max-AGE=60", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie - attributes should be recognized despite case + assert result[0][1]["path"] == "/test" + assert result[0][1]["domain"] == "example.com" + + # Second cookie + assert result[1][1]["secure"] is True + assert result[1][1]["httponly"] is True + assert result[1][1]["max-age"] == "60" + + +def test_parse_cookie_headers_unknown_attrs_ignored() -> None: + """Test that unknown attributes are treated as new cookies (same as SimpleCookie).""" + headers = [ + "cookie=value; Path=/; unknownattr=ignored; HttpOnly", + ] + + result = parse_cookie_headers(headers) + + # SimpleCookie treats unknown attributes with values as new cookies + assert len(result) == 2 + + # First cookie + assert result[0][0] == "cookie" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] == "" # Not set on first cookie + + # Second cookie (the unknown attribute) + assert result[1][0] == "unknownattr" + assert result[1][1].value == "ignored" + assert result[1][1]["httponly"] is True # HttpOnly applies to this cookie + + +def test_parse_cookie_headers_complex_real_world() -> None: + """Test parse_cookie_headers with complex real-world examples.""" + headers = [ + # AWS ELB cookie + "AWSELB=ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890; Path=/", + # Google Analytics + "_ga=GA1.2.1234567890.1234567890; Domain=.example.com; Path=/; Expires=Thu, 31-Dec-2025 23:59:59 GMT", + # Session with all attributes + "session_id=s%3AabcXYZ123.signature123; Path=/; Secure; HttpOnly; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + + # Check each cookie parsed correctly + assert result[0][0] == "AWSELB" + assert result[1][0] == "_ga" + assert result[2][0] == "session_id" + + # Session cookie should have all attributes + session_morsel = result[2][1] + assert session_morsel["secure"] is True + assert session_morsel["httponly"] is True + assert session_morsel.get("samesite") == "Strict" + + +def test_parse_cookie_headers_boolean_attrs() -> None: + """Test that boolean attributes (secure, httponly) work correctly.""" + # Test secure attribute variations + headers = [ + "cookie1=value1; Secure", + "cookie2=value2; Secure=", + "cookie3=value3; Secure=true", # Non-standard but might occur + ] + + result = parse_cookie_headers(headers) + assert len(result) == 3 + + # All should have secure=True + for name, morsel in result: + assert morsel.get("secure") is True, f"{name} should have secure=True" + + # Test httponly attribute variations + headers = [ + "cookie4=value4; HttpOnly", + "cookie5=value5; HttpOnly=", + ] + + result = parse_cookie_headers(headers) + assert len(result) == 2 + + # All should have httponly=True + for name, morsel in result: + assert morsel.get("httponly") is True, f"{name} should have httponly=True" + + +def test_parse_cookie_headers_boolean_attrs_with_partitioned() -> None: + """Test that boolean attributes including partitioned work correctly.""" + # Test secure attribute variations + secure_headers = [ + "cookie1=value1; Secure", + "cookie2=value2; Secure=", + "cookie3=value3; Secure=true", # Non-standard but might occur + ] + + result = parse_cookie_headers(secure_headers) + assert len(result) == 3 + for name, morsel in result: + assert morsel.get("secure") is True, f"{name} should have secure=True" + + # Test httponly attribute variations + httponly_headers = [ + "cookie4=value4; HttpOnly", + "cookie5=value5; HttpOnly=", + ] + + result = parse_cookie_headers(httponly_headers) + assert len(result) == 2 + for name, morsel in result: + assert morsel.get("httponly") is True, f"{name} should have httponly=True" + + # Test partitioned attribute variations + partitioned_headers = [ + "cookie6=value6; Partitioned", + "cookie7=value7; Partitioned=", + "cookie8=value8; Partitioned=yes", # Non-standard but might occur + ] + + result = parse_cookie_headers(partitioned_headers) + assert len(result) == 3 + for name, morsel in result: + assert morsel.get("partitioned") is True, f"{name} should have partitioned=True" + + +def test_parse_cookie_headers_encoded_values() -> None: + """Test that parse_cookie_headers preserves encoded values.""" + headers = [ + "encoded=hello%20world", + "url=https%3A%2F%2Fexample.com%2Fpath", + "special=%21%40%23%24%25%5E%26*%28%29", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + # Values should be preserved as-is (not decoded) + assert result[0][1].value == "hello%20world" + assert result[1][1].value == "https%3A%2F%2Fexample.com%2Fpath" + assert result[2][1].value == "%21%40%23%24%25%5E%26*%28%29" + + +def test_parse_cookie_headers_partitioned() -> None: + """ + Test that parse_cookie_headers handles partitioned attribute correctly. + + This tests the fix for issue #10380 - partitioned cookies support. + The partitioned attribute is a boolean flag like secure and httponly. + """ + headers = [ + "cookie1=value1; Partitioned", + "cookie2=value2; Partitioned=", + "cookie3=value3; Partitioned=true", # Non-standard but might occur + "cookie4=value4; Secure; Partitioned; HttpOnly", + "cookie5=value5; Domain=.example.com; Path=/; Partitioned", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 5 + + # All cookies should have partitioned=True + for i, (name, morsel) in enumerate(result): + assert ( + morsel.get("partitioned") is True + ), f"Cookie {i+1} should have partitioned=True" + assert name == f"cookie{i+1}" + assert morsel.value == f"value{i+1}" + + # Cookie 4 should also have secure and httponly + assert result[3][1].get("secure") is True + assert result[3][1].get("httponly") is True + + # Cookie 5 should also have domain and path + assert result[4][1].get("domain") == ".example.com" + assert result[4][1].get("path") == "/" + + +def test_parse_cookie_headers_partitioned_case_insensitive() -> None: + """Test that partitioned attribute is recognized case-insensitively.""" + headers = [ + "cookie1=value1; partitioned", # lowercase + "cookie2=value2; PARTITIONED", # uppercase + "cookie3=value3; Partitioned", # title case + "cookie4=value4; PaRtItIoNeD", # mixed case + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 4 + + # All should be recognized as partitioned + for i, (_, morsel) in enumerate(result): + assert ( + morsel.get("partitioned") is True + ), f"Cookie {i+1} should have partitioned=True" + + +def test_parse_cookie_headers_partitioned_not_set() -> None: + """Test that cookies without partitioned attribute don't have it set.""" + headers = [ + "normal=value; Secure; HttpOnly", + "regular=cookie; Path=/", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # Check that partitioned is not set (empty string is the default for flags in Morsel) + assert result[0][1].get("partitioned", "") == "" + assert result[1][1].get("partitioned", "") == "" + + +# Tests that don't require partitioned support in SimpleCookie +def test_parse_cookie_headers_partitioned_with_other_attrs_manual() -> None: + """ + Test parsing logic for partitioned cookies combined with all other attributes. + + This test verifies our parsing logic handles partitioned correctly as a boolean + attribute regardless of SimpleCookie support. + """ + # Test that our parser recognizes partitioned in _COOKIE_KNOWN_ATTRS and _COOKIE_BOOL_ATTRS + assert "partitioned" in helpers._COOKIE_KNOWN_ATTRS + assert "partitioned" in helpers._COOKIE_BOOL_ATTRS + + # Test a simple case that won't trigger SimpleCookie errors + headers = ["session=abc123; Secure; HttpOnly"] + result = parse_cookie_headers(headers) + + assert len(result) == 1 + assert result[0][0] == "session" + assert result[0][1]["secure"] is True + assert result[0][1]["httponly"] is True + + +def test_cookie_helpers_constants_include_partitioned() -> None: + """Test that cookie helper constants include partitioned attribute.""" + # Test our constants include partitioned + assert "partitioned" in helpers._COOKIE_KNOWN_ATTRS + assert "partitioned" in helpers._COOKIE_BOOL_ATTRS + + +@pytest.mark.parametrize( + "test_string", + [ + " Partitioned ", + " partitioned ", + " PARTITIONED ", + " Partitioned; ", + " Partitioned= ", + " Partitioned=true ", + ], +) +def test_cookie_pattern_matches_partitioned_attribute(test_string: str) -> None: + """Test that the cookie pattern regex matches various partitioned attribute formats.""" + pattern = helpers._COOKIE_PATTERN + match = pattern.match(test_string) + assert match is not None, f"Pattern should match '{test_string}'" + assert match.group("key").lower() == "partitioned" + + +def test_parse_cookie_headers_issue_7993_double_quotes() -> None: + """ + Test that cookies with unmatched opening quotes don't break parsing of subsequent cookies. + + This reproduces issue #7993 where a cookie containing an unmatched opening double quote + causes subsequent cookies to be silently dropped. + NOTE: This only fixes the specific case where a value starts with a quote but doesn't + end with one (e.g., 'cookie="value'). Other malformed quote cases still behave like + SimpleCookie for compatibility. + """ + # Test case from the issue + headers = ['foo=bar; baz="qux; foo2=bar2'] + + result = parse_cookie_headers(headers) + + # Should parse all cookies correctly + assert len(result) == 3 + assert result[0][0] == "foo" + assert result[0][1].value == "bar" + assert result[1][0] == "baz" + assert result[1][1].value == '"qux' # Unmatched quote included + assert result[2][0] == "foo2" + assert result[2][1].value == "bar2" + + +def test_parse_cookie_headers_empty_headers() -> None: + """Test handling of empty headers in the sequence.""" + # Empty header should be skipped + result = parse_cookie_headers(["", "name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1].value == "value" + + # Multiple empty headers + result = parse_cookie_headers(["", "", ""]) + assert result == [] + + # Empty headers mixed with valid cookies + result = parse_cookie_headers(["", "a=1", "", "b=2", ""]) + assert len(result) == 2 + assert result[0][0] == "a" + assert result[1][0] == "b" + + +def test_parse_cookie_headers_invalid_cookie_syntax() -> None: + """Test handling of invalid cookie syntax.""" + # No valid cookie pattern + result = parse_cookie_headers(["@#$%^&*()"]) + assert result == [] + + # Cookie name without value + result = parse_cookie_headers(["name"]) + assert result == [] + + # Multiple invalid patterns + result = parse_cookie_headers(["!!!!", "????", "name", "@@@"]) + assert result == [] + + +def test_parse_cookie_headers_illegal_cookie_names( + caplog: pytest.LogCaptureFixture, +) -> None: + """ + Test that illegal cookie names are rejected. + + Note: When a known attribute name is used as a cookie name at the start, + parsing stops early (before any warning can be logged). Warnings are only + logged when illegal names appear after a valid cookie. + """ + # Cookie name that is a known attribute (illegal) - parsing stops early + result = parse_cookie_headers(["path=value; domain=test"]) + assert result == [] + + # Cookie name that doesn't match the pattern + result = parse_cookie_headers(["=value"]) + assert result == [] + + # Valid cookie after illegal one - parsing stops at illegal + result = parse_cookie_headers(["domain=bad; good=value"]) + assert result == [] + + # Illegal cookie name that appears after a valid cookie triggers warning + result = parse_cookie_headers(["good=value; Path=/; invalid,cookie=value;"]) + assert len(result) == 1 + assert result[0][0] == "good" + assert "Illegal cookie name 'invalid,cookie'" in caplog.text + + +def test_parse_cookie_headers_attributes_before_cookie() -> None: + """Test that attributes before any cookie are invalid.""" + # Path attribute before cookie + result = parse_cookie_headers(["Path=/; name=value"]) + assert result == [] + + # Domain attribute before cookie + result = parse_cookie_headers(["Domain=.example.com; name=value"]) + assert result == [] + + # Multiple attributes before cookie + result = parse_cookie_headers(["Path=/; Domain=.example.com; Secure; name=value"]) + assert result == [] + + +def test_parse_cookie_headers_attributes_without_values() -> None: + """Test handling of attributes with missing values.""" + # Boolean attribute without value (valid) + result = parse_cookie_headers(["name=value; Secure"]) + assert len(result) == 1 + assert result[0][1]["secure"] is True + + # Non-boolean attribute without value (invalid, stops parsing) + result = parse_cookie_headers(["name=value; Path"]) + assert len(result) == 1 + # Path without value stops further attribute parsing + + # Multiple cookies, invalid attribute in middle + result = parse_cookie_headers(["name=value; Path; Secure"]) + assert len(result) == 1 + # Secure is not parsed because Path without value stops parsing + + +def test_parse_cookie_headers_dollar_prefixed_names() -> None: + """Test handling of cookie names starting with $.""" + # $Version without preceding cookie (ignored) + result = parse_cookie_headers(["$Version=1; name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + + # Multiple $ prefixed without cookie (all ignored) + result = parse_cookie_headers(["$Version=1; $Path=/; $Domain=.com; name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + + # $ prefix at start is ignored, cookie follows + result = parse_cookie_headers(["$Unknown=123; valid=cookie"]) + assert len(result) == 1 + assert result[0][0] == "valid" + + +def test_parse_cookie_headers_dollar_attributes() -> None: + """Test handling of $ prefixed attributes after cookies.""" + # Test multiple $ attributes with cookie (case-insensitive like SimpleCookie) + result = parse_cookie_headers(["name=value; $Path=/test; $Domain=.example.com"]) + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1]["path"] == "/test" + assert result[0][1]["domain"] == ".example.com" + + # Test unknown $ attribute (should be ignored) + result = parse_cookie_headers(["name=value; $Unknown=test"]) + assert len(result) == 1 + assert result[0][0] == "name" + # $Unknown should not be set + + # Test $ attribute with empty value + result = parse_cookie_headers(["name=value; $Path="]) + assert len(result) == 1 + assert result[0][1]["path"] == "" + + # Test case sensitivity compatibility with SimpleCookie + result = parse_cookie_headers(["test=value; $path=/lower; $PATH=/upper"]) + assert len(result) == 1 + # Last one wins, and it's case-insensitive + assert result[0][1]["path"] == "/upper" + + +def test_parse_cookie_headers_attributes_after_illegal_cookie() -> None: + """ + Test that attributes after an illegal cookie name are handled correctly. + + This covers the branches where current_morsel is None because an illegal + cookie name was encountered. + """ + # Illegal cookie followed by $ attribute + result = parse_cookie_headers(["good=value; invalid,cookie=bad; $Path=/test"]) + assert len(result) == 1 + assert result[0][0] == "good" + # $Path should be ignored since current_morsel is None after illegal cookie + + # Illegal cookie followed by boolean attribute + result = parse_cookie_headers(["good=value; invalid,cookie=bad; HttpOnly"]) + assert len(result) == 1 + assert result[0][0] == "good" + # HttpOnly should be ignored since current_morsel is None + + # Illegal cookie followed by regular attribute with value + result = parse_cookie_headers(["good=value; invalid,cookie=bad; Max-Age=3600"]) + assert len(result) == 1 + assert result[0][0] == "good" + # Max-Age should be ignored since current_morsel is None + + # Multiple attributes after illegal cookie + result = parse_cookie_headers( + ["good=value; invalid,cookie=bad; $Path=/; HttpOnly; Max-Age=60; Domain=.com"] + ) + assert len(result) == 1 + assert result[0][0] == "good" + # All attributes should be ignored after illegal cookie + + +def test_parse_cookie_headers_unmatched_quotes_compatibility() -> None: + """ + Test that most unmatched quote scenarios behave like SimpleCookie. + + For compatibility, we only handle the specific case of unmatched opening quotes + (e.g., 'cookie="value'). Other cases behave the same as SimpleCookie. + """ + # Cases that SimpleCookie and our parser both fail to parse completely + incompatible_cases = [ + 'cookie1=val"ue; cookie2=value2', # codespell:ignore + 'cookie1=value"; cookie2=value2', + 'cookie1=va"l"ue"; cookie2=value2', # codespell:ignore + 'cookie1=value1; cookie2=val"ue; cookie3=value3', # codespell:ignore + ] + + for header in incompatible_cases: + # Test SimpleCookie behavior + sc = SimpleCookie() + sc.load(header) + sc_cookies = list(sc.items()) + + # Test our parser behavior + result = parse_cookie_headers([header]) + + # Both should parse the same cookies (partial parsing) + assert len(result) == len(sc_cookies), ( + f"Header: {header}\n" + f"SimpleCookie parsed: {len(sc_cookies)} cookies\n" + f"Our parser parsed: {len(result)} cookies" + ) + + # The case we specifically fix (unmatched opening quote) + fixed_case = 'cookie1=value1; cookie2="unmatched; cookie3=value3' + + # SimpleCookie fails to parse cookie3 + sc = SimpleCookie() + sc.load(fixed_case) + assert len(sc) == 1 # Only cookie1 + + # Our parser handles it better + result = parse_cookie_headers([fixed_case]) + assert len(result) == 3 # All three cookies + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1" + assert result[1][0] == "cookie2" + assert result[1][1].value == '"unmatched' + assert result[2][0] == "cookie3" + assert result[2][1].value == "value3" + + +def test_parse_cookie_headers_expires_attribute() -> None: + """Test parse_cookie_headers handles expires attribute with date formats.""" + headers = [ + "session=abc; Expires=Wed, 09 Jun 2021 10:18:14 GMT", + "user=xyz; expires=Wednesday, 09-Jun-21 10:18:14 GMT", + "token=123; EXPIRES=Wed, 09 Jun 2021 10:18:14 GMT", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + for _, morsel in result: + assert "expires" in morsel + assert "GMT" in morsel["expires"] + + +def test_parse_cookie_headers_edge_cases() -> None: + """Test various edge cases.""" + # Very long cookie values + long_value = "x" * 4096 + result = parse_cookie_headers([f"name={long_value}"]) + assert len(result) == 1 + assert result[0][1].value == long_value + + +def test_parse_cookie_headers_various_date_formats_issue_4327() -> None: + """ + Test that parse_cookie_headers handles various date formats per RFC 6265. + + This tests the fix for issue #4327 - support for RFC 822, RFC 850, + and ANSI C asctime() date formats in cookie expiration. + """ + # Test various date formats + headers = [ + # RFC 822 format (preferred format) + "cookie1=value1; Expires=Wed, 09 Jun 2021 10:18:14 GMT", + # RFC 850 format (obsolete but still used) + "cookie2=value2; Expires=Wednesday, 09-Jun-21 10:18:14 GMT", + # RFC 822 with dashes + "cookie3=value3; Expires=Wed, 09-Jun-2021 10:18:14 GMT", + # ANSI C asctime() format (aiohttp extension - not supported by SimpleCookie) + "cookie4=value4; Expires=Wed Jun 9 10:18:14 2021", + # Various other formats seen in the wild + "cookie5=value5; Expires=Thu, 01 Jan 2030 00:00:00 GMT", + "cookie6=value6; Expires=Mon, 31-Dec-99 23:59:59 GMT", + "cookie7=value7; Expires=Tue, 01-Jan-30 00:00:00 GMT", + ] + + result = parse_cookie_headers(headers) + + # All cookies should be parsed + assert len(result) == 7 + + # Check each cookie was parsed with its expires attribute + expected_cookies = [ + ("cookie1", "value1", "Wed, 09 Jun 2021 10:18:14 GMT"), + ("cookie2", "value2", "Wednesday, 09-Jun-21 10:18:14 GMT"), + ("cookie3", "value3", "Wed, 09-Jun-2021 10:18:14 GMT"), + ("cookie4", "value4", "Wed Jun 9 10:18:14 2021"), + ("cookie5", "value5", "Thu, 01 Jan 2030 00:00:00 GMT"), + ("cookie6", "value6", "Mon, 31-Dec-99 23:59:59 GMT"), + ("cookie7", "value7", "Tue, 01-Jan-30 00:00:00 GMT"), + ] + + for (name, morsel), (exp_name, exp_value, exp_expires) in zip( + result, expected_cookies + ): + assert name == exp_name + assert morsel.value == exp_value + assert morsel.get("expires") == exp_expires + + +def test_parse_cookie_headers_ansi_c_asctime_format() -> None: + """ + Test parsing of ANSI C asctime() format. + + This tests support for ANSI C asctime() format (e.g., "Wed Jun 9 10:18:14 2021"). + NOTE: This is an aiohttp extension - SimpleCookie does NOT support this format. + """ + headers = ["cookie1=value1; Expires=Wed Jun 9 10:18:14 2021"] + + result = parse_cookie_headers(headers) + + # Should parse correctly with the expires attribute preserved + assert len(result) == 1 + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1" + assert result[0][1]["expires"] == "Wed Jun 9 10:18:14 2021" + + +def test_parse_cookie_headers_rfc2822_timezone_issue_4493() -> None: + """ + Test that parse_cookie_headers handles RFC 2822 timezone formats. + + This tests the fix for issue #4493 - support for RFC 2822-compliant dates + with timezone offsets like -0000, +0100, etc. + NOTE: This is an aiohttp extension - SimpleCookie does NOT support this format. + """ + headers = [ + # RFC 2822 with -0000 timezone (common in some APIs) + "hello=world; expires=Wed, 15 Jan 2020 09:45:07 -0000", + # RFC 2822 with positive offset + "session=abc123; expires=Thu, 01 Feb 2024 14:30:00 +0100", + # RFC 2822 with negative offset + "token=xyz789; expires=Fri, 02 Mar 2025 08:15:30 -0500", + # Standard GMT for comparison + "classic=cookie; expires=Sat, 03 Apr 2026 12:00:00 GMT", + ] + + result = parse_cookie_headers(headers) + + # All cookies should be parsed + assert len(result) == 4 + + # Check each cookie was parsed with its expires attribute + assert result[0][0] == "hello" + assert result[0][1].value == "world" + assert result[0][1]["expires"] == "Wed, 15 Jan 2020 09:45:07 -0000" + + assert result[1][0] == "session" + assert result[1][1].value == "abc123" + assert result[1][1]["expires"] == "Thu, 01 Feb 2024 14:30:00 +0100" + + assert result[2][0] == "token" + assert result[2][1].value == "xyz789" + assert result[2][1]["expires"] == "Fri, 02 Mar 2025 08:15:30 -0500" + + assert result[3][0] == "classic" + assert result[3][1].value == "cookie" + assert result[3][1]["expires"] == "Sat, 03 Apr 2026 12:00:00 GMT" + + +def test_parse_cookie_headers_rfc2822_with_attributes() -> None: + """Test that RFC 2822 dates work correctly with other cookie attributes.""" + headers = [ + "session=abc123; expires=Wed, 15 Jan 2020 09:45:07 -0000; Path=/; HttpOnly; Secure", + "token=xyz789; expires=Thu, 01 Feb 2024 14:30:00 +0100; Domain=.example.com; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + assert result[0][0] == "session" + assert result[0][1].value == "abc123" + assert result[0][1]["expires"] == "Wed, 15 Jan 2020 09:45:07 -0000" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] is True + assert result[0][1]["secure"] is True + + # Second cookie + assert result[1][0] == "token" + assert result[1][1].value == "xyz789" + assert result[1][1]["expires"] == "Thu, 01 Feb 2024 14:30:00 +0100" + assert result[1][1]["domain"] == ".example.com" + assert result[1][1]["samesite"] == "Strict" + + +def test_parse_cookie_headers_date_formats_with_attributes() -> None: + """Test that date formats work correctly with other cookie attributes.""" + headers = [ + "session=abc123; Expires=Wed, 09 Jun 2030 10:18:14 GMT; Path=/; HttpOnly; Secure", + "token=xyz789; Expires=Wednesday, 09-Jun-30 10:18:14 GMT; Domain=.example.com; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + assert result[0][0] == "session" + assert result[0][1].value == "abc123" + assert result[0][1]["expires"] == "Wed, 09 Jun 2030 10:18:14 GMT" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] is True + assert result[0][1]["secure"] is True + + # Second cookie + assert result[1][0] == "token" + assert result[1][1].value == "xyz789" + assert result[1][1]["expires"] == "Wednesday, 09-Jun-30 10:18:14 GMT" + assert result[1][1]["domain"] == ".example.com" + assert result[1][1]["samesite"] == "Strict" diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index e1b6e351e3d..15557085b4e 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -371,6 +371,8 @@ async def test_domain_filter_ip_cookie_receive(cookies_to_receive) -> None: ("custom-cookie=value/one;", 'Cookie: custom-cookie="value/one"', True), ("custom-cookie=value1;", "Cookie: custom-cookie=value1", True), ("custom-cookie=value/one;", "Cookie: custom-cookie=value/one", False), + ('foo="quoted_value"', 'Cookie: foo="quoted_value"', True), + ('foo="quoted_value"; domain=127.0.0.1', 'Cookie: foo="quoted_value"', True), ], ids=( "IP domain preserved", @@ -378,6 +380,8 @@ async def test_domain_filter_ip_cookie_receive(cookies_to_receive) -> None: "quoted cookie with special char", "quoted cookie w/o special char", "unquoted cookie with special char", + "pre-quoted cookie", + "pre-quoted cookie with domain", ), ) async def test_quotes_correctly_based_on_input( @@ -1225,7 +1229,7 @@ async def test_update_cookies_from_headers_duplicate_names() -> None: url: URL = URL("http://www.example.com/") # Headers with duplicate names but different domains - headers: List[str] = [ + headers = [ "session-id=123456; Domain=.example.com; Path=/", "session-id=789012; Domain=.www.example.com; Path=/", "user-pref=light; Domain=.example.com", @@ -1255,11 +1259,10 @@ async def test_update_cookies_from_headers_invalid_cookies( url: URL = URL("http://example.com/") # Mix of valid and invalid cookies - headers: List[str] = [ + headers = [ "valid-cookie=value123", - "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" - "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " - "HttpOnly; Path=/", # This cookie with curly braces causes CookieError + "invalid,cookie=value; " # Comma character is not allowed + "HttpOnly; Path=/", "another-valid=value456", ] @@ -1268,7 +1271,7 @@ async def test_update_cookies_from_headers_invalid_cookies( jar.update_cookies_from_headers(headers, url) # Check that we logged warnings for invalid cookies - assert "Can not load response cookies" in caplog.text + assert "Can not load cookies" in caplog.text # Valid cookies should still be added assert len(jar) >= 2 # At least the two clearly valid cookies @@ -1277,6 +1280,52 @@ async def test_update_cookies_from_headers_invalid_cookies( assert "another-valid" in filtered +async def test_update_cookies_from_headers_with_curly_braces() -> None: + """Test that cookies with curly braces in names are now accepted (#2683).""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Cookie names with curly braces should now be accepted + headers = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", + "regular-cookie=value123", + ] + + jar.update_cookies_from_headers(headers, url) + + # Both cookies should be added + assert len(jar) == 2 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}" in filtered + assert "regular-cookie" in filtered + + +async def test_update_cookies_from_headers_with_special_chars() -> None: + """Test that cookies with various special characters are accepted.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Various special characters that should now be accepted + headers = [ + "cookie_with_parens=(value)=test123", + "cookie-with-brackets[index]=value456", + "cookie@with@at=value789", + "cookie:with:colons=value000", + ] + + jar.update_cookies_from_headers(headers, url) + + # All cookies should be added + assert len(jar) == 4 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "cookie_with_parens" in filtered + assert "cookie-with-brackets[index]" in filtered + assert "cookie@with@at" in filtered + assert "cookie:with:colons" in filtered + + async def test_update_cookies_from_headers_empty_list() -> None: """Test that empty header list is handled gracefully.""" jar: CookieJar = CookieJar() @@ -1293,7 +1342,7 @@ async def test_update_cookies_from_headers_with_attributes() -> None: jar: CookieJar = CookieJar() url: URL = URL("https://secure.example.com/app/page") - headers: List[str] = [ + headers = [ "secure-cookie=value1; Secure; HttpOnly; SameSite=Strict", "expiring-cookie=value2; Max-Age=3600; Path=/app", "domain-cookie=value3; Domain=.example.com; Path=/", @@ -1348,7 +1397,7 @@ async def test_update_cookies_from_headers_preserves_existing() -> None: ) # Add more cookies via headers - headers: List[str] = [ + headers = [ "new-cookie1=value3", "new-cookie2=value4", ] @@ -1373,7 +1422,7 @@ async def test_update_cookies_from_headers_overwrites_same_cookie() -> None: jar.update_cookies({"session": "old-value"}, url) # Update with new value via headers - headers: List[str] = ["session=new-value"] + headers = ["session=new-value"] jar.update_cookies_from_headers(headers, url) # Should still have just 1 cookie with updated value @@ -1387,7 +1436,7 @@ async def test_dummy_cookie_jar_update_cookies_from_headers() -> None: jar: DummyCookieJar = DummyCookieJar() url: URL = URL("http://example.com/") - headers: List[str] = [ + headers = [ "cookie1=value1", "cookie2=value2", ] @@ -1398,3 +1447,159 @@ async def test_dummy_cookie_jar_update_cookies_from_headers() -> None: assert len(jar) == 0 filtered: BaseCookie[str] = jar.filter_cookies(url) assert len(filtered) == 0 + + +async def test_shared_cookie_cache_population() -> None: + """Test that shared cookies are cached correctly.""" + jar = CookieJar(unsafe=True) + + # Create a shared cookie (no domain/path restrictions) + sc = SimpleCookie() + sc["shared"] = "value" + sc["shared"]["path"] = "/" # Will be stripped to "" + + # Update with empty URL to avoid domain being set + jar.update_cookies(sc, URL()) + + # Verify cookie is stored at shared key + assert ("", "") in jar._cookies + assert "shared" in jar._cookies[("", "")] + + # Filter cookies to populate cache + filtered = jar.filter_cookies(URL("http://example.com/")) + assert "shared" in filtered + assert filtered["shared"].value == "value" + + # Verify cache was populated + assert ("", "") in jar._morsel_cache + assert "shared" in jar._morsel_cache[("", "")] + + # Verify the cached morsel is the same one returned + cached_morsel = jar._morsel_cache[("", "")]["shared"] + assert cached_morsel is filtered["shared"] + + +async def test_shared_cookie_cache_clearing_on_update() -> None: + """Test that shared cookie cache is cleared when cookie is updated.""" + jar = CookieJar(unsafe=True) + + # Create initial shared cookie + sc = SimpleCookie() + sc["shared"] = "value1" + sc["shared"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Filter to populate cache + filtered1 = jar.filter_cookies(URL("http://example.com/")) + assert filtered1["shared"].value == "value1" + assert "shared" in jar._morsel_cache[("", "")] + + # Update the cookie with new value + sc2 = SimpleCookie() + sc2["shared"] = "value2" + sc2["shared"]["path"] = "/" + jar.update_cookies(sc2, URL()) + + # Verify cache was cleared + assert "shared" not in jar._morsel_cache[("", "")] + + # Filter again to verify new value + filtered2 = jar.filter_cookies(URL("http://example.com/")) + assert filtered2["shared"].value == "value2" + + # Verify cache was repopulated with new value + assert "shared" in jar._morsel_cache[("", "")] + + +async def test_shared_cookie_cache_clearing_on_delete() -> None: + """Test that shared cookie cache is cleared when cookies are deleted.""" + jar = CookieJar(unsafe=True) + + # Create multiple shared cookies + sc = SimpleCookie() + sc["shared1"] = "value1" + sc["shared1"]["path"] = "/" + sc["shared2"] = "value2" + sc["shared2"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Filter to populate cache + jar.filter_cookies(URL("http://example.com/")) + assert "shared1" in jar._morsel_cache[("", "")] + assert "shared2" in jar._morsel_cache[("", "")] + + # Delete one cookie using internal method + jar._delete_cookies([("", "", "shared1")]) + + # Verify cookie and its cache entry were removed + assert "shared1" not in jar._cookies[("", "")] + assert "shared1" not in jar._morsel_cache[("", "")] + + # Verify other cookie remains + assert "shared2" in jar._cookies[("", "")] + assert "shared2" in jar._morsel_cache[("", "")] + + +async def test_shared_cookie_cache_clearing_on_clear() -> None: + """Test that shared cookie cache is cleared when jar is cleared.""" + jar = CookieJar(unsafe=True) + + # Create shared and domain-specific cookies + # Shared cookie + sc1 = SimpleCookie() + sc1["shared"] = "shared_value" + sc1["shared"]["path"] = "/" + jar.update_cookies(sc1, URL()) + + # Domain-specific cookie + sc2 = SimpleCookie() + sc2["domain_cookie"] = "domain_value" + jar.update_cookies(sc2, URL("http://example.com/")) + + # Filter to populate caches + jar.filter_cookies(URL("http://example.com/")) + + # Verify caches are populated + assert ("", "") in jar._morsel_cache + assert "shared" in jar._morsel_cache[("", "")] + assert ("example.com", "") in jar._morsel_cache + assert "domain_cookie" in jar._morsel_cache[("example.com", "")] + + # Clear all cookies + jar.clear() + + # Verify all caches are cleared + assert len(jar._morsel_cache) == 0 + assert len(jar._cookies) == 0 + + # Verify filtering returns no cookies + filtered = jar.filter_cookies(URL("http://example.com/")) + assert len(filtered) == 0 + + +async def test_shared_cookie_with_multiple_domains() -> None: + """Test that shared cookies work across different domains.""" + jar = CookieJar(unsafe=True) + + # Create a truly shared cookie + sc = SimpleCookie() + sc["universal"] = "everywhere" + sc["universal"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Test filtering for different domains + domains = [ + "http://example.com/", + "http://test.org/", + "http://localhost/", + "http://192.168.1.1/", # IP address (requires unsafe=True) + ] + + for domain_url in domains: + filtered = jar.filter_cookies(URL(domain_url)) + assert "universal" in filtered + assert filtered["universal"].value == "everywhere" + + # Verify cache is reused efficiently + assert ("", "") in jar._morsel_cache + assert "universal" in jar._morsel_cache[("", "")] diff --git a/tests/test_web_request.py b/tests/test_web_request.py index 6c9e3826d73..758b8b1f98a 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -303,6 +303,157 @@ def test_request_cookie__set_item() -> None: req.cookies["my"] = "value" +def test_request_cookies_with_special_characters() -> None: + """Test that cookies with special characters in names are accepted. + + This tests the fix for issue #2683 where cookies with special characters + like {, }, / in their names would cause a 500 error. The fix makes the + cookie parser more tolerant to handle real-world cookies. + """ + # Test cookie names with curly braces (e.g., ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}) + headers = CIMultiDict(COOKIE="{test}=value1; normal=value2") + req = make_mocked_request("GET", "/", headers=headers) + # Both cookies should be parsed successfully + assert req.cookies == {"{test}": "value1", "normal": "value2"} + + # Test cookie names with forward slash + headers = CIMultiDict(COOKIE="test/name=value1; valid=value2") + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test/name": "value1", "valid": "value2"} + + # Test cookie names with various special characters + headers = CIMultiDict( + COOKIE="test{foo}bar=value1; test/path=value2; normal_cookie=value3" + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "test{foo}bar": "value1", + "test/path": "value2", + "normal_cookie": "value3", + } + + +def test_request_cookies_real_world_examples() -> None: + """Test handling of real-world cookie examples from issue #2683.""" + # Example from the issue: ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E} + headers = CIMultiDict( + COOKIE="ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}=val1; normal_cookie=val2" + ) + req = make_mocked_request("GET", "/", headers=headers) + # All cookies should be parsed successfully + assert req.cookies == { + "ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}": "val1", + "normal_cookie": "val2", + } + + # Multiple cookies with special characters + headers = CIMultiDict( + COOKIE="{cookie1}=val1; cookie/2=val2; cookie[3]=val3; cookie(4)=val4" + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "{cookie1}": "val1", + "cookie/2": "val2", + "cookie[3]": "val3", + "cookie(4)": "val4", + } + + +def test_request_cookies_edge_cases() -> None: + """Test edge cases for cookie parsing.""" + # Empty cookie value + headers = CIMultiDict(COOKIE="test=; normal=value") + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test": "", "normal": "value"} + + # Cookie with quoted value + headers = CIMultiDict(COOKIE='test="quoted value"; normal=unquoted') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test": "quoted value", "normal": "unquoted"} + + +def test_request_cookies_no_500_error() -> None: + """Test that cookies with special characters don't cause 500 errors. + + This specifically tests that issue #2683 is fixed - previously cookies + with characters like { } would cause CookieError and 500 responses. + """ + # This cookie format previously caused 500 errors + headers = CIMultiDict(COOKIE="ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}=test") + + # Should not raise any exception when accessing cookies + req = make_mocked_request("GET", "/", headers=headers) + cookies = req.cookies # This used to raise CookieError + + # Verify the cookie was parsed successfully + assert "ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}" in cookies + assert cookies["ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}"] == "test" + + +def test_request_cookies_quoted_values() -> None: + """Test that quoted cookie values are handled consistently. + + This tests the fix for issue #5397 where quoted cookie values were + handled inconsistently based on whether domain attributes were present. + The new parser should always unquote cookie values consistently. + """ + # Test simple quoted cookie value + headers = CIMultiDict(COOKIE='sess="quoted_value"') + req = make_mocked_request("GET", "/", headers=headers) + # Quotes should be removed consistently + assert req.cookies == {"sess": "quoted_value"} + + # Test quoted cookie with semicolon in value + headers = CIMultiDict(COOKIE='data="value;with;semicolons"') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"data": "value;with;semicolons"} + + # Test mixed quoted and unquoted cookies + headers = CIMultiDict( + COOKIE='quoted="value1"; unquoted=value2; also_quoted="value3"' + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "quoted": "value1", + "unquoted": "value2", + "also_quoted": "value3", + } + + # Test escaped quotes in cookie value + headers = CIMultiDict(COOKIE=r'escaped="value with \" quote"') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"escaped": 'value with " quote'} + + # Test empty quoted value + headers = CIMultiDict(COOKIE='empty=""') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"empty": ""} + + +def test_request_cookies_with_attributes() -> None: + """Test that cookie attributes don't affect value parsing. + + Related to issue #5397 - ensures that the presence of domain or other + attributes doesn't change how cookie values are parsed. + """ + # Cookie with domain attribute - quotes should still be removed + headers = CIMultiDict(COOKIE='sess="quoted_value"; Domain=.example.com') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"sess": "quoted_value"} + + # Cookie with multiple attributes + headers = CIMultiDict(COOKIE='token="abc123"; Path=/; Secure; HttpOnly') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"token": "abc123"} + + # Multiple cookies with different attributes + headers = CIMultiDict( + COOKIE='c1="v1"; Domain=.example.com; c2="v2"; Path=/api; c3=v3; Secure' + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"c1": "v1", "c2": "v2", "c3": "v3"} + + def test_match_info() -> None: req = make_mocked_request("GET", "/") assert req._match_info is req.match_info From 5facb3d805fa71381efcd452a1bca2ec7a4fd3fa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 10:28:41 +0100 Subject: [PATCH 10/11] Release 3.12.7rc0 (#11119) --- CHANGES.rst | 62 ++++++++++++++++++++++++++++++++++++++++ CHANGES/11105.bugfix.rst | 10 ------- CHANGES/11106.bugfix.rst | 1 - CHANGES/11107.misc.rst | 1 - CHANGES/11112.bugfix.rst | 8 ------ CHANGES/11114.misc.rst | 1 - CHANGES/2683.bugfix.rst | 1 - CHANGES/4486.bugfix.rst | 1 - CHANGES/5397.bugfix.rst | 1 - CHANGES/7993.bugfix.rst | 1 - aiohttp/__init__.py | 2 +- 11 files changed, 63 insertions(+), 26 deletions(-) delete mode 100644 CHANGES/11105.bugfix.rst delete mode 120000 CHANGES/11106.bugfix.rst delete mode 100644 CHANGES/11107.misc.rst delete mode 100644 CHANGES/11112.bugfix.rst delete mode 100644 CHANGES/11114.misc.rst delete mode 120000 CHANGES/2683.bugfix.rst delete mode 120000 CHANGES/4486.bugfix.rst delete mode 120000 CHANGES/5397.bugfix.rst delete mode 120000 CHANGES/7993.bugfix.rst diff --git a/CHANGES.rst b/CHANGES.rst index 0e10454a3d1..b08deb3942c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,68 @@ .. towncrier release notes start +3.12.7rc0 (2025-06-02) +====================== + +Bug fixes +--------- + +- Fixed cookie parsing to be more lenient when handling cookies with special characters + in names or values. Cookies with characters like ``{``, ``}``, and ``/`` in names are now + accepted instead of causing a :exc:`~http.cookies.CookieError` and 500 errors. Additionally, + cookies with mismatched quotes in values are now parsed correctly, and quoted cookie + values are now handled consistently whether or not they include special attributes + like ``Domain``. Also fixed :class:`~aiohttp.CookieJar` to ensure shared cookies (domain="", path="") + respect the ``quote_cookie`` parameter, making cookie quoting behavior consistent for + all cookies -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`2683`, :issue:`5397`, :issue:`7993`, :issue:`11112`. + + + +- Fixed an issue where cookies with duplicate names but different domains or paths + were lost when updating the cookie jar. The :class:`~aiohttp.ClientSession` + cookie jar now correctly stores all cookies even if they have the same name but + different domain or path, following the :rfc:`6265#section-5.3` storage model -- by :user:`bdraco`. + + Note that :attr:`ClientResponse.cookies ` returns + a :class:`~http.cookies.SimpleCookie` which uses the cookie name as a key, so + only the last cookie with each name is accessible via this interface. All cookies + can be accessed via :meth:`ClientResponse.headers.getall('Set-Cookie') + ` if needed. + + + *Related issues and pull requests on GitHub:* + :issue:`4486`, :issue:`11105`, :issue:`11106`. + + + + +Miscellaneous internal changes +------------------------------ + +- Avoided creating closed futures in ``ResponseHandler`` that will never be awaited -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`11107`. + + + +- Downgraded the logging level for connector close errors from ERROR to DEBUG, as these are expected behavior with TLS 1.3 connections -- by :user:`bdraco`. + + + *Related issues and pull requests on GitHub:* + :issue:`11114`. + + + + +---- + + 3.12.6 (2025-05-31) =================== diff --git a/CHANGES/11105.bugfix.rst b/CHANGES/11105.bugfix.rst deleted file mode 100644 index 33578aa7a95..00000000000 --- a/CHANGES/11105.bugfix.rst +++ /dev/null @@ -1,10 +0,0 @@ -Fixed an issue where cookies with duplicate names but different domains or paths -were lost when updating the cookie jar. The :class:`~aiohttp.ClientSession` -cookie jar now correctly stores all cookies even if they have the same name but -different domain or path, following the :rfc:`6265#section-5.3` storage model -- by :user:`bdraco`. - -Note that :attr:`ClientResponse.cookies ` returns -a :class:`~http.cookies.SimpleCookie` which uses the cookie name as a key, so -only the last cookie with each name is accessible via this interface. All cookies -can be accessed via :meth:`ClientResponse.headers.getall('Set-Cookie') -` if needed. diff --git a/CHANGES/11106.bugfix.rst b/CHANGES/11106.bugfix.rst deleted file mode 120000 index 3e5efb0f3f3..00000000000 --- a/CHANGES/11106.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -11105.bugfix.rst \ No newline at end of file diff --git a/CHANGES/11107.misc.rst b/CHANGES/11107.misc.rst deleted file mode 100644 index 37ac4622bd9..00000000000 --- a/CHANGES/11107.misc.rst +++ /dev/null @@ -1 +0,0 @@ -Avoided creating closed futures in ``ResponseHandler`` that will never be awaited -- by :user:`bdraco`. diff --git a/CHANGES/11112.bugfix.rst b/CHANGES/11112.bugfix.rst deleted file mode 100644 index 6edea1c9b23..00000000000 --- a/CHANGES/11112.bugfix.rst +++ /dev/null @@ -1,8 +0,0 @@ -Fixed cookie parsing to be more lenient when handling cookies with special characters -in names or values. Cookies with characters like ``{``, ``}``, and ``/`` in names are now -accepted instead of causing a :exc:`~http.cookies.CookieError` and 500 errors. Additionally, -cookies with mismatched quotes in values are now parsed correctly, and quoted cookie -values are now handled consistently whether or not they include special attributes -like ``Domain``. Also fixed :class:`~aiohttp.CookieJar` to ensure shared cookies (domain="", path="") -respect the ``quote_cookie`` parameter, making cookie quoting behavior consistent for -all cookies -- by :user:`bdraco`. diff --git a/CHANGES/11114.misc.rst b/CHANGES/11114.misc.rst deleted file mode 100644 index 2fcb1468c67..00000000000 --- a/CHANGES/11114.misc.rst +++ /dev/null @@ -1 +0,0 @@ -Downgraded the logging level for connector close errors from ERROR to DEBUG, as these are expected behavior with TLS 1.3 connections -- by :user:`bdraco`. diff --git a/CHANGES/2683.bugfix.rst b/CHANGES/2683.bugfix.rst deleted file mode 120000 index fac3861027d..00000000000 --- a/CHANGES/2683.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/4486.bugfix.rst b/CHANGES/4486.bugfix.rst deleted file mode 120000 index 3e5efb0f3f3..00000000000 --- a/CHANGES/4486.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -11105.bugfix.rst \ No newline at end of file diff --git a/CHANGES/5397.bugfix.rst b/CHANGES/5397.bugfix.rst deleted file mode 120000 index fac3861027d..00000000000 --- a/CHANGES/5397.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/7993.bugfix.rst b/CHANGES/7993.bugfix.rst deleted file mode 120000 index fac3861027d..00000000000 --- a/CHANGES/7993.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -11112.bugfix.rst \ No newline at end of file diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index 78f22b4051f..b1e029241d7 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = "3.12.7.dev0" +__version__ = "3.12.7rc0" from typing import TYPE_CHECKING, Tuple From 80bb38fae175ff3fc7b47b94b5150b069cfbf6b6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 16:45:44 +0100 Subject: [PATCH 11/11] Release 3.12.7 (#11120) --- CHANGES.rst | 4 ++-- aiohttp/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index b08deb3942c..867cd041a55 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,8 +10,8 @@ .. towncrier release notes start -3.12.7rc0 (2025-06-02) -====================== +3.12.7 (2025-06-02) +=================== Bug fixes --------- diff --git a/aiohttp/__init__.py b/aiohttp/__init__.py index b1e029241d7..d4ba8ccb488 100644 --- a/aiohttp/__init__.py +++ b/aiohttp/__init__.py @@ -1,4 +1,4 @@ -__version__ = "3.12.7rc0" +__version__ = "3.12.7" from typing import TYPE_CHECKING, Tuple