Stop resetting Session.created on mutation - #1219
Conversation
Session.created is documented as the timestamp of the session's first access, but __setitem__ and __delitem__ were bumping it on every change, so it actually reflected the last write instead. That behavior was added to keep idle sessions from expiring early (max_age is treated as an inactivity timeout), so simply reverting it would bring back the old expiry bug. Instead, track the last-activity time separately as last_visit and use that for the expiry check, leaving created untouched after the session is first created. Older serialized sessions without last_visit fall back to created for the expiry calculation, so existing cookies keep working. Fixes aio-libs#981
test_created_not_modified_on_mutation previously relied on real wall-clock time, so it happened to pass against the pre-fix code whenever construction and mutation landed within the same second. Mock time.time and advance it between steps so the test deterministically fails if created is ever reset on mutation. Also add the towncrier changelog fragment for aio-libs#981.
a8defd9 to
04fade4
Compare
Confidence Score: 4/5The PostgreSQL storage compatibility issue should be fixed before merging because active sessions can expire according to creation time rather than last activity. The new timeout behavior depends on persisting Files Needing Attention: aiohttp_session/init.py and examples/postgres_storage.py Reviews (1): Last reviewed commit: "Strengthen created-mutation test and add..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return { | ||
| "created": session.created, | ||
| "last_visit": session._last_visit, | ||
| "session": session._mapping, |
There was a problem hiding this comment.
When an application uses the bundled PostgreSQL storage example with max_age, this new field is discarded because that storage persists only session and the now-immutable created value. Reloading therefore falls back to the original creation time, causing actively mutated sessions to expire once max_age has elapsed since creation.
1 participant
What do these changes do?
Session.createdis documented as the timestamp of the session'sfirst access, but
__setitem__/__delitem__were bumping it onevery change, so in practice it reflected the last write instead.
That behavior was added in #671 to fix an idle-timeout bug (
max_ageis treated as an inactivity timeout, so an actively used session
shouldn't expire). Simply reverting #671 would bring that expiry bug
back, so instead this tracks last-activity time separately as
last_visitand uses that for the expiry check, leavingcreateduntouched after the session is first created. Sessions serialized
before this change won't have
last_visit, so the expiry check fallsback to
createdfor them, same as before.Are there changes in behavior for the user?
Session.creatednow stays fixed for the lifetime of a session,matching the documentation. The idle-timeout behavior for
max_ageis unchanged.
Related issue number
Fixes #981
Checklist