Commit f7aae0b
committed
bug symfony#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 errorsFile tree
42 files changed
+625
-261
lines changed- src/Symfony/Component/JsonStreamer
- DataModel
- Read
- Write
- Exception
- Read
- Tests
- DataModel/Write
- Fixtures/stream_writer
- Write
- Write
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 changedLines changed: 10 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
36 | 46 | | |
37 | 47 | | |
38 | 48 | | |
| |||
Lines changed: 10 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
32 | 42 | | |
33 | 43 | | |
34 | 44 | | |
| |||
Lines changed: 4 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
37 | | - | |
| 37 | + | |
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
60 | | - | |
| 60 | + | |
61 | 61 | | |
62 | | - | |
| 62 | + | |
63 | 63 | | |
64 | 64 | | |
Lines changed: 10 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
34 | 44 | | |
35 | 45 | | |
36 | 46 | | |
| |||
Lines changed: 10 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
33 | 43 | | |
34 | 44 | | |
35 | 45 | | |
| |||
Lines changed: 10 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
63 | 73 | | |
64 | 74 | | |
65 | 75 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
26 | 28 | | |
27 | 29 | | |
28 | 30 | | |
| 31 | + | |
| 32 | + | |
29 | 33 | | |
Lines changed: 0 additions & 49 deletions
This file was deleted.
Lines changed: 34 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
15 | 17 | | |
16 | 18 | | |
17 | 19 | | |
| |||
30 | 32 | | |
31 | 33 | | |
32 | 34 | | |
| 35 | + | |
33 | 36 | | |
34 | 37 | | |
35 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
36 | 65 | | |
37 | 66 | | |
38 | 67 | | |
| |||
50 | 79 | | |
51 | 80 | | |
52 | 81 | | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
53 | 87 | | |
Lines changed: 10 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
34 | 44 | | |
35 | 45 | | |
36 | 46 | | |
| |||
0 commit comments