Track session access and modification in SessionMiddleware#3166
Merged
Conversation
Replace the plain `dict` used for `scope["session"]` with a `Session` subclass that tracks `accessed` and `modified` flags. - Only send `Set-Cookie` when the session was actually modified, not on every response. This prevents race conditions with concurrent requests where a slow read-only response clobbers a newer cookie. - Add `Vary: Cookie` header when the session is accessed, so reverse proxies and CDNs don't serve cached responses with session-dependent content to the wrong user. - `pop` and `setdefault` conditionally set `modified` only when the session actually changes, following Django and Flask conventions. Closes #2019
The dict_keys/dict_values/dict_items concrete types are not importable in Python, so use typing.Any as the return type to satisfy mypy strict mode without type: ignore comments.
Kludex
force-pushed
the
track-session-access-modification
branch
4 times, most recently
from
078743c to
cd7d3de
Compare
Move access tracking from per-method overrides (keys/values/items) on the Session dict to the HTTPConnection.session property, matching Flask 3.1.3's approach. This avoids the dict_keys/dict_values/dict_items typing problem entirely since those types are not importable in Python.
Kludex
force-pushed
the
track-session-access-modification
branch
from
cd7d3de to
dce3495
Compare
Kludex
changed the title
SessionMiddleware
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SessionMiddlewarecurrently sendsSet-Cookieon every response when the session is non-empty, even if nothing changed. This causes race conditions with concurrent requests - a slow read-only response can clobber a newer session cookie (see #2019, Chrome bug, authlib#334).This PR replaces the plain
dictused forscope["session"]with aSessionsubclass that tracks two flags:accessed: set inHTTPConnection.sessionwhen the session is accessed, following Flask 3.1.3's approach of tracking at the property level rather than per-method overridesmodified: set on write operations (__setitem__,__delitem__,clear,update), withpopandsetdefaultconditionally marking modified only when the session actually changes (following Django and Flask/Werkzeug conventions)The middleware then uses these flags to:
Set-CookiewhenmodifiedisTrueVary: CookiewhenaccessedisTrue, so reverse proxies and CDNs don't cache session-dependent responses incorrectlymodifiedtracking but bundles unrelated features (partitionedcookies,refresh_window). This PR scopes the change to just access/modification tracking.