Repository navigation
Preserve session records and release buffers after upload-progress decode failures - #24679
Open
programsurf wants to merge 1 commit into
Open
programsurf wants to merge 1 commit into
programsurf wants to merge 1 commit into
Conversation
programsurf
force-pushed
the
fix/gh24493-preserve-upload-session
branch
from
October 25, 2026 12:39
33c275e to
2e165e1
Compare
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.
This extends the session-preservation approach proposed in php/php-src#24521. In addition to preserving the stored session after a rejected upload-progress decode, it releases the retained lazy-write buffer to prevent avoidable memory-limit failures during subsequent progress updates.
Background:
a45ffe71a56741579f648d969193bd027a783e36, proposed aborting the progress update instead of destroying the session. This PR builds on that approach and adds buffer cleanup and regression coverage.The additional problem is that
php_session_abort()leavesPS(session_vars)allocated. The next progress update reads the session again beforephp_session_initialize()releases that buffer. Withsession.lazy_write=1, two serialized records temporarily remain live at once.A valid three-byte upload with a roughly 5 MiB stored session and an 8 MiB upload-request memory limit terminates during request startup:
The session is seeded and subsequently inspected under a 64 MiB limit; the upload request does not call
session_start()in application code. The abort-only version fails, while disabling lazy writing or releasing the unused buffer allows the same upload to complete. The failure also reproduces in a build without AddressSanitizer.This change releases and clears
PS(session_vars)immediately after aborting an upload-progress decode failure. It retains the stored session record and session identifier, and leaves the restricted decoder checks and publicsession_abort()behavior unchanged.Regression coverage:
Validation on PHP 8.4: 268 session PHPTs passed, 1 expected failure, 1 skipped, and 0 unexpected failures. Repeated FPM replay confirms upload completion and session preservation after the fix. Existing session-preservation checks and the original security reproducer retain their expected results.