Skip to content

Add safe wrapper for unserialize#720

Merged
shish merged 1 commit intothecodingmachine:masterfrom
rafaelcx:safe-unserialize
Feb 4, 2026
Merged

Add safe wrapper for unserialize#720
shish merged 1 commit intothecodingmachine:masterfrom
rafaelcx:safe-unserialize

Conversation

@rafaelcx
Copy link
Contributor

@rafaelcx rafaelcx commented Feb 2, 2026

This is my first contribution to this repository, feedback is appreciated.

PHP default behavior for strings that cannot be unserialized is to return a false and emit an E_WARNING (or E_NOTICE depending on PHP version).

This wrapper tries to guarantee that something like that will in fact throw an ErrorException by setting a custom error handler, executing the function, and then restoring it to its previous state.

I believe this is necessary because there is a scenario where an unserialize call, although successful, returns a false because the previous serialized value was indeed a false boolean, and not because an error happened during unserialization.

@shish
Copy link
Collaborator

shish commented Feb 3, 2026

I've approved the automated test workflows to run, but github seems confused (and also another couple of big changes got merged) -- can you rebase and re-push and hopefully then the tests will all run and pass?

@rafaelcx rafaelcx force-pushed the safe-unserialize branch 2 times, most recently from b19cd5e to 055564b Compare February 3, 2026 12:48
@rafaelcx
Copy link
Contributor Author

rafaelcx commented Feb 3, 2026

Yeah, I think Github actions was suffering a major outage yesterday. Rebased and re-pushed.

I see that support for PHP 8.6 has been one of the changes merged into master, should I also re-run the generate script to update /8.6/functionList.php?

@shish
Copy link
Collaborator

shish commented Feb 3, 2026

I see that support for PHP 8.6 has been one of the changes merged into master, should I also re-run the generate script to update /8.6/functionList.php?

Yes please ❤️ (Also the deduplication patch was also merged, so 8.6/functionlist might not exist if it happens to be identical to 8.5/functionlist -- but the tests say something is out of sync either way ^^)

@rafaelcx
Copy link
Contributor Author

rafaelcx commented Feb 4, 2026

I see that support for PHP 8.6 has been one of the changes merged into master, should I also re-run the generate script to update /8.6/functionList.php?

Yes please ❤️ (Also the deduplication patch was also merged, so 8.6/functionlist might not exist if it happens to be identical to 8.5/functionlist -- but the tests say something is out of sync either way ^^)

Awesome! Done, it seems things are on sync now.

@shish shish merged commit 705683a into thecodingmachine:master Feb 4, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants