fix(serializer): require explicit opt-in to decode closure payloads - #7
Merged
Merged
Conversation
opis/closure rebuilds the objects inside a closure payload by reflection (newInstanceWithoutConstructor + __unserialize), which PHP's allowed_classes option does not govern, and restricting allowed_classes breaks closure round-trips altogether. Serializer::unserialize() decoded any payload with the closure prefix, so a caller decoding untrusted data could have arbitrary classes instantiated. unserialize() now refuses closure payloads with a Serialization exception and keeps the allowed_classes => false default for plain data. The new unserializeTrusted() keeps the previous behaviour for payloads from a trusted channel; the Swoole process pool, which exchanges closures only with its own forked workers, uses it. Breaking default, hence 0.2.0 with CHANGELOG and UPGRADE notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Benchmark ResultsSwoole Adapters (Sync, Swoole Thread, Swoole Process, Amp, React)ext-parallel Adapter (Sync, Amp, React, ext-parallel)
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The refusal test now checks only that the payload is refused and that nothing inside it is instantiated, rather than pinning the exception message or the restoration count on the trusted path. UPGRADE notes that Serialization is not a RuntimeException, so callers that caught only RuntimeException around unserialize() know to handle a refused payload. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Why
Serializer::unserialize()decoded every payload carrying the closure prefix throughopis/closure. opis/closure rebuilds the objects inside such a payload by reflection (newInstanceWithoutConstructor()+__unserialize()), which PHP'sallowed_classesoption does not govern, and restrictingallowed_classesbreaks closure round-trips altogether. A caller decoding data it does not fully control could therefore have arbitrary classes instantiated, whatever options it passed.What
Serializer::unserialize()refuses closure payloads withUtopia\Async\Exception\Serializationand keeps theallowed_classes => falsedefault for plain data.Serializer::unserializeTrusted()keeps the previous behaviour, for payloads from a trusted channel (this process or its own workers).unserializeTrusted()on both ends.CHANGELOG.mdandUPGRADE.mdfor 0.2.0 (breaking default), README section on serialization.Tests
tests/Unit/SerializerTest.php:testUnserializeRefusesClosurePayloadByDefault: a closure payload carrying an object whose__unserialize()counts restorations is refused, and nothing is instantiated (fails onmain: the payload was decoded).testUnserializeRefusesClosurePayloadWithAllowedClasses: caller options do not enable closure decoding (fails onmain).testUnserializeTrustedRestoresClosurePayload/testUnserializeTrustedDecodesPlainPayloadWithoutClasses: the opt-in round-trips closures and objects, and plain payloads keepallowed_classes => false.unserializeTrusted().Locally:
SerializerTest.php(23 tests) andtests/E2e/Parallel/Swoole/ProcessTest.php(29 tests, exercises the pool's trusted channel) pass.🤖 Generated with Claude Code