From 6f101daaf3f1e28b9c3039f3cdd42c9c7502622d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 30 Sep 2026 02:53:20 +1300 Subject: [PATCH 1/2] fix(serializer): require explicit opt-in to decode closure payloads 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 --- CHANGELOG.md | 21 +++++++++ README.md | 17 +++++++ UPGRADE.md | 21 +++++++++ src/Parallel/Pool/Swoole/Process.php | 5 +-- src/Serializer.php | 63 ++++++++++++++++++++------ tests/Unit/SerializerProbe.php | 27 ++++++++++++ tests/Unit/SerializerTest.php | 66 +++++++++++++++++++++++++--- 7 files changed, 197 insertions(+), 23 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 UPGRADE.md create mode 100644 tests/Unit/SerializerProbe.php diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..9a77928 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,21 @@ +# Changelog + +## 0.2.0 + +### Breaking + +- `Serializer::unserialize()` refuses closure payloads (data produced by `Serializer::serialize()` for a value containing a closure) and throws `Utopia\Async\Exception\Serialization`. Decoding a closure payload through `opis/closure` rebuilds the objects inside it by reflection, which the `allowed_classes` option does not govern, so it no longer happens by default. Plain payloads keep the `allowed_classes => false` default. + +### Added + +- `Serializer::unserializeTrusted()` decodes closure payloads for data from a trusted channel (this process or its own workers). The Swoole process pool uses it for its own worker channel. + +## 0.1.1 + +- `Promise::all()` records the error before signalling its channel. +- The parallel pool skips SIGKILL for workers that are already reaped on shutdown. +- PHPStan level-max fixes in the Timer and Promise adapters. + +## 0.1.0 + +- Initial release. diff --git a/README.md b/README.md index 59b11c0..d904017 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,23 @@ try { } ``` +## Serialization + +`Serializer::serialize()` encodes data containing closures with `opis/closure` and everything else with PHP's `serialize()`. Decoding a closure payload can rebuild objects of any class, so it is an explicit opt-in: + +```php +use Utopia\Async\Serializer; + +// Plain data only: objects are restored only for the allowed classes (none by default), +// and a closure payload throws Utopia\Async\Exception\Serialization +$data = Serializer::unserialize($payload); +$data = Serializer::unserialize($payload, ['allowed_classes' => [MyValue::class]]); + +// Closures and the objects they carry, for payloads from a trusted channel only +// (this process or its own workers, never user input, caches or queues) +$task = Serializer::unserializeTrusted($payload); +``` + ## Configuration Both `Parallel` and `Promise` facades expose configurable options via static getter/setter methods. diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 0000000..148b2ac --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,21 @@ +# Upgrade Guide + +## 0.1.x to 0.2.0 + +### Closure payloads need trusted decoding + +`Serializer::unserialize()` no longer decodes closure payloads. It throws `Utopia\Async\Exception\Serialization` for them, because `opis/closure` rebuilds the objects inside such a payload by reflection, outside the control of the `allowed_classes` option. + +If you pass closures between processes you control, switch those call sites to `Serializer::unserializeTrusted()`: + +```php +// 0.1.x +$task = Serializer::unserialize($message); + +// 0.2.0 +$task = Serializer::unserializeTrusted($message); +``` + +Keep `Serializer::unserialize()` for everything that does not need closures, and never pass data from users, caches, queues or other shared storage to `Serializer::unserializeTrusted()`. + +`Serializer::unserializeTrusted()` accepts the same `$options` and still applies `allowed_classes => false` to plain payloads. diff --git a/src/Parallel/Pool/Swoole/Process.php b/src/Parallel/Pool/Swoole/Process.php index bb8a50f..d3b511b 100644 --- a/src/Parallel/Pool/Swoole/Process.php +++ b/src/Parallel/Pool/Swoole/Process.php @@ -76,9 +76,8 @@ private function initializePool(): void break; } - // Deserialize the entire message with Serializer (handles closures automatically) try { - $taskData = Serializer::unserialize(\is_string($message) ? $message : ''); + $taskData = Serializer::unserializeTrusted(\is_string($message) ? $message : ''); } catch (\Throwable $e) { continue; } @@ -224,7 +223,7 @@ public function execute(array $tasks): array } try { - $result = Serializer::unserialize(\is_string($response) ? $response : ''); + $result = Serializer::unserializeTrusted(\is_string($response) ? $response : ''); } catch (\Throwable $e) { continue; } diff --git a/src/Serializer.php b/src/Serializer.php index 2277cc0..65b79d5 100644 --- a/src/Serializer.php +++ b/src/Serializer.php @@ -2,11 +2,11 @@ namespace Utopia\Async; +use Utopia\Async\Exception\Serialization; + /** - * High-performance serializer with igbinary support. - * - * Uses igbinary extension if available for 2-3x faster serialization, - * falls back to standard PHP serialize() when not available. + * Serializer for task payloads: opis/closure for data containing closures, + * standard PHP serialize() for everything else. * * @package Utopia\Async */ @@ -40,26 +40,61 @@ public static function serialize(mixed $data): string } /** - * Unserialize data using opis/closure for Closures and standard unserialization for everything else. + * Unserialize plain data. Objects are restored only for the classes allowed by the + * caller's options (none by default) and closure payloads are refused, because decoding + * them can instantiate any class. Use unserializeTrusted() for data from a trusted channel. * * @param string $data * @param array{allowed_classes?: bool|array} $options Options for unserialize * @return mixed + * @throws Serialization If the data is a closure payload * @throws \RuntimeException If unserialization fails or data is invalid */ public static function unserialize(string $data, array $options = []): mixed { - if (empty($data)) { - throw new \RuntimeException('Cannot unserialize empty data'); + if (\str_starts_with($data, self::OPIS_CLOSURE_PREFIX)) { + throw new Serialization('Refusing to decode a closure payload: use Serializer::unserializeTrusted() for data from a trusted channel'); } - // Fast prefix check - only check first 3 bytes - if (\str_starts_with($data, self::OPIS_CLOSURE_PREFIX)) { - $opisData = \substr($data, 3); - $result = @\Opis\Closure\unserialize($opisData, $options); - if ($result !== false || $opisData === \Opis\Closure\serialize(false)) { - return $result; - } + return self::unserializePlain($data, $options); + } + + /** + * Unserialize data from a trusted channel, restoring closures and the objects they carry. + * Only use this for payloads produced by this process or its own workers: a closure payload + * can rebuild objects that the allowed_classes option does not govern. + * + * @param string $data + * @param array{allowed_classes?: bool|array} $options Options for unserialize + * @return mixed + * @throws \RuntimeException If unserialization fails or data is invalid + */ + public static function unserializeTrusted(string $data, array $options = []): mixed + { + if (!\str_starts_with($data, self::OPIS_CLOSURE_PREFIX)) { + return self::unserializePlain($data, $options); + } + + $closureData = \substr($data, \strlen(self::OPIS_CLOSURE_PREFIX)); + $result = @\Opis\Closure\unserialize($closureData, $options); + + if ($result !== false || $closureData === \Opis\Closure\serialize(false)) { + return $result; + } + + throw new \RuntimeException('Failed to unserialize data'); + } + + /** + * @param string $data + * @param array{allowed_classes?: bool|array} $options + * @return mixed + * @throws \RuntimeException If unserialization fails or data is invalid + */ + private static function unserializePlain(string $data, array $options): mixed + { + if ($data === '') { + throw new \RuntimeException('Cannot unserialize empty data'); } /** @var array{allowed_classes?: bool|array} $mergedOptions */ diff --git a/tests/Unit/SerializerProbe.php b/tests/Unit/SerializerProbe.php new file mode 100644 index 0000000..27a9566 --- /dev/null +++ b/tests/Unit/SerializerProbe.php @@ -0,0 +1,27 @@ + $this->value]; + } + + /** + * @param array{value: string} $data + */ + public function __unserialize(array $data): void + { + self::$restored++; + $this->value = $data['value']; + } +} diff --git a/tests/Unit/SerializerTest.php b/tests/Unit/SerializerTest.php index a5596fa..06959f9 100644 --- a/tests/Unit/SerializerTest.php +++ b/tests/Unit/SerializerTest.php @@ -3,6 +3,7 @@ namespace Utopia\Tests\Unit; use PHPUnit\Framework\TestCase; +use Utopia\Async\Exception\Serialization; use Utopia\Async\Serializer; class SerializerTest extends TestCase @@ -48,7 +49,7 @@ public function testSerializeClosure(): void }; $serialized = Serializer::serialize($closure); - $unserialized = Serializer::unserialize($serialized); + $unserialized = Serializer::unserializeTrusted($serialized); $this->assertInstanceOf(\Closure::class, $unserialized); /** @var \Closure(int): int $unserialized */ @@ -66,7 +67,7 @@ public function testSerializeArrayWithClosure(): void ]; $serialized = Serializer::serialize($data); - $unserialized = Serializer::unserialize($serialized); + $unserialized = Serializer::unserializeTrusted($serialized); $this->assertIsArray($unserialized); /** @var array{name: string, value: int, callback: callable(int): int} $unserialized */ @@ -88,7 +89,7 @@ public function testSerializeNestedClosures(): void ]; $serialized = Serializer::serialize($data); - $unserialized = Serializer::unserialize($serialized); + $unserialized = Serializer::unserializeTrusted($serialized); $this->assertIsArray($unserialized); /** @var array{level1: array{level2: array{callback: callable(int): int}}} $unserialized */ @@ -119,7 +120,7 @@ public function testSerializeObjectWithClosure(): void }; $serialized = Serializer::serialize($obj); - $unserialized = Serializer::unserialize($serialized, ['allowed_classes' => true]); + $unserialized = Serializer::unserializeTrusted($serialized, ['allowed_classes' => true]); $this->assertInstanceOf(\stdClass::class, $unserialized); /** @var \stdClass&object{name: string, callback: callable} $unserialized */ @@ -184,7 +185,7 @@ function () { ]]]]]; $serialized = Serializer::serialize($data); - $unserialized = Serializer::unserialize($serialized); + $unserialized = Serializer::unserializeTrusted($serialized); $this->assertIsArray($unserialized); // Closure should be found and properly serialized @@ -318,6 +319,59 @@ public function testFastPathForPrimitives(): void } } + public function testUnserializeRefusesClosurePayloadByDefault(): void + { + SerializerProbe::$restored = 0; + $payload = Serializer::serialize([ + 'task' => fn (): int => 1, + 'probe' => new SerializerProbe(), + ]); + + try { + Serializer::unserialize($payload); + $this->fail('A closure payload must not be decoded without opting in to trusted decoding'); + } catch (Serialization $exception) { + $this->assertStringContainsString('unserializeTrusted', $exception->getMessage()); + } + + $this->assertSame(0, SerializerProbe::$restored, 'No object inside a refused payload may be instantiated'); + } + + public function testUnserializeRefusesClosurePayloadWithAllowedClasses(): void + { + $payload = Serializer::serialize(fn (): SerializerProbe => new SerializerProbe()); + + $this->expectException(Serialization::class); + + Serializer::unserialize($payload, ['allowed_classes' => true]); + } + + public function testUnserializeTrustedRestoresClosurePayload(): void + { + SerializerProbe::$restored = 0; + $payload = Serializer::serialize([ + 'task' => fn (int $x): int => $x * 2, + 'probe' => new SerializerProbe(), + ]); + + $unserialized = Serializer::unserializeTrusted($payload); + + $this->assertIsArray($unserialized); + /** @var array{task: \Closure(int): int, probe: SerializerProbe} $unserialized */ + $this->assertSame(10, $unserialized['task'](5)); + $this->assertInstanceOf(SerializerProbe::class, $unserialized['probe']); + $this->assertSame('probe', $unserialized['probe']->value); + $this->assertSame(1, SerializerProbe::$restored); + } + + public function testUnserializeTrustedDecodesPlainPayloadWithoutClasses(): void + { + $payload = Serializer::serialize(new SerializerProbe()); + + $this->assertInstanceOf(\__PHP_Incomplete_Class::class, Serializer::unserializeTrusted($payload)); + $this->assertSame(['a' => 1], Serializer::unserializeTrusted(Serializer::serialize(['a' => 1]))); + } + /** * Test fast detection of Opis\Closure serialized data. */ @@ -330,7 +384,7 @@ public function testFastOpisClosureDetection(): void $this->assertStringContainsString('Opis\Closure\\', $serialized); // Should deserialize correctly using fast detection - $unserialized = Serializer::unserialize($serialized); + $unserialized = Serializer::unserializeTrusted($serialized); /** @var callable $unserialized */ $this->assertEquals('test', $unserialized()); } From ec52d56608fc15f524e567d89f551c7ae1c58e7b Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 30 Sep 2026 03:03:36 +1300 Subject: [PATCH 2/2] test(serializer): assert refusal by outcome, document the exception type 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 --- UPGRADE.md | 2 ++ tests/Unit/SerializerTest.php | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 148b2ac..97b8009 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -6,6 +6,8 @@ `Serializer::unserialize()` no longer decodes closure payloads. It throws `Utopia\Async\Exception\Serialization` for them, because `opis/closure` rebuilds the objects inside such a payload by reflection, outside the control of the `allowed_classes` option. +`Serialization` extends `Utopia\Async\Exception`, not `\RuntimeException`: code that catches only `\RuntimeException` around `Serializer::unserialize()` must also catch `Serialization` (or `\Exception`) to handle a refused payload. + If you pass closures between processes you control, switch those call sites to `Serializer::unserializeTrusted()`: ```php diff --git a/tests/Unit/SerializerTest.php b/tests/Unit/SerializerTest.php index 06959f9..bbbe704 100644 --- a/tests/Unit/SerializerTest.php +++ b/tests/Unit/SerializerTest.php @@ -327,13 +327,14 @@ public function testUnserializeRefusesClosurePayloadByDefault(): void 'probe' => new SerializerProbe(), ]); + $refused = false; try { Serializer::unserialize($payload); - $this->fail('A closure payload must not be decoded without opting in to trusted decoding'); - } catch (Serialization $exception) { - $this->assertStringContainsString('unserializeTrusted', $exception->getMessage()); + } catch (Serialization) { + $refused = true; } + $this->assertTrue($refused, 'A closure payload must not be decoded without opting in to trusted decoding'); $this->assertSame(0, SerializerProbe::$restored, 'No object inside a refused payload may be instantiated'); } @@ -348,7 +349,6 @@ public function testUnserializeRefusesClosurePayloadWithAllowedClasses(): void public function testUnserializeTrustedRestoresClosurePayload(): void { - SerializerProbe::$restored = 0; $payload = Serializer::serialize([ 'task' => fn (int $x): int => $x * 2, 'probe' => new SerializerProbe(), @@ -361,7 +361,6 @@ public function testUnserializeTrustedRestoresClosurePayload(): void $this->assertSame(10, $unserialized['task'](5)); $this->assertInstanceOf(SerializerProbe::class, $unserialized['probe']); $this->assertSame('probe', $unserialized['probe']->value); - $this->assertSame(1, SerializerProbe::$restored); } public function testUnserializeTrustedDecodesPlainPayloadWithoutClasses(): void