From bbc0f549d8f02b75f6a3c8829b3921d8704feec0 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 13:46:26 +0200 Subject: [PATCH 01/16] fix: serialize non-finite event properties as null --- .changeset/seven-suns-strive.md | 5 ++ api/public-api.json | 39 ++++++++++ lib/Consumer/File.php | 3 +- lib/JsonEncoder.php | 65 +++++++++++++++++ lib/QueueConsumer.php | 2 +- test/JsonEncoderTest.php | 91 +++++++++++++++++++++++ test/NonFinitePropertiesTest.php | 119 +++++++++++++++++++++++++++++++ 7 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 .changeset/seven-suns-strive.md create mode 100644 lib/JsonEncoder.php create mode 100644 test/JsonEncoderTest.php create mode 100644 test/NonFinitePropertiesTest.php diff --git a/.changeset/seven-suns-strive.md b/.changeset/seven-suns-strive.md new file mode 100644 index 00000000..65570351 --- /dev/null +++ b/.changeset/seven-suns-strive.md @@ -0,0 +1,5 @@ +--- +"posthog-php": patch +--- + +Serialize non-finite event property numbers as null so they do not discard events or batches. diff --git a/api/public-api.json b/api/public-api.json index 7490d225..a635ad63 100644 --- a/api/public-api.json +++ b/api/public-api.json @@ -3403,6 +3403,45 @@ } } }, + "PostHog\\JsonEncoder": { + "type": "class", + "abstract": false, + "final": true, + "readonly": false, + "extends": null, + "implements": [ + "JsonSerializable" + ], + "constants": [], + "properties": [], + "methods": { + "encode": { + "static": true, + "abstract": false, + "final": false, + "returnType": "string|false", + "parameters": [ + { + "name": "value", + "type": "mixed", + "byReference": false, + "variadic": false, + "optional": false, + "default": null, + "defaultConstant": null, + "hasDefault": false + } + ] + }, + "jsonSerialize": { + "static": false, + "abstract": false, + "final": false, + "returnType": "mixed", + "parameters": [] + } + } + }, "PostHog\\PostHog": { "type": "class", "abstract": false, diff --git a/lib/Consumer/File.php b/lib/Consumer/File.php index dc4f6e36..eaefaaad 100644 --- a/lib/Consumer/File.php +++ b/lib/Consumer/File.php @@ -4,6 +4,7 @@ use Exception; use PostHog\Consumer; +use PostHog\JsonEncoder; /** * Consumer that writes analytics messages to a local file. @@ -101,7 +102,7 @@ private function write($body) return false; } - $content = json_encode($body); + $content = JsonEncoder::encode($body); $content .= "\n"; return fwrite($this->file_handle, $content) == strlen($content); diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php new file mode 100644 index 00000000..93a5c65c --- /dev/null +++ b/lib/JsonEncoder.php @@ -0,0 +1,65 @@ +value; + if ($value instanceof JsonSerializable) { + $serialized = $value->jsonSerialize(); + if ($serialized !== $value) { + return self::prepare($serialized, $this->depth); + } + } + + // Leave over-depth input to json_encode so recursive arrays and objects still fail safely. + if (is_array($value)) { + $result = []; + foreach ($value as $key => $item) { + $result[$key] = self::prepare($item, $this->depth + 1); + } + } else { + $result = new stdClass(); + foreach ((array) $value as $key => $item) { + if (!str_contains((string) $key, "\0")) { + $result->{(string) $key} = self::prepare($item, $this->depth + 1); + } + } + } + + return $result; + } +} diff --git a/lib/QueueConsumer.php b/lib/QueueConsumer.php index 30434a6e..a431c42e 100644 --- a/lib/QueueConsumer.php +++ b/lib/QueueConsumer.php @@ -213,7 +213,7 @@ private function now(): float */ protected function encodeBatchPayload($batch) { - $payload = json_encode($this->payload($batch)); + $payload = JsonEncoder::encode($this->payload($batch)); if (false === $payload) { $this->handleError(json_last_error(), "Failed to encode batch payload: " . json_last_error_msg()); return false; diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php new file mode 100644 index 00000000..a7540a56 --- /dev/null +++ b/test/JsonEncoderTest.php @@ -0,0 +1,91 @@ + &$number, + 'date' => new \DateTimeImmutable('2024-01-01T00:00:00Z'), + 'arrayObject' => new \ArrayObject(['number' => NAN]), + 'emptyObject' => new \stdClass(), + 'emptyArray' => [], + 'sparseArray' => [2 => INF, 4 => 'kept'], + ]; + self::assertSame( + '{"reference":null,"date":{"date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' + . ',"arrayObject":{"number":null},"emptyObject":{},"emptyArray":[]' + . ',"sparseArray":{"2":null,"4":"kept"}}', + JsonEncoder::encode($input) + ); + self::assertSame(INF, $number); + } + + public function testUnsupportedValuesStillFailEncoding(): void + { + $resource = fopen('php://memory', 'r'); + try { + self::assertFalse(JsonEncoder::encode(['number' => NAN, 'unsupported' => $resource])); + self::assertSame(JSON_ERROR_UNSUPPORTED_TYPE, json_last_error()); + self::assertFalse(JsonEncoder::encode(['number' => INF, 'invalid' => "\xB1\x31"])); + self::assertSame(JSON_ERROR_UTF8, json_last_error()); + } finally { + fclose($resource); + } + } + + public function testJsonSerializableExceptionsAreNotSwallowed(): void + { + $value = new class implements \JsonSerializable { + public function jsonSerialize(): mixed + { + throw new \LogicException('unsupported serialization'); + } + }; + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('unsupported serialization'); + JsonEncoder::encode($value); + } + + public function testRecursiveAndOverDepthInputFailsInBoundedSubprocess(): void + { + $code = <<<'PHP' + require $argv[1]; + $array = []; + $array['self'] = &$array; + $array['again'] = &$array; + $object = new stdClass(); + $object->self = $object; + $deep = ['number' => INF]; + for ($i = 0; $i < 513; ++$i) { + $deep = [$deep]; + } + foreach ([$array, $object, $deep] as $value) { + if (PostHog\JsonEncoder::encode($value) !== false) { + exit(1); + } + } + echo 'rejected'; + PHP; + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=3', '-r', $code, + __DIR__ . '/../vendor/autoload.php'], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $errors); + self::assertSame('rejected', $output); + } +} diff --git a/test/NonFinitePropertiesTest.php b/test/NonFinitePropertiesTest.php new file mode 100644 index 00000000..ef783a5b --- /dev/null +++ b/test/NonFinitePropertiesTest.php @@ -0,0 +1,119 @@ + ['lib_curl', NAN], + 'batch positive infinity' => ['lib_curl', INF], + 'batch negative infinity' => ['lib_curl', -INF], + 'file NaN' => ['file', NAN], + 'file positive infinity' => ['file', INF], + 'file negative infinity' => ['file', -INF], + ]; + } + + #[DataProvider('nonFiniteValues')] + public function testCaptureSerializesNonFinitePropertiesAsNull(string $consumer, float $number): void + { + $filename = tempnam(sys_get_temp_dir(), 'posthog-nonfinite-'); + $httpClient = new MockedHttpClient('unused.invalid'); + $client = new Client('test-key', [ + 'consumer' => $consumer, + 'filename' => $filename, + 'batch_size' => 10, + 'flush_interval_seconds' => 3600, + ], $httpClient); + $object = (object) ['number' => $number, 'finite' => 4.5]; + $typedObject = new class ($number) { + private float $hidden = INF; + + public function __construct(public readonly float $number) + { + } + }; + $serializable = new class ($object) implements \JsonSerializable { + public function __construct(private object $value) + { + } + + public function jsonSerialize(): mixed + { + return ['visible' => $this->value]; + } + }; + $properties = [ + 'number' => $number, + 'nested' => ['number' => $number, 'object' => $object], + 'items' => [1, $number, $object, 3], + 'finite' => 2.5, + 'zero' => 0, + 'enabled' => false, + 'nullable' => null, + 'strings' => ['NaN', 'Infinity', '-Infinity'], + 'typed' => $typedObject, + 'serializable' => $serializable, + ]; + try { + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'non-finite', 'properties' => $properties, + ])); + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], + ])); + self::assertTrue($client->flush()); + if ($consumer === 'file') { + self::assertNull($httpClient->calls); + $events = array_map( + static fn(string $line): array => json_decode($line, true, 512, JSON_THROW_ON_ERROR), + file($filename, FILE_IGNORE_NEW_LINES) + ); + } else { + self::assertCount(1, $httpClient->calls); + self::assertSame('/batch/', $httpClient->calls[0]['path']); + $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); + self::assertSame('test-key', $payload['api_key']); + $events = $payload['batch']; + } + self::assertSame(['non-finite', 'neighbor'], array_column($events, 'event')); + self::assertSame([ + 'number' => null, + 'nested' => ['number' => null, 'object' => ['number' => null, 'finite' => 4.5]], + 'items' => [1, null, ['number' => null, 'finite' => 4.5], 3], + 'finite' => 2.5, + 'zero' => 0, + 'enabled' => false, + 'nullable' => null, + 'strings' => ['NaN', 'Infinity', '-Infinity'], + 'typed' => ['number' => null], + 'serializable' => ['visible' => ['number' => null, 'finite' => 4.5]], + ], array_intersect_key($events[0]['properties'], $properties)); + self::assertSame(7, $events[1]['properties']['finite']); + self::assertSame($object, $properties['nested']['object']); + self::assertSame($object, $properties['items'][2]); + if (is_nan($number)) { + self::assertNan($properties['number']); + self::assertNan($properties['nested']['number']); + self::assertNan($properties['items'][1]); + self::assertNan($object->number); + self::assertNan($typedObject->number); + } else { + self::assertSame($number, $properties['number']); + self::assertSame($number, $properties['nested']['number']); + self::assertSame($number, $properties['items'][1]); + self::assertSame($number, $object->number); + self::assertSame($number, $typedObject->number); + } + } finally { + $client->shutdown(); + unlink($filename); + } + } +} From 04d9f63439541067e5357550be171e9cc3e64477 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 13:49:57 +0200 Subject: [PATCH 02/16] fix: bound recursive JSON serializer chains --- lib/JsonEncoder.php | 15 +++++++++------ test/JsonEncoderTest.php | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 93a5c65c..5f702d4e 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -13,8 +13,11 @@ */ final class JsonEncoder implements JsonSerializable { - private function __construct(private mixed $value, private int $depth) - { + private function __construct( + private mixed $value, + private int $depth, + private int $serializerHops + ) { } public static function encode(mixed $value): string|false @@ -22,14 +25,14 @@ public static function encode(mixed $value): string|false return json_encode(self::prepare($value, 0)); } - private static function prepare(mixed $value, int $depth): mixed + private static function prepare(mixed $value, int $depth, int $serializerHops = 0): mixed { if (is_float($value) && !is_finite($value)) { return null; } if (is_array($value) || (is_object($value) && !$value instanceof UnitEnum)) { - return $depth < 512 ? new self($value, $depth) : $value; + return $depth < 512 && $serializerHops < 512 ? new self($value, $depth, $serializerHops) : $value; } return $value; @@ -41,11 +44,11 @@ public function jsonSerialize(): mixed if ($value instanceof JsonSerializable) { $serialized = $value->jsonSerialize(); if ($serialized !== $value) { - return self::prepare($serialized, $this->depth); + return self::prepare($serialized, $this->depth, $this->serializerHops + 1); } } - // Leave over-depth input to json_encode so recursive arrays and objects still fail safely. + // Container depth and serializer hops are bounded before handing recursive input to json_encode. if (is_array($value)) { $result = []; foreach ($value as $key => $item) { diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index a7540a56..8a52ee69 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -62,6 +62,23 @@ public function testRecursiveAndOverDepthInputFailsInBoundedSubprocess(): void $array['again'] = &$array; $object = new stdClass(); $object->self = $object; + $first = new class implements JsonSerializable { + public object $next; + + public function jsonSerialize(): mixed + { + return $this->next; + } + }; + $second = clone $first; + $first->next = $second; + $second->next = $first; + if (json_encode($first) !== false || json_last_error() !== JSON_ERROR_RECURSION) { + exit(2); + } + if (PostHog\JsonEncoder::encode($first) !== false || json_last_error() !== JSON_ERROR_RECURSION) { + exit(3); + } $deep = ['number' => INF]; for ($i = 0; $i < 513; ++$i) { $deep = [$deep]; From 6a9282d7c891ca4bb23105563dde9c148b683259 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 16:14:57 +0200 Subject: [PATCH 03/16] fix: preserve native object serialization and cycle detection --- lib/JsonEncoder.php | 50 ++++++++++++++++++++++------ test/JsonEncoderTest.php | 70 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 11 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 5f702d4e..6c860387 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -2,7 +2,10 @@ namespace PostHog; +use ArrayObject; use JsonSerializable; +use ReflectionClass; +use ReflectionReference; use stdClass; use UnitEnum; @@ -16,23 +19,43 @@ final class JsonEncoder implements JsonSerializable private function __construct( private mixed $value, private int $depth, - private int $serializerHops + private ArrayObject $wrappers ) { } public static function encode(mixed $value): string|false { - return json_encode(self::prepare($value, 0)); + $wrappers = new ArrayObject(); + try { + return json_encode(self::prepare($value, 0, $wrappers)); + } finally { + $wrappers->exchangeArray([]); + } } - private static function prepare(mixed $value, int $depth, int $serializerHops = 0): mixed - { + private static function prepare( + mixed $value, + int $depth, + ArrayObject $wrappers, + ?string $referenceId = null + ): mixed { if (is_float($value) && !is_finite($value)) { return null; } if (is_array($value) || (is_object($value) && !$value instanceof UnitEnum)) { - return $depth < 512 && $serializerHops < 512 ? new self($value, $depth, $serializerHops) : $value; + $identity = is_object($value) ? 'object:' . spl_object_id($value) : $referenceId; + if ($identity !== null && isset($wrappers[$identity])) { + return $wrappers[$identity]; + } + if ($depth >= 512) { + return $value; + } + $wrapper = new self($value, $depth, $wrappers); + if ($identity !== null) { + $wrappers[$identity] = $wrapper; + } + return $wrapper; } return $value; @@ -44,21 +67,28 @@ public function jsonSerialize(): mixed if ($value instanceof JsonSerializable) { $serialized = $value->jsonSerialize(); if ($serialized !== $value) { - return self::prepare($serialized, $this->depth, $this->serializerHops + 1); + return self::prepare($serialized, $this->depth, $this->wrappers); } } - // Container depth and serializer hops are bounded before handing recursive input to json_encode. + // Reuse wrappers for original identities so json_encode can detect recursion before expanding it. if (is_array($value)) { $result = []; foreach ($value as $key => $item) { - $result[$key] = self::prepare($item, $this->depth + 1); + $reference = ReflectionReference::fromArrayElement($value, $key); + $referenceId = $reference !== null ? 'reference:' . $reference->getId() : null; + $result[$key] = self::prepare($item, $this->depth + 1, $this->wrappers, $referenceId); } } else { $result = new stdClass(); - foreach ((array) $value as $key => $item) { + $class = new ReflectionClass($value); + while (!$class->isInternal() && ($parent = $class->getParentClass()) !== false) { + $class = $parent; + } + $properties = $class->isInternal() ? (array) $value : get_object_vars($value); + foreach ($properties as $key => $item) { if (!str_contains((string) $key, "\0")) { - $result->{(string) $key} = self::prepare($item, $this->depth + 1); + $result->{(string) $key} = self::prepare($item, $this->depth + 1, $this->wrappers); } } } diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index 8a52ee69..d45d5bcd 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -14,19 +14,74 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v 'reference' => &$number, 'date' => new \DateTimeImmutable('2024-01-01T00:00:00Z'), 'arrayObject' => new \ArrayObject(['number' => NAN]), + 'dateSubclass' => new class ('2024-01-01T00:00:00Z') extends \DateTimeImmutable { + public string $label = 'kept'; + }, + 'arrayObjectSubclass' => new class (['number' => INF]) extends \ArrayObject { + public string $ignored = 'ignored'; + }, 'emptyObject' => new \stdClass(), 'emptyArray' => [], 'sparseArray' => [2 => INF, 4 => 'kept'], ]; self::assertSame( '{"reference":null,"date":{"date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' - . ',"arrayObject":{"number":null},"emptyObject":{},"emptyArray":[]' + . ',"arrayObject":{"number":null}' + . ',"dateSubclass":{"label":"kept","date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' + . ',"arrayObjectSubclass":{"number":null},"emptyObject":{},"emptyArray":[]' . ',"sparseArray":{"2":null,"4":"kept"}}', JsonEncoder::encode($input) ); self::assertSame(INF, $number); } + public function testPropertyHooksControlTheSerializedPublicView(): void + { + if (PHP_VERSION_ID < 80400) { + self::markTestSkipped('Property hooks require PHP 8.4.'); + } + $object = eval(<<<'PHP' + return new class { + public string $value = 'private fixture' { + get => $this->value === '' ? '' : 'filtered'; + } + public string $virtual { + get => 'visible'; + } + }; + PHP); + $native = json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR); + self::assertTrue($native === ['value' => 'filtered', 'virtual' => 'visible']); + $encoded = json_decode( + JsonEncoder::encode(['number' => NAN, 'object' => $object]), + true, + 512, + JSON_THROW_ON_ERROR + ); + self::assertTrue( + $encoded['object'] === ['value' => 'filtered', 'virtual' => 'visible'], + 'Serialization must use the public property getters, not backing storage.' + ); + self::assertNull($encoded['number']); + } + + public function testPropertyGetterExceptionsAreNotSwallowed(): void + { + if (PHP_VERSION_ID < 80400) { + self::markTestSkipped('Property hooks require PHP 8.4.'); + } + $object = eval(<<<'PHP' + return new class { + public string $value { + get => throw new LogicException('unsupported getter'); + } + }; + PHP); + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('unsupported getter'); + JsonEncoder::encode($object); + } + public function testUnsupportedValuesStillFailEncoding(): void { $resource = fopen('php://memory', 'r'); @@ -79,6 +134,19 @@ public function jsonSerialize(): mixed if (PostHog\JsonEncoder::encode($first) !== false || json_last_error() !== JSON_ERROR_RECURSION) { exit(3); } + $wideObject = new stdClass(); + $wideArray = []; + for ($i = 0; $i < 10000; ++$i) { + $wideObject->{'p' . $i} = $i; + $wideArray['p' . $i] = $i; + } + $wideObject->self = $wideObject; + $wideArray['self'] = &$wideArray; + foreach ([$wideObject, $wideArray] as $value) { + if (PostHog\JsonEncoder::encode($value) !== false || json_last_error() !== JSON_ERROR_RECURSION) { + exit(4); + } + } $deep = ['number' => INF]; for ($i = 0; $i < 513; ++$i) { $deep = [$deep]; From 25ad08448cfeb5dab5b06094930adcc496e9aeab Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 16:39:56 +0200 Subject: [PATCH 04/16] fix: preserve embedded-NUL keys and normalize serializable enums --- lib/JsonEncoder.php | 8 +++-- test/JsonEncoderTest.php | 11 +++++++ test/NonFiniteJsonSerializableEnum.php | 25 ++++++++++++++++ test/NonFinitePropertiesTest.php | 41 ++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 test/NonFiniteJsonSerializableEnum.php diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 6c860387..bc87935d 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -43,7 +43,11 @@ private static function prepare( return null; } - if (is_array($value) || (is_object($value) && !$value instanceof UnitEnum)) { + if ( + is_array($value) + || $value instanceof JsonSerializable + || (is_object($value) && !$value instanceof UnitEnum) + ) { $identity = is_object($value) ? 'object:' . spl_object_id($value) : $referenceId; if ($identity !== null && isset($wrappers[$identity])) { return $wrappers[$identity]; @@ -87,7 +91,7 @@ public function jsonSerialize(): mixed } $properties = $class->isInternal() ? (array) $value : get_object_vars($value); foreach ($properties as $key => $item) { - if (!str_contains((string) $key, "\0")) { + if (!str_starts_with((string) $key, "\0")) { $result->{(string) $key} = self::prepare($item, $this->depth + 1, $this->wrappers); } } diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index d45d5bcd..fbff5508 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -16,6 +16,7 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v 'arrayObject' => new \ArrayObject(['number' => NAN]), 'dateSubclass' => new class ('2024-01-01T00:00:00Z') extends \DateTimeImmutable { public string $label = 'kept'; + private float $hidden = INF; }, 'arrayObjectSubclass' => new class (['number' => INF]) extends \ArrayObject { public string $ignored = 'ignored'; @@ -35,6 +36,16 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v self::assertSame(INF, $number); } + public function testPreservesPublicPropertyNamesContainingEmbeddedNullBytes(): void + { + $object = (object) ["a\0b" => 1]; + $expected = '{"a\u0000b":1}'; + + self::assertSame($expected, json_encode($object)); + self::assertSame($expected, JsonEncoder::encode($object)); + self::assertSame(1, $object->{"a\0b"}); + } + public function testPropertyHooksControlTheSerializedPublicView(): void { if (PHP_VERSION_ID < 80400) { diff --git a/test/NonFiniteJsonSerializableEnum.php b/test/NonFiniteJsonSerializableEnum.php new file mode 100644 index 00000000..21fff121 --- /dev/null +++ b/test/NonFiniteJsonSerializableEnum.php @@ -0,0 +1,25 @@ + match ($this) { + self::NotANumber => NAN, + self::PositiveInfinity => INF, + self::NegativeInfinity => -INF, + }, + 'finite' => 2.5, + 'literal' => 'NaN', + ]; + } +} diff --git a/test/NonFinitePropertiesTest.php b/test/NonFinitePropertiesTest.php index ef783a5b..077d023f 100644 --- a/test/NonFinitePropertiesTest.php +++ b/test/NonFinitePropertiesTest.php @@ -20,6 +20,47 @@ public static function nonFiniteValues(): array ]; } + public static function serializableEnums(): array + { + return [ + 'NaN' => [NonFiniteJsonSerializableEnum::NotANumber], + 'positive infinity' => [NonFiniteJsonSerializableEnum::PositiveInfinity], + 'negative infinity' => [NonFiniteJsonSerializableEnum::NegativeInfinity], + ]; + } + + #[DataProvider('serializableEnums')] + public function testCaptureNormalizesNonFiniteJsonSerializableEnums(NonFiniteJsonSerializableEnum $enum): void + { + $httpClient = new MockedHttpClient('unused.invalid'); + $client = new Client('test-key', [ + 'batch_size' => 10, + 'flush_interval_seconds' => 3600, + ], $httpClient); + $properties = ['enum' => $enum, 'enabled' => false]; + try { + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'enum-event', 'properties' => $properties, + ])); + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], + ])); + self::assertTrue($client->flush()); + self::assertCount(1, $httpClient->calls); + $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); + self::assertSame(['enum-event', 'neighbor'], array_column($payload['batch'], 'event')); + self::assertSame( + ['number' => null, 'finite' => 2.5, 'literal' => 'NaN'], + $payload['batch'][0]['properties']['enum'] + ); + self::assertFalse($payload['batch'][0]['properties']['enabled']); + self::assertSame(7, $payload['batch'][1]['properties']['finite']); + self::assertSame(['enum' => $enum, 'enabled' => false], $properties); + } finally { + $client->shutdown(); + } + } + #[DataProvider('nonFiniteValues')] public function testCaptureSerializesNonFinitePropertiesAsNull(string $consumer, float $number): void { From 4059fad653c297e072899053ec3504e2f180a9d2 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 16:49:17 +0200 Subject: [PATCH 05/16] fix: preserve property hooks on stdClass subclasses --- lib/JsonEncoder.php | 4 ++- test/JsonEncoderTest.php | 27 ++++++++++++------ test/NonFinitePropertiesTest.php | 48 ++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index bc87935d..cb7e0ad8 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -89,7 +89,9 @@ public function jsonSerialize(): mixed while (!$class->isInternal() && ($parent = $class->getParentClass()) !== false) { $class = $parent; } - $properties = $class->isInternal() ? (array) $value : get_object_vars($value); + $properties = $class->isInternal() && $class->getName() !== stdClass::class + ? (array) $value + : get_object_vars($value); foreach ($properties as $key => $item) { if (!str_starts_with((string) $key, "\0")) { $result->{(string) $key} = self::prepare($item, $this->depth + 1, $this->wrappers); diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index fbff5508..ecef707e 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -2,6 +2,7 @@ namespace PostHog\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use PostHog\JsonEncoder; @@ -46,13 +47,22 @@ public function testPreservesPublicPropertyNamesContainingEmbeddedNullBytes(): v self::assertSame(1, $object->{"a\0b"}); } - public function testPropertyHooksControlTheSerializedPublicView(): void + public static function propertyHookParents(): array + { + return [ + 'ordinary class' => [''], + 'stdClass subclass' => [' extends \\stdClass'], + ]; + } + + #[DataProvider('propertyHookParents')] + public function testPropertyHooksControlTheSerializedPublicView(string $parentDeclaration): void { if (PHP_VERSION_ID < 80400) { self::markTestSkipped('Property hooks require PHP 8.4.'); } - $object = eval(<<<'PHP' - return new class { + $object = eval(sprintf(<<<'PHP' + return new class%s { public string $value = 'private fixture' { get => $this->value === '' ? '' : 'filtered'; } @@ -60,7 +70,7 @@ public function testPropertyHooksControlTheSerializedPublicView(): void get => 'visible'; } }; - PHP); + PHP, $parentDeclaration)); $native = json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR); self::assertTrue($native === ['value' => 'filtered', 'virtual' => 'visible']); $encoded = json_decode( @@ -76,18 +86,19 @@ public function testPropertyHooksControlTheSerializedPublicView(): void self::assertNull($encoded['number']); } - public function testPropertyGetterExceptionsAreNotSwallowed(): void + #[DataProvider('propertyHookParents')] + public function testPropertyGetterExceptionsAreNotSwallowed(string $parentDeclaration): void { if (PHP_VERSION_ID < 80400) { self::markTestSkipped('Property hooks require PHP 8.4.'); } - $object = eval(<<<'PHP' - return new class { + $object = eval(sprintf(<<<'PHP' + return new class%s { public string $value { get => throw new LogicException('unsupported getter'); } }; - PHP); + PHP, $parentDeclaration)); $this->expectException(\LogicException::class); $this->expectExceptionMessage('unsupported getter'); JsonEncoder::encode($object); diff --git a/test/NonFinitePropertiesTest.php b/test/NonFinitePropertiesTest.php index 077d023f..34814039 100644 --- a/test/NonFinitePropertiesTest.php +++ b/test/NonFinitePropertiesTest.php @@ -61,6 +61,54 @@ public function testCaptureNormalizesNonFiniteJsonSerializableEnums(NonFiniteJso } } + public function testCapturePreservesStdClassSubclassPropertyHooks(): void + { + if (PHP_VERSION_ID < 80400) { + self::markTestSkipped('Property hooks require PHP 8.4.'); + } + $object = eval(<<<'PHP' + return new class extends stdClass { + public string $value = 'private fixture' { + get => $this->value === '' ? '' : 'filtered'; + } + public string $virtual { + get => 'visible'; + } + public float $finite = 2.5; + }; + PHP); + $expected = ['value' => 'filtered', 'virtual' => 'visible', 'finite' => 2.5]; + self::assertTrue(json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR) === $expected); + $httpClient = new MockedHttpClient('unused.invalid'); + $client = new Client('test-key', [ + 'batch_size' => 10, + 'flush_interval_seconds' => 3600, + ], $httpClient); + try { + self::assertTrue($client->capture([ + 'distinctId' => 'user', + 'event' => 'hooked-subclass', + 'properties' => ['object' => $object, 'number' => NAN], + ])); + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], + ])); + self::assertTrue($client->flush()); + self::assertCount(1, $httpClient->calls); + $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); + self::assertSame(['hooked-subclass', 'neighbor'], array_column($payload['batch'], 'event')); + self::assertTrue( + $payload['batch'][0]['properties']['object'] === $expected, + 'Wire serialization must preserve the filtered public property view.' + ); + self::assertNull($payload['batch'][0]['properties']['number']); + self::assertSame(7, $payload['batch'][1]['properties']['finite']); + self::assertTrue(((array) $object)['value'] === 'private fixture'); + } finally { + $client->shutdown(); + } + } + #[DataProvider('nonFiniteValues')] public function testCaptureSerializesNonFinitePropertiesAsNull(string $consumer, float $number): void { From a3fd115f656dc650121a498157bbe98c567cb941 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 17:01:03 +0200 Subject: [PATCH 06/16] fix: preserve native JSON encoding for closures --- lib/JsonEncoder.php | 3 +- test/JsonEncoderTest.php | 15 ++++++++++ test/NonFinitePropertiesTest.php | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index cb7e0ad8..a52c1a7e 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -3,6 +3,7 @@ namespace PostHog; use ArrayObject; +use Closure; use JsonSerializable; use ReflectionClass; use ReflectionReference; @@ -46,7 +47,7 @@ private static function prepare( if ( is_array($value) || $value instanceof JsonSerializable - || (is_object($value) && !$value instanceof UnitEnum) + || (is_object($value) && !$value instanceof UnitEnum && !$value instanceof Closure) ) { $identity = is_object($value) ? 'object:' . spl_object_id($value) : $referenceId; if ($identity !== null && isset($wrappers[$identity])) { diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index ecef707e..0539f430 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -37,6 +37,21 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v self::assertSame(INF, $number); } + public function testClosuresKeepTheirNativeEmptyObjectRepresentation(): void + { + $invoked = false; + $callback = static function () use (&$invoked): void { + $invoked = true; + }; + $input = ['callback' => $callback]; + $expected = '{"callback":{}}'; + + self::assertSame($expected, json_encode($input)); + self::assertSame($expected, JsonEncoder::encode($input)); + self::assertSame($callback, $input['callback']); + self::assertFalse($invoked); + } + public function testPreservesPublicPropertyNamesContainingEmbeddedNullBytes(): void { $object = (object) ["a\0b" => 1]; diff --git a/test/NonFinitePropertiesTest.php b/test/NonFinitePropertiesTest.php index 34814039..5a10e2cb 100644 --- a/test/NonFinitePropertiesTest.php +++ b/test/NonFinitePropertiesTest.php @@ -61,6 +61,53 @@ public function testCaptureNormalizesNonFiniteJsonSerializableEnums(NonFiniteJso } } + public function testCapturePreservesClosuresAlongsideNonFiniteNumbersAndNeighbor(): void + { + $invoked = false; + $callback = static function () use (&$invoked): void { + $invoked = true; + }; + $properties = [ + 'callback' => $callback, + 'numbers' => [NAN, INF, -INF], + 'finite' => 2.5, + 'enabled' => false, + 'literal' => 'NaN', + ]; + $httpClient = new MockedHttpClient('unused.invalid'); + $client = new Client('test-key', [ + 'batch_size' => 10, + 'flush_interval_seconds' => 3600, + ], $httpClient); + try { + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'callback-event', 'properties' => $properties, + ])); + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], + ])); + self::assertTrue($client->flush()); + self::assertCount(1, $httpClient->calls); + $payload = json_decode($httpClient->calls[0]['payload'], false, 512, JSON_THROW_ON_ERROR); + self::assertSame(['callback-event', 'neighbor'], array_column($payload->batch, 'event')); + $actual = $payload->batch[0]->properties; + self::assertInstanceOf(\stdClass::class, $actual->callback); + self::assertSame([], (array) $actual->callback); + self::assertSame([null, null, null], $actual->numbers); + self::assertSame(2.5, $actual->finite); + self::assertFalse($actual->enabled); + self::assertSame('NaN', $actual->literal); + self::assertSame(7, $payload->batch[1]->properties->finite); + self::assertSame($callback, $properties['callback']); + self::assertNan($properties['numbers'][0]); + self::assertSame(INF, $properties['numbers'][1]); + self::assertSame(-INF, $properties['numbers'][2]); + self::assertFalse($invoked); + } finally { + $client->shutdown(); + } + } + public function testCapturePreservesStdClassSubclassPropertyHooks(): void { if (PHP_VERSION_ID < 80400) { From 2155f5a1bf878bb990aa19f8a34ab4bba3ccbb22 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Oct 2026 18:32:42 +0200 Subject: [PATCH 07/16] fix: preserve native getters and release JSON wrappers --- lib/JsonEncoder.php | 25 +++++++++------ test/JsonEncoderTest.php | 53 +++++++++++++++++++++++++++++++- test/NonFinitePropertiesTest.php | 17 +++++++--- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index a52c1a7e..f8501770 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -2,13 +2,16 @@ namespace PostHog; +use ArrayIterator; use ArrayObject; use Closure; +use DateTimeInterface; +use DateTimeZone; use JsonSerializable; -use ReflectionClass; use ReflectionReference; use stdClass; use UnitEnum; +use WeakReference; /** * Encodes event payloads with non-finite numbers represented as null. @@ -51,14 +54,17 @@ private static function prepare( ) { $identity = is_object($value) ? 'object:' . spl_object_id($value) : $referenceId; if ($identity !== null && isset($wrappers[$identity])) { - return $wrappers[$identity]; + $wrapper = $wrappers[$identity]->get(); + if ($wrapper !== null) { + return $wrapper; + } } if ($depth >= 512) { return $value; } $wrapper = new self($value, $depth, $wrappers); if ($identity !== null) { - $wrappers[$identity] = $wrapper; + $wrappers[$identity] = WeakReference::create($wrapper); } return $wrapper; } @@ -86,13 +92,12 @@ public function jsonSerialize(): mixed } } else { $result = new stdClass(); - $class = new ReflectionClass($value); - while (!$class->isInternal() && ($parent = $class->getParentClass()) !== false) { - $class = $parent; - } - $properties = $class->isInternal() && $class->getName() !== stdClass::class - ? (array) $value - : get_object_vars($value); + // These native JSON handlers expose storage rather than the standard public property view. + $usesStorage = $value instanceof DateTimeInterface + || $value instanceof DateTimeZone + || (($value instanceof ArrayObject || $value instanceof ArrayIterator) + && !($value->getFlags() & ArrayObject::STD_PROP_LIST)); + $properties = $usesStorage ? (array) $value : get_object_vars($value); foreach ($properties as $key => $item) { if (!str_starts_with((string) $key, "\0")) { $result->{(string) $key} = self::prepare($item, $this->depth + 1, $this->wrappers); diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index 0539f430..de369ab2 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -15,6 +15,8 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v 'reference' => &$number, 'date' => new \DateTimeImmutable('2024-01-01T00:00:00Z'), 'arrayObject' => new \ArrayObject(['number' => NAN]), + 'arrayIterator' => new \ArrayIterator(['number' => INF]), + 'timezone' => new \DateTimeZone('UTC'), 'dateSubclass' => new class ('2024-01-01T00:00:00Z') extends \DateTimeImmutable { public string $label = 'kept'; private float $hidden = INF; @@ -28,7 +30,8 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v ]; self::assertSame( '{"reference":null,"date":{"date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' - . ',"arrayObject":{"number":null}' + . ',"arrayObject":{"number":null},"arrayIterator":{"number":null}' + . ',"timezone":{"timezone_type":3,"timezone":"UTC"}' . ',"dateSubclass":{"label":"kept","date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' . ',"arrayObjectSubclass":{"number":null},"emptyObject":{},"emptyArray":[]' . ',"sparseArray":{"2":null,"4":"kept"}}', @@ -67,6 +70,11 @@ public static function propertyHookParents(): array return [ 'ordinary class' => [''], 'stdClass subclass' => [' extends \\stdClass'], + 'Exception subclass' => [' extends \\Exception'], + 'Error subclass' => [' extends \\Error'], + 'SplObjectStorage subclass' => [' extends \\SplObjectStorage'], + 'ArrayObject with standard properties' => [' extends \\ArrayObject'], + 'ArrayIterator with standard properties' => [' extends \\ArrayIterator'], ]; } @@ -86,6 +94,9 @@ public function testPropertyHooksControlTheSerializedPublicView(string $parentDe } }; PHP, $parentDeclaration)); + if ($object instanceof \ArrayObject || $object instanceof \ArrayIterator) { + $object->setFlags(\ArrayObject::STD_PROP_LIST); + } $native = json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR); self::assertTrue($native === ['value' => 'filtered', 'virtual' => 'visible']); $encoded = json_decode( @@ -114,6 +125,9 @@ public function testPropertyGetterExceptionsAreNotSwallowed(string $parentDeclar } }; PHP, $parentDeclaration)); + if ($object instanceof \ArrayObject || $object instanceof \ArrayIterator) { + $object->setFlags(\ArrayObject::STD_PROP_LIST); + } $this->expectException(\LogicException::class); $this->expectExceptionMessage('unsupported getter'); JsonEncoder::encode($object); @@ -145,6 +159,43 @@ public function jsonSerialize(): mixed JsonEncoder::encode($value); } + public function testTemporarySerializedObjectsFitWithinMemoryLimit(): void + { + $code = <<<'PHP' + require $argv[1]; + $event = new class implements JsonSerializable { + public function jsonSerialize(): mixed + { + $items = []; + for ($i = 0; $i < 1000; ++$i) { + $items[] = (object) ['value' => $i]; + } + return $items; + } + }; + $batch = array_fill(0, 100, $event); + $native = json_encode($batch); + if ($native === false || PostHog\JsonEncoder::encode($batch) !== $native) { + exit(1); + } + echo 'encoded'; + PHP; + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=64M', '-d', 'max_execution_time=10', '-r', $code, + __DIR__ . '/../vendor/autoload.php'], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $errors); + self::assertSame('encoded', $output); + } + public function testRecursiveAndOverDepthInputFailsInBoundedSubprocess(): void { $code = <<<'PHP' diff --git a/test/NonFinitePropertiesTest.php b/test/NonFinitePropertiesTest.php index 5a10e2cb..9f5f3513 100644 --- a/test/NonFinitePropertiesTest.php +++ b/test/NonFinitePropertiesTest.php @@ -108,13 +108,22 @@ public function testCapturePreservesClosuresAlongsideNonFiniteNumbersAndNeighbor } } - public function testCapturePreservesStdClassSubclassPropertyHooks(): void + public static function propertyHookParents(): array + { + return [ + 'stdClass subclass' => ['stdClass'], + 'Exception subclass' => ['Exception'], + ]; + } + + #[DataProvider('propertyHookParents')] + public function testCapturePreservesSubclassPropertyHooks(string $parent): void { if (PHP_VERSION_ID < 80400) { self::markTestSkipped('Property hooks require PHP 8.4.'); } - $object = eval(<<<'PHP' - return new class extends stdClass { + $object = eval(sprintf(<<<'PHP' + return new class extends %s { public string $value = 'private fixture' { get => $this->value === '' ? '' : 'filtered'; } @@ -123,7 +132,7 @@ public function testCapturePreservesStdClassSubclassPropertyHooks(): void } public float $finite = 2.5; }; - PHP); + PHP, $parent)); $expected = ['value' => 'filtered', 'virtual' => 'visible', 'finite' => 2.5]; self::assertTrue(json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR) === $expected); $httpClient = new MockedHttpClient('unused.invalid'); From e41458905d03ae65f830848e01b87e3e8fd1ca2f Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 08:31:38 +0200 Subject: [PATCH 08/16] fix: preserve native flags and reduce plain-data encoding allocations --- lib/JsonEncoder.php | 130 ++++++++++++++-- test/JsonEncoderTest.php | 189 +++++++++++++++++++++++- test/fixtures/json-encoder-siblings.php | 69 +++++++++ 3 files changed, 376 insertions(+), 12 deletions(-) create mode 100644 test/fixtures/json-encoder-siblings.php diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index f8501770..29a70cd3 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -8,7 +8,9 @@ use DateTimeInterface; use DateTimeZone; use JsonSerializable; +use ReflectionMethod; use ReflectionReference; +use SplObjectStorage; use stdClass; use UnitEnum; use WeakReference; @@ -23,7 +25,8 @@ final class JsonEncoder implements JsonSerializable private function __construct( private mixed $value, private int $depth, - private ArrayObject $wrappers + private ArrayObject $wrappers, + private bool $plainData ) { } @@ -31,27 +34,86 @@ public static function encode(mixed $value): string|false { $wrappers = new ArrayObject(); try { - return json_encode(self::prepare($value, 0, $wrappers)); + $plainData = self::isPlainData($value, 0, new SplObjectStorage()); + return json_encode(self::prepare($value, 0, $wrappers, $plainData)); } finally { $wrappers->exchangeArray([]); } } + // Original objects are safe to reuse only when no callback can mutate a later sibling. + // References and unknown/native object types conservatively retain deferred preparation. + private static function isPlainData(mixed $value, int $depth, SplObjectStorage $active): bool + { + if ($depth >= 512) { + return false; + } + if (is_object($value)) { + if (get_class($value) !== stdClass::class || isset($active[$value])) { + return false; + } + $active[$value] = true; + try { + return self::isPlainData(get_object_vars($value), $depth + 1, $active); + } finally { + unset($active[$value]); + } + } + if (is_array($value)) { + foreach ($value as $key => $item) { + if ( + ReflectionReference::fromArrayElement($value, $key) !== null + || (is_string($key) && str_starts_with($key, "\0")) + || !self::isPlainData($item, $depth + 1, $active) + ) { + return false; + } + } + return true; + } + return $value === null || is_scalar($value); + } + private static function prepare( mixed $value, int $depth, ArrayObject $wrappers, - ?string $referenceId = null + bool $plainData, + ?string $referenceId = null, + ?bool &$changed = null ): mixed { + $changed = false; if (is_float($value) && !is_finite($value)) { + $changed = true; return null; } + if ($depth >= 512) { + return $value; + } + + if (is_array($value) && $referenceId === null) { + $hasReferences = false; + foreach ($value as $key => $item) { + if (ReflectionReference::fromArrayElement($value, $key) !== null) { + $hasReferences = true; + break; + } + } + if (!$hasReferences) { + return self::prepareArray($value, $depth, $wrappers, $plainData, $changed); + } + } + if ($plainData && is_object($value) && get_class($value) === stdClass::class) { + $properties = self::prepareArray(get_object_vars($value), $depth, $wrappers, true, $changed); + return $changed ? (object) $properties : $value; + } if ( is_array($value) || $value instanceof JsonSerializable || (is_object($value) && !$value instanceof UnitEnum && !$value instanceof Closure) ) { + $changed = true; $identity = is_object($value) ? 'object:' . spl_object_id($value) : $referenceId; if ($identity !== null && isset($wrappers[$identity])) { $wrapper = $wrappers[$identity]->get(); @@ -59,10 +121,7 @@ private static function prepare( return $wrapper; } } - if ($depth >= 512) { - return $value; - } - $wrapper = new self($value, $depth, $wrappers); + $wrapper = new self($value, $depth, $wrappers, $plainData); if ($identity !== null) { $wrappers[$identity] = WeakReference::create($wrapper); } @@ -72,13 +131,51 @@ private static function prepare( return $value; } + private static function prepareArray( + array $value, + int $depth, + ArrayObject $wrappers, + bool $plainData, + bool &$changed + ): array { + $changed = false; + $result = []; + $position = 0; + foreach ($value as $key => $item) { + $childChanged = false; + $prepared = self::prepare($item, $depth + 1, $wrappers, $plainData, null, $childChanged); + if ($childChanged && !$changed) { + // Copy by value into a new array: writing a COW copy can mutate caller reference cells. + $remaining = $position; + foreach ($value as $prefixKey => $prefixValue) { + if ($remaining-- === 0) { + break; + } + $result[$prefixKey] = $prefixValue; + } + $changed = true; + } + if ($changed) { + $result[$key] = $prepared; + } + ++$position; + } + return $changed ? $result : $value; + } + + private static function nativeArrayFlags(ArrayObject|ArrayIterator $value): int + { + $nativeClass = $value instanceof ArrayObject ? ArrayObject::class : ArrayIterator::class; + return (new ReflectionMethod($nativeClass, 'getFlags'))->invoke($value); + } + public function jsonSerialize(): mixed { $value = $this->value; if ($value instanceof JsonSerializable) { $serialized = $value->jsonSerialize(); if ($serialized !== $value) { - return self::prepare($serialized, $this->depth, $this->wrappers); + return self::prepare($serialized, $this->depth, $this->wrappers, $this->plainData); } } @@ -88,7 +185,13 @@ public function jsonSerialize(): mixed foreach ($value as $key => $item) { $reference = ReflectionReference::fromArrayElement($value, $key); $referenceId = $reference !== null ? 'reference:' . $reference->getId() : null; - $result[$key] = self::prepare($item, $this->depth + 1, $this->wrappers, $referenceId); + $result[$key] = self::prepare( + $item, + $this->depth + 1, + $this->wrappers, + $this->plainData, + $referenceId + ); } } else { $result = new stdClass(); @@ -96,11 +199,16 @@ public function jsonSerialize(): mixed $usesStorage = $value instanceof DateTimeInterface || $value instanceof DateTimeZone || (($value instanceof ArrayObject || $value instanceof ArrayIterator) - && !($value->getFlags() & ArrayObject::STD_PROP_LIST)); + && !(self::nativeArrayFlags($value) & ArrayObject::STD_PROP_LIST)); $properties = $usesStorage ? (array) $value : get_object_vars($value); foreach ($properties as $key => $item) { if (!str_starts_with((string) $key, "\0")) { - $result->{(string) $key} = self::prepare($item, $this->depth + 1, $this->wrappers); + $result->{(string) $key} = self::prepare( + $item, + $this->depth + 1, + $this->wrappers, + $this->plainData + ); } } } diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index de369ab2..de021163 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -40,6 +40,133 @@ public function testSupportedObjectRepresentationsAndReferencesArePreserved(): v self::assertSame(INF, $number); } + public static function nativeArrayFlagOverrides(): array + { + return [ + 'ArrayObject wrong flags' => ['ArrayObject', 'return self::STD_PROP_LIST;'], + 'ArrayIterator wrong flags' => ['ArrayIterator', 'return self::STD_PROP_LIST;'], + 'ArrayObject throwing flags' => ['ArrayObject', 'throw new LogicException("unexpected override");'], + 'ArrayIterator throwing flags' => ['ArrayIterator', 'throw new LogicException("unexpected override");'], + ]; + } + + #[DataProvider('nativeArrayFlagOverrides')] + public function testNativeArrayFlagOverridesAreNotInvoked(string $parent, string $override): void + { + $object = eval(sprintf(<<<'PHP' + return new class (['stored' => 7]) extends %s { + public string $excluded = 'private fixture'; + private int $overrideCalls = 0; + + public function getFlags(): int + { + ++$this->overrideCalls; + %s + } + + public function overrideCalls(): int + { + return $this->overrideCalls; + } + }; + PHP, $parent, $override)); + self::assertSame('{"stored":7}', json_encode($object)); + self::assertSame(0, $object->overrideCalls()); + self::assertTrue( + JsonEncoder::encode($object) === '{"stored":7}', + 'Native flags must preserve storage and exclude ignored public properties.' + ); + self::assertSame(0, $object->overrideCalls()); + } + + public static function nativeArrayParents(): array + { + return ['ArrayObject' => ['ArrayObject'], 'ArrayIterator' => ['ArrayIterator']]; + } + + #[DataProvider('nativeArrayParents')] + public function testNativeStandardPropertyFlagsPreserveHooksDespiteOverride(string $parent): void + { + if (PHP_VERSION_ID < 80400) { + self::markTestSkipped('Property hooks require PHP 8.4.'); + } + $object = eval(sprintf(<<<'PHP' + return new class (['stored' => 7]) extends %s { + public string $value = 'private fixture' { + get => $this->value === '' ? '' : 'filtered'; + } + public string $virtual { + get => 'visible'; + } + public function getFlags(): int + { + return 0; + } + }; + PHP, $parent)); + $object->setFlags(\ArrayObject::STD_PROP_LIST); + $expected = ['value' => 'filtered', 'virtual' => 'visible']; + self::assertTrue(json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR) === $expected); + self::assertTrue( + json_decode(JsonEncoder::encode($object), true, 512, JSON_THROW_ON_ERROR) === $expected, + 'Genuine native STD_PROP_LIST must retain the getter-filtered public view.' + ); + } + + public function testReferencedNonFiniteArraySlotDoesNotMutateCallerOrReorderList(): void + { + $number = INF; + $input = ['prefix' => 1, 'items' => [0, &$number, 2], 'suffix' => false]; + self::assertSame('{"prefix":1,"items":[0,null,2],"suffix":false}', JsonEncoder::encode($input)); + self::assertSame(INF, $number); + self::assertSame([0, INF, 2], $input['items']); + } + + public function testCustomSerializerRunsOnceBeforeLaterObjectValuesAreRead(): void + { + $createInput = static function (float $next): array { + $object = (object) ['number' => 1]; + $calls = (object) ['count' => 0]; + $serializer = new class ($object, $calls, $next) implements \JsonSerializable { + public function __construct(private object $object, private object $calls, private float $next) + { + } + + public function jsonSerialize(): mixed + { + ++$this->calls->count; + $this->object->number = $this->next; + return 'done'; + } + }; + return [['mutator' => $serializer, 'object' => $object], $calls]; + }; + [$nativeInput, $nativeCalls] = $createInput(4); + self::assertSame('{"mutator":"done","object":{"number":4}}', json_encode($nativeInput)); + self::assertSame(1, $nativeCalls->count); + [$input, $calls] = $createInput(NAN); + self::assertSame('{"mutator":"done","object":{"number":null}}', JsonEncoder::encode($input)); + self::assertSame(1, $calls->count); + } + + public function testPlainDataObjectsNormalizeNestedNumbersWithoutMutatingInput(): void + { + $object = (object) [ + 'number' => NAN, + 'nested' => (object) ['number' => INF], + 'items' => [-INF, 0], + 'finite' => 2.5, + 'literal' => 'NaN', + ]; + self::assertSame( + '{"number":null,"nested":{"number":null},"items":[null,0],"finite":2.5,"literal":"NaN"}', + JsonEncoder::encode($object) + ); + self::assertNan($object->number); + self::assertSame(INF, $object->nested->number); + self::assertSame([-INF, 0], $object->items); + } + public function testClosuresKeepTheirNativeEmptyObjectRepresentation(): void { $invoked = false; @@ -86,12 +213,20 @@ public function testPropertyHooksControlTheSerializedPublicView(string $parentDe } $object = eval(sprintf(<<<'PHP' return new class%s { + private int $getterCalls = 0; public string $value = 'private fixture' { - get => $this->value === '' ? '' : 'filtered'; + get { + ++$this->getterCalls; + return $this->value === '' ? '' : 'filtered'; + } } public string $virtual { get => 'visible'; } + public function getterCalls(): int + { + return $this->getterCalls; + } }; PHP, $parentDeclaration)); if ($object instanceof \ArrayObject || $object instanceof \ArrayIterator) { @@ -99,6 +234,7 @@ public function testPropertyHooksControlTheSerializedPublicView(string $parentDe } $native = json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR); self::assertTrue($native === ['value' => 'filtered', 'virtual' => 'visible']); + self::assertSame(1, $object->getterCalls()); $encoded = json_decode( JsonEncoder::encode(['number' => NAN, 'object' => $object]), true, @@ -110,6 +246,7 @@ public function testPropertyHooksControlTheSerializedPublicView(string $parentDe 'Serialization must use the public property getters, not backing storage.' ); self::assertNull($encoded['number']); + self::assertSame(2, $object->getterCalls()); } #[DataProvider('propertyHookParents')] @@ -159,6 +296,56 @@ public function jsonSerialize(): mixed JsonEncoder::encode($value); } + public static function siblingContainers(): array + { + return [ + 'empty arrays' => ['empty-arrays', 300000, false], + 'shared finite scalar arrays' => ['shared-scalar-arrays', 200000, false], + 'distinct finite scalar arrays' => ['distinct-scalar-arrays', 100000, false], + 'shared empty object' => ['shared-empty-object', 300000, false], + 'distinct empty objects' => ['distinct-empty-objects', 200000, false], + 'distinct finite scalar objects' => ['distinct-scalar-objects', 50000, false], + 'dense arrays with non-finite sibling' => ['shared-scalar-arrays', 200000, true], + 'dense objects with non-finite sibling' => ['distinct-empty-objects', 200000, true], + ]; + } + + #[DataProvider('siblingContainers')] + public function testSiblingContainersFitWithinNativeMemoryLimit( + string $shape, + int $count, + bool $includeNonFinite + ): void { + $native = $this->runSiblingMemoryProbe($shape, $count, 'native', $includeNonFinite); + self::assertSame(0, $native['exit'], 'Native baseline failed: ' . $native['stderr']); + $encoded = $this->runSiblingMemoryProbe($shape, $count, 'sdk', $includeNonFinite); + self::assertSame(0, $encoded['exit'], 'SDK encoding failed: ' . $encoded['stderr']); + $nativeLines = explode("\n", trim($native['stdout'])); + $encodedLines = explode("\n", trim($encoded['stdout'])); + $expected = json_decode(end($nativeLines), true, 512, JSON_THROW_ON_ERROR); + $actual = json_decode(end($encodedLines), true, 512, JSON_THROW_ON_ERROR); + self::assertLessThan(1024 * 1024, $expected['bytes']); + self::assertSame($expected, $actual); + } + + private function runSiblingMemoryProbe(string $shape, int $count, string $mode, bool $includeNonFinite): array + { + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=10', + __DIR__ . '/fixtures/json-encoder-siblings.php', __DIR__ . '/../vendor/autoload.php', + $shape, (string) $count, $mode, $includeNonFinite ? '1' : '0'], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + return ['exit' => proc_close($process), 'stdout' => $output, 'stderr' => $errors]; + } + public function testTemporarySerializedObjectsFitWithinMemoryLimit(): void { $code = <<<'PHP' diff --git a/test/fixtures/json-encoder-siblings.php b/test/fixtures/json-encoder-siblings.php new file mode 100644 index 00000000..da0082fe --- /dev/null +++ b/test/fixtures/json-encoder-siblings.php @@ -0,0 +1,69 @@ + $i % 10]; + } + break; +} +$properties = ['items' => $items]; +if ($includeNonFinite) { + $properties['numbers'] = [NAN, INF, -INF]; + $properties['finite'] = 2.5; + $properties['literal'] = 'NaN'; +} +$batch = [['event' => 'memory', 'distinct_id' => 'u', 'properties' => $properties]]; +echo "prepared\n"; +if ($mode === 'native') { + $expectedBatch = $batch; + if ($includeNonFinite) { + $expectedBatch[0]['properties']['numbers'] = [null, null, null]; + } + $encoded = json_encode(['batch' => $expectedBatch, 'api_key' => 'test-key']); +} else { + $consumer = new PostHog\Test\QueueConsumerTestConsumer([]); + $encoded = $consumer->encodedBatchPayload($batch); +} +if ($encoded === false) { + exit(1); +} +if ($includeNonFinite) { + if ( + !is_nan($batch[0]['properties']['numbers'][0]) + || $batch[0]['properties']['numbers'][1] !== INF + || $batch[0]['properties']['numbers'][2] !== -INF + ) { + exit(2); + } +} +echo json_encode([ + 'bytes' => strlen($encoded), + 'sha256' => hash('sha256', $encoded), +], JSON_THROW_ON_ERROR), "\n"; From 446beecf5943ff46aa9545bc372ef75f5de320d4 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 09:07:48 +0200 Subject: [PATCH 09/16] fix: preserve live reference reads and safe native eligibility --- lib/JsonEncoder.php | 100 ++++++++++---- test/JsonEncoderTest.php | 171 +++++++++++++++++++++++- test/fixtures/json-encoder-siblings.php | 15 +++ 3 files changed, 256 insertions(+), 30 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 29a70cd3..55dfe869 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -5,6 +5,8 @@ use ArrayIterator; use ArrayObject; use Closure; +use DateTime; +use DateTimeImmutable; use DateTimeInterface; use DateTimeZone; use JsonSerializable; @@ -26,7 +28,8 @@ private function __construct( private mixed $value, private int $depth, private ArrayObject $wrappers, - private bool $plainData + private bool $plainData, + private ?string $readReferenceId = null ) { } @@ -42,19 +45,25 @@ public static function encode(mixed $value): string|false } // Original objects are safe to reuse only when no callback can mutate a later sibling. - // References and unknown/native object types conservatively retain deferred preparation. + // References and unknown/callback-capable types conservatively retain deferred preparation. private static function isPlainData(mixed $value, int $depth, SplObjectStorage $active): bool { if ($depth >= 512) { return false; } if (is_object($value)) { - if (get_class($value) !== stdClass::class || isset($active[$value])) { + $class = get_class($value); + $native = $class === DateTime::class + || $class === DateTimeImmutable::class + || $class === DateTimeZone::class + || $class === ArrayObject::class + || $class === ArrayIterator::class; + if (($class !== stdClass::class && !$native) || isset($active[$value])) { return false; } $active[$value] = true; try { - return self::isPlainData(get_object_vars($value), $depth + 1, $active); + return self::isPlainData(self::nativeProperties($value), $depth + 1, $active); } finally { unset($active[$value]); } @@ -163,6 +172,38 @@ private static function prepareArray( return $changed ? $result : $value; } + private static function prepareReference( + array $parent, + int|string $key, + int $depth, + ArrayObject $wrappers, + bool $plainData, + ReflectionReference $reference + ): self { + $referenceId = 'reference:' . $reference->getId(); + $readerId = 'reader:' . $referenceId; + if (isset($wrappers[$readerId])) { + $reader = $wrappers[$readerId]->get(); + if ($reader !== null) { + return $reader; + } + } + $reader = new self(null, $depth, $wrappers, $plainData, $referenceId); + // ReflectionReference proves this cell already exists; never bind ordinary input slots. + $reader->value = &$parent[$key]; + $wrappers[$readerId] = WeakReference::create($reader); + return $reader; + } + + private static function nativeProperties(object $value): array + { + $usesStorage = $value instanceof DateTimeInterface + || $value instanceof DateTimeZone + || (($value instanceof ArrayObject || $value instanceof ArrayIterator) + && !(self::nativeArrayFlags($value) & ArrayObject::STD_PROP_LIST)); + return $usesStorage ? (array) $value : get_object_vars($value); + } + private static function nativeArrayFlags(ArrayObject|ArrayIterator $value): int { $nativeClass = $value instanceof ArrayObject ? ArrayObject::class : ArrayIterator::class; @@ -171,6 +212,15 @@ private static function nativeArrayFlags(ArrayObject|ArrayIterator $value): int public function jsonSerialize(): mixed { + if ($this->readReferenceId !== null) { + return self::prepare( + $this->value, + $this->depth, + $this->wrappers, + $this->plainData, + $this->readReferenceId + ); + } $value = $this->value; if ($value instanceof JsonSerializable) { $serialized = $value->jsonSerialize(); @@ -184,31 +234,33 @@ public function jsonSerialize(): mixed $result = []; foreach ($value as $key => $item) { $reference = ReflectionReference::fromArrayElement($value, $key); - $referenceId = $reference !== null ? 'reference:' . $reference->getId() : null; - $result[$key] = self::prepare( - $item, - $this->depth + 1, - $this->wrappers, - $this->plainData, - $referenceId - ); + $result[$key] = $reference !== null + ? self::prepareReference( + $value, + $key, + $this->depth + 1, + $this->wrappers, + $this->plainData, + $reference + ) + : self::prepare($item, $this->depth + 1, $this->wrappers, $this->plainData); } } else { $result = new stdClass(); - // These native JSON handlers expose storage rather than the standard public property view. - $usesStorage = $value instanceof DateTimeInterface - || $value instanceof DateTimeZone - || (($value instanceof ArrayObject || $value instanceof ArrayIterator) - && !(self::nativeArrayFlags($value) & ArrayObject::STD_PROP_LIST)); - $properties = $usesStorage ? (array) $value : get_object_vars($value); + $properties = self::nativeProperties($value); foreach ($properties as $key => $item) { if (!str_starts_with((string) $key, "\0")) { - $result->{(string) $key} = self::prepare( - $item, - $this->depth + 1, - $this->wrappers, - $this->plainData - ); + $reference = ReflectionReference::fromArrayElement($properties, $key); + $result->{(string) $key} = $reference !== null + ? self::prepareReference( + $properties, + $key, + $this->depth + 1, + $this->wrappers, + $this->plainData, + $reference + ) + : self::prepare($item, $this->depth + 1, $this->wrappers, $this->plainData); } } } diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index de021163..43a8a9a1 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -149,6 +149,152 @@ public function jsonSerialize(): mixed self::assertSame(1, $calls->count); } + public static function referencedScalarChanges(): array + { + $changes = [ + 'finite to finite' => [1, 2, '2'], + 'finite to NaN' => [1, NAN, 'null'], + 'finite to positive infinity' => [1, INF, 'null'], + 'finite to negative infinity' => [1, -INF, 'null'], + 'NaN to finite' => [NAN, 2, '2'], + 'positive infinity to finite' => [INF, 2, '2'], + 'negative infinity to finite' => [-INF, 2, '2'], + ]; + $cases = []; + foreach (['array', 'stdClass', 'ArrayObject', 'ArrayObject public view'] as $container) { + foreach ($changes as $label => $values) { + $cases[$container . ' ' . $label] = [$container, ...$values]; + } + } + return $cases; + } + + #[DataProvider('referencedScalarChanges')] + public function testReferencedScalarIsReadAfterEarlierSerializer( + string $container, + int|float $initial, + int|float $next, + string $expectedNumber + ): void { + $encode = static function (bool $native) use ($container, $initial, $next): array { + $number = $initial; + $calls = 0; + $serializer = new class ($number, $calls, $next) implements \JsonSerializable { + private int|float $number; + private int $calls; + + public function __construct(int|float &$number, int &$calls, private int|float $next) + { + $this->number = &$number; + $this->calls = &$calls; + } + + public function jsonSerialize(): mixed + { + ++$this->calls; + $this->number = $this->next; + return null; + } + }; + $fields = ['mutator' => $serializer, 'n' => &$number, 'after' => 7]; + $input = match ($container) { + 'array' => $fields, + 'stdClass' => (object) $fields, + 'ArrayObject' => new \ArrayObject($fields), + 'ArrayObject public view' => new class extends \ArrayObject { + public mixed $mutator; + public mixed $n; + public int $after = 7; + }, + }; + if ($container === 'ArrayObject public view') { + $input->setFlags(\ArrayObject::STD_PROP_LIST); + $input->mutator = $serializer; + $input->n = &$number; + } + $before = is_array($input) ? $input : (array) $input; + $referenceId = \ReflectionReference::fromArrayElement($before, 'n')->getId(); + $json = $native ? json_encode($input) : JsonEncoder::encode($input); + $error = json_last_error(); + $after = is_array($input) ? $input : (array) $input; + self::assertSame(['mutator', 'n', 'after'], array_keys($after)); + self::assertSame($referenceId, \ReflectionReference::fromArrayElement($after, 'n')->getId()); + self::assertNull(\ReflectionReference::fromArrayElement($after, 'mutator')); + self::assertNull(\ReflectionReference::fromArrayElement($after, 'after')); + self::assertSame(1, $calls); + if (is_nan($next)) { + self::assertNan($number); + } else { + self::assertSame($next, $number); + } + return [$json, $error]; + }; + [$native, $error] = $encode(true); + if (is_finite($next)) { + self::assertSame('{"mutator":null,"n":2,"after":7}', $native); + } else { + self::assertFalse($native); + self::assertSame(JSON_ERROR_INF_OR_NAN, $error); + } + [$actual] = $encode(false); + self::assertSame('{"mutator":null,"n":' . $expectedNumber . ',"after":7}', $actual); + } + + public function testSharedReferenceIsReadAtEachWirePosition(): void + { + $number = 1; + $serializer = new class ($number) implements \JsonSerializable { + private int $number; + + public function __construct(int &$number) + { + $this->number = &$number; + } + + public function jsonSerialize(): mixed + { + $this->number = 2; + return null; + } + }; + $input = ['first' => &$number, 'mutator' => $serializer, 'last' => &$number]; + self::assertSame('{"first":1,"mutator":null,"last":2}', JsonEncoder::encode($input)); + self::assertSame(2, $number); + self::assertSame( + \ReflectionReference::fromArrayElement($input, 'first')->getId(), + \ReflectionReference::fromArrayElement($input, 'last')->getId() + ); + } + + public function testNativeSubclassSerializerRemainsDeferred(): void + { + $object = (object) ['number' => 1]; + $calls = (object) ['count' => 0]; + $serializer = new class ($object, $calls) extends \ArrayObject implements \JsonSerializable { + public function __construct(private object $object, private object $calls) + { + parent::__construct(['stored' => 7]); + } + + public function jsonSerialize(): mixed + { + ++$this->calls->count; + $this->object->number = NAN; + return 'done'; + } + + public function getFlags(): int + { + throw new \LogicException('must not dispatch'); + } + }; + self::assertSame( + '{"mutator":"done","object":{"number":null}}', + JsonEncoder::encode(['mutator' => $serializer, 'object' => $object]) + ); + self::assertSame(1, $calls->count); + } + public function testPlainDataObjectsNormalizeNestedNumbersWithoutMutatingInput(): void { $object = (object) [ @@ -307,6 +453,13 @@ public static function siblingContainers(): array 'distinct finite scalar objects' => ['distinct-scalar-objects', 50000, false], 'dense arrays with non-finite sibling' => ['shared-scalar-arrays', 200000, true], 'dense objects with non-finite sibling' => ['distinct-empty-objects', 200000, true], + 'dense objects with DateTime' => ['distinct-empty-objects', 200000, true, 'DateTime'], + 'dense objects with DateTimeImmutable' => ['distinct-empty-objects', 200000, true, 'DateTimeImmutable'], + 'dense objects with DateTimeZone' => ['distinct-empty-objects', 200000, true, 'DateTimeZone'], + 'dense objects with ArrayObject storage' => ['distinct-empty-objects', 200000, true, 'ArrayObject'], + 'dense objects with ArrayIterator storage' => ['distinct-empty-objects', 200000, true, 'ArrayIterator'], + 'ArrayObject public sibling' => ['distinct-empty-objects', 200000, true, 'ArrayObject-properties'], + 'ArrayIterator public sibling' => ['distinct-empty-objects', 200000, true, 'ArrayIterator-properties'], ]; } @@ -314,11 +467,12 @@ public static function siblingContainers(): array public function testSiblingContainersFitWithinNativeMemoryLimit( string $shape, int $count, - bool $includeNonFinite + bool $includeNonFinite, + string $nativeType = '' ): void { - $native = $this->runSiblingMemoryProbe($shape, $count, 'native', $includeNonFinite); + $native = $this->runSiblingMemoryProbe($shape, $count, 'native', $includeNonFinite, $nativeType); self::assertSame(0, $native['exit'], 'Native baseline failed: ' . $native['stderr']); - $encoded = $this->runSiblingMemoryProbe($shape, $count, 'sdk', $includeNonFinite); + $encoded = $this->runSiblingMemoryProbe($shape, $count, 'sdk', $includeNonFinite, $nativeType); self::assertSame(0, $encoded['exit'], 'SDK encoding failed: ' . $encoded['stderr']); $nativeLines = explode("\n", trim($native['stdout'])); $encodedLines = explode("\n", trim($encoded['stdout'])); @@ -328,12 +482,17 @@ public function testSiblingContainersFitWithinNativeMemoryLimit( self::assertSame($expected, $actual); } - private function runSiblingMemoryProbe(string $shape, int $count, string $mode, bool $includeNonFinite): array - { + private function runSiblingMemoryProbe( + string $shape, + int $count, + string $mode, + bool $includeNonFinite, + string $nativeType + ): array { $process = proc_open( [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=10', __DIR__ . '/fixtures/json-encoder-siblings.php', __DIR__ . '/../vendor/autoload.php', - $shape, (string) $count, $mode, $includeNonFinite ? '1' : '0'], + $shape, (string) $count, $mode, $includeNonFinite ? '1' : '0', $nativeType], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes ); diff --git a/test/fixtures/json-encoder-siblings.php b/test/fixtures/json-encoder-siblings.php index da0082fe..8eb4783c 100644 --- a/test/fixtures/json-encoder-siblings.php +++ b/test/fixtures/json-encoder-siblings.php @@ -6,6 +6,7 @@ $count = (int) $argv[3]; $mode = $argv[4]; $includeNonFinite = ($argv[5] ?? '0') === '1'; +$nativeType = $argv[6] ?? ''; $items = []; switch ($shape) { case 'empty-arrays': @@ -39,6 +40,20 @@ $properties['finite'] = 2.5; $properties['literal'] = 'NaN'; } +if ($nativeType !== '') { + $native = match ($nativeType) { + 'DateTime' => new DateTime('2024-01-01T00:00:00Z'), + 'DateTimeImmutable' => new DateTimeImmutable('2024-01-01T00:00:00Z'), + 'DateTimeZone' => new DateTimeZone('UTC'), + 'ArrayObject', 'ArrayObject-properties' => new ArrayObject(['stored' => 7]), + 'ArrayIterator', 'ArrayIterator-properties' => new ArrayIterator(['stored' => 7]), + }; + if (str_ends_with($nativeType, '-properties')) { + $native->setFlags(ArrayObject::STD_PROP_LIST); + $native->visible = 7; + } + $properties['native'] = $native; +} $batch = [['event' => 'memory', 'distinct_id' => 'u', 'properties' => $properties]]; echo "prepared\n"; if ($mode === 'native') { From 7e232ad94344c71006f9165c27db22098836c036 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 09:36:40 +0200 Subject: [PATCH 10/16] fix: defer declared property reads and bound JSON preparation --- lib/JsonEncoder.php | 80 +++++++++++++++++++++++++++++++------ test/JsonEncoderTest.php | 86 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 55dfe869..e6664839 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -10,6 +10,7 @@ use DateTimeInterface; use DateTimeZone; use JsonSerializable; +use ReflectionClass; use ReflectionMethod; use ReflectionReference; use SplObjectStorage; @@ -24,20 +25,24 @@ */ final class JsonEncoder implements JsonSerializable { + private const MAX_PREPARE_STEPS = 1000000; + private function __construct( private mixed $value, private int $depth, private ArrayObject $wrappers, private bool $plainData, - private ?string $readReferenceId = null + private ?string $readReferenceId = null, + private ?string $readProperty = null ) { } public static function encode(mixed $value): string|false { - $wrappers = new ArrayObject(); + $wrappers = new ArrayObject(['remaining' => self::MAX_PREPARE_STEPS]); try { - $plainData = self::isPlainData($value, 0, new SplObjectStorage()); + $remaining = self::MAX_PREPARE_STEPS; + $plainData = self::isPlainData($value, 0, new SplObjectStorage(), $remaining); return json_encode(self::prepare($value, 0, $wrappers, $plainData)); } finally { $wrappers->exchangeArray([]); @@ -46,9 +51,14 @@ public static function encode(mixed $value): string|false // Original objects are safe to reuse only when no callback can mutate a later sibling. // References and unknown/callback-capable types conservatively retain deferred preparation. - private static function isPlainData(mixed $value, int $depth, SplObjectStorage $active): bool - { - if ($depth >= 512) { + private static function isPlainData( + mixed $value, + int $depth, + SplObjectStorage $active, + int &$remaining + ): bool { + // Bounded lookahead must yield to native encoding before expanding a shared graph. + if (--$remaining < 0 || $depth >= 512) { return false; } if (is_object($value)) { @@ -63,7 +73,7 @@ private static function isPlainData(mixed $value, int $depth, SplObjectStorage $ } $active[$value] = true; try { - return self::isPlainData(self::nativeProperties($value), $depth + 1, $active); + return self::isPlainData(self::nativeProperties($value), $depth + 1, $active, $remaining); } finally { unset($active[$value]); } @@ -73,7 +83,7 @@ private static function isPlainData(mixed $value, int $depth, SplObjectStorage $ if ( ReflectionReference::fromArrayElement($value, $key) !== null || (is_string($key) && str_starts_with($key, "\0")) - || !self::isPlainData($item, $depth + 1, $active) + || !self::isPlainData($item, $depth + 1, $active, $remaining) ) { return false; } @@ -100,7 +110,8 @@ private static function prepare( return $value; } - if (is_array($value) && $referenceId === null) { + $eager = $wrappers['remaining']-- > 0; + if ($eager && is_array($value) && $referenceId === null) { $hasReferences = false; foreach ($value as $key => $item) { if (ReflectionReference::fromArrayElement($value, $key) !== null) { @@ -112,7 +123,7 @@ private static function prepare( return self::prepareArray($value, $depth, $wrappers, $plainData, $changed); } } - if ($plainData && is_object($value) && get_class($value) === stdClass::class) { + if ($eager && $plainData && is_object($value) && get_class($value) === stdClass::class) { $properties = self::prepareArray(get_object_vars($value), $depth, $wrappers, true, $changed); return $changed ? (object) $properties : $value; } @@ -197,11 +208,36 @@ private static function prepareReference( private static function nativeProperties(object $value): array { - $usesStorage = $value instanceof DateTimeInterface + return self::usesStorage($value) ? (array) $value : get_object_vars($value); + } + + private static function livePropertyNames(object $value): array + { + if (self::usesStorage($value)) { + return []; + } + $names = []; + $class = new ReflectionClass($value); + do { + foreach ($class->getProperties() as $property) { + // Native encoding snapshots all property hooks before invoking child serializers. + if (PHP_VERSION_ID >= 80400 && $property->hasHooks()) { + return []; + } + if ($property->isPublic() && !$property->isStatic()) { + $names[$property->getName()] = true; + } + } + } while (($class = $class->getParentClass()) !== false); + return $names; + } + + private static function usesStorage(object $value): bool + { + return $value instanceof DateTimeInterface || $value instanceof DateTimeZone || (($value instanceof ArrayObject || $value instanceof ArrayIterator) && !(self::nativeArrayFlags($value) & ArrayObject::STD_PROP_LIST)); - return $usesStorage ? (array) $value : get_object_vars($value); } private static function nativeArrayFlags(ArrayObject|ArrayIterator $value): int @@ -212,6 +248,14 @@ private static function nativeArrayFlags(ArrayObject|ArrayIterator $value): int public function jsonSerialize(): mixed { + if ($this->readProperty !== null) { + return self::prepare( + $this->value->{$this->readProperty}, + $this->depth, + $this->wrappers, + $this->plainData + ); + } if ($this->readReferenceId !== null) { return self::prepare( $this->value, @@ -248,8 +292,20 @@ public function jsonSerialize(): mixed } else { $result = new stdClass(); $properties = self::nativeProperties($value); + $liveProperties = self::livePropertyNames($value); foreach ($properties as $key => $item) { if (!str_starts_with((string) $key, "\0")) { + if (isset($liveProperties[$key])) { + $result->{(string) $key} = new self( + $value, + $this->depth + 1, + $this->wrappers, + $this->plainData, + null, + (string) $key + ); + continue; + } $reference = ReflectionReference::fromArrayElement($properties, $key); $result->{(string) $key} = $reference !== null ? self::prepareReference( diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index 43a8a9a1..4817f621 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -149,6 +149,60 @@ public function jsonSerialize(): mixed self::assertSame(1, $calls->count); } + public static function declaredPropertyChanges(): array + { + return [ + 'ordinary class redaction' => ['', 'redacted'], + 'stdClass subclass redaction' => [' extends \\stdClass', 'redacted'], + 'Exception subclass redaction' => [' extends \\Exception', 'redacted'], + 'ordinary class non-finite update' => ['', NAN], + ]; + } + + #[DataProvider('declaredPropertyChanges')] + public function testDeclaredPropertyIsReadAfterEarlierSerializer(string $parent, mixed $next): void + { + $make = static function () use ($parent, $next): object { + $object = eval(sprintf(<<<'PHP' + return new class%s { + public mixed $mutator; + public mixed $value = 'secret'; + }; + PHP, $parent)); + $object->mutator = new class ($object, $next) implements \JsonSerializable { + public int $calls = 0; + + public function __construct(private object $parent, private mixed $next) + { + } + + public function jsonSerialize(): mixed + { + ++$this->calls; + $this->parent->value = $this->next; + return 'done'; + } + }; + return $object; + }; + $native = json_encode($make()); + if (is_float($next)) { + self::assertFalse($native); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + $expected = '{"mutator":"done","value":null}'; + } else { + $expected = '{"mutator":"done","value":"redacted"}'; + self::assertSame($expected, $native); + } + $object = $make(); + self::assertTrue( + JsonEncoder::encode($object) === $expected, + 'Declared properties must be read after earlier serializers have run.' + ); + self::assertSame(1, $object->mutator->calls); + self::assertNull(\ReflectionReference::fromArrayElement((array) $object, 'value')); + } + public static function referencedScalarChanges(): array { $changes = [ @@ -542,6 +596,38 @@ public function jsonSerialize(): mixed self::assertSame('encoded', $output); } + public function testSharedInvalidArrayGraphIsRejectedWithoutEagerExpansion(): void + { + $code = <<<'PHP' + require $argv[1]; + $value = ["\xB1"]; + for ($i = 0; $i < 24; ++$i) { + $value = [$value, $value]; + } + if (json_encode($value) !== false || json_last_error() !== JSON_ERROR_UTF8) { + exit(1); + } + if (PostHog\JsonEncoder::encode($value) !== false || json_last_error() !== JSON_ERROR_UTF8) { + exit(2); + } + echo 'rejected'; + PHP; + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=2', '-r', $code, + __DIR__ . '/../vendor/autoload.php'], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $errors); + self::assertSame('rejected', $output); + } + public function testRecursiveAndOverDepthInputFailsInBoundedSubprocess(): void { $code = <<<'PHP' From c5d55ce50d6c4038d090e64189a6b1d9351fdd2b Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 09:46:42 +0200 Subject: [PATCH 11/16] fix: reject invalid UTF-8 before expanding shared JSON graphs --- lib/JsonEncoder.php | 15 +++++++++++++-- test/JsonEncoderTest.php | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index e6664839..313fa539 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -43,6 +43,9 @@ public static function encode(mixed $value): string|false try { $remaining = self::MAX_PREPARE_STEPS; $plainData = self::isPlainData($value, 0, new SplObjectStorage(), $remaining); + if ($plainData === null) { + return false; + } return json_encode(self::prepare($value, 0, $wrappers, $plainData)); } finally { $wrappers->exchangeArray([]); @@ -56,7 +59,7 @@ private static function isPlainData( int $depth, SplObjectStorage $active, int &$remaining - ): bool { + ): ?bool { // Bounded lookahead must yield to native encoding before expanding a shared graph. if (--$remaining < 0 || $depth >= 512) { return false; @@ -83,13 +86,21 @@ private static function isPlainData( if ( ReflectionReference::fromArrayElement($value, $key) !== null || (is_string($key) && str_starts_with($key, "\0")) - || !self::isPlainData($item, $depth + 1, $active, $remaining) ) { return false; } + $plainData = self::isPlainData($item, $depth + 1, $active, $remaining); + if ($plainData !== true) { + return $plainData; + } } return true; } + if (is_string($value) && preg_match('//u', $value) !== 1) { + // Preserve the native error without expanding a graph that is already invalid. + json_encode($value); + return null; + } return $value === null || is_scalar($value); } diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index 4817f621..61fbeaf1 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -596,6 +596,25 @@ public function jsonSerialize(): mixed self::assertSame('encoded', $output); } + public function testInvalidUtf8DoesNotChangeSerializerInvocationOrder(): void + { + $serializer = new class implements \JsonSerializable { + public int $calls = 0; + + public function jsonSerialize(): mixed + { + ++$this->calls; + return 'kept'; + } + }; + self::assertFalse(JsonEncoder::encode(['invalid' => "\xB1", 'serializer' => $serializer])); + self::assertSame(JSON_ERROR_UTF8, json_last_error()); + self::assertSame(0, $serializer->calls); + self::assertFalse(JsonEncoder::encode(['serializer' => $serializer, 'invalid' => "\xB1"])); + self::assertSame(JSON_ERROR_UTF8, json_last_error()); + self::assertSame(1, $serializer->calls); + } + public function testSharedInvalidArrayGraphIsRejectedWithoutEagerExpansion(): void { $code = <<<'PHP' From 5a4a5401f11a312d7dd3d155e87debf668c11414 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 11:12:44 +0200 Subject: [PATCH 12/16] fix: emit JSON sequentially without wrapper graph allocation --- api/public-api.json | 11 +- lib/JsonEncoder.php | 447 +++++++++++---------------- test/JsonEncoderTest.php | 205 ++++++++++++ test/fixtures/json-encoder-scale.php | 32 ++ 4 files changed, 425 insertions(+), 270 deletions(-) create mode 100644 test/fixtures/json-encoder-scale.php diff --git a/api/public-api.json b/api/public-api.json index a635ad63..f5dbc3a7 100644 --- a/api/public-api.json +++ b/api/public-api.json @@ -3409,9 +3409,7 @@ "final": true, "readonly": false, "extends": null, - "implements": [ - "JsonSerializable" - ], + "implements": [], "constants": [], "properties": [], "methods": { @@ -3432,13 +3430,6 @@ "hasDefault": false } ] - }, - "jsonSerialize": { - "static": false, - "abstract": false, - "final": false, - "returnType": "mixed", - "parameters": [] } } }, diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 313fa539..bc77ba95 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -5,10 +5,9 @@ use ArrayIterator; use ArrayObject; use Closure; -use DateTime; -use DateTimeImmutable; use DateTimeInterface; use DateTimeZone; +use JsonException; use JsonSerializable; use ReflectionClass; use ReflectionMethod; @@ -16,322 +15,250 @@ use SplObjectStorage; use stdClass; use UnitEnum; -use WeakReference; /** * Encodes event payloads with non-finite numbers represented as null. * * @internal */ -final class JsonEncoder implements JsonSerializable +final class JsonEncoder { - private const MAX_PREPARE_STEPS = 1000000; + private string $output = ''; + private int $error = JSON_ERROR_NONE; + private mixed $errorValue = null; + private array $references = []; + private array $propertyLayouts = []; + private SplObjectStorage $objects; - private function __construct( - private mixed $value, - private int $depth, - private ArrayObject $wrappers, - private bool $plainData, - private ?string $readReferenceId = null, - private ?string $readProperty = null - ) { + private function __construct() + { + $this->objects = new SplObjectStorage(); } public static function encode(mixed $value): string|false { - $wrappers = new ArrayObject(['remaining' => self::MAX_PREPARE_STEPS]); - try { - $remaining = self::MAX_PREPARE_STEPS; - $plainData = self::isPlainData($value, 0, new SplObjectStorage(), $remaining); - if ($plainData === null) { - return false; - } - return json_encode(self::prepare($value, 0, $wrappers, $plainData)); - } finally { - $wrappers->exchangeArray([]); + $encoder = new self(); + if ($encoder->writeValue($value, 0) && $encoder->error === JSON_ERROR_NONE) { + json_encode(null); + return $encoder->output; } + $encoder->restoreError(); + return false; } - // Original objects are safe to reuse only when no callback can mutate a later sibling. - // References and unknown/callback-capable types conservatively retain deferred preparation. - private static function isPlainData( - mixed $value, - int $depth, - SplObjectStorage $active, - int &$remaining - ): ?bool { - // Bounded lookahead must yield to native encoding before expanding a shared graph. - if (--$remaining < 0 || $depth >= 512) { - return false; + private function writeValue(mixed $value, int $depth, ?string $referenceId = null): bool + { + if (is_float($value) && !is_finite($value)) { + $value = null; } - if (is_object($value)) { - $class = get_class($value); - $native = $class === DateTime::class - || $class === DateTimeImmutable::class - || $class === DateTimeZone::class - || $class === ArrayObject::class - || $class === ArrayIterator::class; - if (($class !== stdClass::class && !$native) || isset($active[$value])) { + if (is_array($value)) { + if ($referenceId !== null && isset($this->references[$referenceId])) { + $this->error = JSON_ERROR_RECURSION; return false; } - $active[$value] = true; + if ($depth >= 512) { + $this->error = JSON_ERROR_DEPTH; + return false; + } + if ($value === []) { + $this->output .= '[]'; + return true; + } + if ($referenceId !== null) { + $this->references[$referenceId] = true; + } try { - return self::isPlainData(self::nativeProperties($value), $depth + 1, $active, $remaining); + return $this->writeArray($value, $depth); } finally { - unset($active[$value]); - } - } - if (is_array($value)) { - foreach ($value as $key => $item) { - if ( - ReflectionReference::fromArrayElement($value, $key) !== null - || (is_string($key) && str_starts_with($key, "\0")) - ) { - return false; - } - $plainData = self::isPlainData($item, $depth + 1, $active, $remaining); - if ($plainData !== true) { - return $plainData; + if ($referenceId !== null) { + unset($this->references[$referenceId]); } } - return true; - } - if (is_string($value) && preg_match('//u', $value) !== 1) { - // Preserve the native error without expanding a graph that is already invalid. - json_encode($value); - return null; } - return $value === null || is_scalar($value); - } - - private static function prepare( - mixed $value, - int $depth, - ArrayObject $wrappers, - bool $plainData, - ?string $referenceId = null, - ?bool &$changed = null - ): mixed { - $changed = false; - if (is_float($value) && !is_finite($value)) { - $changed = true; - return null; + if ( + is_object($value) + && ($value instanceof JsonSerializable || (!$value instanceof UnitEnum && !$value instanceof Closure)) + ) { + return $this->writeObject($value, $depth); } - if ($depth >= 512) { - return $value; + $encoded = $this->encodeScalar($value); + if ($encoded === false) { + return false; } + $this->output .= $encoded; + return true; + } - $eager = $wrappers['remaining']-- > 0; - if ($eager && is_array($value) && $referenceId === null) { - $hasReferences = false; - foreach ($value as $key => $item) { - if (ReflectionReference::fromArrayElement($value, $key) !== null) { - $hasReferences = true; - break; - } + private function writeArray(array $value, int $depth, bool $object = false): bool + { + $list = !$object && array_is_list($value); + $this->output .= $list ? '[' : '{'; + $first = true; + foreach ($value as $key => $item) { + if ($object && str_starts_with((string) $key, "\0")) { + continue; } - if (!$hasReferences) { - return self::prepareArray($value, $depth, $wrappers, $plainData, $changed); + if (!$first) { + $this->output .= ','; } - } - if ($eager && $plainData && is_object($value) && get_class($value) === stdClass::class) { - $properties = self::prepareArray(get_object_vars($value), $depth, $wrappers, true, $changed); - return $changed ? (object) $properties : $value; - } - - if ( - is_array($value) - || $value instanceof JsonSerializable - || (is_object($value) && !$value instanceof UnitEnum && !$value instanceof Closure) - ) { - $changed = true; - $identity = is_object($value) ? 'object:' . spl_object_id($value) : $referenceId; - if ($identity !== null && isset($wrappers[$identity])) { - $wrapper = $wrappers[$identity]->get(); - if ($wrapper !== null) { - return $wrapper; - } + $first = false; + if (!$list) { + $this->writeKey((string) $key); } - $wrapper = new self($value, $depth, $wrappers, $plainData); - if ($identity !== null) { - $wrappers[$identity] = WeakReference::create($wrapper); + $reference = ReflectionReference::fromArrayElement($value, $key); + if (!$this->writeValue($item, $depth + 1, $reference?->getId())) { + return false; } - return $wrapper; } - - return $value; + $this->output .= $list ? ']' : '}'; + return true; } - private static function prepareArray( - array $value, - int $depth, - ArrayObject $wrappers, - bool $plainData, - bool &$changed - ): array { - $changed = false; - $result = []; - $position = 0; - foreach ($value as $key => $item) { - $childChanged = false; - $prepared = self::prepare($item, $depth + 1, $wrappers, $plainData, null, $childChanged); - if ($childChanged && !$changed) { - // Copy by value into a new array: writing a COW copy can mutate caller reference cells. - $remaining = $position; - foreach ($value as $prefixKey => $prefixValue) { - if ($remaining-- === 0) { - break; - } - $result[$prefixKey] = $prefixValue; - } - $changed = true; - } - if ($changed) { - $result[$key] = $prepared; - } - ++$position; + private function encodeScalar(mixed $value): string|false + { + try { + return json_encode($value, JSON_THROW_ON_ERROR); + } catch (JsonException $error) { + $this->error = $error->getCode(); + $this->errorValue = $value; + return false; } - return $changed ? $result : $value; } - private static function prepareReference( - array $parent, - int|string $key, - int $depth, - ArrayObject $wrappers, - bool $plainData, - ReflectionReference $reference - ): self { - $referenceId = 'reference:' . $reference->getId(); - $readerId = 'reader:' . $referenceId; - if (isset($wrappers[$readerId])) { - $reader = $wrappers[$readerId]->get(); - if ($reader !== null) { - return $reader; - } + private function writeKey(string $key): void + { + $encoded = $this->encodeScalar($key); + if ($encoded === false) { + // Native JSON continues visiting members after a malformed key, but never returns partial output. + $encoded = '""'; } - $reader = new self(null, $depth, $wrappers, $plainData, $referenceId); - // ReflectionReference proves this cell already exists; never bind ordinary input slots. - $reader->value = &$parent[$key]; - $wrappers[$readerId] = WeakReference::create($reader); - return $reader; + $this->output .= $encoded . ':'; } - private static function nativeProperties(object $value): array + private function writeObject(object $value, int $depth): bool { - return self::usesStorage($value) ? (array) $value : get_object_vars($value); + if (isset($this->objects[$value])) { + $this->error = JSON_ERROR_RECURSION; + return false; + } + $this->objects[$value] = true; + try { + if ($value instanceof JsonSerializable) { + $serialized = $value->jsonSerialize(); + if ($serialized !== $value) { + return $this->writeValue($serialized, $depth); + } + } + if ($depth >= 512) { + $this->error = JSON_ERROR_DEPTH; + return false; + } + if (self::usesStorage($value)) { + return $this->writeArray((array) $value, $depth, true); + } + if (get_class($value) === stdClass::class || (new ReflectionClass($value))->isInternal()) { + return $this->writeArray(get_object_vars($value), $depth, true); + } + $layout = $this->propertyLayout($value); + // Snapshot dynamic members without invoking declared getters ahead of their wire position. + $properties = get_mangled_object_vars($value); + $this->output .= '{'; + $first = true; + foreach ($layout as $name => $property) { + unset($properties[$name]); + if (PHP_VERSION_ID >= 80400 && $property->hasHooks()) { + if ($property->isVirtual() && !$property->hasHook(\PropertyHookType::Get)) { + continue; + } + } elseif (!$property->isInitialized($value)) { + continue; + } + if (!$first) { + $this->output .= ','; + } + $first = false; + $this->writeKey($name); + if (!$this->writeValue($value->{$name}, $depth + 1)) { + return false; + } + } + foreach ($properties as $name => $item) { + if (str_starts_with((string) $name, "\0")) { + continue; + } + if (!$first) { + $this->output .= ','; + } + $first = false; + $this->writeKey((string) $name); + $reference = ReflectionReference::fromArrayElement($properties, $name); + if (!$this->writeValue($item, $depth + 1, $reference?->getId())) { + return false; + } + } + $this->output .= '}'; + return true; + } finally { + unset($this->objects[$value]); + } } - private static function livePropertyNames(object $value): array + private function propertyLayout(object $value): array { - if (self::usesStorage($value)) { - return []; + $name = get_class($value); + if (array_key_exists($name, $this->propertyLayouts)) { + return $this->propertyLayouts[$name]; } - $names = []; + $classes = []; $class = new ReflectionClass($value); do { + $classes[] = $class; + } while (($class = $class->getParentClass()) !== false); + $slots = []; + foreach (array_reverse($classes) as $class) { foreach ($class->getProperties() as $property) { - // Native encoding snapshots all property hooks before invoking child serializers. - if (PHP_VERSION_ID >= 80400 && $property->hasHooks()) { - return []; - } - if ($property->isPublic() && !$property->isStatic()) { - $names[$property->getName()] = true; + if ($property->isStatic() || $property->getDeclaringClass()->getName() !== $class->getName()) { + continue; } + $key = $property->isPrivate() + ? "\0" . $class->getName() . "\0" . $property->getName() + : $property->getName(); + $slots[$key] = $property; } - } while (($class = $class->getParentClass()) !== false); - return $names; + } + return $this->propertyLayouts[$name] = array_filter( + $slots, + static fn ($property) => $property->isPublic() + ); } private static function usesStorage(object $value): bool { - return $value instanceof DateTimeInterface - || $value instanceof DateTimeZone - || (($value instanceof ArrayObject || $value instanceof ArrayIterator) - && !(self::nativeArrayFlags($value) & ArrayObject::STD_PROP_LIST)); - } - - private static function nativeArrayFlags(ArrayObject|ArrayIterator $value): int - { - $nativeClass = $value instanceof ArrayObject ? ArrayObject::class : ArrayIterator::class; - return (new ReflectionMethod($nativeClass, 'getFlags'))->invoke($value); - } - - public function jsonSerialize(): mixed - { - if ($this->readProperty !== null) { - return self::prepare( - $this->value->{$this->readProperty}, - $this->depth, - $this->wrappers, - $this->plainData - ); - } - if ($this->readReferenceId !== null) { - return self::prepare( - $this->value, - $this->depth, - $this->wrappers, - $this->plainData, - $this->readReferenceId - ); + if ($value instanceof DateTimeInterface || $value instanceof DateTimeZone) { + return true; } - $value = $this->value; - if ($value instanceof JsonSerializable) { - $serialized = $value->jsonSerialize(); - if ($serialized !== $value) { - return self::prepare($serialized, $this->depth, $this->wrappers, $this->plainData); - } + if ($value instanceof ArrayObject || $value instanceof ArrayIterator) { + $nativeClass = $value instanceof ArrayObject ? ArrayObject::class : ArrayIterator::class; + $flags = (new ReflectionMethod($nativeClass, 'getFlags'))->invoke($value); + return !($flags & ArrayObject::STD_PROP_LIST); } + return false; + } - // Reuse wrappers for original identities so json_encode can detect recursion before expanding it. - if (is_array($value)) { - $result = []; - foreach ($value as $key => $item) { - $reference = ReflectionReference::fromArrayElement($value, $key); - $result[$key] = $reference !== null - ? self::prepareReference( - $value, - $key, - $this->depth + 1, - $this->wrappers, - $this->plainData, - $reference - ) - : self::prepare($item, $this->depth + 1, $this->wrappers, $this->plainData); + private function restoreError(): void + { + // PHP has no json_last_error setter; replay only callback-free failures. + if ($this->error === JSON_ERROR_RECURSION) { + $value = []; + $value[] = &$value; + } elseif ($this->error === JSON_ERROR_DEPTH) { + $value = []; + for ($i = 0; $i < 513; ++$i) { + $value = [$value]; } } else { - $result = new stdClass(); - $properties = self::nativeProperties($value); - $liveProperties = self::livePropertyNames($value); - foreach ($properties as $key => $item) { - if (!str_starts_with((string) $key, "\0")) { - if (isset($liveProperties[$key])) { - $result->{(string) $key} = new self( - $value, - $this->depth + 1, - $this->wrappers, - $this->plainData, - null, - (string) $key - ); - continue; - } - $reference = ReflectionReference::fromArrayElement($properties, $key); - $result->{(string) $key} = $reference !== null - ? self::prepareReference( - $properties, - $key, - $this->depth + 1, - $this->wrappers, - $this->plainData, - $reference - ) - : self::prepare($item, $this->depth + 1, $this->wrappers, $this->plainData); - } - } + $value = $this->errorValue; } - - return $result; + json_encode($value); } } diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index 61fbeaf1..6bc98417 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -203,6 +203,63 @@ public function jsonSerialize(): mixed self::assertNull(\ReflectionReference::fromArrayElement((array) $object, 'value')); } + public static function declaredPropertyPresenceChanges(): array + { + return [ + 'remove' => [false, false], + 'remove with magic getter' => [false, true], + 'initialize' => [true, false], + 'initialize with magic getter' => [true, true], + ]; + } + + #[DataProvider('declaredPropertyPresenceChanges')] + public function testDeclaredPropertyPresenceMatchesNativeEncoding(bool $initialize, bool $magic): void + { + $make = static function () use ($initialize, $magic): object { + $getter = $magic + ? 'public function __get(string $name): mixed { throw new LogicException("magic read"); }' + : ''; + $object = eval(sprintf(<<<'PHP' + return new class { + public mixed $mutator; + public string $value; + public int $after = 7; + %s + }; + PHP, $getter)); + if (!$initialize) { + $object->value = 'before'; + } + $object->mutator = new class ($object, $initialize) implements \JsonSerializable { + public int $calls = 0; + + public function __construct(private object $parent, private bool $initialize) + { + } + + public function jsonSerialize(): mixed + { + ++$this->calls; + if ($this->initialize) { + $this->parent->value = 'ready'; + } else { + unset($this->parent->value); + } + return 'done'; + } + }; + return $object; + }; + $expected = $initialize + ? '{"mutator":"done","value":"ready","after":7}' + : '{"mutator":"done","after":7}'; + self::assertSame($expected, json_encode($make())); + $object = $make(); + self::assertSame($expected, JsonEncoder::encode($object)); + self::assertSame(1, $object->mutator->calls); + } + public static function referencedScalarChanges(): array { $changes = [ @@ -367,6 +424,30 @@ public function testPlainDataObjectsNormalizeNestedNumbersWithoutMutatingInput() self::assertSame([-INF, 0], $object->items); } + public function testInheritedNativePublicPropertiesKeepTheirWireOrder(): void + { + if (!class_exists(\PDOException::class)) { + self::markTestSkipped('PDOException requires the PDO extension.'); + } + $value = new class extends \PDOException { + public string $value = 'kept'; + }; + $value->errorInfo = ['HY000']; + $expected = '{"errorInfo":["HY000"],"value":"kept"}'; + self::assertSame($expected, json_encode($value)); + self::assertSame($expected, JsonEncoder::encode($value)); + } + + public function testNativeVirtualPropertyHandlersArePreserved(): void + { + if (!class_exists(\BcMath\Number::class)) { + self::markTestSkipped('BcMath Number requires PHP 8.4 and the bcmath extension.'); + } + $value = new \BcMath\Number('1.23'); + self::assertSame('{"value":"1.23","scale":2}', json_encode($value)); + self::assertSame(json_encode($value), JsonEncoder::encode($value)); + } + public function testClosuresKeepTheirNativeEmptyObjectRepresentation(): void { $invoked = false; @@ -470,6 +551,90 @@ public function testPropertyGetterExceptionsAreNotSwallowed(string $parentDeclar JsonEncoder::encode($object); } + public function testHookedPropertyIsReadAfterEarlierSerializer(): void + { + if (PHP_VERSION_ID < 80400) { + self::markTestSkipped('Property hooks require PHP 8.4.'); + } + $make = static function (): object { + $object = eval(<<<'PHP' + return new class { + public mixed $mutator; + public string $value = 'before' { + get => strtoupper($this->value); + } + }; + PHP); + $object->mutator = new class ($object) implements \JsonSerializable { + public function __construct(private object $parent) + { + } + + public function jsonSerialize(): mixed + { + $this->parent->value = 'after'; + return 'done'; + } + }; + return $object; + }; + $expected = '{"mutator":"done","value":"AFTER"}'; + self::assertSame($expected, json_encode($make())); + $object = $make(); + self::assertSame($expected, JsonEncoder::encode($object)); + self::assertSame('AFTER', $object->value); + } + + public function testScalarEncodingDoesNotResetErrorStateSeenByCallbacks(): void + { + $serializer = new class implements \JsonSerializable { + public function jsonSerialize(): mixed + { + return json_last_error(); + } + }; + json_decode('{'); + $expected = json_encode(['serializer' => $serializer]); + json_decode('{'); + self::assertSame($expected, JsonEncoder::encode(['serializer' => $serializer])); + self::assertSame(JSON_ERROR_NONE, json_last_error()); + } + + public function testMalformedKeysRetainErrorsAndNativeCallbackOrdering(): void + { + $serializer = new class implements \JsonSerializable { + public int $calls = 0; + + public function jsonSerialize(): mixed + { + ++$this->calls; + return 7; + } + }; + self::assertFalse(json_encode(["\xB1" => $serializer])); + self::assertSame(JSON_ERROR_UTF8, json_last_error()); + self::assertSame(1, $serializer->calls); + self::assertFalse(JsonEncoder::encode(["\xB1" => $serializer])); + self::assertSame(JSON_ERROR_UTF8, json_last_error()); + self::assertSame(2, $serializer->calls); + self::assertSame('7', JsonEncoder::encode(7)); + self::assertSame(JSON_ERROR_NONE, json_last_error()); + } + + public function testDepthBoundariesMatchNativeErrors(): void + { + foreach ([511, 512, 513] as $depth) { + $value = 0; + for ($i = 0; $i < $depth; ++$i) { + $value = [$value]; + } + $expected = json_encode($value); + $error = json_last_error(); + self::assertSame($expected, JsonEncoder::encode($value)); + self::assertSame($error, json_last_error()); + } + } + public function testUnsupportedValuesStillFailEncoding(): void { $resource = fopen('php://memory', 'r'); @@ -496,6 +661,19 @@ public function jsonSerialize(): mixed JsonEncoder::encode($value); } + public function testUserJsonExceptionsAreNotConvertedIntoEncodingFailures(): void + { + $value = new class implements \JsonSerializable { + public function jsonSerialize(): mixed + { + throw new \JsonException('user serialization exception'); + } + }; + $this->expectException(\JsonException::class); + $this->expectExceptionMessage('user serialization exception'); + JsonEncoder::encode($value); + } + public static function siblingContainers(): array { return [ @@ -559,6 +737,33 @@ private function runSiblingMemoryProbe( return ['exit' => proc_close($process), 'stdout' => $output, 'stderr' => $errors]; } + public static function largePayloadShapes(): array + { + return ['sibling allocation' => ['siblings', 10], 'reference traversal' => ['references', 5]]; + } + + #[DataProvider('largePayloadShapes')] + public function testLargePayloadEncodingMatchesNativeWithinResourceLimits(string $shape, int $seconds): void + { + $outputs = []; + foreach (['native', 'sdk'] as $mode) { + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=128M', '-d', 'max_execution_time=' . $seconds, + __DIR__ . '/fixtures/json-encoder-scale.php', __DIR__ . '/../vendor/autoload.php', $shape, $mode], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $outputs[$mode] = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $mode . ': ' . $errors); + } + self::assertSame($outputs['native'], $outputs['sdk']); + } + public function testTemporarySerializedObjectsFitWithinMemoryLimit(): void { $code = <<<'PHP' diff --git a/test/fixtures/json-encoder-scale.php b/test/fixtures/json-encoder-scale.php new file mode 100644 index 00000000..deb33a2a --- /dev/null +++ b/test/fixtures/json-encoder-scale.php @@ -0,0 +1,32 @@ + 'scale', 'properties' => ['items' => $items]]]; +if ($argv[3] === 'native') { + $encoded = json_encode(['batch' => $batch, 'api_key' => 'test-key']); +} else { + $encoded = PostHog\JsonEncoder::encode(['batch' => $batch, 'api_key' => 'test-key']); + $consumer = new PostHog\Test\QueueConsumerTestConsumer([]); + $payload = $consumer->encodedBatchPayload($batch); + $expectedPayload = $argv[2] === 'siblings' ? false : $encoded; + if ($payload !== $expectedPayload || json_last_error() !== JSON_ERROR_NONE) { + exit(3); + } +} +if ($encoded === false) { + exit(1); +} +if ($argv[2] === 'references' && ($numbers[0] !== 1 || $numbers[59999] !== 1)) { + exit(2); +} +echo json_encode(['bytes' => strlen($encoded), 'sha256' => hash('sha256', $encoded)]); From 577a8b72c0c38906d1390772c189ac614d5d0d57 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 12:04:22 +0200 Subject: [PATCH 13/16] fix: preserve native object views and replaced array references --- lib/JsonEncoder.php | 56 +++++++++++++-- test/JsonEncoderTest.php | 93 +++++++++++++++++++++++++ test/fixtures/json-encoder-siblings.php | 8 +++ 3 files changed, 150 insertions(+), 7 deletions(-) diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index bc77ba95..71139d11 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -23,6 +23,8 @@ */ final class JsonEncoder { + private const MAX_REFERENCE_SCAN_STEPS = 1000000; + private string $output = ''; private int $error = JSON_ERROR_NONE; private mixed $errorValue = null; @@ -53,8 +55,13 @@ private function writeValue(mixed $value, int $depth, ?string $referenceId = nul } if (is_array($value)) { if ($referenceId !== null && isset($this->references[$referenceId])) { - $this->error = JSON_ERROR_RECURSION; - return false; + $references = [$referenceId => true]; + $remaining = self::MAX_REFERENCE_SCAN_STEPS; + $cycle = self::hasArrayCycle($value, $references, $remaining, 0); + if ($cycle !== false) { + $this->error = $cycle === true ? JSON_ERROR_RECURSION : JSON_ERROR_DEPTH; + return false; + } } if ($depth >= 512) { $this->error = JSON_ERROR_DEPTH; @@ -65,12 +72,12 @@ private function writeValue(mixed $value, int $depth, ?string $referenceId = nul return true; } if ($referenceId !== null) { - $this->references[$referenceId] = true; + $this->references[$referenceId] = ($this->references[$referenceId] ?? 0) + 1; } try { return $this->writeArray($value, $depth); } finally { - if ($referenceId !== null) { + if ($referenceId !== null && --$this->references[$referenceId] === 0) { unset($this->references[$referenceId]); } } @@ -89,6 +96,37 @@ private function writeValue(mixed $value, int $depth, ?string $referenceId = nul return true; } + private static function hasArrayCycle(array $value, array &$references, int &$remaining, int $depth): ?bool + { + // Unlike encoding, this bounded scan invokes no callbacks, so reference cells cannot be reassigned mid-scan. + if ($depth >= 512) { + return null; + } + foreach ($value as $key => $item) { + if (--$remaining < 0) { + return null; + } + if (!is_array($item)) { + continue; + } + $id = ReflectionReference::fromArrayElement($value, $key)?->getId(); + if ($id !== null && isset($references[$id])) { + return true; + } + if ($id !== null) { + $references[$id] = true; + } + $cycle = self::hasArrayCycle($item, $references, $remaining, $depth + 1); + if ($id !== null) { + unset($references[$id]); + } + if ($cycle !== false) { + return $cycle; + } + } + return false; + } + private function writeArray(array $value, int $depth, bool $object = false): bool { $list = !$object && array_is_list($value); @@ -160,8 +198,8 @@ private function writeObject(object $value, int $depth): bool return $this->writeArray(get_object_vars($value), $depth, true); } $layout = $this->propertyLayout($value); - // Snapshot dynamic members without invoking declared getters ahead of their wire position. - $properties = get_mangled_object_vars($value); + // Casting avoids permanent property tables and does not invoke declared getters ahead of their slot. + $properties = (array) $value; $this->output .= '{'; $first = true; foreach ($layout as $name => $property) { @@ -234,7 +272,11 @@ private function propertyLayout(object $value): array private static function usesStorage(object $value): bool { - if ($value instanceof DateTimeInterface || $value instanceof DateTimeZone) { + if ( + $value instanceof DateTimeInterface + || $value instanceof DateTimeZone + || $value instanceof \SimpleXMLElement + ) { return true; } if ($value instanceof ArrayObject || $value instanceof ArrayIterator) { diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index 6bc98417..f6c8bad5 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -448,6 +448,97 @@ public function testNativeVirtualPropertyHandlersArePreserved(): void self::assertSame(json_encode($value), JsonEncoder::encode($value)); } + public function testSimpleXmlSubclassesKeepTheirNativePropertyView(): void + { + if (!class_exists(\SimpleXMLElement::class)) { + self::markTestSkipped('SimpleXML extension is required.'); + } + $object = new class ('') extends \SimpleXMLElement { + public string $label; + }; + self::assertSame('{"label":"A"}', json_encode($object)); + self::assertSame('{"label":"A"}', JsonEncoder::encode($object)); + } + + public static function replacementArrays(): array + { + return [ + 'map' => [['safe' => 3]], + 'empty list' => [[]], + 'nested list' => [[['safe' => 3]]], + ]; + } + + #[DataProvider('replacementArrays')] + public function testReplacedReferencedArrayIsNotTreatedAsTheActiveArray(array $replacement): void + { + $make = static function () use ($replacement): array { + $root = []; + $serializer = new class ($root, $replacement) implements \JsonSerializable { + private array $root; + public int $calls = 0; + + public function __construct(array &$root, private array $replacement) + { + $this->root = &$root; + } + + public function jsonSerialize(): mixed + { + ++$this->calls; + $this->root = $this->replacement; + return 'done'; + } + }; + $root = ['mutator' => $serializer, 'later' => &$root]; + return [['root' => &$root], $serializer]; + }; + [$native] = $make(); + $expected = json_encode($native); + self::assertIsString($expected); + [$input, $serializer] = $make(); + self::assertSame($expected, JsonEncoder::encode($input)); + self::assertSame(JSON_ERROR_NONE, json_last_error()); + self::assertSame(1, $serializer->calls); + self::assertSame($replacement, $input['root']); + } + + public function testOuterReferenceRemainsActiveAfterTemporaryReplacement(): void + { + $make = static function (): array { + $root = []; + $replace = new class ($root) implements \JsonSerializable { + private array $root; + public array $replacement = ['safe' => 3]; + public int $calls = 0; + + public function __construct(array &$root) + { + $this->root = &$root; + } + + public function jsonSerialize(): mixed + { + ++$this->calls; + $this->root = $this->replacement; + return 'done'; + } + }; + $restore = clone $replace; + $root = ['replace' => $replace, 'first' => &$root, 'restore' => $restore, 'last' => &$root]; + $restore->replacement = $root; + return [['root' => &$root], $replace, $restore]; + }; + [$native] = $make(); + self::assertFalse(json_encode($native)); + self::assertSame(JSON_ERROR_RECURSION, json_last_error()); + [$input, $replace, $restore] = $make(); + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame(JSON_ERROR_RECURSION, json_last_error()); + self::assertSame(1, $replace->calls); + self::assertSame(1, $restore->calls); + } + public function testClosuresKeepTheirNativeEmptyObjectRepresentation(): void { $invoked = false; @@ -683,6 +774,8 @@ public static function siblingContainers(): array 'shared empty object' => ['shared-empty-object', 300000, false], 'distinct empty objects' => ['distinct-empty-objects', 200000, false], 'distinct finite scalar objects' => ['distinct-scalar-objects', 50000, false], + 'declared-only objects' => ['declared-scalar-objects', 100000, false], + 'declared objects with non-finite sibling' => ['declared-scalar-objects', 100000, true], 'dense arrays with non-finite sibling' => ['shared-scalar-arrays', 200000, true], 'dense objects with non-finite sibling' => ['distinct-empty-objects', 200000, true], 'dense objects with DateTime' => ['distinct-empty-objects', 200000, true, 'DateTime'], diff --git a/test/fixtures/json-encoder-siblings.php b/test/fixtures/json-encoder-siblings.php index 8eb4783c..b2607537 100644 --- a/test/fixtures/json-encoder-siblings.php +++ b/test/fixtures/json-encoder-siblings.php @@ -28,6 +28,14 @@ $items[] = new stdClass(); } break; + case 'declared-scalar-objects': + $prototype = new class { + public int $n = 1; + }; + for ($i = 0; $i < $count; ++$i) { + $items[] = clone $prototype; + } + break; case 'distinct-scalar-objects': for ($i = 0; $i < $count; ++$i) { $items[] = (object) ['n' => $i % 10]; From d16a8b1af3596d4d5154bc322adaac14c4dcdb57 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 13:44:14 +0200 Subject: [PATCH 14/16] fix: restrict non-finite repair to bounded data-only payloads --- .changeset/seven-suns-strive.md | 2 +- README.md | 1 + docs/json-serialization-design-review.md | 202 +++++++++++++ docs/json-serialization.md | 33 +++ lib/JsonEncoder.php | 306 ++++---------------- test/JsonEncoderDataOnlyTest.php | 203 +++++++++++++ test/JsonEncoderReviewRegressionTest.php | 135 +++++++++ test/JsonEncoderTest.php | 141 +++++---- test/NonFinitePropertiesTest.php | 239 +++++++++------ test/fixtures/json-encoder-data-only.php | 26 ++ test/fixtures/json-encoder-large-string.php | 19 ++ test/fixtures/json-encoder-siblings.php | 15 +- 12 files changed, 923 insertions(+), 399 deletions(-) create mode 100644 docs/json-serialization-design-review.md create mode 100644 docs/json-serialization.md create mode 100644 test/JsonEncoderDataOnlyTest.php create mode 100644 test/JsonEncoderReviewRegressionTest.php create mode 100644 test/fixtures/json-encoder-data-only.php create mode 100644 test/fixtures/json-encoder-large-string.php diff --git a/.changeset/seven-suns-strive.md b/.changeset/seven-suns-strive.md index 65570351..93f3322e 100644 --- a/.changeset/seven-suns-strive.md +++ b/.changeset/seven-suns-strive.md @@ -2,4 +2,4 @@ "posthog-php": patch --- -Serialize non-finite event property numbers as null so they do not discard events or batches. +Normalize NaN and infinity to null in bounded data-only JSON payloads, while preserving native encoding for objects, references and out-of-budget payloads. diff --git a/README.md b/README.md index 44a19b6d..38955043 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ SDK usage examples and code snippets live in the official documentation so they - [PHP library docs](https://posthog.com/docs/libraries/php) - [Laravel integration docs](https://posthog.com/docs/libraries/laravel) - [Upgrade notes](UPGRADING.md), including GeoIP configuration and compatibility +- [Event JSON encoding](docs/json-serialization.md), including non-finite repair and native fallback ## Questions? diff --git a/docs/json-serialization-design-review.md b/docs/json-serialization-design-review.md new file mode 100644 index 00000000..505843a0 --- /dev/null +++ b/docs/json-serialization-design-review.md @@ -0,0 +1,202 @@ +# PHP event serialization: design checkpoint + +## Status and scope + +- PR: https://github.com/PostHog/posthog-php/pull/276 +- Evaluated revision: `577a8b72c0c38906d1390772c189ac614d5d0d57`. +- Worktree: `/Users/marandaneto/Github/worktrees/posthog-php-nonfinite-20261022-123914`. +- The user subsequently approved the conservative, data-only repair direction with “go for it.” The proposed contract below is now implemented as documented in [Event JSON encoding](json-serialization.md). +- The original checkpoint evidence and reproduction inventory below are retained as a historical baseline. The new implementation does not reconstruct Zend serialization behavior: it either repairs a bounded callback-free data tree or delegates the original whole payload to native JSON. + +## What went wrong + +The review threads are not repetitions of the same unresolved comment. New revisions have exposed different violations of the same underlying invariants: native property views and ordering, callback execution, array identity, caller ownership, and resource use. + +The original recursive-wrapper approach introduced snapshots and retained wrappers. Fixes added weak references, native-type handling, preflight scans and budgets. The sequential writer removed the wrapper graph but assumed responsibility for Zend's property traversal, recursion detection, primitive buffering and error state. That moved the failure surface rather than establishing a stable contract. + +Passing the accumulated regression suite showed that individual examples had been repaired. It did not establish that either implementation was a native-equivalent JSON encoder. In particular: + +- A test asserting recursion failure without checking callback counts missed duplicate execution and stateful acceptance of a cycle. +- Ordinary inheritance tests did not cover abstract virtual properties becoming backed slots. +- Ordinary native-object tests did not cover a property-view read changing a DateInterval object's subsequent native output. +- Sibling-container memory probes did not cover duplication of a single large encoded string. + +The first implementation commit, `bbc0f54`, already tested normalization inside stdClass, a typed readonly object and JsonSerializable output. Object normalization was therefore part of the original promise, not merely accidental scope added during review. Narrowing it now requires explicit agreement. + +## Contract assessed at the checkpoint + +### Compatibility requirements + +For inputs with no non-finite numbers, native `json_encode()` is the independent oracle. Compare fresh equivalent fixtures, including: + +1. Wire values, object/list distinctions and omission of uninitialized or removed properties. +2. Operational property order whenever getters or serializers affect later values, especially redaction. +3. Callback/getter invocation counts, ordering and propagated user exceptions. +4. Native recursion/depth rejection, JSON error state and absence of partial output. +5. Caller-visible state after encoding. User callbacks may intentionally mutate state; the SDK must not add mutations beyond native behavior. +6. Material memory/time regressions, including normal rejection by the existing batch-size guard rather than a process-fatal allocation before that guard. + +These requirements are distinct from the feature expansion: converting an otherwise unsupported non-finite number into JSON null without changing the caller's value. + +### Behavior requiring an explicit scope decision + +| Input category | Before this PR | Promise before the checkpoint | Approved conservative contract | +|---|---|---|---| +| Eligible scalar/array JSON data containing NaN or infinity | Native encoding fails | Convert numbers to null recursively | Preserve this core feature within a defined safe domain | +| Finite custom/native objects, hooks and reference graphs | Native traversal and handlers | Native-equivalent behavior | Delegate the original payload to native encoding unchanged | +| Non-finite values inside objects, getters, serializer results or reference cells | Native failure or user exception | Also normalize these values | Keep native behavior outside the safe domain; this narrows the original promise | +| A batch mixing opaque values and otherwise repairable non-finite data | Native batch failure | Preserve healthy neighboring events | Whole-payload native fallback may still fail the batch; this limitation needs agreement | +| Malformed UTF-8, resources and actual cycles | Native errors | No partial JSON | Preserve native error handling rather than introduce synthetic successful output | + +The [capture spec](https://github.com/PostHog/sdk-specs/blob/main/openspec/specs/capture/spec.md) covers capture, serialization safety and a separate null/undefined-property cleanup contract. It does not define PHP object traversal or this NaN/infinity policy. Do not infer a new non-finite policy from the null-member rule or mix another cleanup into this reassessment. + +## Design alternatives + +### 1. Continue the general-purpose writer + +This preserves the broad promise only if we can demonstrate native-equivalent behavior across the supported PHP versions. The current implementation has not demonstrated that. + +Its risky responsibilities include: + +- Reconstructing stored-slot order from reflection metadata, including virtual/abstract implementations. +- Selecting native extension property views without materializing extra caller state. +- Treating reference-cell IDs as a substitute for native array identity across callbacks. [ReflectionReference::getId](https://www.php.net/manual/en/reflectionreference.getid.php) identifies a reference, not a copy-on-write array value. +- Preserving native callback execution while detecting cycles before re-entry. +- Avoiding a second full string allocation while assembling the output. + +**Not recommended as another incremental patch round.** Four more example-specific fixes would not prove these invariants. + +### 2. Try native JSON first, then retry after normalization + +Rejected for stateful values. A local probe with a JsonSerializable returning NaN recorded two callback invocations for two native attempts. Retrying after a failed first pass can observe changed data, repeat side effects or alter redaction behavior. + +This is not a safe fallback for arbitrary objects or reference graphs. + +### 3. Use partial output or replace tokens in encoded JSON + +Rejected. The executed probe `json_encode([0, NAN, INF, -INF], JSON_PARTIAL_OUTPUT_ON_ERROR)` returned `[0,0,0,0]` with `JSON_ERROR_INF_OR_NAN`. The output no longer identifies which zeros were non-finite. Other encoding failures must not become partial successful payloads. Literal strings such as `"NaN"` must remain unchanged. + +### 4. Narrow the repair domain and preserve native delegation + +**Approved direction.** The current bounds and fallback policy are documented in [Event JSON encoding](json-serialization.md). + +- Define an eligible, callback-free data domain rather than emulate arbitrary PHP object serialization. +- Detect eligibility without reading object properties, invoking callbacks or binding caller reference cells. +- Delegate an ineligible **whole payload** unchanged to one native encoding operation. Merely passing objects through while copying surrounding arrays is not enough: callbacks can retain or return ancestor arrays, so copying can change native identity/recursion behavior. +- Repair only eligible values, preserve object/list distinctions and numeric null positions, and leave caller values unchanged. +- If eligibility cannot be established within its limits, use unchanged native delegation rather than reject an otherwise healthy finite payload or allocate a fallback wrapper graph. +- Prove memory behavior for dense non-finite data as well as unchanged data. Native delegation alone does not prove that a normalization copy is cheap. +- Do not add a public option or another user hook. `Client::applyBeforeSend()` already supplies `before_send`; callers can explicitly provide JSON-safe data there. It is an escape hatch, not permission for the SDK to change native behavior silently. + +This candidate does **not** automatically satisfy the existing object/non-finite feature tests. In particular, normalization of typed-object fields, stdClass fields, JsonSerializable enum/results, non-finite reference-cell updates, and mixed opaque/non-finite payloads must be explicitly retained or taken out of scope. Their examples must not be deleted or silently skipped to make a prototype green. + +### Decision gate + +The checkpoint required this decision before production implementation. The user subsequently chose the first option: + +- Approve a precisely bounded data-only repair contract, including opaque/mixed-batch fallback and unsupported non-finite behavior; then prototype and test it. +- Require the full original object/reference normalization contract; then first establish a feasible native-compatible mechanism. If we cannot establish one, defer the feature rather than claim compatibility from another patch series. + +No production change, commit, push or review-thread resolution was performed during the checkpoint itself. The later implementation follows the user's explicit approval rather than treating the narrowed feature expectations as incidental test fixes. + +## Reported reproduction inventory + +There are 18 top-level review comments representing 17 distinct concerns; the second Exception-hook comment duplicates the first. The mapping below concerns concrete reproductions, not proof that the whole defect family is solved. + +`J` means `test/JsonEncoderTest.php`; `N` means `test/NonFinitePropertiesTest.php`; `R` means the new `test/JsonEncoderReviewRegressionTest.php`. + +| Review comment ID | Reproduction and retained regression | Result on `577a8b7` at checkpoint | +|---|---|---| +| `4350801113`, `4350929352` | Internal-parent getter redaction and virtual properties: J `testPropertyHooksControlTheSerializedPublicView` / `testPropertyGetterExceptionsAreNotSwallowed`; N `testCapturePreservesSubclassPropertyHooks` | Existing cases pass | +| `4350801125` | Temporary serializer output retained across 100 x 1,000 objects: J `testTemporarySerializedObjectsFitWithinMemoryLimit` (64 MB) | Pass | +| `4350952993` | ArrayObject/ArrayIterator override dispatch: J `testNativeArrayFlagOverridesAreNotInvoked` and `testNativeStandardPropertyFlagsPreserveHooksDespiteOverride` | Pass | +| `4350953015` | Eager sibling wrappers: J `testSiblingContainersFitWithinNativeMemoryLimit`, including `empty arrays` (300,000; 32 MB) | Pass | +| `4356816316` | Earlier serializer redacts a later declared scalar: J `testDeclaredPropertyIsReadAfterEarlierSerializer`, `testHookedPropertyIsReadAfterEarlierSerializer` | Pass | +| `4356816321` | Shared 24-level invalid-UTF8 graph: J `testSharedInvalidArrayGraphIsRejectedWithoutEagerExpansion` (32 MB; two seconds) | Pass | +| `4357239411` | Earlier serializer unsets a later field, with/without magic getter: J `testDeclaredPropertyPresenceMatchesNativeEncoding`, `remove` cases | Pass | +| `4357265958` | Budget fallback creates a million wrappers: J `testLargePayloadEncodingMatchesNativeWithinResourceLimits`, `sibling allocation`; `json-encoder-scale.php` (2,000,000 arrays; 128 MB) | Pass | +| `4357701895` | Earlier serializer initializes a previously uninitialized field: J `testDeclaredPropertyPresenceMatchesNativeEncoding`, `initialize` cases | Pass | +| `4357701903` | Reference preparation copies the parent per element: J `testLargePayloadEncodingMatchesNativeWithinResourceLimits`, `reference traversal` (60,000 references; five seconds) | Pass | +| `4358008186` | Permanent tables on declared-only objects: J `testSiblingContainersFitWithinNativeMemoryLimit`, `declared-only objects` and neighboring non-finite case (100,000 objects; 32 MB) | Pass | +| `4358008192` | SimpleXML subclass native view: J `testSimpleXmlSubclassesKeepTheirNativePropertyView` | Pass | +| `4358008196` | A referenced slot points to a replacement array: J `testReplacedReferencedArrayIsNotTreatedAsTheActiveArray` (map/empty/nested); `testOuterReferenceRemainsActiveAfterTemporaryReplacement` | Pass | +| `4358414871` | Abstract virtual property becomes a backed slot before inherited redaction: R `testBackedImplementationOfAbstractPropertyPreservesNativeRedactionOrder` | **Fails:** SDK reads the unredacted value first | +| `4358414885` | Initial active array has no reference ID: R `testActiveArrayDoesNotReenterItsSerializer` (stateless/stateful) | **Both fail:** callback count is two; stateful case is accepted instead of rejected | +| `4358414893` | Hooked DateInterval gains extra fields and changes subsequent native output: R `testHookedDateIntervalPreservesNativeViewAndCallerState` | **Fails:** interval fields are added; the native before/after probe also shows caller mutation | +| `4358414903` | Full encoded string duplicated in the buffer: R `testLargeStringMatchesNativeWithin32MbBeforeBatchSizeRejection`; `json-encoder-large-string.php` (12 MiB ASCII; 32 MB) | **Fails:** native succeeds; SDK exits with memory exhaustion | + +Review links use `https://github.com/PostHog/posthog-php/pull/276#discussion_r`. + +Additional pre-review reproductions are also retained: the paired recursive serializer chain added in `04d9f63`, native object formats, embedded-NUL property names and serializable enums (`25ad084`), stdClass-subclass hooks (`4059fad`), and Closure representation (`a3fd115`). Their cases remain in J/N and `test/NonFiniteJsonSerializableEnum.php`. + +No existing tests or assertions were removed, weakened or skipped by this checkpoint. Historical RED evidence for earlier findings is recorded in the PR replies/history; this audit reran the current retained cases, not every historical pre-fix revision. + +## Historical checkpoint validation + +Environment: PHP 8.5.10, PHPUnit 11.5.55, local offline/loopback fixtures. No production code was changed. The new hook/abstract-property tests are gated below PHP 8.4. These new tests were not run locally on PHP 8.2/8.3/8.4 or with Xdebug; published-head CI results do not include them. + +```sh +./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage \ + test/JsonEncoderTest.php test/NonFinitePropertiesTest.php +# PASS: 113 tests, 862 assertions. + +./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage \ + test/JsonEncoderReviewRegressionTest.php +# EXPECTED RED: 5 tests, 20 assertions, 5 behavior/resource failures. +# All native-oracle/setup checks pass before the SDK failures. + +./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage +# RED: 1,548 tests, 8,011 assertions, exactly the five new failures. +# Existing notices: 27 deprecations and 42 PHPUnit deprecations. + +./vendor/bin/phpcs --standard=phpcs.xml \ + test/JsonEncoderReviewRegressionTest.php test/fixtures/json-encoder-large-string.php +# PASS. + +git diff --exit-code -- lib api composer.json phpunit.xml +# PASS: production, API snapshot, dependencies and runner configuration unchanged. +``` + +The large-string fixture compares independent native/SDK processes, frees the first encoded string before checking the batch guard, and keeps the existing 32 MB limit. Its failure is not caused by retaining two completed encodings in the test harness. + +## Approved implementation and coverage changes + +The custom writer, native-handler cases, property-layout reconstruction, active-reference maps and synthetic error restoration were removed. The encoder now has a read-only eligibility pass (10,000 values, 64 array levels, 1 MiB raw string/key bytes), a copy-on-change data normalizer, and one final native payload encoding. Opaque values, references, malformed strings and exhausted budgets cause unchanged native delegation, not a new encoding rejection. Primitive string validation uses `JSON_THROW_ON_ERROR` so it does not reset JSON error state seen by user callbacks. + +No concrete reported reproduction was deleted or skipped. The five new finite/native-parity tests in `JsonEncoderReviewRegressionTest.php` pass without changing their expectations. The following feature expectations changed only because the user approved narrowing the normalization domain: + +- The original mixed native-object/reference fixture in `testSupportedObjectRepresentationsAndReferencesArePreserved` remains and now asserts native failure for its non-finite mode; a finite counterpart preserves native format coverage. +- Non-finite reference slots, object fields and serializer output now assert native failure/error parity and caller identity/value preservation. The original stdClass/typed-object/serializer fixtures remain present. +- All 28 reference-mutation cases remain. Transitions to non-finite values assert native failure; transitions to finite values retain native wire output and callback counts. +- Hook privacy tests retain the original opaque/non-finite combinations as native-failure cases and add finite companions that still assert filtered wire values and getter counts. +- The large sibling/native-handler memory shapes remain under their original limits. Opaque or over-budget non-finite combinations compare native failure instead of pre-sanitized null data. Finite native-handler companions preserve successful-output coverage. +- The original six batch/File object-normalization integration fixtures remain in `testOpaqueNonFinitePropertiesKeepNativeFailureAndCallerState`. Six new data-only integration cases prove NaN/infinity repair, healthy-neighbor delivery and caller immutability in the supported domain. +- Serializable-enum and Closure examples remain. Opaque non-finite payloads assert native failure; finite Closure wire-format and non-invocation coverage is retained. + +`JsonEncoderDataOnlyTest.php` defines the new policy independently: eligible numeric repair, whole-payload opaque/reference fallback, no serializer retry, exact value/byte/depth boundaries, successful finite fallback outside those bounds, and 32 MiB repair probes for dense arrays and escaped strings. The new policy was run against the previous writer before replacement and produced six expected failures; all five latest review reproductions were also red at the checkpoint. + +The approved contract deliberately does not isolate mixed batches event by event, normalize values produced by arbitrary objects, or repair payloads beyond the eligibility bounds. These limitations are customer-visible in the encoding contract rather than hidden in test changes. + +## Exit criteria for the replacement design + +1. Approve the normalization domain and fallback policy in the PR before modifying production code or changing feature-test expectations. +2. Preserve every concrete reproduction above. Native-compatibility/privacy tests are not negotiable merely because a feature is narrowed. +3. Define the oracle for each category: native result/error/callback/state parity versus explicitly agreed non-finite-to-null transformation. +4. Validate the complete matrix on PHP 8.2-8.5, with applicable hook skips, Xdebug coverage and independent native resource baselines. +5. Retain batch/File integration coverage and demonstrate healthy-neighbor behavior for the explicitly supported domain. Do not assume isolated event encoding preserves all whole-batch callback semantics. +6. Review the design's state/ownership/resource invariants before requesting another automatic review. Another passing test count alone is not the exit criterion. + +## Approved revision validation + +Local validation of the approved data-only implementation on PHP 8.5.10: + +- 159 focused tests / 1,183 assertions pass across `JsonEncoderDataOnlyTest`, `JsonEncoderReviewRegressionTest`, `JsonEncoderTest` and `NonFinitePropertiesTest`. +- The complete suite passes with Xdebug 3.5.3 coverage: 1,589 tests / 8,312 assertions. Existing notices remain 27 deprecations and 42 PHPUnit deprecations. +- All 16 adapter tests pass. +- Changed PHP files pass PHP_CodeSniffer; the public API snapshot is unchanged/up to date; strict Composer validation passes. +- The five newest native-parity regressions pass with their original expectations. No reported reproduction was deleted or skipped to obtain this result. +- New 32 MiB probes compare eligible dense non-finite arrays and escaped strings with independently constructed native-null payloads. Oversized finite strings and finite payloads outside each repair budget retain native success. + +The design invariant review checks that copying happens only after full eligibility, that no eligible graph can run a user serializer/getter or contain reference cells, that unchanged and ineligible payloads use their original native graph, and that string validation cannot overwrite the JSON error state seen by native callbacks. Resource bounds limit both inspection work and potential normalization allocations; reaching a bound falls back rather than building a wrapper graph or inventing an encoding error. + +PHP 8.2-8.5 CI is required as the cross-version acceptance gate; local results alone are not a matrix-wide validation. diff --git a/docs/json-serialization.md b/docs/json-serialization.md new file mode 100644 index 00000000..5bc4f23c --- /dev/null +++ b/docs/json-serialization.md @@ -0,0 +1,33 @@ +# Event JSON encoding + +NaN and positive/negative infinity are encoded as JSON `null` when the **entire payload** is an eligible data-only tree. Array positions and keys, finite values, literal strings such as `"NaN"`, and caller-owned values are preserved. + +An eligible payload contains only PHP arrays, strings, numbers, booleans and null, with no objects, resources or retained array-element references. Repair is bounded to: + +| Limit | Scope | +|---|---| +| 10,000 values | Includes the root, each container and each array value | +| 64 array levels | The complete payload, not just custom properties | +| 1 MiB of string bytes | Combined raw bytes of string values and string keys, before JSON escaping | + +The inspection is read-only and invokes no getters or serializers. Repair starts only after the entire payload qualifies, and copies only containers containing changed values. Individual strings are validated without resetting the JSON error state seen by callbacks. + +## Native fallback + +If any value is opaque or unsupported, a reference is found, a string is malformed, or a bound is exceeded, the **original whole payload** is passed unchanged to native `json_encode()`. It is not partially repaired and is not retried. Finite payloads outside the repair bounds are still encoded normally. + +Objects include stdClass, enums, Closure, JsonSerializable implementations and native extension objects. The SDK does not inspect their properties or reproduce their serialization handlers. Native property ordering, callback counts, redaction, recursion detection, user exceptions and JSON errors remain authoritative. + +Consequently, non-finite numbers in an ineligible payload still cause the native encoding failure. This includes numbers inside objects or references, values produced by a serializer, and plain non-finite fields next to an opaque value. Capture callers needing a different policy can supply JSON-safe data through the existing `before_send` hook; no new SDK option is introduced. + +## Batch and File behavior + +For queued consumers, the payload includes the entire batch, API key and SDK metadata. One opaque value or reference in any event disables repair for the whole batch. A mixed batch containing an object and a non-finite number can therefore still fail native encoding, including its healthy neighboring events. This is not per-event error isolation. + +For the File consumer, eligibility applies to each complete event. Ineligible events retain the File consumer's existing native-failure handling, including its empty-line output when encoding fails; this change does not replace that handling. + +The existing 1 MiB batch **encoded body** limit is separate from the raw-string repair budget. Escaping can enlarge JSON. Oversized valid batches are still rejected by the existing size guard after encoding, and large finite strings use native encoding without a second SDK output buffer. + +No partial-output flags, string-token replacement, object casting, reflection-based property traversal, reference binding or serializer retries are used. + +The design history and complete review-reproduction inventory are recorded in [the design checkpoint](json-serialization-design-review.md). diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 71139d11..3020c055 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -2,305 +2,99 @@ namespace PostHog; -use ArrayIterator; -use ArrayObject; -use Closure; -use DateTimeInterface; -use DateTimeZone; use JsonException; -use JsonSerializable; -use ReflectionClass; -use ReflectionMethod; use ReflectionReference; -use SplObjectStorage; -use stdClass; -use UnitEnum; /** - * Encodes event payloads with non-finite numbers represented as null. + * Repairs non-finite numbers in bounded, object/reference-free data payloads. * * @internal */ final class JsonEncoder { - private const MAX_REFERENCE_SCAN_STEPS = 1000000; - - private string $output = ''; - private int $error = JSON_ERROR_NONE; - private mixed $errorValue = null; - private array $references = []; - private array $propertyLayouts = []; - private SplObjectStorage $objects; + private const MAX_DEPTH = 64; + private int $remainingValues = 10000; + private int $remainingBytes = 1048576; + private bool $hasNonFinite = false; private function __construct() { - $this->objects = new SplObjectStorage(); } public static function encode(mixed $value): string|false { - $encoder = new self(); - if ($encoder->writeValue($value, 0) && $encoder->error === JSON_ERROR_NONE) { - json_encode(null); - return $encoder->output; + // Opaque callbacks may retain ancestor arrays, so eligibility must precede every copy. + $inspection = new self(); + if ($inspection->eligible($value, 0) && $inspection->hasNonFinite) { + $changed = false; + $value = self::normalize($value, $changed); } - $encoder->restoreError(); - return false; + return json_encode($value); } - private function writeValue(mixed $value, int $depth, ?string $referenceId = null): bool + private function eligible(mixed $value, int $depth): bool { - if (is_float($value) && !is_finite($value)) { - $value = null; + if (--$this->remainingValues < 0) { + return false; } if (is_array($value)) { - if ($referenceId !== null && isset($this->references[$referenceId])) { - $references = [$referenceId => true]; - $remaining = self::MAX_REFERENCE_SCAN_STEPS; - $cycle = self::hasArrayCycle($value, $references, $remaining, 0); - if ($cycle !== false) { - $this->error = $cycle === true ? JSON_ERROR_RECURSION : JSON_ERROR_DEPTH; - return false; - } - } - if ($depth >= 512) { - $this->error = JSON_ERROR_DEPTH; + if ($depth >= self::MAX_DEPTH) { return false; } - if ($value === []) { - $this->output .= '[]'; - return true; - } - if ($referenceId !== null) { - $this->references[$referenceId] = ($this->references[$referenceId] ?? 0) + 1; - } - try { - return $this->writeArray($value, $depth); - } finally { - if ($referenceId !== null && --$this->references[$referenceId] === 0) { - unset($this->references[$referenceId]); + foreach ($value as $key => $item) { + if ( + ReflectionReference::fromArrayElement($value, $key) !== null + || (is_string($key) && !$this->eligibleString($key)) + || !$this->eligible($item, $depth + 1) + ) { + return false; } } + return true; } - if ( - is_object($value) - && ($value instanceof JsonSerializable || (!$value instanceof UnitEnum && !$value instanceof Closure)) - ) { - return $this->writeObject($value, $depth); - } - $encoded = $this->encodeScalar($value); - if ($encoded === false) { - return false; - } - $this->output .= $encoded; - return true; - } - - private static function hasArrayCycle(array $value, array &$references, int &$remaining, int $depth): ?bool - { - // Unlike encoding, this bounded scan invokes no callbacks, so reference cells cannot be reassigned mid-scan. - if ($depth >= 512) { - return null; - } - foreach ($value as $key => $item) { - if (--$remaining < 0) { - return null; - } - if (!is_array($item)) { - continue; - } - $id = ReflectionReference::fromArrayElement($value, $key)?->getId(); - if ($id !== null && isset($references[$id])) { - return true; - } - if ($id !== null) { - $references[$id] = true; - } - $cycle = self::hasArrayCycle($item, $references, $remaining, $depth + 1); - if ($id !== null) { - unset($references[$id]); - } - if ($cycle !== false) { - return $cycle; - } - } - return false; - } - - private function writeArray(array $value, int $depth, bool $object = false): bool - { - $list = !$object && array_is_list($value); - $this->output .= $list ? '[' : '{'; - $first = true; - foreach ($value as $key => $item) { - if ($object && str_starts_with((string) $key, "\0")) { - continue; - } - if (!$first) { - $this->output .= ','; - } - $first = false; - if (!$list) { - $this->writeKey((string) $key); - } - $reference = ReflectionReference::fromArrayElement($value, $key); - if (!$this->writeValue($item, $depth + 1, $reference?->getId())) { - return false; - } + if (is_string($value)) { + return $this->eligibleString($value); } - $this->output .= $list ? ']' : '}'; - return true; - } - - private function encodeScalar(mixed $value): string|false - { - try { - return json_encode($value, JSON_THROW_ON_ERROR); - } catch (JsonException $error) { - $this->error = $error->getCode(); - $this->errorValue = $value; - return false; - } - } - - private function writeKey(string $key): void - { - $encoded = $this->encodeScalar($key); - if ($encoded === false) { - // Native JSON continues visiting members after a malformed key, but never returns partial output. - $encoded = '""'; + if (is_float($value) && !is_finite($value)) { + $this->hasNonFinite = true; } - $this->output .= $encoded . ':'; + return $value === null || is_scalar($value); } - private function writeObject(object $value, int $depth): bool + private function eligibleString(string $value): bool { - if (isset($this->objects[$value])) { - $this->error = JSON_ERROR_RECURSION; + $this->remainingBytes -= strlen($value); + if ($this->remainingBytes < 0) { return false; } - $this->objects[$value] = true; + // Validation must not reset the JSON error state visible to subsequent native callbacks. try { - if ($value instanceof JsonSerializable) { - $serialized = $value->jsonSerialize(); - if ($serialized !== $value) { - return $this->writeValue($serialized, $depth); - } - } - if ($depth >= 512) { - $this->error = JSON_ERROR_DEPTH; - return false; - } - if (self::usesStorage($value)) { - return $this->writeArray((array) $value, $depth, true); - } - if (get_class($value) === stdClass::class || (new ReflectionClass($value))->isInternal()) { - return $this->writeArray(get_object_vars($value), $depth, true); - } - $layout = $this->propertyLayout($value); - // Casting avoids permanent property tables and does not invoke declared getters ahead of their slot. - $properties = (array) $value; - $this->output .= '{'; - $first = true; - foreach ($layout as $name => $property) { - unset($properties[$name]); - if (PHP_VERSION_ID >= 80400 && $property->hasHooks()) { - if ($property->isVirtual() && !$property->hasHook(\PropertyHookType::Get)) { - continue; - } - } elseif (!$property->isInitialized($value)) { - continue; - } - if (!$first) { - $this->output .= ','; - } - $first = false; - $this->writeKey($name); - if (!$this->writeValue($value->{$name}, $depth + 1)) { - return false; - } - } - foreach ($properties as $name => $item) { - if (str_starts_with((string) $name, "\0")) { - continue; - } - if (!$first) { - $this->output .= ','; - } - $first = false; - $this->writeKey((string) $name); - $reference = ReflectionReference::fromArrayElement($properties, $name); - if (!$this->writeValue($item, $depth + 1, $reference?->getId())) { - return false; - } - } - $this->output .= '}'; + json_encode($value, JSON_THROW_ON_ERROR); return true; - } finally { - unset($this->objects[$value]); - } - } - - private function propertyLayout(object $value): array - { - $name = get_class($value); - if (array_key_exists($name, $this->propertyLayouts)) { - return $this->propertyLayouts[$name]; - } - $classes = []; - $class = new ReflectionClass($value); - do { - $classes[] = $class; - } while (($class = $class->getParentClass()) !== false); - $slots = []; - foreach (array_reverse($classes) as $class) { - foreach ($class->getProperties() as $property) { - if ($property->isStatic() || $property->getDeclaringClass()->getName() !== $class->getName()) { - continue; - } - $key = $property->isPrivate() - ? "\0" . $class->getName() . "\0" . $property->getName() - : $property->getName(); - $slots[$key] = $property; - } + } catch (JsonException) { + return false; } - return $this->propertyLayouts[$name] = array_filter( - $slots, - static fn ($property) => $property->isPublic() - ); } - private static function usesStorage(object $value): bool + private static function normalize(mixed $value, bool &$changed): mixed { - if ( - $value instanceof DateTimeInterface - || $value instanceof DateTimeZone - || $value instanceof \SimpleXMLElement - ) { - return true; + $changed = is_float($value) && !is_finite($value); + if ($changed) { + return null; } - if ($value instanceof ArrayObject || $value instanceof ArrayIterator) { - $nativeClass = $value instanceof ArrayObject ? ArrayObject::class : ArrayIterator::class; - $flags = (new ReflectionMethod($nativeClass, 'getFlags'))->invoke($value); - return !($flags & ArrayObject::STD_PROP_LIST); + if (!is_array($value)) { + return $value; } - return false; - } - - private function restoreError(): void - { - // PHP has no json_last_error setter; replay only callback-free failures. - if ($this->error === JSON_ERROR_RECURSION) { - $value = []; - $value[] = &$value; - } elseif ($this->error === JSON_ERROR_DEPTH) { - $value = []; - for ($i = 0; $i < 513; ++$i) { - $value = [$value]; + $result = $value; + foreach ($value as $key => $item) { + $childChanged = false; + $normalized = self::normalize($item, $childChanged); + if ($childChanged) { + // Eligibility excludes reference cells, so copy-on-write cannot alter caller values. + $result[$key] = $normalized; + $changed = true; } - } else { - $value = $this->errorValue; } - json_encode($value); + return $result; } } diff --git a/test/JsonEncoderDataOnlyTest.php b/test/JsonEncoderDataOnlyTest.php new file mode 100644 index 00000000..c651a2be --- /dev/null +++ b/test/JsonEncoderDataOnlyTest.php @@ -0,0 +1,203 @@ + [NAN], 'positive infinity' => [INF], 'negative infinity' => [-INF]]; + } + + #[DataProvider('nonFiniteNumbers')] + public function testDataOnlyNumbersBecomeNullWithoutChangingCallerValues(float $number): void + { + self::assertSame('null', JsonEncoder::encode($number)); + $input = ['number' => $number, 'nested' => ['items' => [false, $number, 0]], 'literal' => 'NaN']; + self::assertSame( + '{"number":null,"nested":{"items":[false,null,0]},"literal":"NaN"}', + JsonEncoder::encode($input) + ); + if (is_nan($number)) { + self::assertNan($input['number']); + self::assertNan($input['nested']['items'][1]); + } else { + self::assertSame($number, $input['number']); + self::assertSame($number, $input['nested']['items'][1]); + } + } + + public function testOpaqueObjectDisablesRepairForTheWholePayload(): void + { + $serializer = new class implements \JsonSerializable { + public int $calls = 0; + + public function jsonSerialize(): mixed + { + ++$this->calls; + return 'kept'; + } + }; + $input = ['number' => NAN, 'opaque' => $serializer]; + self::assertFalse(json_encode($input)); + $error = json_last_error(); + $nativeCalls = $serializer->calls; + $serializer->calls = 0; + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame($error, json_last_error()); + self::assertSame($nativeCalls, $serializer->calls); + self::assertNan($input['number']); + } + + public function testReferenceSlotDisablesRepairForTheWholePayload(): void + { + $finite = 7; + $input = ['number' => NAN, 'reference' => &$finite]; + $identity = \ReflectionReference::fromArrayElement($input, 'reference')->getId(); + self::assertFalse(json_encode($input)); + $error = json_last_error(); + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame($error, json_last_error()); + self::assertSame($identity, \ReflectionReference::fromArrayElement($input, 'reference')->getId()); + self::assertSame(7, $finite); + self::assertNan($input['number']); + } + + public function testNativeSerializerFailureIsNotRetriedOrNormalized(): void + { + $serializer = new class implements \JsonSerializable { + public int $calls = 0; + + public function jsonSerialize(): mixed + { + ++$this->calls; + return ['number' => NAN]; + } + }; + self::assertFalse(JsonEncoder::encode($serializer)); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + self::assertSame(1, $serializer->calls); + } + + public static function valueBudgets(): array + { + return ['within budget' => [9999, true], 'over budget' => [10000, false]]; + } + + #[DataProvider('valueBudgets')] + public function testValueBudgetBoundary(int $count, bool $eligible): void + { + $input = array_fill(0, $count, 0); + $input[0] = NAN; + if ($eligible) { + $expected = $input; + $expected[0] = null; + self::assertSame(json_encode($expected), JsonEncoder::encode($input)); + } else { + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + } + self::assertNan($input[0]); + } + + public static function byteBudgets(): array + { + return ['within budget' => [1048574, true], 'over budget' => [1048575, false]]; + } + + #[DataProvider('byteBudgets')] + public function testStringAndKeyByteBudgetBoundary(int $bytes, bool $eligible): void + { + $input = ['n' => NAN, 's' => str_repeat('A', $bytes)]; + if ($eligible) { + $expected = ['n' => null, 's' => $input['s']]; + self::assertTrue( + JsonEncoder::encode($input) === json_encode($expected), + 'A payload at the byte boundary must be repaired exactly.' + ); + } else { + self::assertTrue( + JsonEncoder::encode($input) === false, + 'An over-budget payload must retain native failure.' + ); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + } + self::assertNan($input['n']); + } + + public static function outsideBudgetShapes(): array + { + return ['values' => ['values'], 'bytes' => ['bytes'], 'depth' => ['depth']]; + } + + #[DataProvider('outsideBudgetShapes')] + public function testFinitePayloadsOutsideRepairBudgetsStillUseNativeEncoding(string $shape): void + { + $input = match ($shape) { + 'values' => array_fill(0, 10000, 0), + 'bytes' => str_repeat('A', 1048577), + 'depth' => 0, + }; + if ($shape === 'depth') { + for ($i = 0; $i < 65; ++$i) { + $input = [$input]; + } + } + $expected = json_encode($input); + self::assertIsString($expected); + self::assertTrue(JsonEncoder::encode($input) === $expected, 'Finite fallback must preserve native output.'); + self::assertSame(JSON_ERROR_NONE, json_last_error()); + } + + public static function eligibleMemoryShapes(): array + { + return ['dense non-finite arrays' => ['dense'], 'escaped string' => ['escaped']]; + } + + #[DataProvider('eligibleMemoryShapes')] + public function testEligibleRepairMatchesNativeNullDataWithin32Mb(string $shape): void + { + $outputs = []; + foreach (['native', 'sdk'] as $mode) { + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=10', + __DIR__ . '/fixtures/json-encoder-data-only.php', + __DIR__ . '/../vendor/autoload.php', $shape, $mode], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $outputs[$mode] = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $mode . ': ' . $errors); + } + self::assertSame($outputs['native'], $outputs['sdk']); + } + + public static function depthBudgets(): array + { + return ['within budget' => [64, true], 'over budget' => [65, false]]; + } + + #[DataProvider('depthBudgets')] + public function testArrayDepthBudgetBoundary(int $depth, bool $eligible): void + { + $input = NAN; + for ($i = 0; $i < $depth; ++$i) { + $input = [$input]; + } + if ($eligible) { + self::assertSame(str_repeat('[', $depth) . 'null' . str_repeat(']', $depth), JsonEncoder::encode($input)); + } else { + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + } + } +} diff --git a/test/JsonEncoderReviewRegressionTest.php b/test/JsonEncoderReviewRegressionTest.php new file mode 100644 index 00000000..44865e24 --- /dev/null +++ b/test/JsonEncoderReviewRegressionTest.php @@ -0,0 +1,135 @@ +mutator = new class ($object) implements \JsonSerializable { + public function __construct(private object $parent) + { + } + + public function jsonSerialize(): mixed + { + $this->parent->value = 'redacted'; + return 'done'; + } + }; + return $object; + }; + $expected = '{"mutator":"done","value":"redacted"}'; + self::assertSame($expected, json_encode($make())); + self::assertTrue( + JsonEncoder::encode($make()) === $expected, + 'A backed implementation must not be read before an inherited redacting serializer.' + ); + } + + public static function cyclicSerializerModes(): array + { + return ['stateless' => [false], 'break on second invocation' => [true]]; + } + + #[DataProvider('cyclicSerializerModes')] + public function testActiveArrayDoesNotReenterItsSerializer(bool $breakOnSecondInvocation): void + { + $make = static function () use ($breakOnSecondInvocation): array { + $properties = []; + $calls = (object) ['count' => 0]; + $serializer = new class ($properties, $calls, $breakOnSecondInvocation) implements \JsonSerializable { + private array $properties; + + public function __construct( + array &$properties, + private object $calls, + private bool $breakOnSecondInvocation + ) { + $this->properties = &$properties; + } + + public function jsonSerialize(): mixed + { + ++$this->calls->count; + if ($this->breakOnSecondInvocation && $this->calls->count === 2) { + $this->properties = ['safe' => 3]; + } + return 'done'; + } + }; + $properties = ['mutator' => $serializer, 'self' => &$properties]; + return [['event' => 'cycle', 'properties' => $properties], $calls]; + }; + [$native, $nativeCalls] = $make(); + self::assertNull(\ReflectionReference::fromArrayElement($native, 'properties')); + self::assertFalse(json_encode($native)); + self::assertSame(JSON_ERROR_RECURSION, json_last_error()); + self::assertSame(1, $nativeCalls->count); + [$input, $calls] = $make(); + $actual = JsonEncoder::encode($input); + $error = json_last_error(); + self::assertFalse($actual); + self::assertSame(JSON_ERROR_RECURSION, $error); + self::assertSame(1, $calls->count); + } + + public function testHookedDateIntervalPreservesNativeViewAndCallerState(): void + { + if (PHP_VERSION_ID < 80400) { + self::markTestSkipped('Property hooks require PHP 8.4.'); + } + $object = eval(<<<'PHP' + return new class ('P1D') extends DateInterval { + public string $view { + get => 'visible'; + } + }; + PHP); + $before = json_encode($object); + self::assertSame('{"view":"visible"}', $before); + $actual = JsonEncoder::encode($object); + $after = json_encode($object); + self::assertSame($before, $actual); + self::assertSame($before, $after, 'Encoding must not materialize interval fields on the caller object.'); + } + + public function testLargeStringMatchesNativeWithin32MbBeforeBatchSizeRejection(): void + { + $outputs = []; + foreach (['native', 'sdk'] as $mode) { + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=10', + __DIR__ . '/fixtures/json-encoder-large-string.php', __DIR__ . '/../vendor/autoload.php', $mode], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $outputs[$mode] = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $mode . ': ' . $errors); + } + self::assertSame($outputs['native'], $outputs['sdk']); + } +} diff --git a/test/JsonEncoderTest.php b/test/JsonEncoderTest.php index f6c8bad5..4a0dddfb 100644 --- a/test/JsonEncoderTest.php +++ b/test/JsonEncoderTest.php @@ -8,36 +8,42 @@ class JsonEncoderTest extends TestCase { - public function testSupportedObjectRepresentationsAndReferencesArePreserved(): void + public static function supportedObjectModes(): array { - $number = INF; + return ['finite values' => [false], 'original non-finite values' => [true]]; + } + + #[DataProvider('supportedObjectModes')] + public function testSupportedObjectRepresentationsAndReferencesArePreserved(bool $nonFinite): void + { + $number = $nonFinite ? INF : 2.5; $input = [ 'reference' => &$number, 'date' => new \DateTimeImmutable('2024-01-01T00:00:00Z'), - 'arrayObject' => new \ArrayObject(['number' => NAN]), - 'arrayIterator' => new \ArrayIterator(['number' => INF]), + 'arrayObject' => new \ArrayObject(['number' => $nonFinite ? NAN : $number]), + 'arrayIterator' => new \ArrayIterator(['number' => $number]), 'timezone' => new \DateTimeZone('UTC'), 'dateSubclass' => new class ('2024-01-01T00:00:00Z') extends \DateTimeImmutable { public string $label = 'kept'; private float $hidden = INF; }, - 'arrayObjectSubclass' => new class (['number' => INF]) extends \ArrayObject { + 'arrayObjectSubclass' => new class (['number' => $number]) extends \ArrayObject { public string $ignored = 'ignored'; }, 'emptyObject' => new \stdClass(), 'emptyArray' => [], - 'sparseArray' => [2 => INF, 4 => 'kept'], + 'sparseArray' => [2 => $number, 4 => 'kept'], ]; - self::assertSame( - '{"reference":null,"date":{"date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' - . ',"arrayObject":{"number":null},"arrayIterator":{"number":null}' - . ',"timezone":{"timezone_type":3,"timezone":"UTC"}' - . ',"dateSubclass":{"label":"kept","date":"2024-01-01 00:00:00.000000","timezone_type":2,"timezone":"Z"}' - . ',"arrayObjectSubclass":{"number":null},"emptyObject":{},"emptyArray":[]' - . ',"sparseArray":{"2":null,"4":"kept"}}', - JsonEncoder::encode($input) - ); - self::assertSame(INF, $number); + $expected = json_encode($input); + $error = json_last_error(); + if ($nonFinite) { + self::assertFalse($expected); + } else { + self::assertIsString($expected); + } + self::assertSame($expected, JsonEncoder::encode($input)); + self::assertSame($error, json_last_error()); + self::assertSame($nonFinite ? INF : 2.5, $number); } public static function nativeArrayFlagOverrides(): array @@ -113,11 +119,14 @@ public function getFlags(): int ); } - public function testReferencedNonFiniteArraySlotDoesNotMutateCallerOrReorderList(): void + public function testReferencedNonFiniteArraySlotKeepsNativeFailureWithoutMutatingCaller(): void { $number = INF; $input = ['prefix' => 1, 'items' => [0, &$number, 2], 'suffix' => false]; - self::assertSame('{"prefix":1,"items":[0,null,2],"suffix":false}', JsonEncoder::encode($input)); + self::assertFalse(json_encode($input)); + $error = json_last_error(); + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame($error, json_last_error()); self::assertSame(INF, $number); self::assertSame([0, INF, 2], $input['items']); } @@ -144,9 +153,14 @@ public function jsonSerialize(): mixed [$nativeInput, $nativeCalls] = $createInput(4); self::assertSame('{"mutator":"done","object":{"number":4}}', json_encode($nativeInput)); self::assertSame(1, $nativeCalls->count); + [$finiteInput, $finiteCalls] = $createInput(4); + self::assertSame('{"mutator":"done","object":{"number":4}}', JsonEncoder::encode($finiteInput)); + self::assertSame(1, $finiteCalls->count); [$input, $calls] = $createInput(NAN); - self::assertSame('{"mutator":"done","object":{"number":null}}', JsonEncoder::encode($input)); + self::assertFalse(JsonEncoder::encode($input)); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); self::assertSame(1, $calls->count); + self::assertNan($input['object']->number); } public static function declaredPropertyChanges(): array @@ -189,7 +203,7 @@ public function jsonSerialize(): mixed if (is_float($next)) { self::assertFalse($native); self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); - $expected = '{"mutator":"done","value":null}'; + $expected = false; } else { $expected = '{"mutator":"done","value":"redacted"}'; self::assertSame($expected, $native); @@ -264,9 +278,9 @@ public static function referencedScalarChanges(): array { $changes = [ 'finite to finite' => [1, 2, '2'], - 'finite to NaN' => [1, NAN, 'null'], - 'finite to positive infinity' => [1, INF, 'null'], - 'finite to negative infinity' => [1, -INF, 'null'], + 'finite to NaN' => [1, NAN, null], + 'finite to positive infinity' => [1, INF, null], + 'finite to negative infinity' => [1, -INF, null], 'NaN to finite' => [NAN, 2, '2'], 'positive infinity to finite' => [INF, 2, '2'], 'negative infinity to finite' => [-INF, 2, '2'], @@ -285,7 +299,7 @@ public function testReferencedScalarIsReadAfterEarlierSerializer( string $container, int|float $initial, int|float $next, - string $expectedNumber + ?string $expectedNumber ): void { $encode = static function (bool $native) use ($container, $initial, $next): array { $number = $initial; @@ -347,8 +361,14 @@ public function jsonSerialize(): mixed self::assertFalse($native); self::assertSame(JSON_ERROR_INF_OR_NAN, $error); } - [$actual] = $encode(false); - self::assertSame('{"mutator":null,"n":' . $expectedNumber . ',"after":7}', $actual); + [$actual, $actualError] = $encode(false); + self::assertSame($native, $actual); + self::assertSame($error, $actualError); + if ($expectedNumber === null) { + self::assertFalse($actual); + } else { + self::assertSame('{"mutator":null,"n":' . $expectedNumber . ',"after":7}', $actual); + } } public function testSharedReferenceIsReadAtEachWirePosition(): void @@ -399,14 +419,13 @@ public function getFlags(): int throw new \LogicException('must not dispatch'); } }; - self::assertSame( - '{"mutator":"done","object":{"number":null}}', - JsonEncoder::encode(['mutator' => $serializer, 'object' => $object]) - ); + self::assertFalse(JsonEncoder::encode(['mutator' => $serializer, 'object' => $object])); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); self::assertSame(1, $calls->count); + self::assertNan($object->number); } - public function testPlainDataObjectsNormalizeNestedNumbersWithoutMutatingInput(): void + public function testOpaqueDataObjectsKeepNativeFailureWithoutMutatingInput(): void { $object = (object) [ 'number' => NAN, @@ -415,10 +434,10 @@ public function testPlainDataObjectsNormalizeNestedNumbersWithoutMutatingInput() 'finite' => 2.5, 'literal' => 'NaN', ]; - self::assertSame( - '{"number":null,"nested":{"number":null},"items":[null,0],"finite":2.5,"literal":"NaN"}', - JsonEncoder::encode($object) - ); + self::assertFalse(json_encode($object)); + $error = json_last_error(); + self::assertFalse(JsonEncoder::encode($object)); + self::assertSame($error, json_last_error()); self::assertNan($object->number); self::assertSame(INF, $object->nested->number); self::assertSame([-INF, 0], $object->items); @@ -577,8 +596,18 @@ public static function propertyHookParents(): array ]; } - #[DataProvider('propertyHookParents')] - public function testPropertyHooksControlTheSerializedPublicView(string $parentDeclaration): void + public static function propertyHookPayloads(): array + { + $cases = []; + foreach (self::propertyHookParents() as $name => [$parent]) { + $cases[$name . ' finite'] = [$parent, false]; + $cases[$name . ' original non-finite'] = [$parent, true]; + } + return $cases; + } + + #[DataProvider('propertyHookPayloads')] + public function testPropertyHooksControlTheSerializedPublicView(string $parentDeclaration, bool $nonFinite): void { if (PHP_VERSION_ID < 80400) { self::markTestSkipped('Property hooks require PHP 8.4.'); @@ -607,17 +636,18 @@ public function getterCalls(): int $native = json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR); self::assertTrue($native === ['value' => 'filtered', 'virtual' => 'visible']); self::assertSame(1, $object->getterCalls()); - $encoded = json_decode( - JsonEncoder::encode(['number' => NAN, 'object' => $object]), - true, - 512, - JSON_THROW_ON_ERROR - ); - self::assertTrue( - $encoded['object'] === ['value' => 'filtered', 'virtual' => 'visible'], - 'Serialization must use the public property getters, not backing storage.' - ); - self::assertNull($encoded['number']); + $encoded = JsonEncoder::encode(['number' => $nonFinite ? NAN : 2.5, 'object' => $object]); + if ($nonFinite) { + self::assertFalse($encoded); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + } else { + $decoded = json_decode($encoded, true, 512, JSON_THROW_ON_ERROR); + self::assertTrue( + $decoded['object'] === ['value' => 'filtered', 'virtual' => 'visible'], + 'Serialization must use the public property getters, not backing storage.' + ); + self::assertSame(2.5, $decoded['number']); + } self::assertSame(2, $object->getterCalls()); } @@ -767,7 +797,7 @@ public function jsonSerialize(): mixed public static function siblingContainers(): array { - return [ + $cases = [ 'empty arrays' => ['empty-arrays', 300000, false], 'shared finite scalar arrays' => ['shared-scalar-arrays', 200000, false], 'distinct finite scalar arrays' => ['distinct-scalar-arrays', 100000, false], @@ -786,6 +816,13 @@ public static function siblingContainers(): array 'ArrayObject public sibling' => ['distinct-empty-objects', 200000, true, 'ArrayObject-properties'], 'ArrayIterator public sibling' => ['distinct-empty-objects', 200000, true, 'ArrayIterator-properties'], ]; + foreach ($cases as $name => $case) { + if (isset($case[3])) { + $case[2] = false; + $cases[$name . ' finite'] = $case; + } + } + return $cases; } #[DataProvider('siblingContainers')] @@ -803,7 +840,11 @@ public function testSiblingContainersFitWithinNativeMemoryLimit( $encodedLines = explode("\n", trim($encoded['stdout'])); $expected = json_decode(end($nativeLines), true, 512, JSON_THROW_ON_ERROR); $actual = json_decode(end($encodedLines), true, 512, JSON_THROW_ON_ERROR); - self::assertLessThan(1024 * 1024, $expected['bytes']); + if ($includeNonFinite) { + self::assertSame(['error' => JSON_ERROR_INF_OR_NAN], $expected); + } else { + self::assertLessThan(1024 * 1024, $expected['bytes']); + } self::assertSame($expected, $actual); } diff --git a/test/NonFinitePropertiesTest.php b/test/NonFinitePropertiesTest.php index 9f5f3513..94ba0b6d 100644 --- a/test/NonFinitePropertiesTest.php +++ b/test/NonFinitePropertiesTest.php @@ -30,13 +30,11 @@ public static function serializableEnums(): array } #[DataProvider('serializableEnums')] - public function testCaptureNormalizesNonFiniteJsonSerializableEnums(NonFiniteJsonSerializableEnum $enum): void - { + public function testCaptureKeepsNativeFailureForNonFiniteSerializableEnums( + NonFiniteJsonSerializableEnum $enum + ): void { $httpClient = new MockedHttpClient('unused.invalid'); - $client = new Client('test-key', [ - 'batch_size' => 10, - 'flush_interval_seconds' => 3600, - ], $httpClient); + $client = new Client('test-key', ['batch_size' => 10, 'flush_interval_seconds' => 3600], $httpClient); $properties = ['enum' => $enum, 'enabled' => false]; try { self::assertTrue($client->capture([ @@ -45,40 +43,38 @@ public function testCaptureNormalizesNonFiniteJsonSerializableEnums(NonFiniteJso self::assertTrue($client->capture([ 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], ])); - self::assertTrue($client->flush()); - self::assertCount(1, $httpClient->calls); - $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); - self::assertSame(['enum-event', 'neighbor'], array_column($payload['batch'], 'event')); - self::assertSame( - ['number' => null, 'finite' => 2.5, 'literal' => 'NaN'], - $payload['batch'][0]['properties']['enum'] - ); - self::assertFalse($payload['batch'][0]['properties']['enabled']); - self::assertSame(7, $payload['batch'][1]['properties']['finite']); + json_encode(null); + self::assertFalse($client->flush()); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + self::assertNull($httpClient->calls); self::assertSame(['enum' => $enum, 'enabled' => false], $properties); } finally { $client->shutdown(); } } - public function testCapturePreservesClosuresAlongsideNonFiniteNumbersAndNeighbor(): void + public static function closureModes(): array + { + return ['finite numbers' => [false], 'original non-finite numbers' => [true]]; + } + + #[DataProvider('closureModes')] + public function testCaptureKeepsNativeClosureBehavior(bool $nonFinite): void { $invoked = false; $callback = static function () use (&$invoked): void { $invoked = true; }; + $numbers = $nonFinite ? [NAN, INF, -INF] : [1.5, 2.5, -2.5]; $properties = [ 'callback' => $callback, - 'numbers' => [NAN, INF, -INF], + 'numbers' => $numbers, 'finite' => 2.5, 'enabled' => false, 'literal' => 'NaN', ]; $httpClient = new MockedHttpClient('unused.invalid'); - $client = new Client('test-key', [ - 'batch_size' => 10, - 'flush_interval_seconds' => 3600, - ], $httpClient); + $client = new Client('test-key', ['batch_size' => 10, 'flush_interval_seconds' => 3600], $httpClient); try { self::assertTrue($client->capture([ 'distinctId' => 'user', 'event' => 'callback-event', 'properties' => $properties, @@ -86,22 +82,29 @@ public function testCapturePreservesClosuresAlongsideNonFiniteNumbersAndNeighbor self::assertTrue($client->capture([ 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], ])); - self::assertTrue($client->flush()); - self::assertCount(1, $httpClient->calls); - $payload = json_decode($httpClient->calls[0]['payload'], false, 512, JSON_THROW_ON_ERROR); - self::assertSame(['callback-event', 'neighbor'], array_column($payload->batch, 'event')); - $actual = $payload->batch[0]->properties; - self::assertInstanceOf(\stdClass::class, $actual->callback); - self::assertSame([], (array) $actual->callback); - self::assertSame([null, null, null], $actual->numbers); - self::assertSame(2.5, $actual->finite); - self::assertFalse($actual->enabled); - self::assertSame('NaN', $actual->literal); - self::assertSame(7, $payload->batch[1]->properties->finite); + if ($nonFinite) { + json_encode(null); + self::assertFalse($client->flush()); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + self::assertNull($httpClient->calls); + self::assertNan($properties['numbers'][0]); + self::assertSame(INF, $properties['numbers'][1]); + self::assertSame(-INF, $properties['numbers'][2]); + } else { + self::assertTrue($client->flush()); + self::assertCount(1, $httpClient->calls); + $payload = json_decode($httpClient->calls[0]['payload'], false, 512, JSON_THROW_ON_ERROR); + self::assertSame(['callback-event', 'neighbor'], array_column($payload->batch, 'event')); + $actual = $payload->batch[0]->properties; + self::assertInstanceOf(\stdClass::class, $actual->callback); + self::assertSame([], (array) $actual->callback); + self::assertSame($numbers, $actual->numbers); + self::assertSame(2.5, $actual->finite); + self::assertFalse($actual->enabled); + self::assertSame('NaN', $actual->literal); + self::assertSame(7, $payload->batch[1]->properties->finite); + } self::assertSame($callback, $properties['callback']); - self::assertNan($properties['numbers'][0]); - self::assertSame(INF, $properties['numbers'][1]); - self::assertSame(-INF, $properties['numbers'][2]); self::assertFalse($invoked); } finally { $client->shutdown(); @@ -111,13 +114,15 @@ public function testCapturePreservesClosuresAlongsideNonFiniteNumbersAndNeighbor public static function propertyHookParents(): array { return [ - 'stdClass subclass' => ['stdClass'], - 'Exception subclass' => ['Exception'], + 'stdClass finite' => ['stdClass', false], + 'stdClass original non-finite' => ['stdClass', true], + 'Exception finite' => ['Exception', false], + 'Exception original non-finite' => ['Exception', true], ]; } #[DataProvider('propertyHookParents')] - public function testCapturePreservesSubclassPropertyHooks(string $parent): void + public function testCapturePreservesSubclassPropertyHooks(string $parent, bool $nonFinite): void { if (PHP_VERSION_ID < 80400) { self::markTestSkipped('Property hooks require PHP 8.4.'); @@ -136,45 +141,117 @@ public function testCapturePreservesSubclassPropertyHooks(string $parent): void $expected = ['value' => 'filtered', 'virtual' => 'visible', 'finite' => 2.5]; self::assertTrue(json_decode(json_encode($object), true, 512, JSON_THROW_ON_ERROR) === $expected); $httpClient = new MockedHttpClient('unused.invalid'); + $client = new Client('test-key', ['batch_size' => 10, 'flush_interval_seconds' => 3600], $httpClient); + try { + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'hooked-subclass', + 'properties' => ['object' => $object, 'number' => $nonFinite ? NAN : 2.5], + ])); + self::assertTrue($client->capture([ + 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], + ])); + if ($nonFinite) { + json_encode(null); + self::assertFalse($client->flush()); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + self::assertNull($httpClient->calls); + } else { + self::assertTrue($client->flush()); + self::assertCount(1, $httpClient->calls); + $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); + self::assertSame(['hooked-subclass', 'neighbor'], array_column($payload['batch'], 'event')); + self::assertTrue( + $payload['batch'][0]['properties']['object'] === $expected, + 'Wire serialization must preserve the filtered public property view.' + ); + self::assertSame(2.5, $payload['batch'][0]['properties']['number']); + self::assertSame(7, $payload['batch'][1]['properties']['finite']); + } + self::assertTrue(((array) $object)['value'] === 'private fixture'); + } finally { + $client->shutdown(); + } + } + + #[DataProvider('nonFiniteValues')] + public function testCaptureSerializesDataOnlyNonFinitePropertiesAsNull(string $consumer, float $number): void + { + $filename = tempnam(sys_get_temp_dir(), 'posthog-nonfinite-'); + $httpClient = new MockedHttpClient('unused.invalid'); $client = new Client('test-key', [ - 'batch_size' => 10, - 'flush_interval_seconds' => 3600, + 'consumer' => $consumer, 'filename' => $filename, + 'batch_size' => 10, 'flush_interval_seconds' => 3600, ], $httpClient); + $properties = [ + 'number' => $number, + 'nested' => ['number' => $number], + 'items' => [1, $number, [], 3], + 'sparse' => [2 => $number, 4 => 'kept'], + 'finite' => 2.5, + 'zero' => 0, + 'enabled' => false, + 'nullable' => null, + 'strings' => ['NaN', 'Infinity', '-Infinity'], + ]; try { self::assertTrue($client->capture([ - 'distinctId' => 'user', - 'event' => 'hooked-subclass', - 'properties' => ['object' => $object, 'number' => NAN], + 'distinctId' => 'user', 'event' => 'non-finite', 'properties' => $properties, ])); self::assertTrue($client->capture([ 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], ])); self::assertTrue($client->flush()); - self::assertCount(1, $httpClient->calls); - $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); - self::assertSame(['hooked-subclass', 'neighbor'], array_column($payload['batch'], 'event')); - self::assertTrue( - $payload['batch'][0]['properties']['object'] === $expected, - 'Wire serialization must preserve the filtered public property view.' - ); - self::assertNull($payload['batch'][0]['properties']['number']); - self::assertSame(7, $payload['batch'][1]['properties']['finite']); - self::assertTrue(((array) $object)['value'] === 'private fixture'); + if ($consumer === 'file') { + self::assertNull($httpClient->calls); + $events = array_map( + static fn(string $line): array => json_decode($line, true, 512, JSON_THROW_ON_ERROR), + file($filename, FILE_IGNORE_NEW_LINES) + ); + } else { + self::assertCount(1, $httpClient->calls); + self::assertSame('/batch/', $httpClient->calls[0]['path']); + $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); + self::assertSame('test-key', $payload['api_key']); + $events = $payload['batch']; + } + self::assertSame(['non-finite', 'neighbor'], array_column($events, 'event')); + self::assertSame([ + 'number' => null, + 'nested' => ['number' => null], + 'items' => [1, null, [], 3], + 'sparse' => [2 => null, 4 => 'kept'], + 'finite' => 2.5, + 'zero' => 0, + 'enabled' => false, + 'nullable' => null, + 'strings' => ['NaN', 'Infinity', '-Infinity'], + ], array_intersect_key($events[0]['properties'], $properties)); + self::assertSame(7, $events[1]['properties']['finite']); + if (is_nan($number)) { + self::assertNan($properties['number']); + self::assertNan($properties['nested']['number']); + self::assertNan($properties['items'][1]); + self::assertNan($properties['sparse'][2]); + } else { + self::assertSame($number, $properties['number']); + self::assertSame($number, $properties['nested']['number']); + self::assertSame($number, $properties['items'][1]); + self::assertSame($number, $properties['sparse'][2]); + } } finally { $client->shutdown(); + unlink($filename); } } #[DataProvider('nonFiniteValues')] - public function testCaptureSerializesNonFinitePropertiesAsNull(string $consumer, float $number): void + public function testOpaqueNonFinitePropertiesKeepNativeFailureAndCallerState(string $consumer, float $number): void { $filename = tempnam(sys_get_temp_dir(), 'posthog-nonfinite-'); $httpClient = new MockedHttpClient('unused.invalid'); $client = new Client('test-key', [ - 'consumer' => $consumer, - 'filename' => $filename, - 'batch_size' => 10, - 'flush_interval_seconds' => 3600, + 'consumer' => $consumer, 'filename' => $filename, + 'batch_size' => 10, 'flush_interval_seconds' => 3600, ], $httpClient); $object = (object) ['number' => $number, 'finite' => 4.5]; $typedObject = new class ($number) { @@ -207,40 +284,32 @@ public function jsonSerialize(): mixed 'serializable' => $serializable, ]; try { + self::assertFalse(json_encode($properties)); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + json_encode(null); self::assertTrue($client->capture([ 'distinctId' => 'user', 'event' => 'non-finite', 'properties' => $properties, ])); + if ($consumer === 'file') { + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); + } self::assertTrue($client->capture([ 'distinctId' => 'user', 'event' => 'neighbor', 'properties' => ['finite' => 7], ])); - self::assertTrue($client->flush()); if ($consumer === 'file') { - self::assertNull($httpClient->calls); - $events = array_map( - static fn(string $line): array => json_decode($line, true, 512, JSON_THROW_ON_ERROR), - file($filename, FILE_IGNORE_NEW_LINES) - ); + self::assertTrue($client->flush()); + $lines = file($filename, FILE_IGNORE_NEW_LINES); + self::assertCount(2, $lines); + self::assertSame('', $lines[0]); + $neighbor = json_decode($lines[1], true, 512, JSON_THROW_ON_ERROR); + self::assertSame('neighbor', $neighbor['event']); + self::assertSame(7, $neighbor['properties']['finite']); } else { - self::assertCount(1, $httpClient->calls); - self::assertSame('/batch/', $httpClient->calls[0]['path']); - $payload = json_decode($httpClient->calls[0]['payload'], true, 512, JSON_THROW_ON_ERROR); - self::assertSame('test-key', $payload['api_key']); - $events = $payload['batch']; + json_encode(null); + self::assertFalse($client->flush()); + self::assertSame(JSON_ERROR_INF_OR_NAN, json_last_error()); } - self::assertSame(['non-finite', 'neighbor'], array_column($events, 'event')); - self::assertSame([ - 'number' => null, - 'nested' => ['number' => null, 'object' => ['number' => null, 'finite' => 4.5]], - 'items' => [1, null, ['number' => null, 'finite' => 4.5], 3], - 'finite' => 2.5, - 'zero' => 0, - 'enabled' => false, - 'nullable' => null, - 'strings' => ['NaN', 'Infinity', '-Infinity'], - 'typed' => ['number' => null], - 'serializable' => ['visible' => ['number' => null, 'finite' => 4.5]], - ], array_intersect_key($events[0]['properties'], $properties)); - self::assertSame(7, $events[1]['properties']['finite']); + self::assertNull($httpClient->calls); self::assertSame($object, $properties['nested']['object']); self::assertSame($object, $properties['items'][2]); if (is_nan($number)) { diff --git a/test/fixtures/json-encoder-data-only.php b/test/fixtures/json-encoder-data-only.php new file mode 100644 index 00000000..48d25faa --- /dev/null +++ b/test/fixtures/json-encoder-data-only.php @@ -0,0 +1,26 @@ + array_fill(0, 4000, ['number' => $number])] + : ['number' => $number, 'text' => str_repeat("\0", 128 * 1024)]; +$batch = [['event' => 'eligible-memory', 'properties' => $properties]]; +if ($argv[3] === 'native') { + $encoded = json_encode(['batch' => $batch, 'api_key' => 'test-key']); +} else { + $consumer = new PostHog\Test\QueueConsumerTestConsumer([]); + $encoded = $consumer->encodedBatchPayload($batch); + if ($argv[2] === 'dense') { + if (!is_nan($properties['items'][0]['number']) || !is_nan($properties['items'][3999]['number'])) { + exit(2); + } + } elseif (!is_nan($properties['number'])) { + exit(2); + } +} +if ($encoded === false) { + exit(1); +} +echo json_encode(['bytes' => strlen($encoded), 'sha256' => hash('sha256', $encoded)]); diff --git a/test/fixtures/json-encoder-large-string.php b/test/fixtures/json-encoder-large-string.php new file mode 100644 index 00000000..895eee31 --- /dev/null +++ b/test/fixtures/json-encoder-large-string.php @@ -0,0 +1,19 @@ + 'large-string', 'properties' => ['value' => str_repeat('A', 12 * 1024 * 1024)]]]; +$payload = ['batch' => $batch, 'api_key' => 'test-key']; +$encoded = $argv[2] === 'native' ? json_encode($payload) : PostHog\JsonEncoder::encode($payload); +if ($encoded === false) { + exit(1); +} +$summary = ['bytes' => strlen($encoded), 'sha256' => hash('sha256', $encoded)]; +unset($encoded); +if ($argv[2] === 'sdk') { + $consumer = new PostHog\Test\QueueConsumerTestConsumer([]); + if ($consumer->encodedBatchPayload($batch) !== false || json_last_error() !== JSON_ERROR_NONE) { + exit(2); + } +} +echo json_encode($summary); diff --git a/test/fixtures/json-encoder-siblings.php b/test/fixtures/json-encoder-siblings.php index b2607537..d08f1975 100644 --- a/test/fixtures/json-encoder-siblings.php +++ b/test/fixtures/json-encoder-siblings.php @@ -65,16 +65,17 @@ $batch = [['event' => 'memory', 'distinct_id' => 'u', 'properties' => $properties]]; echo "prepared\n"; if ($mode === 'native') { - $expectedBatch = $batch; - if ($includeNonFinite) { - $expectedBatch[0]['properties']['numbers'] = [null, null, null]; - } - $encoded = json_encode(['batch' => $expectedBatch, 'api_key' => 'test-key']); + $encoded = json_encode(['batch' => $batch, 'api_key' => 'test-key']); } else { $consumer = new PostHog\Test\QueueConsumerTestConsumer([]); $encoded = $consumer->encodedBatchPayload($batch); } -if ($encoded === false) { +$error = json_last_error(); +if ($includeNonFinite) { + if ($encoded !== false || $error !== JSON_ERROR_INF_OR_NAN) { + exit(1); + } +} elseif ($encoded === false) { exit(1); } if ($includeNonFinite) { @@ -86,7 +87,7 @@ exit(2); } } -echo json_encode([ +echo json_encode($includeNonFinite ? ['error' => $error] : [ 'bytes' => strlen($encoded), 'sha256' => hash('sha256', $encoded), ], JSON_THROW_ON_ERROR), "\n"; From 4df58ef0c83d52c7ffa404e5a119d79da0d8ec6e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 14:23:29 +0200 Subject: [PATCH 15/16] fix: compact retained array storage before non-finite repair --- docs/json-serialization-design-review.md | 18 ++++- docs/json-serialization.md | 2 +- lib/JsonEncoder.php | 16 ++--- test/JsonEncoderDataOnlyTest.php | 69 +++++++++++++++++++ .../json-encoder-retained-capacity.php | 41 +++++++++++ 5 files changed, 133 insertions(+), 13 deletions(-) create mode 100644 test/fixtures/json-encoder-retained-capacity.php diff --git a/docs/json-serialization-design-review.md b/docs/json-serialization-design-review.md index 505843a0..a98d49ef 100644 --- a/docs/json-serialization-design-review.md +++ b/docs/json-serialization-design-review.md @@ -161,7 +161,7 @@ The large-string fixture compares independent native/SDK processes, frees the fi ## Approved implementation and coverage changes -The custom writer, native-handler cases, property-layout reconstruction, active-reference maps and synthetic error restoration were removed. The encoder now has a read-only eligibility pass (10,000 values, 64 array levels, 1 MiB raw string/key bytes), a copy-on-change data normalizer, and one final native payload encoding. Opaque values, references, malformed strings and exhausted budgets cause unchanged native delegation, not a new encoding rejection. Primitive string validation uses `JSON_THROW_ON_ERROR` so it does not reset JSON error state seen by user callbacks. +The custom writer, native-handler cases, property-layout reconstruction, active-reference maps and synthetic error restoration were removed. The encoder now has a read-only eligibility pass (10,000 values, 64 array levels, 1 MiB raw string/key bytes), a compact data normalizer with unchanged-subtree reuse, and one final native payload encoding. Opaque values, references, malformed strings and exhausted budgets cause unchanged native delegation, not a new encoding rejection. Primitive string validation uses `JSON_THROW_ON_ERROR` so it does not reset JSON error state seen by user callbacks. No concrete reported reproduction was deleted or skipped. The five new finite/native-parity tests in `JsonEncoderReviewRegressionTest.php` pass without changing their expectations. The following feature expectations changed only because the user approved narrowing the normalization domain: @@ -186,7 +186,7 @@ The approved contract deliberately does not isolate mixed batches event by event 5. Retain batch/File integration coverage and demonstrate healthy-neighbor behavior for the explicitly supported domain. Do not assume isolated event encoding preserves all whole-batch callback semantics. 6. Review the design's state/ownership/resource invariants before requesting another automatic review. Another passing test count alone is not the exit criterion. -## Approved revision validation +## First approved revision validation (`d16a8b1`) Local validation of the approved data-only implementation on PHP 8.5.10: @@ -197,6 +197,18 @@ Local validation of the approved data-only implementation on PHP 8.5.10: - The five newest native-parity regressions pass with their original expectations. No reported reproduction was deleted or skipped to obtain this result. - New 32 MiB probes compare eligible dense non-finite arrays and escaped strings with independently constructed native-null payloads. Oversized finite strings and finite payloads outside each repair budget retain native success. -The design invariant review checks that copying happens only after full eligibility, that no eligible graph can run a user serializer/getter or contain reference cells, that unchanged and ineligible payloads use their original native graph, and that string validation cannot overwrite the JSON error state seen by native callbacks. Resource bounds limit both inspection work and potential normalization allocations; reaching a bound falls back rather than building a wrapper graph or inventing an encoding error. +The design invariant review checks that copying happens only after full eligibility, that no eligible graph can run a user serializer/getter or contain reference cells, that unchanged and ineligible payloads use their original native graph, and that string validation cannot overwrite the JSON error state seen by native callbacks. The initial allocation review incorrectly assumed that copy-on-write storage was bounded by live entries; the correction below records the counterexample and removes that assumption. Budget exhaustion still falls back rather than inventing an encoding error. PHP 8.2-8.5 CI is required as the cross-version acceptance gate; local results alone are not a matrix-wide validation. + +## Retained-capacity correction + +Two subsequent reviews reported the same allocation defect: [4359164748](https://github.com/PostHog/posthog-php/pull/276#discussion_r4359164748) and [4359218233](https://github.com/PostHog/posthog-php/pull/276#discussion_r4359218233). A million-slot array pruned to one live non-finite value passes the value budget, but PHP copy-on-write duplicates its retained 16 MiB backing allocation. The repeated-child example amplifies the same defect. This invalidated the first revision's allocation argument without changing the approved eligibility/serialization contract. + +Three retained-capacity tests were red on `d16a8b1`: the reported million-slot root, 16 repeated pruned children, and a sparse high live key. Under the same 32 MiB limit, each native non-finite baseline returned its normal error and each equivalent native-null baseline encoded successfully; SDK encoding fatally attempted an extra 16 MiB allocation. The tests and fixture remain in `JsonEncoderDataOnlyTest.php` and `test/fixtures/json-encoder-retained-capacity.php`. + +Normalization now compacts live entries with key-preserving `array_slice` before mapping values. It never writes into a copy seeded from the original table. Compaction must precede mapping: `array_map` alone can retain a high packed index extent. Mapping also preserves genuine numeric-string keys from symbol tables; ordinary PHP keyed assignment coerces them to integers and can change an object-shaped JSON value into a list. A separate red/green test protects this behavior, and a prefix/sparse-key test protects unchanged values and insertion order. + +The revised allocation invariant concerns newly allocated storage, not the caller's existing capacity: candidate tables are constructed from live entries, unchanged subtrees are reused without writes, and each repeated changed occurrence receives compact storage. Budgets cap recursive value visits and string validation. PHP iteration over holes can still reflect existing backing extent, so live-entry counts alone are not a bound on every internal scan. + +Local validation of the correction on PHP 8.5.10: 164 focused tests / 1,216 assertions; 1,594 full tests / 8,345 assertions with Xdebug 3.5.3 coverage; 16 adapter tests; changed-file PHP_CodeSniffer, API snapshot, strict Composer and whitespace checks pass. The original five latest native-parity regressions remain unchanged and green. Existing deprecation counts remain 27 and 42. The PHP 8.2-8.5 matrix must also pass before thread resolution. diff --git a/docs/json-serialization.md b/docs/json-serialization.md index 5bc4f23c..9430ad3f 100644 --- a/docs/json-serialization.md +++ b/docs/json-serialization.md @@ -10,7 +10,7 @@ An eligible payload contains only PHP arrays, strings, numbers, booleans and nul | 64 array levels | The complete payload, not just custom properties | | 1 MiB of string bytes | Combined raw bytes of string values and string keys, before JSON escaping | -The inspection is read-only and invokes no getters or serializers. Repair starts only after the entire payload qualifies, and copies only containers containing changed values. Individual strings are validated without resetting the JSON error state seen by callbacks. +The inspection is read-only and invokes no getters or serializers. Repair starts only after the entire payload qualifies. It builds compact candidate arrays from live entries, preserving key types and order, and reuses unchanged subtrees. It never seeds a writable copy from the caller's array, whose retained backing capacity can greatly exceed its live entry count. Individual strings are validated without resetting the JSON error state seen by callbacks. ## Native fallback diff --git a/lib/JsonEncoder.php b/lib/JsonEncoder.php index 3020c055..c572fa06 100644 --- a/lib/JsonEncoder.php +++ b/lib/JsonEncoder.php @@ -85,16 +85,14 @@ private static function normalize(mixed $value, bool &$changed): mixed if (!is_array($value)) { return $value; } - $result = $value; - foreach ($value as $key => $item) { + // Compact retained storage first; array_map also preserves numeric-string keys without PHP key coercion. + $compact = array_slice($value, 0, null, true); + $result = array_map(static function (mixed $item) use (&$changed): mixed { $childChanged = false; $normalized = self::normalize($item, $childChanged); - if ($childChanged) { - // Eligibility excludes reference cells, so copy-on-write cannot alter caller values. - $result[$key] = $normalized; - $changed = true; - } - } - return $result; + $changed = $changed || $childChanged; + return $normalized; + }, $compact); + return $changed ? $result : $value; } } diff --git a/test/JsonEncoderDataOnlyTest.php b/test/JsonEncoderDataOnlyTest.php index c651a2be..106bc9ec 100644 --- a/test/JsonEncoderDataOnlyTest.php +++ b/test/JsonEncoderDataOnlyTest.php @@ -181,6 +181,75 @@ public function testEligibleRepairMatchesNativeNullDataWithin32Mb(string $shape) self::assertSame($outputs['native'], $outputs['sdk']); } + public function testRepairPreservesUnchangedPrefixesAndSparseKeys(): void + { + $input = [ + 'before' => ['nested' => [2.5]], + 2 => 'kept', + "\0name" => 0, + 5 => ['unchanged' => [], 'bad' => NAN, 'suffix' => false], + 'after' => [0, INF], + ]; + $expected = $input; + $expected[5]['bad'] = null; + $expected['after'][1] = null; + self::assertSame(json_encode($expected), JsonEncoder::encode($input)); + self::assertNan($input[5]['bad']); + self::assertSame(INF, $input['after'][1]); + } + + public function testRepairPreservesNumericStringKeysFromSymbolTables(): void + { + $input = (static function (): array { + ${'0'} = NAN; + ${'1'} = INF; + return get_defined_vars(); + })(); + $expected = (static function (): array { + ${'0'} = null; + ${'1'} = null; + return get_defined_vars(); + })(); + self::assertSame(['0', '1'], array_keys($input)); + self::assertSame('{"0":null,"1":null}', json_encode($expected)); + self::assertSame(json_encode($expected), JsonEncoder::encode($input)); + self::assertSame(['0', '1'], array_keys($input)); + self::assertNan(array_values($input)[0]); + self::assertSame(INF, array_values($input)[1]); + } + + public static function retainedCapacityShapes(): array + { + return ['pruned root' => ['root'], 'repeated children' => ['siblings'], 'sparse high key' => ['high-key']]; + } + + #[DataProvider('retainedCapacityShapes')] + public function testRetainedArrayCapacityDoesNotExpandRepairAllocations(string $shape): void + { + $outputs = []; + foreach (['native-error', 'native-null', 'sdk'] as $mode) { + $process = proc_open( + [PHP_BINARY, '-d', 'memory_limit=32M', '-d', 'max_execution_time=10', + __DIR__ . '/fixtures/json-encoder-retained-capacity.php', + __DIR__ . '/../vendor/autoload.php', $shape, $mode], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes + ); + self::assertIsResource($process); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + $errors = stream_get_contents($pipes[2]); + fclose($pipes[1]); + fclose($pipes[2]); + self::assertSame(0, proc_close($process), $mode . ': ' . $errors); + $outputs[$mode] = json_decode($output, true, 512, JSON_THROW_ON_ERROR); + if ($mode === 'native-error') { + self::assertSame(['error' => JSON_ERROR_INF_OR_NAN], $outputs[$mode]); + } + } + self::assertSame($outputs['native-null'], $outputs['sdk']); + } + public static function depthBudgets(): array { return ['within budget' => [64, true], 'over budget' => [65, false]]; diff --git a/test/fixtures/json-encoder-retained-capacity.php b/test/fixtures/json-encoder-retained-capacity.php new file mode 100644 index 00000000..2079bdd5 --- /dev/null +++ b/test/fixtures/json-encoder-retained-capacity.php @@ -0,0 +1,41 @@ + 0; --$i) { + unset($items[$i]); + } +} +$number = $shape === 'siblings' ? INF : NAN; +$items[$index] = $mode === 'native-null' ? null : $number; +$input = $shape === 'siblings' ? ['items' => array_fill(0, 16, $items)] : $items; +$encoded = $mode === 'sdk' ? PostHog\JsonEncoder::encode($input) : json_encode($input); +if ($mode === 'native-error') { + if ($encoded !== false || json_last_error() !== JSON_ERROR_INF_OR_NAN) { + exit(1); + } + echo json_encode(['error' => JSON_ERROR_INF_OR_NAN]); + exit(0); +} +if ($encoded === false) { + exit(1); +} +if ($mode === 'sdk') { + if (count($items) !== 1 || array_key_first($items) !== $index) { + exit(2); + } + if (is_nan($number) ? !is_nan($items[$index]) : $items[$index] !== $number) { + exit(2); + } +} +echo json_encode(['bytes' => strlen($encoded), 'sha256' => hash('sha256', $encoded)]); From 9f690edb5c83dfa72ff38926ccff0b3b95d92da6 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Oct 2026 14:57:16 +0200 Subject: [PATCH 16/16] refactor: keep JSON encoding internal and remove serialization docs --- README.md | 1 - api/public-api.json | 30 --- docs/json-serialization-design-review.md | 214 ------------------ docs/json-serialization.md | 33 --- lib/Consumer/File.php | 2 +- lib/{ => Internal}/JsonEncoder.php | 2 +- lib/QueueConsumer.php | 1 + scripts/PublicApiSnapshot.php | 5 +- test/JsonEncoderDataOnlyTest.php | 2 +- test/JsonEncoderReviewRegressionTest.php | 2 +- test/JsonEncoderTest.php | 12 +- test/PublicApiSnapshotTest.php | 26 +++ test/fixtures/json-encoder-large-string.php | 2 +- .../json-encoder-retained-capacity.php | 2 +- test/fixtures/json-encoder-scale.php | 2 +- 15 files changed, 44 insertions(+), 292 deletions(-) delete mode 100644 docs/json-serialization-design-review.md delete mode 100644 docs/json-serialization.md rename lib/{ => Internal}/JsonEncoder.php (99%) create mode 100644 test/PublicApiSnapshotTest.php diff --git a/README.md b/README.md index 38955043..44a19b6d 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,6 @@ SDK usage examples and code snippets live in the official documentation so they - [PHP library docs](https://posthog.com/docs/libraries/php) - [Laravel integration docs](https://posthog.com/docs/libraries/laravel) - [Upgrade notes](UPGRADING.md), including GeoIP configuration and compatibility -- [Event JSON encoding](docs/json-serialization.md), including non-finite repair and native fallback ## Questions? diff --git a/api/public-api.json b/api/public-api.json index f5dbc3a7..7490d225 100644 --- a/api/public-api.json +++ b/api/public-api.json @@ -3403,36 +3403,6 @@ } } }, - "PostHog\\JsonEncoder": { - "type": "class", - "abstract": false, - "final": true, - "readonly": false, - "extends": null, - "implements": [], - "constants": [], - "properties": [], - "methods": { - "encode": { - "static": true, - "abstract": false, - "final": false, - "returnType": "string|false", - "parameters": [ - { - "name": "value", - "type": "mixed", - "byReference": false, - "variadic": false, - "optional": false, - "default": null, - "defaultConstant": null, - "hasDefault": false - } - ] - } - } - }, "PostHog\\PostHog": { "type": "class", "abstract": false, diff --git a/docs/json-serialization-design-review.md b/docs/json-serialization-design-review.md deleted file mode 100644 index a98d49ef..00000000 --- a/docs/json-serialization-design-review.md +++ /dev/null @@ -1,214 +0,0 @@ -# PHP event serialization: design checkpoint - -## Status and scope - -- PR: https://github.com/PostHog/posthog-php/pull/276 -- Evaluated revision: `577a8b72c0c38906d1390772c189ac614d5d0d57`. -- Worktree: `/Users/marandaneto/Github/worktrees/posthog-php-nonfinite-20261022-123914`. -- The user subsequently approved the conservative, data-only repair direction with “go for it.” The proposed contract below is now implemented as documented in [Event JSON encoding](json-serialization.md). -- The original checkpoint evidence and reproduction inventory below are retained as a historical baseline. The new implementation does not reconstruct Zend serialization behavior: it either repairs a bounded callback-free data tree or delegates the original whole payload to native JSON. - -## What went wrong - -The review threads are not repetitions of the same unresolved comment. New revisions have exposed different violations of the same underlying invariants: native property views and ordering, callback execution, array identity, caller ownership, and resource use. - -The original recursive-wrapper approach introduced snapshots and retained wrappers. Fixes added weak references, native-type handling, preflight scans and budgets. The sequential writer removed the wrapper graph but assumed responsibility for Zend's property traversal, recursion detection, primitive buffering and error state. That moved the failure surface rather than establishing a stable contract. - -Passing the accumulated regression suite showed that individual examples had been repaired. It did not establish that either implementation was a native-equivalent JSON encoder. In particular: - -- A test asserting recursion failure without checking callback counts missed duplicate execution and stateful acceptance of a cycle. -- Ordinary inheritance tests did not cover abstract virtual properties becoming backed slots. -- Ordinary native-object tests did not cover a property-view read changing a DateInterval object's subsequent native output. -- Sibling-container memory probes did not cover duplication of a single large encoded string. - -The first implementation commit, `bbc0f54`, already tested normalization inside stdClass, a typed readonly object and JsonSerializable output. Object normalization was therefore part of the original promise, not merely accidental scope added during review. Narrowing it now requires explicit agreement. - -## Contract assessed at the checkpoint - -### Compatibility requirements - -For inputs with no non-finite numbers, native `json_encode()` is the independent oracle. Compare fresh equivalent fixtures, including: - -1. Wire values, object/list distinctions and omission of uninitialized or removed properties. -2. Operational property order whenever getters or serializers affect later values, especially redaction. -3. Callback/getter invocation counts, ordering and propagated user exceptions. -4. Native recursion/depth rejection, JSON error state and absence of partial output. -5. Caller-visible state after encoding. User callbacks may intentionally mutate state; the SDK must not add mutations beyond native behavior. -6. Material memory/time regressions, including normal rejection by the existing batch-size guard rather than a process-fatal allocation before that guard. - -These requirements are distinct from the feature expansion: converting an otherwise unsupported non-finite number into JSON null without changing the caller's value. - -### Behavior requiring an explicit scope decision - -| Input category | Before this PR | Promise before the checkpoint | Approved conservative contract | -|---|---|---|---| -| Eligible scalar/array JSON data containing NaN or infinity | Native encoding fails | Convert numbers to null recursively | Preserve this core feature within a defined safe domain | -| Finite custom/native objects, hooks and reference graphs | Native traversal and handlers | Native-equivalent behavior | Delegate the original payload to native encoding unchanged | -| Non-finite values inside objects, getters, serializer results or reference cells | Native failure or user exception | Also normalize these values | Keep native behavior outside the safe domain; this narrows the original promise | -| A batch mixing opaque values and otherwise repairable non-finite data | Native batch failure | Preserve healthy neighboring events | Whole-payload native fallback may still fail the batch; this limitation needs agreement | -| Malformed UTF-8, resources and actual cycles | Native errors | No partial JSON | Preserve native error handling rather than introduce synthetic successful output | - -The [capture spec](https://github.com/PostHog/sdk-specs/blob/main/openspec/specs/capture/spec.md) covers capture, serialization safety and a separate null/undefined-property cleanup contract. It does not define PHP object traversal or this NaN/infinity policy. Do not infer a new non-finite policy from the null-member rule or mix another cleanup into this reassessment. - -## Design alternatives - -### 1. Continue the general-purpose writer - -This preserves the broad promise only if we can demonstrate native-equivalent behavior across the supported PHP versions. The current implementation has not demonstrated that. - -Its risky responsibilities include: - -- Reconstructing stored-slot order from reflection metadata, including virtual/abstract implementations. -- Selecting native extension property views without materializing extra caller state. -- Treating reference-cell IDs as a substitute for native array identity across callbacks. [ReflectionReference::getId](https://www.php.net/manual/en/reflectionreference.getid.php) identifies a reference, not a copy-on-write array value. -- Preserving native callback execution while detecting cycles before re-entry. -- Avoiding a second full string allocation while assembling the output. - -**Not recommended as another incremental patch round.** Four more example-specific fixes would not prove these invariants. - -### 2. Try native JSON first, then retry after normalization - -Rejected for stateful values. A local probe with a JsonSerializable returning NaN recorded two callback invocations for two native attempts. Retrying after a failed first pass can observe changed data, repeat side effects or alter redaction behavior. - -This is not a safe fallback for arbitrary objects or reference graphs. - -### 3. Use partial output or replace tokens in encoded JSON - -Rejected. The executed probe `json_encode([0, NAN, INF, -INF], JSON_PARTIAL_OUTPUT_ON_ERROR)` returned `[0,0,0,0]` with `JSON_ERROR_INF_OR_NAN`. The output no longer identifies which zeros were non-finite. Other encoding failures must not become partial successful payloads. Literal strings such as `"NaN"` must remain unchanged. - -### 4. Narrow the repair domain and preserve native delegation - -**Approved direction.** The current bounds and fallback policy are documented in [Event JSON encoding](json-serialization.md). - -- Define an eligible, callback-free data domain rather than emulate arbitrary PHP object serialization. -- Detect eligibility without reading object properties, invoking callbacks or binding caller reference cells. -- Delegate an ineligible **whole payload** unchanged to one native encoding operation. Merely passing objects through while copying surrounding arrays is not enough: callbacks can retain or return ancestor arrays, so copying can change native identity/recursion behavior. -- Repair only eligible values, preserve object/list distinctions and numeric null positions, and leave caller values unchanged. -- If eligibility cannot be established within its limits, use unchanged native delegation rather than reject an otherwise healthy finite payload or allocate a fallback wrapper graph. -- Prove memory behavior for dense non-finite data as well as unchanged data. Native delegation alone does not prove that a normalization copy is cheap. -- Do not add a public option or another user hook. `Client::applyBeforeSend()` already supplies `before_send`; callers can explicitly provide JSON-safe data there. It is an escape hatch, not permission for the SDK to change native behavior silently. - -This candidate does **not** automatically satisfy the existing object/non-finite feature tests. In particular, normalization of typed-object fields, stdClass fields, JsonSerializable enum/results, non-finite reference-cell updates, and mixed opaque/non-finite payloads must be explicitly retained or taken out of scope. Their examples must not be deleted or silently skipped to make a prototype green. - -### Decision gate - -The checkpoint required this decision before production implementation. The user subsequently chose the first option: - -- Approve a precisely bounded data-only repair contract, including opaque/mixed-batch fallback and unsupported non-finite behavior; then prototype and test it. -- Require the full original object/reference normalization contract; then first establish a feasible native-compatible mechanism. If we cannot establish one, defer the feature rather than claim compatibility from another patch series. - -No production change, commit, push or review-thread resolution was performed during the checkpoint itself. The later implementation follows the user's explicit approval rather than treating the narrowed feature expectations as incidental test fixes. - -## Reported reproduction inventory - -There are 18 top-level review comments representing 17 distinct concerns; the second Exception-hook comment duplicates the first. The mapping below concerns concrete reproductions, not proof that the whole defect family is solved. - -`J` means `test/JsonEncoderTest.php`; `N` means `test/NonFinitePropertiesTest.php`; `R` means the new `test/JsonEncoderReviewRegressionTest.php`. - -| Review comment ID | Reproduction and retained regression | Result on `577a8b7` at checkpoint | -|---|---|---| -| `4350801113`, `4350929352` | Internal-parent getter redaction and virtual properties: J `testPropertyHooksControlTheSerializedPublicView` / `testPropertyGetterExceptionsAreNotSwallowed`; N `testCapturePreservesSubclassPropertyHooks` | Existing cases pass | -| `4350801125` | Temporary serializer output retained across 100 x 1,000 objects: J `testTemporarySerializedObjectsFitWithinMemoryLimit` (64 MB) | Pass | -| `4350952993` | ArrayObject/ArrayIterator override dispatch: J `testNativeArrayFlagOverridesAreNotInvoked` and `testNativeStandardPropertyFlagsPreserveHooksDespiteOverride` | Pass | -| `4350953015` | Eager sibling wrappers: J `testSiblingContainersFitWithinNativeMemoryLimit`, including `empty arrays` (300,000; 32 MB) | Pass | -| `4356816316` | Earlier serializer redacts a later declared scalar: J `testDeclaredPropertyIsReadAfterEarlierSerializer`, `testHookedPropertyIsReadAfterEarlierSerializer` | Pass | -| `4356816321` | Shared 24-level invalid-UTF8 graph: J `testSharedInvalidArrayGraphIsRejectedWithoutEagerExpansion` (32 MB; two seconds) | Pass | -| `4357239411` | Earlier serializer unsets a later field, with/without magic getter: J `testDeclaredPropertyPresenceMatchesNativeEncoding`, `remove` cases | Pass | -| `4357265958` | Budget fallback creates a million wrappers: J `testLargePayloadEncodingMatchesNativeWithinResourceLimits`, `sibling allocation`; `json-encoder-scale.php` (2,000,000 arrays; 128 MB) | Pass | -| `4357701895` | Earlier serializer initializes a previously uninitialized field: J `testDeclaredPropertyPresenceMatchesNativeEncoding`, `initialize` cases | Pass | -| `4357701903` | Reference preparation copies the parent per element: J `testLargePayloadEncodingMatchesNativeWithinResourceLimits`, `reference traversal` (60,000 references; five seconds) | Pass | -| `4358008186` | Permanent tables on declared-only objects: J `testSiblingContainersFitWithinNativeMemoryLimit`, `declared-only objects` and neighboring non-finite case (100,000 objects; 32 MB) | Pass | -| `4358008192` | SimpleXML subclass native view: J `testSimpleXmlSubclassesKeepTheirNativePropertyView` | Pass | -| `4358008196` | A referenced slot points to a replacement array: J `testReplacedReferencedArrayIsNotTreatedAsTheActiveArray` (map/empty/nested); `testOuterReferenceRemainsActiveAfterTemporaryReplacement` | Pass | -| `4358414871` | Abstract virtual property becomes a backed slot before inherited redaction: R `testBackedImplementationOfAbstractPropertyPreservesNativeRedactionOrder` | **Fails:** SDK reads the unredacted value first | -| `4358414885` | Initial active array has no reference ID: R `testActiveArrayDoesNotReenterItsSerializer` (stateless/stateful) | **Both fail:** callback count is two; stateful case is accepted instead of rejected | -| `4358414893` | Hooked DateInterval gains extra fields and changes subsequent native output: R `testHookedDateIntervalPreservesNativeViewAndCallerState` | **Fails:** interval fields are added; the native before/after probe also shows caller mutation | -| `4358414903` | Full encoded string duplicated in the buffer: R `testLargeStringMatchesNativeWithin32MbBeforeBatchSizeRejection`; `json-encoder-large-string.php` (12 MiB ASCII; 32 MB) | **Fails:** native succeeds; SDK exits with memory exhaustion | - -Review links use `https://github.com/PostHog/posthog-php/pull/276#discussion_r`. - -Additional pre-review reproductions are also retained: the paired recursive serializer chain added in `04d9f63`, native object formats, embedded-NUL property names and serializable enums (`25ad084`), stdClass-subclass hooks (`4059fad`), and Closure representation (`a3fd115`). Their cases remain in J/N and `test/NonFiniteJsonSerializableEnum.php`. - -No existing tests or assertions were removed, weakened or skipped by this checkpoint. Historical RED evidence for earlier findings is recorded in the PR replies/history; this audit reran the current retained cases, not every historical pre-fix revision. - -## Historical checkpoint validation - -Environment: PHP 8.5.10, PHPUnit 11.5.55, local offline/loopback fixtures. No production code was changed. The new hook/abstract-property tests are gated below PHP 8.4. These new tests were not run locally on PHP 8.2/8.3/8.4 or with Xdebug; published-head CI results do not include them. - -```sh -./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage \ - test/JsonEncoderTest.php test/NonFinitePropertiesTest.php -# PASS: 113 tests, 862 assertions. - -./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage \ - test/JsonEncoderReviewRegressionTest.php -# EXPECTED RED: 5 tests, 20 assertions, 5 behavior/resource failures. -# All native-oracle/setup checks pass before the SDK failures. - -./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage -# RED: 1,548 tests, 8,011 assertions, exactly the five new failures. -# Existing notices: 27 deprecations and 42 PHPUnit deprecations. - -./vendor/bin/phpcs --standard=phpcs.xml \ - test/JsonEncoderReviewRegressionTest.php test/fixtures/json-encoder-large-string.php -# PASS. - -git diff --exit-code -- lib api composer.json phpunit.xml -# PASS: production, API snapshot, dependencies and runner configuration unchanged. -``` - -The large-string fixture compares independent native/SDK processes, frees the first encoded string before checking the batch guard, and keeps the existing 32 MB limit. Its failure is not caused by retaining two completed encodings in the test harness. - -## Approved implementation and coverage changes - -The custom writer, native-handler cases, property-layout reconstruction, active-reference maps and synthetic error restoration were removed. The encoder now has a read-only eligibility pass (10,000 values, 64 array levels, 1 MiB raw string/key bytes), a compact data normalizer with unchanged-subtree reuse, and one final native payload encoding. Opaque values, references, malformed strings and exhausted budgets cause unchanged native delegation, not a new encoding rejection. Primitive string validation uses `JSON_THROW_ON_ERROR` so it does not reset JSON error state seen by user callbacks. - -No concrete reported reproduction was deleted or skipped. The five new finite/native-parity tests in `JsonEncoderReviewRegressionTest.php` pass without changing their expectations. The following feature expectations changed only because the user approved narrowing the normalization domain: - -- The original mixed native-object/reference fixture in `testSupportedObjectRepresentationsAndReferencesArePreserved` remains and now asserts native failure for its non-finite mode; a finite counterpart preserves native format coverage. -- Non-finite reference slots, object fields and serializer output now assert native failure/error parity and caller identity/value preservation. The original stdClass/typed-object/serializer fixtures remain present. -- All 28 reference-mutation cases remain. Transitions to non-finite values assert native failure; transitions to finite values retain native wire output and callback counts. -- Hook privacy tests retain the original opaque/non-finite combinations as native-failure cases and add finite companions that still assert filtered wire values and getter counts. -- The large sibling/native-handler memory shapes remain under their original limits. Opaque or over-budget non-finite combinations compare native failure instead of pre-sanitized null data. Finite native-handler companions preserve successful-output coverage. -- The original six batch/File object-normalization integration fixtures remain in `testOpaqueNonFinitePropertiesKeepNativeFailureAndCallerState`. Six new data-only integration cases prove NaN/infinity repair, healthy-neighbor delivery and caller immutability in the supported domain. -- Serializable-enum and Closure examples remain. Opaque non-finite payloads assert native failure; finite Closure wire-format and non-invocation coverage is retained. - -`JsonEncoderDataOnlyTest.php` defines the new policy independently: eligible numeric repair, whole-payload opaque/reference fallback, no serializer retry, exact value/byte/depth boundaries, successful finite fallback outside those bounds, and 32 MiB repair probes for dense arrays and escaped strings. The new policy was run against the previous writer before replacement and produced six expected failures; all five latest review reproductions were also red at the checkpoint. - -The approved contract deliberately does not isolate mixed batches event by event, normalize values produced by arbitrary objects, or repair payloads beyond the eligibility bounds. These limitations are customer-visible in the encoding contract rather than hidden in test changes. - -## Exit criteria for the replacement design - -1. Approve the normalization domain and fallback policy in the PR before modifying production code or changing feature-test expectations. -2. Preserve every concrete reproduction above. Native-compatibility/privacy tests are not negotiable merely because a feature is narrowed. -3. Define the oracle for each category: native result/error/callback/state parity versus explicitly agreed non-finite-to-null transformation. -4. Validate the complete matrix on PHP 8.2-8.5, with applicable hook skips, Xdebug coverage and independent native resource baselines. -5. Retain batch/File integration coverage and demonstrate healthy-neighbor behavior for the explicitly supported domain. Do not assume isolated event encoding preserves all whole-batch callback semantics. -6. Review the design's state/ownership/resource invariants before requesting another automatic review. Another passing test count alone is not the exit criterion. - -## First approved revision validation (`d16a8b1`) - -Local validation of the approved data-only implementation on PHP 8.5.10: - -- 159 focused tests / 1,183 assertions pass across `JsonEncoderDataOnlyTest`, `JsonEncoderReviewRegressionTest`, `JsonEncoderTest` and `NonFinitePropertiesTest`. -- The complete suite passes with Xdebug 3.5.3 coverage: 1,589 tests / 8,312 assertions. Existing notices remain 27 deprecations and 42 PHPUnit deprecations. -- All 16 adapter tests pass. -- Changed PHP files pass PHP_CodeSniffer; the public API snapshot is unchanged/up to date; strict Composer validation passes. -- The five newest native-parity regressions pass with their original expectations. No reported reproduction was deleted or skipped to obtain this result. -- New 32 MiB probes compare eligible dense non-finite arrays and escaped strings with independently constructed native-null payloads. Oversized finite strings and finite payloads outside each repair budget retain native success. - -The design invariant review checks that copying happens only after full eligibility, that no eligible graph can run a user serializer/getter or contain reference cells, that unchanged and ineligible payloads use their original native graph, and that string validation cannot overwrite the JSON error state seen by native callbacks. The initial allocation review incorrectly assumed that copy-on-write storage was bounded by live entries; the correction below records the counterexample and removes that assumption. Budget exhaustion still falls back rather than inventing an encoding error. - -PHP 8.2-8.5 CI is required as the cross-version acceptance gate; local results alone are not a matrix-wide validation. - -## Retained-capacity correction - -Two subsequent reviews reported the same allocation defect: [4359164748](https://github.com/PostHog/posthog-php/pull/276#discussion_r4359164748) and [4359218233](https://github.com/PostHog/posthog-php/pull/276#discussion_r4359218233). A million-slot array pruned to one live non-finite value passes the value budget, but PHP copy-on-write duplicates its retained 16 MiB backing allocation. The repeated-child example amplifies the same defect. This invalidated the first revision's allocation argument without changing the approved eligibility/serialization contract. - -Three retained-capacity tests were red on `d16a8b1`: the reported million-slot root, 16 repeated pruned children, and a sparse high live key. Under the same 32 MiB limit, each native non-finite baseline returned its normal error and each equivalent native-null baseline encoded successfully; SDK encoding fatally attempted an extra 16 MiB allocation. The tests and fixture remain in `JsonEncoderDataOnlyTest.php` and `test/fixtures/json-encoder-retained-capacity.php`. - -Normalization now compacts live entries with key-preserving `array_slice` before mapping values. It never writes into a copy seeded from the original table. Compaction must precede mapping: `array_map` alone can retain a high packed index extent. Mapping also preserves genuine numeric-string keys from symbol tables; ordinary PHP keyed assignment coerces them to integers and can change an object-shaped JSON value into a list. A separate red/green test protects this behavior, and a prefix/sparse-key test protects unchanged values and insertion order. - -The revised allocation invariant concerns newly allocated storage, not the caller's existing capacity: candidate tables are constructed from live entries, unchanged subtrees are reused without writes, and each repeated changed occurrence receives compact storage. Budgets cap recursive value visits and string validation. PHP iteration over holes can still reflect existing backing extent, so live-entry counts alone are not a bound on every internal scan. - -Local validation of the correction on PHP 8.5.10: 164 focused tests / 1,216 assertions; 1,594 full tests / 8,345 assertions with Xdebug 3.5.3 coverage; 16 adapter tests; changed-file PHP_CodeSniffer, API snapshot, strict Composer and whitespace checks pass. The original five latest native-parity regressions remain unchanged and green. Existing deprecation counts remain 27 and 42. The PHP 8.2-8.5 matrix must also pass before thread resolution. diff --git a/docs/json-serialization.md b/docs/json-serialization.md deleted file mode 100644 index 9430ad3f..00000000 --- a/docs/json-serialization.md +++ /dev/null @@ -1,33 +0,0 @@ -# Event JSON encoding - -NaN and positive/negative infinity are encoded as JSON `null` when the **entire payload** is an eligible data-only tree. Array positions and keys, finite values, literal strings such as `"NaN"`, and caller-owned values are preserved. - -An eligible payload contains only PHP arrays, strings, numbers, booleans and null, with no objects, resources or retained array-element references. Repair is bounded to: - -| Limit | Scope | -|---|---| -| 10,000 values | Includes the root, each container and each array value | -| 64 array levels | The complete payload, not just custom properties | -| 1 MiB of string bytes | Combined raw bytes of string values and string keys, before JSON escaping | - -The inspection is read-only and invokes no getters or serializers. Repair starts only after the entire payload qualifies. It builds compact candidate arrays from live entries, preserving key types and order, and reuses unchanged subtrees. It never seeds a writable copy from the caller's array, whose retained backing capacity can greatly exceed its live entry count. Individual strings are validated without resetting the JSON error state seen by callbacks. - -## Native fallback - -If any value is opaque or unsupported, a reference is found, a string is malformed, or a bound is exceeded, the **original whole payload** is passed unchanged to native `json_encode()`. It is not partially repaired and is not retried. Finite payloads outside the repair bounds are still encoded normally. - -Objects include stdClass, enums, Closure, JsonSerializable implementations and native extension objects. The SDK does not inspect their properties or reproduce their serialization handlers. Native property ordering, callback counts, redaction, recursion detection, user exceptions and JSON errors remain authoritative. - -Consequently, non-finite numbers in an ineligible payload still cause the native encoding failure. This includes numbers inside objects or references, values produced by a serializer, and plain non-finite fields next to an opaque value. Capture callers needing a different policy can supply JSON-safe data through the existing `before_send` hook; no new SDK option is introduced. - -## Batch and File behavior - -For queued consumers, the payload includes the entire batch, API key and SDK metadata. One opaque value or reference in any event disables repair for the whole batch. A mixed batch containing an object and a non-finite number can therefore still fail native encoding, including its healthy neighboring events. This is not per-event error isolation. - -For the File consumer, eligibility applies to each complete event. Ineligible events retain the File consumer's existing native-failure handling, including its empty-line output when encoding fails; this change does not replace that handling. - -The existing 1 MiB batch **encoded body** limit is separate from the raw-string repair budget. Escaping can enlarge JSON. Oversized valid batches are still rejected by the existing size guard after encoding, and large finite strings use native encoding without a second SDK output buffer. - -No partial-output flags, string-token replacement, object casting, reflection-based property traversal, reference binding or serializer retries are used. - -The design history and complete review-reproduction inventory are recorded in [the design checkpoint](json-serialization-design-review.md). diff --git a/lib/Consumer/File.php b/lib/Consumer/File.php index eaefaaad..0454b4fa 100644 --- a/lib/Consumer/File.php +++ b/lib/Consumer/File.php @@ -4,7 +4,7 @@ use Exception; use PostHog\Consumer; -use PostHog\JsonEncoder; +use PostHog\Internal\JsonEncoder; /** * Consumer that writes analytics messages to a local file. diff --git a/lib/JsonEncoder.php b/lib/Internal/JsonEncoder.php similarity index 99% rename from lib/JsonEncoder.php rename to lib/Internal/JsonEncoder.php index c572fa06..a3f279f3 100644 --- a/lib/JsonEncoder.php +++ b/lib/Internal/JsonEncoder.php @@ -1,6 +1,6 @@ self = $wideObject; $wideArray['self'] = &$wideArray; foreach ([$wideObject, $wideArray] as $value) { - if (PostHog\JsonEncoder::encode($value) !== false || json_last_error() !== JSON_ERROR_RECURSION) { + if (PostHog\Internal\JsonEncoder::encode($value) !== false || json_last_error() !== JSON_ERROR_RECURSION) { exit(4); } } @@ -1030,7 +1030,7 @@ public function jsonSerialize(): mixed $deep = [$deep]; } foreach ([$array, $object, $deep] as $value) { - if (PostHog\JsonEncoder::encode($value) !== false) { + if (PostHog\Internal\JsonEncoder::encode($value) !== false) { exit(1); } } diff --git a/test/PublicApiSnapshotTest.php b/test/PublicApiSnapshotTest.php new file mode 100644 index 00000000..7a1fc478 --- /dev/null +++ b/test/PublicApiSnapshotTest.php @@ -0,0 +1,26 @@ +invoke(null, __DIR__ . '/../lib', 'PostHog\\'); + + self::assertArrayHasKey(Client::class, $api['symbols']); + self::assertArrayHasKey(Consumer::class, $api['symbols']); + self::assertArrayNotHasKey(JsonEncoder::class, $api['symbols']); + self::assertArrayNotHasKey('PostHog\\JsonEncoder', $api['symbols']); + } +} diff --git a/test/fixtures/json-encoder-large-string.php b/test/fixtures/json-encoder-large-string.php index 895eee31..587e3cce 100644 --- a/test/fixtures/json-encoder-large-string.php +++ b/test/fixtures/json-encoder-large-string.php @@ -4,7 +4,7 @@ $batch = [['event' => 'large-string', 'properties' => ['value' => str_repeat('A', 12 * 1024 * 1024)]]]; $payload = ['batch' => $batch, 'api_key' => 'test-key']; -$encoded = $argv[2] === 'native' ? json_encode($payload) : PostHog\JsonEncoder::encode($payload); +$encoded = $argv[2] === 'native' ? json_encode($payload) : PostHog\Internal\JsonEncoder::encode($payload); if ($encoded === false) { exit(1); } diff --git a/test/fixtures/json-encoder-retained-capacity.php b/test/fixtures/json-encoder-retained-capacity.php index 2079bdd5..e54ea689 100644 --- a/test/fixtures/json-encoder-retained-capacity.php +++ b/test/fixtures/json-encoder-retained-capacity.php @@ -19,7 +19,7 @@ $number = $shape === 'siblings' ? INF : NAN; $items[$index] = $mode === 'native-null' ? null : $number; $input = $shape === 'siblings' ? ['items' => array_fill(0, 16, $items)] : $items; -$encoded = $mode === 'sdk' ? PostHog\JsonEncoder::encode($input) : json_encode($input); +$encoded = $mode === 'sdk' ? PostHog\Internal\JsonEncoder::encode($input) : json_encode($input); if ($mode === 'native-error') { if ($encoded !== false || json_last_error() !== JSON_ERROR_INF_OR_NAN) { exit(1); diff --git a/test/fixtures/json-encoder-scale.php b/test/fixtures/json-encoder-scale.php index deb33a2a..5687793c 100644 --- a/test/fixtures/json-encoder-scale.php +++ b/test/fixtures/json-encoder-scale.php @@ -15,7 +15,7 @@ if ($argv[3] === 'native') { $encoded = json_encode(['batch' => $batch, 'api_key' => 'test-key']); } else { - $encoded = PostHog\JsonEncoder::encode(['batch' => $batch, 'api_key' => 'test-key']); + $encoded = PostHog\Internal\JsonEncoder::encode(['batch' => $batch, 'api_key' => 'test-key']); $consumer = new PostHog\Test\QueueConsumerTestConsumer([]); $payload = $consumer->encodedBatchPayload($batch); $expectedPayload = $argv[2] === 'siblings' ? false : $encoded;