[PR #11106/cfb9931 backport][3.13] Fix cookies with duplicate names being lost when updating cookie jar (#11109)

bdraco · web-flow · commit 1f02d7db7e13 · 2025-06-01T09:43:01.000-05:00
diff --git a/CHANGES/11105.bugfix.rst 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 <aiohttp.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')
+<multidict.MultiDictProxy.getall>` if needed.
diff --git a/CHANGES/11106.bugfix.rst b/CHANGES/11106.bugfix.rst
@@ -0,0 +1 @@
+11105.bugfix.rst
diff --git a/CHANGES/4486.bugfix.rst b/CHANGES/4486.bugfix.rst
@@ -0,0 +1 @@
+11105.bugfix.rst
diff --git 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
@@ -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
@@ -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
@@ -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') <multidict.MultiDictProxy.getall>`.
+
+         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
@@ -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
diff --git a/tests/test_client_session.py b/tests/test_client_session.py
diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py
