[session] abort strict-mode re-creation when create_sid fails - #286
Open
iliaal wants to merge 2 commits into
Open
[session] abort strict-mode re-creation when create_sid fails#286iliaal wants to merge 2 commits into
iliaal wants to merge 2 commits into
Conversation
When a save handler rejects the session ID in strict mode, the re-created ID must be non-NULL or initialization must fail; previously a failing s_create_sid() was silently papered over by generating a fresh internal ID and continuing, running reset_id() side effects (Set-Cookie, SID constant) for a session that then failed to start, and proceeding with a NULL PS(id) if the fallback generation also failed. Mirror the primary no-ID branch's abort/error handling; sibling call sites audited: session_regenerate_id() collision loop and session_create_id() already NULL-check with their own error paths, mod_mm's strict re-create returns FAILURE into the initialize abort path.
After the successful s_close()/s_open() pair, a failing s_create_sid() reset the status and threw without closing the freshly opened handler, leaving mod_data open until request shutdown and double-opening it on the next start; the collision-retry failure path in the same function already closes. Mirror that close on the null-id path. Sibling audit: session_create_id() breaks out to its own error path, mod_mm's strict re-create returns FAILURE into the initialize abort path, and the strict-mode re-create branch of php_session_initialize() now aborts via the preceding commit.
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.
When session_start() runs in strict mode and the save handler's validateId() rejects the incoming ID, the re-creation branch released PS(id) and called s_create_sid(), but on failure it silently substituted a freshly generated internal ID instead of aborting like the primary no-ID branch does; if that fallback generation also failed, PS(id) stayed NULL and initialization continued with it, skipping s_close(). Even in the recoverable case, reset_id() already ran side effects such as the Set-Cookie header and the SID constant for a session that then failed to start because the pending exception broke s_read(). This makes handler create_sid() failures fatal in strict-mode re-creation, mirroring the primary branch's php_session_abort()/error/FAILURE handling, and adds a regression test asserting that no SID artifact survives a failed start. Sibling call sites audited: session_regenerate_id() and session_create_id() already NULL-check with their own error paths, mod_mm's strict re-create returns FAILURE into the initialize abort path.