fix(session): merge a save into the stored session instead of overwriting it - #108
Merged
tina4stack merged 3 commits intoOct 1, 2026
Conversation
…ting it A save wrote back the whole session as this request loaded it. When requests on one session overlapped, the one that finished later put its stale copy back over the other's changes, and re-created the session if the other request had destroyed it. regenerate() moved the same stale copy to the new id. A save now re-reads the stored session and applies only the keys this request set, changed or removed since it loaded; after clear() it replaces the session. If the stored session is gone, it stays gone: nothing is written and no cookie is sent. If the store cannot be read, nothing is written and the change is kept for a retry. regenerate() re-reads the same way before moving the session, and mints no id for a session that has ended. regenerate() returns null when there is no session to carry, and the SessionLike type in sso.ts now says so. Signed-off-by: Michael <[email protected]>
This was referenced Oct 1, 2026
Contributor
Author
|
I have read the Tina4 Contributor Licence Agreement and I agree to it. |
…ion merge The merge on save adds complexity to this one file. Only its entry in the baseline changes. Signed-off-by: Michael <[email protected]>
A request that started a session the store already holds now re-writes the re-read, merged record on save() even when nothing changed, so the backend deadline is re-stamped to now + TINA4_SESSION_TTL. A session therefore expires after that span of INACTIVITY rather than that long after its last change, matching tina4-php (the reference for ADR-0087) and closing the per-framework session-lifetime divergence. The slide preserves every concurrent-save guard and its precedence: no session writes nothing; a record another request ended is not re-created (the session ends for this request too); a failed backend read keeps the dirty flag for a later retry; a cleared session replaces rather than merges; and TINA4_SESSION_TTL=0 still re-stamps a never-expires deadline (ADR-0027). The re-write is the merged current record, so sliding is concurrent-safe. The read-only test now reads the stored deadline off disk before and after a read-only save on a real FileSessionHandler and asserts it advanced while the stored data is unchanged, mirroring php's testAnUnchangedSessionIsStillWrittenSoExpiryCountsFromTheLastRequest. Proven by mutation: it fails on the old write-nothing behaviour. The real dead-port Redis retry scenario and every other case are unchanged. Signed-off-by: Andre van Zuydam <[email protected]> Co-Authored-By: Claude Opus 4.8 <[email protected]> Co-Authored-By: Tina4 <[email protected]>
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
3 participants
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.
A session save wrote back the whole session as the request loaded it. When requests on one session overlap, the one that finishes later puts its stale copy back over the other's changes, and re-creates the session if the other request destroyed it.
regenerate()moved the same stale copy to the new id.Change. A save re-reads the stored session and applies only the keys this request set, changed or removed since it loaded; after
clear()it replaces the session. If the stored session is gone, it stays gone: nothing is written and no cookie is sent. If the store cannot be read, nothing is written and the change is kept for a retry.regenerate()re-reads the same way before moving the session, and returnsnullinstead of minting an id for a session that has ended.SessionLike.regenerate()insso.tsis typedstring | nullto match.Test.
test/sessionConcurrentRequests.test.ts: two sessions on one store stand in for overlapping requests (48 checks, including the documented login flow and an SSO callback). 22 fail onv3without the change. Each part of the change, reverted on its own, turns a case red. The full suite shows no new failures againstv3on the same machine. Also run end to end over HTTP on the built-in server, with the file and the database (SQLite) session backends.Parity. The same change in all four frameworks: tina4stack/tina4-python#189, tina4stack/tina4-php#260, tina4stack/tina4-ruby#94, #108.