Skip to content

ext/session: Truncate session file after successful write - #23653

Closed
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/session-write-truncate-84
Closed

iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/session-write-truncate-84

Conversation

@iliaal

@iliaal iliaal commented Sep 10, 2026

Copy link
Copy Markdown
Member

The files session save handler truncated the session file to zero before writing, so a short or failed write returned failure with the previous data already gone. It now writes first and truncates to the new length only after the full buffer lands.

Comment thread ext/session/tests/session_write_failure_keeps_data.phpt Outdated
Comment thread ext/session/tests/session_write_failure_keeps_data.phpt Outdated
@iliaal
iliaal force-pushed the fix/session-write-truncate-84 branch from 4ca4585 to 870a4b2 Compare September 11, 2026 11:37
@iliaal iliaal changed the title [Session] Truncate session file after successful write ext/session: Truncate session file after successful write Sep 12, 2026

@jorgsowa jorgsowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH the test is not very intuitive to read. May be improved. Also there is missing test case for failing partial write, but it could require more changes to the code than just this fix, so it may be left for another PR.

Comment thread ext/session/mod_files.c
@iliaal
iliaal force-pushed the fix/session-write-truncate-84 branch from 870a4b2 to 37922e1 Compare October 15, 2026 11:45
@iliaal

iliaal commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Leaving the partial-write test case for a follow-up.

Truncating before a shorter write discards the old session data even when
the write fails without storing any bytes. Truncate only after the full
write and update the cached size for subsequent writes.
@iliaal
iliaal force-pushed the fix/session-write-truncate-84 branch from 37922e1 to d6b1147 Compare October 15, 2026 16:43
@iliaal iliaal closed this in c2b02f2 Oct 15, 2026
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.

4 participants