Repository navigation
[Server] Answer 404 for a session that a handler cleared or that holds corrupt data - #682
Merged
chr-hertel merged 2 commits intoNov 8, 2026
Conversation
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
November 8, 2026 13:08
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Readable empty session files and sessions cleared or hydrated before their first read are incorrectly classified as unreadable, causing erroneous 503 responses.
2 open issues
What changed in this PR
This follow-up to #627 distinguishes empty or corrupt session data from failed store reads, allowing clients to restart invalid sessions rather than keep retrying.
Changes:
- Add
SessionInterface::isUnreadable()and use it to distinguish HTTP 404 from 503 responses. - Add regression coverage for cleared sessions, corrupt payloads, and failed reads.
- Document the interface compatibility break.
| File | Description |
|---|---|
tests/Unit/Server/Transport/SessionBindingTest.php |
Verify cleared and corrupt sessions return 404. |
tests/Unit/Server/Session/SessionTest.php |
Cover readable, unreadable, and new session states. |
src/Server/Session/SessionInterface.php |
Declare the unreadable-session check. |
src/Server/Session/Session.php |
Implement unreadable-session detection. |
src/Server/Protocol.php |
Use the new check when selecting failure responses. |
CHANGELOG.md |
Document response behavior and the interface break. |
Review effort: Balanced
…ther principal Treat a zero-byte session file as gone, keep isUnreadable() right after clear() or hydrate(), and log why a session is refused.
chr-hertel
force-pushed
the
session-empty-vs-unreadable
branch
from
November 8, 2026 13:35
8396725 to
7ccc3a7
Compare
chr-hertel
added
Server
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
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.
Follow-up to #627
After #627, a session that exists but holds no attributes is treated like a failed read and answered with
503. So when a handler calls$session->clear(), or the stored data is corrupt, the client keeps retrying until the TTL is over instead of opening a new session. Now only a session the store fails to return gets the503, those two get a404.Protocolcan't tell these apart throughall(), hence the newSessionInterface::isUnreadable().[BC Break] A custom
SessionInterfaceneeds to implement it - documented in the CHANGELOG.