Fix GH-20727: User code can run after module request shutdown via the output layer#20824
Open
ndossche wants to merge 2 commits intophp:masterfrom
Open
Fix GH-20727: User code can run after module request shutdown via the output layer#20824ndossche wants to merge 2 commits intophp:masterfrom
ndossche wants to merge 2 commits intophp:masterfrom
Conversation
…the output layer If user code runs after modules executed RSHUTDOWN, it can be dangerous because user code can rely on module globals that have already been invalidated. We should not run user code after RSHUTDOWN. This is shown by the test by using putenv(). The original report demonstrated a silent failure via mbstring. There are more test cases possible but this is by far the simplest. An alternative solution would be to try to separate the user code running via php_header() from the output layer shutdown, to make sure user code runs earlier. However, that becomes an ugly complex solution. This PR's solution keeps things simple but this can be a BC break if extensions produce output in their RSHUTDOWN handler (However, that may have been unsafe in the first place).
Member
|
@ndossche This is just a test.. Did you forget to push something else? |
Member
Author
Woops! Indeed, now pushed. |
bukka
approved these changes
Jan 3, 2026
iluuu1994
reviewed
Jan 3, 2026
Member
|
I don't mind the BC break for master if it's documented and attempting to produce output in RSHUTDOWN triggers an error/assertion. |
Member
Author
|
I'm discovering some more things now:
|
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.
If user code runs after modules executed RSHUTDOWN, it can be dangerous because user code can rely on module globals that have already been invalidated.
We should not run user code after RSHUTDOWN.
This is shown by the test by using putenv(). The original report demonstrated a silent failure via mbstring. There are more test cases possible but this is by far the simplest.
An alternative solution would be to try to separate the user code running via php_header() from the output layer shutdown, to make sure user code runs earlier. However, that becomes an ugly complex solution. This PR's solution keeps things simple but this can be a BC break if extensions produce output in their RSHUTDOWN handler (However, that may have been unsafe in the first place).