SessionMiddleware sends a new set-cookie for every request, with unintended results #2018

sm-Fifteen · 2023-01-27T17:42:55Z

sm-Fifteen
Jan 27, 2023

The way SessionMiddleware currently works is that it encodes session data in a signed base64 object with timestamp, in a format not entirely dissimilar to (but incompatible with) JWT. Because of the way it works, being run right before the headers for an HTTP response are sent, and because it doesn't perform many checks besides whether or not there is session data to encode, it will try to update the session cookie whenever a route is called. This is somewhat wasteful, as session data is not usually updated on every request, but it leads to interesting issues as well.

I have a FastAPI application where login is handeled via a session cookie with a separate login process. Several routes are unlocked based on whether or not the cookie is present and based on the user authorizations recorded in said session cookie. When the frontend fetches API data, if the user is logged in, every response contains a new Set-Cookie header, which Firefox ignores, but Chrome acknowledges. None of the API routes alter session data, so the cookie value is always the same, save for the timestamp and signature at the end. This actually runs into an interesting Chrome issue where responses to fetch requests that arrive after the page has been navigated away from still alter the session cookie, even if if the new page changed the session data on load. This means I have this strange, Chrome-exclusive, race-condition-y bug that matches lepture/authlib#334, where the session state data setup by the Oauth flow before redirecting to the third party login form may get clobbered if some /api/slow_route query was pending when the user clicked the login button.

It's unclear if it's actually a chrome bug or unintended behavior from everything working as intended. If Starlette wasn't updating cookies for every request, and the Set-Cookie was actually important here, it's difficult to argue whether or not Chrome should have ignored it.

See here the Starlette test case from the Chromium bug report, which doesn't use SessionMiddleware, but shows the Chrome behavior in action.

from starlette.applications import Starlette
from starlette.responses import HTMLResponse, RedirectResponse, JSONResponse, PlainTextResponse
from starlette.routing import Route
import uvicorn
import datetime
from asyncio import sleep

html_content = """
    <!DOCTYPE html>
    <html>
        <head>
            <meta http-equiv="refresh" content="2;url=/set_cookie_and_redirect" />
            <script>fetch("/set_cookie_on_fetch")</script>
        </head>
        <body>
            Calling slow route via fetch() before changing page.
        </body>
    </html>
"""


async def homepage(request):
    return HTMLResponse(html_content)

async def set_cookie_on_fetch(request):
    res = JSONResponse({'hello':'world'})
    await sleep(5)
    res.set_cookie("my_cookie", "a__" + datetime.datetime.now().isoformat())
    return res

async def set_cookie_and_redirect(request):
    res = RedirectResponse("/final")
    res.set_cookie("my_cookie", "b__" + datetime.datetime.now().isoformat())
    return res

async def final(request):
    cookie_val = request.cookies.get('my_cookie')
    cookie_is_b = cookie_val.partition("__")[0] == 'b'

    if (cookie_is_b):
        res = PlainTextResponse("Cookie value is \"" + cookie_val + "\", all is well. Try refreshing the page.", 200)
    else:
        res = PlainTextResponse("Cookie value is \"" + cookie_val + "\", it shouldn't be.", 400)
    return res

routes = [
    Route('/', homepage),
    Route('/set_cookie_on_fetch', set_cookie_on_fetch),
    Route('/set_cookie_and_redirect', set_cookie_and_redirect),
    Route('/final', final),
]

app = Starlette(debug=True, routes=routes)
uvicorn.run(app, host="0.0.0.0", port=8000)

Kludex · 2023-01-27T19:50:06Z

Kludex
Jan 27, 2023
Maintainer Sponsor

I've created #2019 from this discussion.

@sm-Fifteen you are an old player here, if you feel like it should be an issue, no need to create a discussion first. Just saying. 👀

1 reply

sm-Fifteen Jan 27, 2023
Author

Heh, I was about to create it as an issue, but then started wondering whether or not discussion should take place here, since it's kind of unclear in who's court the responsibility for this bug falls. I guess the cause on Starlette's end is outlined clearly enough that it justifies an issue regardless. Thank you for confirming.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SessionMiddleware sends a new set-cookie for every request, with unintended results #2018

{{title}}

Replies: 1 comment 1 reply

{{title}}

{{title}}

Select a reply

SessionMiddleware sends a new set-cookie for every request, with unintended results #2018

sm-Fifteen Jan 27, 2023

Replies: 1 comment · 1 reply

Kludex Jan 27, 2023 Maintainer Sponsor

sm-Fifteen Jan 27, 2023 Author

sm-Fifteen
Jan 27, 2023

Replies: 1 comment 1 reply

Kludex
Jan 27, 2023
Maintainer Sponsor

sm-Fifteen Jan 27, 2023
Author