[Serializer] PHP Warning - undefined array key in AbstractObjectNormalizer::isMaxDepthReached #54378

jaydiablo · 2024-03-22T18:05:11Z

Symfony version(s) affected

6.4.5

Description

A bundle we use (rompetompe/inertia-bundle) calls the Symfony serializer on any data that's passed to it (that will be converted to JSON for use by Inertia.js), after upgrading to Symfony 6 we're seeing PHP warnings in some situations. It seems to only happen when there is a stdClass object in the data that's passed to the serializer (from a json_decode call somewhere else in the app).

The line that causes the warning is this one:

if (!($enableMaxDepth = $context[self::ENABLE_MAX_DEPTH] ?? $this->defaultContext[self::ENABLE_MAX_DEPTH] ?? false)
            || null === $maxDepth = $attributesMetadata[$attribute]?->getMaxDepth()
        ) {
            return false;
        }

It happens because $attributesMetadata is an empty array in this context and $attribute is a string (which is one of the properties of the stdClass object it's trying to normalize). Earlier in the normalizer there is this chunk of code:

$class = ($this->objectClassResolver)($object);
$classMetadata = $this->classMetadataFactory?->getMetadataFor($class);
$attributesMetadata = $this->classMetadataFactory?->getMetadataFor($class)->getAttributesMetadata();

I mention this, because there is a check a little bit below this to see if $attributesMetadata is null and then it bypasses the isMaxDepthReached call:

foreach ($attributes as $attribute) {
            $maxDepthReached = false;
            if (null !== $attributesMetadata && ($maxDepthReached = $this->isMaxDepthReached($attributesMetadata, $class, $attribute, $context)) && !$maxDepthHandler) {
                continue;
            }

In the application $attributesMetadata is an empty array, but in an isolated test case that I've tried to put together, $attributesMetadata ends up being null so I can't reproduce the warning in an isolated test case.

Perhaps there's something else to this (why does stdClass have attributes?) in the context of the app, but it does seem like there should be a check if the $attribute is set on $attributesMetadata in the isMaxDepthReached method to avoid this warning.

Aside from the Warning being thrown, the serialization/normalization works as expected.

How to reproduce

I put this test case together to try to reproduce in the simplest way possible but it doesn't throw the warning as mentioned above:

<?php

ini_set('display_errors', 1);
ini_set('error_reporting', E_ALL);

use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;

include_once 'vendor/autoload.php';

$object = new stdClass();
$object->string = 'yes';

$serializer = new Symfony\Component\Serializer\Serializer([
    new Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer(),
    new Symfony\Component\Serializer\Normalizer\ProblemNormalizer(),
    new Symfony\Component\Serializer\Normalizer\UidNormalizer(),
    new Symfony\Component\Serializer\Normalizer\DateTimeNormalizer(),
    new Symfony\Component\Serializer\Normalizer\DateTimeZoneNormalizer(),
    new Symfony\Component\Serializer\Normalizer\DateIntervalNormalizer(),
    new Symfony\Component\Serializer\Normalizer\FormErrorNormalizer(),
    new Symfony\Component\Serializer\Normalizer\BackedEnumNormalizer(),
    new Symfony\Component\Serializer\Normalizer\DataUriNormalizer(),
    new Symfony\Component\Serializer\Normalizer\JsonSerializableNormalizer(),
    new Symfony\Component\Serializer\Normalizer\ArrayDenormalizer(),
    new Symfony\Component\Serializer\Normalizer\ObjectNormalizer(),
], [
    new Symfony\Component\Serializer\Encoder\XmlEncoder(),
    new Symfony\Component\Serializer\Encoder\JsonEncoder(),
    new Symfony\Component\Serializer\Encoder\YamlEncoder(),
    new Symfony\Component\Serializer\Encoder\CsvEncoder(),
]);

$page = [
    'props' => [
        'string' => 'yes',
        'array' => ['string' => 'yes'],
        'object' => $object,
        'nested_object' => [
            'string' => 'yes',
            'array' => ['string' => 'yes'],
            'object' => $object,
        ],
    ]
];

$context = [];

$json = $serializer->serialize($page, 'json', array_merge([
    'json_encode_options' => JsonResponse::DEFAULT_ENCODING_OPTIONS,
    AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER => function () { return null; },
    AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true,
    AbstractObjectNormalizer::ENABLE_MAX_DEPTH => true,
], $context));

var_dump($json);

I don't know how to make this test case set attributes on the stdClass class like it's doing in the app, but I've tried to replicate how the serializer is configured as best as possible (as far as I can tell, the inertia-bundle just uses @serializer for the serializer dependency, which we're not modifying at all, so should be the Symfony default Serializer config).

Possible Solution

This could probably be fixed in a couple of ways, first, if $attributesMetadata is an empty array, it seems like it should probably be treated the same as if it were null.

In the normalize method of AbstractObjectNormalizer this check:

foreach ($attributes as $attribute) {
            $maxDepthReached = false;
            if (null !== $attributesMetadata && ($maxDepthReached = $this->isMaxDepthReached($attributesMetadata, $class, $attribute, $context)) && !$maxDepthHandler) {
                continue;
            }

could be changed to also check the count of the metadata variable so that the isMaxDepthReached method isn't called with an empty array:

foreach ($attributes as $attribute) {
            $maxDepthReached = false;
            if (null !== $attributesMetadata && count($attributesMetadata) > 0 && ($maxDepthReached = $this->isMaxDepthReached($attributesMetadata, $class, $attribute, $context)) && !$maxDepthHandler) {
                continue;
            }

This does fix it in our application.

Alternatively, the isMaxDepthReached method could be modified in a similar way to check if the $attribute is a key on the passed in $attributesMetadata.

Additional Context

No response

The text was updated successfully, but these errors were encountered:

jaydiablo · 2024-03-22T18:51:56Z

FWIW, here's what $classMetadata looks like in the case where the warning is thrown:

In my test case, this variable is null.

jaydiablo · 2024-03-22T19:26:57Z

Ah, here we go, this test case causes the warning:

<?php

ini_set('display_errors', 1);
ini_set('error_reporting', E_ALL);

use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;

include_once 'vendor/autoload.php';

$object = new stdClass();
$object->string = 'yes';
$object->test = 'true';

$loader = new Symfony\Component\Serializer\Mapping\Loader\LoaderChain([
    new Symfony\Component\Serializer\Mapping\Loader\AttributeLoader(),
]);
$classMetadataFactory = new Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory($loader);

$serializer = new Symfony\Component\Serializer\Serializer([
    new Symfony\Component\Serializer\Normalizer\ObjectNormalizer($classMetadataFactory),
], [
    new Symfony\Component\Serializer\Encoder\JsonEncoder(),
]);

$page = [
    'props' => [
        'string' => 'yes',
        'array' => ['string' => 'yes'],
        'nested_object' => [
            'string' => 'yes',
            'array' => ['string' => 'yes'],
            'object' => $object,
        ],
    ]
];

$context = [];

$json = $serializer->serialize($page, 'json', array_merge([
    'json_encode_options' => JsonResponse::DEFAULT_ENCODING_OPTIONS,
    AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER => function () { return null; },
    AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true,
    AbstractObjectNormalizer::ENABLE_MAX_DEPTH => true,
], $context));

var_dump($json);

Previous attempt didn't have a class metadata factory injected.

chalasr · 2024-03-23T16:51:51Z

Would you mind sending a PR with test ? It should target the lowest active branch where the bug exists (5.4 or 6.4)

- if (null !== $attributesMetadata && count($attributesMetadata) > 0 && ($maxDepthReached = $this->isMaxDepthReached($attributesMetadata, $class, $attribute, $context)) && !$maxDepthHandler) {
+ if (!$attributesMetadata && ($maxDepthReached = $this->isMaxDepthReached($attributesMetadata, $class, $attribute, $context)) && !$maxDepthHandler) {
    continue;
}

jaydiablo · 2024-03-25T17:34:01Z

@chalasr sure, PR coming shortly. It doesn't seem to be an issue in Symfony 5.4. I tried the similar test case there (5.4 doesn't have the AttributeLoader, so I changed that to the AnnotationLoader) and the warning is not thrown.

…th MaxDepth enabled (jaydiablo) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Serializer] Fixing PHP warning in the ObjectNormalizer with MaxDepth enabled | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #54378 | License | MIT This fixes a specific issue I ran into when testing an application in Symfony 6 that uses the rompetompe/inertia-bundle to serialize PHP data to JSON for use by Inertia.js. Added a test that is as minimal as necessary for the Warning to be thrown. I tested against Symfony 5.4 as well, but the issue doesn't seem to be present there (could have something to do with 5.4 not having the AttributeLoader). Commits ------- 31e3bde [Serializer] Fixing PHP warning in the ObjectNormalizer with MaxDepth enabled

jaydiablo added the Bug label Mar 22, 2024

carsonbot added Status: Needs Review Serializer labels Mar 22, 2024

jaydiablo mentioned this issue Mar 25, 2024

[Serializer] Fixing PHP warning in the ObjectNormalizer with MaxDepth enabled #54395

Merged

nicolas-grekas closed this as completed Apr 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Serializer] PHP Warning - undefined array key in AbstractObjectNormalizer::isMaxDepthReached #54378

[Serializer] PHP Warning - undefined array key in AbstractObjectNormalizer::isMaxDepthReached #54378

jaydiablo commented Mar 22, 2024

jaydiablo commented Mar 22, 2024

Uh oh!

jaydiablo commented Mar 22, 2024 •

edited

Loading

Uh oh!

chalasr commented Mar 23, 2024

Uh oh!

jaydiablo commented Mar 25, 2024

Uh oh!

Uh oh!

[Serializer] PHP Warning - undefined array key in AbstractObjectNormalizer::isMaxDepthReached #54378

[Serializer] PHP Warning - undefined array key in AbstractObjectNormalizer::isMaxDepthReached #54378

Comments

jaydiablo commented Mar 22, 2024

Symfony version(s) affected

Description

How to reproduce

Possible Solution

Additional Context

jaydiablo commented Mar 22, 2024

Uh oh!

jaydiablo commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chalasr commented Mar 23, 2024

Uh oh!

jaydiablo commented Mar 25, 2024

Uh oh!

jaydiablo commented Mar 22, 2024 •

edited

Loading