[PR aio-libs#12824/60b85e98 backport][3.15] Preserve host-only cookie scope across CookieJar save/load (aio-libs#12834)

Author: bdraco

CHANGES/12824.bugfix.rst

@@ -0,0 +1 @@
+Fixed :class:`~aiohttp.CookieJar` dropping the host-only flag of cookies when persisted with :meth:`~aiohttp.CookieJar.save` and reloaded with :meth:`~aiohttp.CookieJar.load`, so a cookie set without a ``Domain`` attribute is again scoped to the exact host that set it after a reload; the absolute expiration deadline is now persisted as well, so a reloaded cookie keeps its original lifetime instead of being rescheduled from the load time. :meth:`~aiohttp.CookieJar.load` now replaces the jar contents rather than merging onto prior state, and loaded cookies pass through the same acceptance rules as :meth:`~aiohttp.CookieJar.update_cookies`, so a cookie for an IP-address host is dropped when loaded into a jar created without ``unsafe=True`` -- by :user:`bdraco`.

aiohttp/cookiejar.py

@@ -39,6 +39,9 @@
 _MIN_SCHEDULED_COOKIE_EXPIRATION = 100
 _SIMPLE_COOKIE = SimpleCookie()
 
+# Not persisted; the absolute deadline is saved instead.
+_RELATIVE_EXPIRY_ATTRS = frozenset(("max-age", "expires"))
+
 
 class _RestrictedCookieUnpickler(pickle._Unpickler):
     """A restricted unpickler that only allows cookie-related types.
@@ -174,21 +177,28 @@ def save(self, file_path: PathLike) -> None:
             :class:`str` or :class:`pathlib.Path` instance.
         """
         file_path = pathlib.Path(file_path)
-        data: dict[str, dict[str, dict[str, str | bool]]] = {}
+        data: dict[str, dict[str, dict[str, str | bool | float]]] = {}
         for (domain, path), cookie in self._cookies.items():
             key = f"{domain}|{path}"
             data[key] = {}
             for name, morsel in cookie.items():
-                morsel_data: dict[str, str | bool] = {
+                morsel_data: dict[str, str | bool | float] = {
                     "key": morsel.key,
                     "value": morsel.value,
                     "coded_value": morsel.coded_value,
                 }
-                # Save all morsel attributes that have values
+                # Skip relative expiry; the absolute deadline is saved below.
                 for attr in morsel._reserved:  # type: ignore[attr-defined]
+                    if attr in _RELATIVE_EXPIRY_ATTRS:
+                        continue
                     attr_val = morsel[attr]
                     if attr_val:
                         morsel_data[attr] = attr_val
+                # Persist or it reloads as a domain cookie and leaks to subdomains.
+                if (domain, name) in self._host_only_cookies:
+                    morsel_data["host_only"] = True
+                if (exp := self._expirations.get((domain, path, name))) is not None:
+                    morsel_data["expires_timestamp"] = exp
                 data[key][name] = morsel_data
 
         # Cookie persistence may include authentication/session tokens.
@@ -209,6 +219,9 @@ def load(self, file_path: PathLike) -> None:
         pickle format (using a restricted unpickler) for backward
         compatibility with existing cookie files.
 
+        Replaces the current jar contents; loaded cookies pass through the
+        same acceptance rules as :meth:`update_cookies`.
+
         :param file_path: Path to file from where cookies will be
             imported, :class:`str` or :class:`pathlib.Path` instance.
         """
@@ -217,32 +230,28 @@ def load(self, file_path: PathLike) -> None:
         try:
             with file_path.open(mode="r", encoding="utf-8") as f:
                 data = json.load(f)
-            self._cookies = self._load_json_data(data)
+            self._load_json_data(data)
         except (json.JSONDecodeError, UnicodeDecodeError, ValueError):
             # Fall back to legacy pickle format with restricted unpickler
             with file_path.open(mode="rb") as f:
                 self._cookies = _RestrictedCookieUnpickler(f).load()
 
     def _load_json_data(
-        self, data: dict[str, dict[str, dict[str, str | bool]]]
-    ) -> defaultdict[tuple[str, str], SimpleCookie]:
-        """Load cookies from parsed JSON data."""
-        cookies: defaultdict[tuple[str, str], SimpleCookie] = defaultdict(SimpleCookie)
+        self, data: dict[str, dict[str, dict[str, str | bool | float]]]
+    ) -> None:
+        """Replace contents, routing cookies through update_cookies()."""
+        self.clear()
         for compound_key, cookie_data in data.items():
             domain, path = compound_key.split("|", 1)
-            key = (domain, path)
             for name, morsel_data in cookie_data.items():
                 morsel: Morsel[str] = Morsel()
-                morsel_key = morsel_data["key"]
-                morsel_value = morsel_data["value"]
-                morsel_coded_value = morsel_data["coded_value"]
                 # Use __setstate__ to bypass validation, same pattern
                 # used in _build_morsel and _cookie_helpers.
                 morsel.__setstate__(  # type: ignore[attr-defined]
                     {
-                        "key": morsel_key,
-                        "value": morsel_value,
-                        "coded_value": morsel_coded_value,
+                        "key": morsel_data["key"],
+                        "value": morsel_data["value"],
+                        "coded_value": morsel_data["coded_value"],
                     }
                 )
                 # Restore morsel attributes
@@ -253,8 +262,17 @@ def _load_json_data(
                         "coded_value",
                     ):
                         morsel[attr] = morsel_data[attr]
-                cookies[key][name] = morsel
-        return cookies
+                # Drop the domain so update_cookies() re-marks it host-only.
+                if morsel_data.get("host_only"):
+                    morsel["domain"] = ""
+                response_url = (
+                    URL.build(scheme="https", host=domain) if domain else URL()
+                )
+                self.update_cookies({name: morsel}, response_url)
+                # Restore the absolute deadline; update_cookies() schedules none.
+                if (exp := morsel_data.get("expires_timestamp")) is not None:
+                    self._expire_cookie(float(exp), domain, path, name)
+        self._do_expiration()
 
     def clear(self, predicate: ClearCookiePredicate | None = None) -> None:
         if predicate is None:

tests/test_cookiejar.py

@@ -2,6 +2,7 @@
 import datetime
 import heapq
 import itertools
+import json
 import logging
 import os
 import pathlib
@@ -1814,6 +1815,140 @@ async def test_save_load_json_partitioned_cookies(tmp_path: Path) -> None:
         assert s["path"] == lo["path"]
 
 
+async def test_save_load_json_preserves_host_only_scope(tmp_path: Path) -> None:
+    """Verify save/load keeps host-only cookies off subdomains."""
+    file_path = tmp_path / "host_only.json"
+    issuer = URL("https://auth.example.com/login")
+    subdomain = URL("https://sub.auth.example.com/")
+
+    jar_save = CookieJar()
+    jar_save.update_cookies({"sid": "hostonly"}, response_url=issuer)
+    assert "sid" not in jar_save.filter_cookies(subdomain)
+    jar_save.save(file_path=file_path)
+
+    jar_load = CookieJar()
+    jar_load.load(file_path=file_path)
+
+    assert jar_load.host_only_cookies == frozenset({("auth.example.com", "sid")})
+    assert "sid" not in jar_load.filter_cookies(subdomain)
+    assert "sid" in jar_load.filter_cookies(issuer)
+
+
+async def test_save_load_json_domain_cookie_still_matches_subdomain(
+    tmp_path: Path,
+) -> None:
+    """Verify save/load keeps an explicit Domain cookie valid for subdomains."""
+    file_path = tmp_path / "domain.json"
+    subdomain = URL("https://sub.example.com/")
+
+    jar_save = CookieJar()
+    jar_save.update_cookies_from_headers(
+        ["sid=domaincookie; Domain=example.com"], URL("https://example.com/")
+    )
+    jar_save.save(file_path=file_path)
+
+    jar_load = CookieJar()
+    jar_load.load(file_path=file_path)
+
+    assert jar_load.host_only_cookies == frozenset()
+    assert "sid" in jar_load.filter_cookies(subdomain)
+
+
+async def test_save_load_json_preserves_max_age_deadline(tmp_path: Path) -> None:
+    """Verify save/load restores the absolute deadline without resetting it."""
+    file_path = tmp_path / "max_age.json"
+    url = URL("https://example.com/")
+
+    jar_save = CookieJar()
+    jar_save.update_cookies_from_headers(
+        ["sid=x; Max-Age=3600; Domain=example.com"], url
+    )
+    expirations = dict(jar_save._expirations)
+    jar_save.save(file_path=file_path)
+
+    jar_load = CookieJar()
+    jar_load.load(file_path=file_path)
+
+    # The deadline is restored as the original absolute time, not now + Max-Age.
+    assert dict(jar_load._expirations) == expirations
+    assert "sid" in jar_load.filter_cookies(url)
+
+
+async def test_save_load_json_drops_expired_cookie(tmp_path: Path) -> None:
+    """Verify a cookie whose persisted deadline is in the past is dropped on load."""
+    file_path = tmp_path / "expired.json"
+    url = URL("https://example.com/")
+
+    # Save a future-expiring cookie, then rewrite its persisted deadline to the
+    # past so the cookie survives save() and the drop happens on the load path.
+    jar_save = CookieJar()
+    jar_save.update_cookies_from_headers(
+        ["sid=x; Expires=Tue, 1 Jan 2999 12:00:00 GMT; Domain=example.com"], url
+    )
+    jar_save.save(file_path=file_path)
+    data = json.loads(file_path.read_text())
+    _, cookies = next(iter(data.items()))
+    cookies["sid"]["expires_timestamp"] = 0.0
+    file_path.write_text(json.dumps(data))
+
+    jar_load = CookieJar()
+    jar_load.load(file_path=file_path)
+
+    assert len(jar_load) == 0
+    assert "sid" not in jar_load.filter_cookies(url)
+
+
+async def test_save_load_json_preserves_expires_deadline(tmp_path: Path) -> None:
+    """Verify a future Expires deadline survives a save/load roundtrip."""
+    file_path = tmp_path / "expires.json"
+    url = URL("https://example.com/")
+
+    jar_save = CookieJar()
+    jar_save.update_cookies_from_headers(
+        ["sid=x; Expires=Tue, 1 Jan 2999 12:00:00 GMT; Domain=example.com"], url
+    )
+    expirations = dict(jar_save._expirations)
+    jar_save.save(file_path=file_path)
+
+    jar_load = CookieJar()
+    jar_load.load(file_path=file_path)
+
+    assert dict(jar_load._expirations) == expirations
+    assert "sid" in jar_load.filter_cookies(url)
+
+
+async def test_load_json_old_format_without_new_keys(tmp_path: Path) -> None:
+    """Verify a file written by an older version (no host_only/expires_timestamp) loads."""
+    file_path = tmp_path / "old.json"
+    # Old schema: no host_only, no expires_timestamp; relative max-age morsel attr.
+    file_path.write_text(
+        json.dumps(
+            {
+                "example.com|/": {
+                    "sid": {
+                        "key": "sid",
+                        "value": "x",
+                        "coded_value": "x",
+                        "domain": "example.com",
+                        "max-age": "3600",
+                    }
+                }
+            }
+        )
+    )
+    url = URL("https://example.com/")
+
+    jar_load = CookieJar()
+    # No exception when the new keys are absent.
+    jar_load.load(file_path=file_path)
+
+    # A host-only cookie saved without Domain by an older version had no domain
+    # field, so it now loads as a domain cookie (the documented migration loss).
+    assert "sid" in jar_load.filter_cookies(url)
+    # max-age is rescheduled from load time rather than an absolute deadline.
+    assert any(key[2] == "sid" for key in jar_load._expirations)
+
+
 async def test_json_format_is_safe(tmp_path: Path) -> None:
     """Verify the JSON file format cannot execute code on load."""
     import json