[Serializer] Fix denormalizing empty string into object|null parameter

[Jeroeny]

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
License	MIT

In XML and CSV all basic datatypes are represented as strings

The AbstractObjectNormalizer handles this for bool, int and float types. But if a parameter is typed Object|null, a serialization and then deserialization will fail.

This will throw:

final class Xml
{
    public function __construct(public Uuid|null $element = null)
    {
    }
}

$test = new Xml(null);
$serialized = $this->serializer->serialize($test, XmlEncoder::FORMAT);
$deserialized = $this->serializer->deserialize($serialized, Xml::class, XmlEncoder::FORMAT);

  [Symfony\Component\Serializer\Exception\NotNormalizableValueException]       
  The data is not a valid "Symfony\Component\Uid\Uuid" string representation.  

Reproducer: https://github.com/Jeroeny/reproduce/blob/xmlnull/src/Test.php

[OskarStark]

Looks like a bugfix to me, in this case 🎯 6.3, but let's wait for advice of @nicolas-grekas

[nicolas-grekas]

This would need tests for sure. But I'm no expert in the Serializer component so I'd appreciate if someone more familiar with it could review.

[Jeroeny]

I've added tests. I also discovered that $builtinType isn't reset in the for-loop in some cases. Which means a Object|bool type wouldn't hit this switch-case: https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php#L471 (The added tests also cover this).

[Jeroeny]

Another interesting failure scenario:

// this fails
$this->serializer->denormalize(['element' => '0'], Example::class, context: [AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true]);

class Example {
    public function __construct(public Args|int $element) {}
}

class Args {
    public function __construct(public Uuid $nested, private string $test)
}

//  [Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException]
//  Cannot create an instance of "App\Args" from serialized data because its constructor requires the following parameters to be present : "$nested", "$test".

// whereas this returns an instance succesfully
$this->serializer->denormalize(['element' => '0'], Test::class, context: [AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true]);

class Test {
    public function __construct(public Uuid|int $element)
}

Because the first denormalization throws MissingConstructorArgumentsException, which goes before the return on DISABLE_TYPE_ENFORCEMENT.
The second throws NotNormalizableValueException, but is caught and will return 0 because of DISABLE_TYPE_ENFORCEMENT.

I'm not sure what the reasoning was here or if this is actually not intended.

It could be fixed by adding $hasTypeMismatch = $builtinType !== Type::BUILTIN_TYPE_OBJECT here:
https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php#L565. To establish that it did not throw for at least one of the $types.
And the later using that to return before MissingConstructorArgumentsException. (Or always return before that exception when DISABLE_TYPE_ENFORCEMENT is true?)

[nicolas-grekas]

Let's resume this one? Is this a bugfix as it's documented in the PR description? If yes can you please target the lowest affected branch?

[Jeroeny]

Applied feedback and targeting 6.3

[nicolas-grekas]

Isn't 5.4 affected also?

[Jeroeny]

Eh yes, I was looking into the wrong part when checking that branch. Now based on 5.4.

[nicolas-grekas]

Parse error on PHP 7.2 ;)

[Jeroeny]

Serializer tests are ok now

[nicolas-grekas]

(rebase needed ;) )

[nicolas-grekas]

Thank you @Jeroeny.