diff --git a/CHANGES/13637.bugfix.rst b/CHANGES/13637.bugfix.rst new file mode 100644 index 00000000000..49a8ac91986 --- /dev/null +++ b/CHANGES/13637.bugfix.rst @@ -0,0 +1 @@ +Fixed ``CookieJar.update_cookies()`` to copy user-passed mutable ``Morsel`` objects -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index d4e474ab98a..a5a1052970d 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -10,14 +10,14 @@ import time import warnings from collections import defaultdict -from collections.abc import Iterable, Iterator, Mapping +from collections.abc import Iterable, Iterator, Mapping, Sequence from http.cookies import BaseCookie, Morsel, SimpleCookie from types import MappingProxyType -from typing import Union +from typing import Union, cast from yarl import URL -from ._cookie_helpers import preserve_morsel_with_coded_value +from ._cookie_helpers import parse_set_cookie_headers, preserve_morsel_with_coded_value from .abc import AbstractCookieJar, ClearCookiePredicate from .helpers import is_ip_address from .typedefs import LooseCookies, PathLike, StrOrURL @@ -324,6 +324,20 @@ def _expire_cookie(self, when: float, domain: str, path: str, name: str) -> None def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> None: """Update cookies.""" + self._update_cookies(cookies, response_url, copy_morsels=True) + + def update_cookies_from_headers( + self, headers: Sequence[str], response_url: URL + ) -> None: + """Update cookies from raw Set-Cookie headers.""" + if headers and (cookies_to_update := parse_set_cookie_headers(headers)): + # The freshly parsed Morsels are not shared with the caller, + # so they can be stored and normalized without a defensive copy. + self._update_cookies(cookies_to_update, response_url, copy_morsels=False) + + def _update_cookies( + self, cookies: LooseCookies, response_url: URL, *, copy_morsels: bool + ) -> None: hostname = response_url.raw_host if not self._unsafe and is_ip_address(hostname): @@ -338,6 +352,9 @@ def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> No tmp = SimpleCookie() tmp[name] = cookie # type: ignore[assignment] cookie = tmp[name] + elif copy_morsels: + # TODO(https://github.com/python/typeshed/pull/16346): Remove cast + cookie = cast("Morsel[str]", cookie.copy()) domain = cookie["domain"] diff --git a/requirements/constraints.txt b/requirements/constraints.txt index d669aec7ba8..d47666572c3 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -168,7 +168,7 @@ pip-tools==7.6.1 # via -r requirements/dev.in pkgconfig==1.6.0 # via -r requirements/test-common-base.in -platformdirs==4.11.5 +platformdirs==4.11.6 # via virtualenv pluggy==1.6.0 # via diff --git a/requirements/dev.txt b/requirements/dev.txt index 190ae2cfc3b..5df52c1016d 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -165,7 +165,7 @@ pip-tools==7.6.1 # via -r requirements/dev.in pkgconfig==1.6.0 # via -r requirements/test-common-base.in -platformdirs==4.11.5 +platformdirs==4.11.6 # via virtualenv pluggy==1.6.0 # via diff --git a/requirements/lint.txt b/requirements/lint.txt index 3158ce7b061..f32cf63b327 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -86,7 +86,7 @@ packaging==26.3 # via pytest pathspec==1.1.1 # via mypy -platformdirs==4.11.5 +platformdirs==4.11.6 # via virtualenv pluggy==1.6.0 # via pytest diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 9fa150a5d9a..d020305f3d3 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -166,11 +166,13 @@ async def test_constructor( ) -> None: jar = CookieJar() jar.update_cookies(cookies_to_send) - jar_cookies = SimpleCookie() - for cookie in jar: - dict.__setitem__(jar_cookies, cookie.key, cookie) - expected_cookies = cookies_to_send - assert jar_cookies == expected_cookies + jar_cookies = {cookie.key: cookie for cookie in jar} + assert jar_cookies.keys() == cookies_to_send.keys() + for name, expected in cookies_to_send.items(): + # The jar stores normalized copies, so only the parts that + # normalization must not touch are compared here. + assert jar_cookies[name].value == expected.value + assert jar_cookies[name].coded_value == expected.coded_value async def test_constructor_with_expired( @@ -204,7 +206,12 @@ def test_save_load( for cookie in jar_load: jar_test[cookie.key] = cookie - assert jar_test == cookies_to_receive + # The jar stores normalized copies of the received cookies, so the + # round-tripped contents are compared against the saved jar itself. + jar_expected = SimpleCookie() + for cookie in jar_save: + dict.__setitem__(jar_expected, cookie.key, cookie) + assert jar_test == jar_expected def test_save_load_partitioned_cookies(tmp_path: Path) -> None: @@ -1425,6 +1432,41 @@ def test_dummy_cookie_jar_update_cookies_from_headers() -> None: assert len(filtered) == 0 +def test_update_cookies_copies_caller_morsel() -> None: + """Test that mutating a Morsel after update_cookies() does not change the jar. + + https://github.com/aio-libs/aiohttp/issues/13634 + """ + jar = CookieJar() + url = URL("http://example.com/") + sc = SimpleCookie() + sc["auth"] = "original-value" + jar.update_cookies({"auth": sc["auth"]}, url) + + # Mutate the caller's Morsel after the jar has stored it. + sc["auth"].set("auth", "mutated-value", "mutated-value") + + assert jar.filter_cookies(url)["auth"].value == "original-value" + + +def test_update_cookies_does_not_mutate_caller_morsel() -> None: + """Test that update_cookies() normalization does not leak into the caller's Morsel. + + https://github.com/aio-libs/aiohttp/issues/13634 + """ + jar = CookieJar() + sc = SimpleCookie() + sc["sid"] = "value" + jar.update_cookies({"sid": sc["sid"]}, URL("http://example.com/sub/page")) + + # The jar normalizes its private copy, not the caller's object. + assert sc["sid"]["domain"] == "" + assert sc["sid"]["path"] == "" + + filtered = jar.filter_cookies(URL("http://example.com/sub/page")) + assert filtered["sid"].value == "value" + + async def test_shared_cookie_cache_population() -> None: """Test that shared cookies are cached correctly.""" jar = CookieJar(unsafe=True)