8000 bug #59646 [JsonStreamer] Max depth handling and JSON errors (mtarld) · symfony/symfony@f7aae0b · GitHub
[go: up one dir, main page]
Skip to content

Commit f7aae0b

Browse files
fabpotfabpot
committed
bug #59646 [JsonStreamer] Max depth handling and JSON errors (mtarld)
This PR was merged into the 7.3 branch. Discussion ---------- [JsonStreamer] Max depth handling and JSON errors | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | yes | Deprecations? | no | Issues | | License | MIT Until now, max depth was handled like the following: 1. Creating a `DataModelNodeInterface` that is at most 512 deep, and adding a `ExceptionNode` when this depth is reached. 1. Generating an encoder based on that tree, that adds a throw line whenever encountering an `ExceptionNode`. But this approach creates some issues: - When encoding an object that has a property containing a 512 deep "json encodable data" such as a very nested array, this won't throw any exception even though the encoded data is 513 deep. - The generated encoders are very big files in case of self-referencing objects: ```php return static function (mixed $data, \Psr\Container\ContainerInterface $normalizers, array $options): \Traversable { yield '{"self":'; if (null === $data->self) { yield 'null'; } else { yield '{"self":'; if (null === $data->self->self) { yield 'null'; } else { // 510 more times until we throw an exception } yield '}'; } yield '}'; }; ``` - The created `DataModelNodeInterface` contains useless repeated nodes and is memory-consuming in case of self-referencing objects. --- This PR fixes these issues by using another approach: - Creating "ghost objects" when encountering an already seen object while constructing a `DataModelNodeInterface` (like what is done for decoding). These ghosts will prevent nodes to be repeated. - Creating "generators" callable in the encoders that are responsible to encode these ghost objects. In that way, encoder PHP files will be way lighter (and will be generated faster, for example the test suite went from `00:03.877` to `00:00.122`). ```php return static function (mixed $data, \Psr\Container\ContainerInterface $normalizers, array $options): \Traversable { $generators['Foo'] = static function ($data, $depth) use ($normalizers, $options, &$generators) { if ($depth >= 512) { throw new NotEncodableValueException('Maximum stack depth exceeded'); } yield '{"`@self`":'; if ($data->self instanceof Foo) { yield from $generators['Foo']($data->self, $depth + 1); } else (null === $data->self) { yield 'null'; } yield '}'; }; yield from $generators['Foo']($data, 0); }; ``` - Use these generators only for ghost objects, as calling functions costs a bit in terms of performances. - Specify the proper depth in `json_encode` depending on the current depth: ```php return static function (mixed $data, \Psr\Container\ContainerInterface $normalizers, array $options): \Traversable { yield '{"`@id`":'; yield \json_encode($data->id, depth: 511); yield '}'; }; ``` --- Moreover, JSON encoding errors were not gathered at all. That's why this PR updates the `json_encode` call to use the `JSON_THROW_ON_ERROR` flag, and catches/convert the resulting exceptions to `NotEncodableValueException`: ```php return static function (mixed $data, \Psr\Container\ContainerInterface $normalizers, array $options): \Traversable { try { yield \json_encode($data, \JSON_THROW_ON_ERROR, 512); } catch (\JsonException $e) { throw new \Symfony\Component\JsonEncoder\Exception\NotEncodableValueException($e->getMessage(), 0, $e); } }; ``` And the `MaxDepthException` has been converted to a `NotEncodableValueException` with the `"Maximum stack depth exceeded"` message to be the same as the native `json_encode` max depth exception. Commits ------- fb7fac1 [JsonEncoder] Fix max depth handling and json errors
2 parents 7100c7b + fb7fac1 commit f7aae0b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+625
-261
lines changed

‎src/Symfony/Component/JsonStreamer/DataModel/FunctionDataAccessor.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,16 @@ public function __construct(
3333
) {
3434
}
3535

36+
public function getObjectAccessor(): ?DataAccessorInterface
37+
{
38+
return $this->objectAccessor;
39+
}
40+
41+
public function withObjectAccessor(?DataAccessorInterface $accessor): self
42+
{
43+
return new self($this->functionName, $this->arguments, $accessor);
44+
}
45+
3646
public function toPhpExpr(): Expr
3747
{
3848
$builder = new BuilderFactory();

‎src/Symfony/Component/JsonStreamer/DataModel/PropertyDataAccessor.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ public function __construct(
2929
) {
3030
}
3131

32+
public function getObjectAccessor(): DataAccessorInterface
33+
{
34+
return $this->objectAccessor;
35+
}
36+
37+
public function withObjectAccessor(DataAccessorInterface $accessor): self
38+
{
39+
return new self($accessor, $this->propertyName);
40+
}
41+
3242
public function toPhpExpr(): Expr
3343
{
3444
return (new BuilderFactory())->propertyFetch($this->objectAccessor->toPhpExpr(), $this->propertyName);

‎src/Symfony/Component/JsonStreamer/DataModel/Read/ObjectNode.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ final class ObjectNode implements DataModelNodeInterface
3030
public function __construct(
3131
private ObjectType $type,
3232
private array $properties,
33-
private bool $ghost = false,
33+
private bool $mock = false,
3434
) {
3535
}
3636

37-
public static function createGhost(ObjectType|UnionType $type): self
37+
public static function createMock(ObjectType|UnionType $type): self
3838
{
3939
return new self($type, [], true);
4040
}
@@ -57,8 +57,8 @@ public function getProperties(): array
5757
return $this->properties;
5858
}
5959

60-
public function isGhost(): bool
60+
public function isMock(): bool
6161
{
62-
return $this->ghost;
62+
return $this->mock;
6363
}
6464
}

‎src/Symfony/Component/JsonStreamer/DataModel/Write/BackedEnumNode.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ public function __construct(
3131
) {
3232
}
3333

34+
public function withAccessor(DataAccessorInterface $accessor): self
35+
{
36+
return new self($accessor, $this->type);
37+
}
38+
39+
public function getIdentifier(): string
40+
{
41+
return (string) $this->getType();
42+
}
43+
3444
public function getAccessor(): DataAccessorInterface
3545
{
3646
return $this->accessor;

‎src/Symfony/Component/JsonStreamer/DataModel/Write/CollectionNode.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ public function __construct(
3030
) {
3131
}
3232

33+
public function withAccessor(DataAccessorInterface $accessor): self
34+
{
35+
return new self($accessor, $this->type, $this->item);
36+
}
37+
38+
public function getIdentifier(): string
39+
{
40+
return (string) $this->getType();
41+
}
42+
3343
public function getAccessor(): DataAccessorInterface
3444
{
3545
return $this->accessor;

‎src/Symfony/Component/JsonStreamer/DataModel/Write/CompositeNode.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ public function __construct(
6060
$this->nodes = $nodes;
6161
}
6262

63+
public function withAccessor(DataAccessorInterface $accessor): self
64+
{
65+
return new self($accessor, array_map(static fn (DataModelNodeInterface $n): DataModelNodeInterface => $n->withAccessor($accessor), $this->nodes));
66+
}
67+
68+
public function getIdentifier(): string
69+
{
70+
return (string) $this->getType();
71+
}
72+
6373
public function getAccessor(): DataAccessorInterface
6474
{
6575
return $this->accessor;

‎src/Symfony/Component/JsonStreamer/DataModel/Write/DataModelNodeInterface.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323
*/
2424
interface DataModelNodeInterface
2525
{
26+
public function getIdentifier(): string;
27+
2628
public function getType(): Type;
2729

2830
public function getAccessor(): DataAccessorInterface;
31+
32+
public function withAccessor(DataAccessorInterface $accessor): self;
2933
}

‎src/Symfony/Component/JsonStreamer/DataModel/Write/ExceptionNode.php

Lines changed: 0 additions & 49 deletions
This file was deleted.

‎src/Symfony/Component/JsonStreamer/DataModel/Write/ObjectNode.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Component\JsonStreamer\DataModel\Write;
1313

1414
use Symfony\Component\JsonStreamer\DataModel\DataAccessorInterface;
15+
use Symfony\Component\JsonStreamer\DataModel\FunctionDataAccessor;
16+
use Symfony\Component\JsonStreamer\DataModel\PropertyDataAccessor;
1517
use Symfony\Component\TypeInfo\Type\ObjectType;
1618

1719
/**
@@ -30,9 +32,36 @@ public function __construct(
3032
private DataAccessorInterface $accessor,
3133
private ObjectType $type,
3234
private array $properties,
35+
private bool $mock = false,
3336
) {
3437
}
3538

39+
public static function createMock(DataAccessorInterface $accessor, ObjectType $type): self
40+
{
41+
return new self($accessor, $type, [], true);
42+
}
43+
44+
public function withAccessor(DataAccessorInterface $accessor): self
45+
{
46+
$properties = [];
47+
foreach ($this->properties as $key => $property) {
48+
$propertyAccessor = $property->getAccessor();
49+
50+
if ($propertyAccessor instanceof PropertyDataAccessor || $propertyAccessor instanceof FunctionDataAccessor && $propertyAccessor->getObjectAccessor()) {
51+
$propertyAccessor = $propertyAccessor->withObjectAccessor($accessor);
52+
}
53+
54+
$properties[$key] = $property->withAccessor($propertyAccessor);
55+
}
56+
57+
return new self($accessor, $this->type, $properties, $this->mock);
58+
}
59+
60+
public function getIdentifier(): string
61+
{
62+
return (string) $this->getType();
63+
}
64+
3665
public function getAccessor(): DataAccessorInterface
3766
{
3867
return $this->accessor;
@@ -50,4 +79,9 @@ public function getProperties(): array
5079
{
5180
return $this->properties;
5281
}
82+
83+
public function isMock(): bool
84+
{
85+
return $this->mock;
86+
}
5387
}

‎src/Symfony/Component/JsonStreamer/DataModel/Write/ScalarNode.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ public function __construct(
3131
) {
3232
}
3333

34+
public function withAccessor(DataAccessorInterface $accessor): self
35+
{
36+
return new self($accessor, $this->type);
37+
}
38+
39+
public function getIdentifier(): string
40+
{
41+
return (string) $this->getType();
42+
}
43+
3444
public function getAccessor(): DataAccessorInterface
3545
{
3646
return $this->accessor;

‎src/Symfony/Component/JsonStreamer/Exception/MaxDepthException.php renamed to ‎src/Symfony/Component/JsonStreamer/Exception/NotEncodableValueException.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@
1616
*
1717
* @experimental
1818
*/
19-
final class MaxDepthException extends RuntimeException
19+
class NotEncodableValueException extends UnexpectedValueException
2020
{
21-
public function __construct()
22-
{
23-
parent::__construct('Max depth of 512 has been reached.');
24-
}
2521
}

‎src/Symfony/Component/JsonStreamer/Read/PhpAstBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ private function buildCollectionNodeStatements(CollectionNode $node, bool $decod
421421
*/
422422
private function buildObjectNodeStatements(ObjectNode $node, bool $decodeFromStream, array &$context): array
423423
{
424-
if ($node->isGhost()) {
424+
if ($node->isMock()) {
425425
return [];
426426
}
427427

‎src/Symfony/Component/JsonStreamer/Read/StreamReaderGenerator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public function createDataModel(Type $type, array $options = [], array $context
123123
$className = $type->getClassName();
124124

125125
if ($context['generated_classes'][$typeString] ??= false) {
126-
return ObjectNode::createGhost($type);
126+
return ObjectNode::createMock($type);
127127
}
128128

129129
$propertiesNodes = [];

‎src/Symfony/Component/JsonStreamer/Tests/DataModel/Write/CompositeNodeTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,17 @@ public function testSortNodesOnCreation()
5454

5555
$this->assertSame([$collection, $object, $scalar], $composite->getNodes());
5656
}
57+
58+
public function testWithAccessor()
59+
{
60+
$composite = new CompositeNode(new VariableDataAccessor('data'), [
61+
new ScalarNode(new VariableDataAccessor('foo'), Type::int()),
62+
new ScalarNode(new VariableDataAccessor('bar'), Type::int()),
63+
]);
64+
$composite = $composite->withAccessor($newAccessor = new VariableDataAccessor('baz'));
65+
66+
foreach ($composite->getNodes() as $node) {
67+
$this->assertSame($newAccessor, $node->getAccessor());
68+
}
69+
}
5770
}

‎src/Symfony/Component/JsonStreamer/Tests/DataModel/Write/ObjectNodeTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\JsonStreamer\Tests\DataModel\Write;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\JsonStreamer\DataModel\FunctionDataAccessor;
16+
use Symfony\Component\JsonStreamer\DataModel\PropertyDataAccessor;
17+
use Symfony\Component\JsonStreamer\DataModel\VariableDataAccessor;
18+
use Symfony\Component\JsonStreamer\DataModel\Write\ObjectNode;
19+
use Symfony\Component\JsonStreamer\DataModel\Write\ScalarNode;
20+
use Symfony\Component\TypeInfo\Type;
21+
22+
class ObjectNodeTest extends TestCase
23+
{
24+
public function testWithAccessor()
25+
{
26+
$object = new ObjectNode(new VariableDataAccessor('foo'), Type::object(self::class), [
27+
new ScalarNode(new PropertyDataAccessor(new VariableDataAccessor('foo'), 'property'), Type::int()),
28+
new ScalarNode(new FunctionDataAccessor('function', [], new VariableDataAccessor('foo')), Type::int()),
29+
new ScalarNode(new FunctionDataAccessor('function', []), Type::int()),
30+
new ScalarNode(new VariableDataAccessor('bar'), Type::int()),
31+
]);
32+
$object = $object->withAccessor($newAccessor = new VariableDataAccessor('baz'));
33+
34+
$this->assertSame($newAccessor, $object->getAccessor());
35+
$this->assertSame($newAccessor, $object->getProperties()[0]->getAccessor()->getObjectAccessor());
36+
$this->assertSame($newAccessor, $object->getProperties()[1]->getAccessor()->getObjectAccessor());
37+
$this->assertNull($object->getProperties()[2]->getAccessor()->getObjectAccessor());
38+
$this->assertNotSame($newAccessor, $object->getProperties()[3]->getAccessor());
39+
}
40+
}

‎src/Symfony/Component/JsonStreamer/Tests/Fixtures/stream_writer/backed_enum.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

33
return static function (mixed $data, \Psr\Container\ContainerInterface $valueTransformers, array $options): \Traversable {
4-
yield \json_encode($data->value);
4+
try {
5+
yield \json_encode($data->value, \JSON_THROW_ON_ERROR, 512);
6+
} catch (\JsonException $e) {
7+
throw new \Symfony\Component\JsonStreamer\Exception\NotEncodableValueException($e->getMessage(), 0, $e);
8+
}
59
};

‎src/Symfony/Component/JsonStreamer/Tests/Fixtures/stream_writer/bool.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

33
return static function (mixed $data, \Psr\Container\ContainerInterface $valueTransformers, array $options): \Traversable {
4-
yield $data ? 'true' : 'false';
4+
try {
5+
yield $data ? 'true' : 'false';
6+
} catch (\JsonException $e) {
7+
throw new \Symfony\Component\JsonStreamer\Exception\NotEncodableValueException($e->getMessage(), 0, $e);
8+
}
59
};

‎src/Symfony/Component/JsonStreamer/Tests/Fixtures/stream_writer/bool_list.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

33
return static function (mixed $data, \Psr\Container\ContainerInterface $valueTransformers, array $options): \Traversable {
4-
yield \json_encode($data);
4+
try {
5+
yield \json_encode($data, \JSON_THROW_ON_ERROR, 512);
6+
} catch (\JsonException $e) {
7+
throw new \Symfony\Component\JsonStreamer\Exception\NotEncodableValueException($e->getMessage(), 0, $e);
8+
}
59
};

‎src/Symfony/Component/JsonStreamer/Tests/Fixtures/stream_writer/dict.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

33
return static function (mixed $data, \Psr\Container\ContainerInterface $valueTransformers, array $options): \Traversable {
4-
yield \json_encode($data);
4+
try {
5+
yield \json_encode($data, \JSON_THROW_ON_ERROR, 512);
6+
} catch (\JsonException $e) {
7+
throw new \Symfony\Component\JsonStreamer\Exception\NotEncodableValueException($e->getMessage(), 0, $e);
8+
}
59
};

‎src/Symfony/Component/JsonStreamer/Tests/Fixtures/stream_writer/iterable.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

33
return static function (mixed $data, \Psr\Container\ContainerInterface $valueTransformers, array $options): \Traversable {
4-
yield \json_encode($data);
4+
try {
5+
yield \json_encode($data, \JSON_THROW_ON_ERROR, 512);
6+
} catch (\JsonException $e) {
7+
throw new \Symfony\Component\JsonStreamer\Exception\NotEncodableValueException($e->getMessage(), 0, $e);
8+
}
59
};

‎src/Symfony/Component/JsonStreamer/Tests/Fixtures/stream_writer/list.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
<?php
22

33
return static function (mixed $data, \Psr\Container\ContainerInterface $valueTransformers, array $options): \Traversable {
4-
yield \json_encode($data);
4+
try {
5+
yield \json_encode($data, \JSON_THROW_ON_ERROR, 512);
6+
} catch (\JsonException $e) {
7+
throw new \Symfony\Component\JsonStreamer\Exception\NotEncodableValueException($e->getMessage(), 0, $e);
8+
}
59
};

0 commit comments

Comments
 (0)
0