Skip to content

fix(session): Prevent stale requests from replacing a regenerated session - #65928

Open
DerDreschner wants to merge 1 commit into
masterfrom
fix/stale-session-ids
Open

DerDreschner wants to merge 1 commit into
masterfrom
fix/stale-session-ids

Conversation

@DerDreschner

Copy link
Copy Markdown
Member

Summary

When the session id is being regenerated (for example when you unlock a public share while a request from another tab is already on its way), the session can flap back to the old one due to stale requests. When PHP receives a stale request with the previous session id, it starts a new session with the old session id and sets a new cookie with the old session id. The browser is pushed back onto the old session and loses everything the regenerating request did, which leads to intermittent 401 errors - for a password-protected public share, the file list fails with a 401 on PROPFIND /public.php/dav/files/<token>/.

To fix the issue, I no longer delete the old session: it keeps a marker that its id was replaced. A request that still arrives on that id gets an empty session and, for a grace period of 60 seconds, no session cookie at all, so it can no longer replace the id the browser just stored. After the grace period it gets a fresh session with a cookie as usual. Internal::reopen() now starts the session without cookies for the same reason - until now it re-emitted the cookie on every late session write.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@DerDreschner DerDreschner self-assigned this Oct 26, 2026
@DerDreschner
DerDreschner requested a review from a team as a code owner October 26, 2026 13:45
@DerDreschner
DerDreschner requested review from provokateurin, salmart-dev and sorbaugh and a balanced review from Copilot and removed request for a team October 26, 2026 13:45
@DerDreschner

Copy link
Copy Markdown
Member Author

/backport to stable35

@DerDreschner

Copy link
Copy Markdown
Member Author

/backport to stable34

@DerDreschner

Copy link
Copy Markdown
Member Author

/backport to stable33

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

@DerDreschner
DerDreschner force-pushed the fix/stale-session-ids branch from d94241c to 783eb52 Compare October 27, 2026 07:04
@DerDreschner
DerDreschner requested a balanced review from Copilot October 27, 2026 07:04

This comment was marked as off-topic.

…sion

Regenerating the session id - on login, on unlocking a public share, on a
remember-me login - leaves the requests that are already on their way with
the previous session cookie. PHP starts a session for that id again and
sends it back in a Set-Cookie header, so the id the browser had just stored
is overwritten and everything the regenerating request did is lost. For a
logged-in user who unlocks a password-protected public share this shows up
as a 401 on PROPFIND /public.php/dav/files/<token>/.

Mark the old session as superseded instead of deleting it, and move the
requests that still arrive on it to an empty session. Within a grace period
of 60 seconds they get no session cookie at all, so they can no longer
replace the regenerated id the browser holds. reopen() does the same and
only sends a cookie when the id it was given is gone for good.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
@DerDreschner
DerDreschner force-pushed the fix/stale-session-ids branch from 783eb52 to 25c1ba3 Compare October 27, 2026 08:01

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants