Loading saved CookieJar breaks in 3.8.4

[Traktormaster]

Describe the bug

I save the contents of a CookieJar to persist a session. Loading a file saved in <3.8.4 will not work in aiohttp 3.8.4 as expected. When I'd iterate through the cookies the expiration update will raise an unhandled exception.

This issue completely breaks the authentication flow and bricks the session of a user in my application.

To Reproduce

1. Create and save a simple CookieJar in 3.8.3:

from aiohttp import CookieJar
from http.cookies import SimpleCookie
cj = CookieJar()
cooks = SimpleCookie({"coo": "kie"})
cj.update_cookies(cooks)
cj.save("/tmp/aiohttp383s.cj")

2. Load it in 3.8.4 and trigger expiration-update by iterating over it:

from aiohttp import CookieJar
cj = CookieJar()
cj.load("/tmp/aiohttp383s.cj")
list(cj)

Expected behavior

I expected the save/load to work across versions, but at least when upgrading to a newer version. The documentation does not mention anything to be afraid of either.

Logs/tracebacks

Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages/aiohttp/cookiejar.py", line 147, in __len__
    return sum(1 for i in self)
  File "/home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages/aiohttp/cookiejar.py", line 147, in <genexpr>
    return sum(1 for i in self)
  File "/home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages/aiohttp/cookiejar.py", line 142, in __iter__
    self._do_expiration()
  File "/home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages/aiohttp/cookiejar.py", line 150, in _do_expiration
    self.clear(lambda x: False)
  File "/home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages/aiohttp/cookiejar.py", line 115, in clear
    for (domain, path), cookie in self._cookies.items():
ValueError: not enough values to unpack (expected 2, got 0)

Python Version

Python 3.10.9

aiohttp Version

Name: aiohttp
Version: 3.8.4
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages
Requires: aiosignal, async-timeout, attrs, charset-normalizer, frozenlist, multidict, yarl

multidict Version

Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages
Requires: 
Required-by: aiohttp, yarl

yarl Version

Name: yarl
Version: 1.8.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /home/gg/PycharmProjects/.venv/x310/lib/python3.10/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

Arch Linux

Related component

Client

Additional context

No response

Code of Conduct

• I agree to follow the aio-libs Code of Conduct

[webknjaz]

@Dreamsorcerer is this related to the PR you wanted released recently?

[Dreamsorcerer]

There was a secondary PR in 3.8 which looked pretty safe, but I had no idea there was functionality for saving cookies to disk:
8cf01ad

Looks like the problem is that the expected keys changed from domain to (domain, path). We could try and revert that PR, but the same issue would certainly occur in 3.9 when it is reintroduced.

Should maybe add a test that loads a cookiejar from a premade dump, then we can get alerted when the pickle format changes.

[Dreamsorcerer]

I think some migration code could look like this:

import pickle

with file_path.open(mode="rb") as f:
    p_cookies = pickle.load(f)

cookies = [(name, c) for sc in p_cookies.values() for name, c in sc.items()]

p_cookies.clear()
for name, c in cookies:
    p_cookies[(c["domain"], c["path"])][name] = c

with file_path.open(mode="wb") as f:
    pickle.dump(p_cookies, f, pickle.HIGHEST_PROTOCOL)

[Walter-o]

Coming here to alert that going from:
aiohttp==3.8.3 to aiohttp==3.8.4
completely breaks loading of existing cookiejar files

[Dreamsorcerer]

See further discussion in #7221.
It's unlikely that we can provide a solid guarantee of backwards compatibility (with the current implementation), but the tests introduced should atleast stop us doing it accidentally in a patch release again.