Skip to content

Commit 1bc8d26

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#52172 [Serializer] Fix denormalizing empty string into object|null parameter (Jeroeny)
This PR was merged into the 5.4 branch. Discussion ---------- [Serializer] Fix denormalizing empty string into `object|null` parameter | 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: ```php 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 Commits ------- 62f2203 Fix denormalizing empty string into object|null parameter
2 parents 7b40a95 + 62f2203 commit 1bc8d26

File tree

9 files changed

+245
-7
lines changed

9 files changed

+245
-7
lines changed

‎src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
467467
{
468468
$expectedTypes = [];
469469
$isUnionType = \count($types) > 1;
470+
$e = null;
470471
$extraAttributesException = null;
471472
$missingConstructorArgumentException = null;
473+
$isNullable = false;
472474
foreach ($types as $type) {
473475
if (null === $data && $type->isNullable()) {
474476
return null;
@@ -491,18 +493,22 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
491493
// In XML and CSV all basic datatypes are represented as strings, it is e.g. not possible to determine,
492494
// if a value is meant to be a string, float, int or a boolean value from the serialized representation.
493495
// That's why we have to transform the values, if one of these non-string basic datatypes is expected.
496+
$builtinType = $type->getBuiltinType();
494497
if (\is_string($data) && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) {
495498
if ('' === $data) {
496-
if (Type::BUILTIN_TYPE_ARRAY === $builtinType = $type->getBuiltinType()) {
499+
if (Type::BUILTIN_TYPE_ARRAY === $builtinType) {
497500
return [];
498501
}
499502

500-
if ($type->isNullable() && \in_array($builtinType, [Type::BUILTIN_TYPE_BOOL, Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT], true)) {
501-
return null;
503+
if (Type::BUILTIN_TYPE_STRING === $builtinType) {
504+
return '';
502505
}
506+
507+
// Don't return null yet because Object-types that come first may accept empty-string too
508+
$isNullable = $isNullable ?: $type->isNullable();
503509
}
504510

505-
switch ($builtinType ?? $type->getBuiltinType()) {
511+
switch ($builtinType) {
506512
case Type::BUILTIN_TYPE_BOOL:
507513
// according to https://www.w3.org/TR/xmlschema-2/#boolean, valid representations are "false", "true", "0" and "1"
508514
if ('false' === $data || '0' === $data) {
@@ -603,19 +609,19 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
603609
return $data;
604610
}
605611
} catch (NotNormalizableValueException $e) {
606-
if (!$isUnionType) {
612+
if (!$isUnionType && !$isNullable) {
607613
throw $e;
608614
}
609615
} catch (ExtraAttributesException $e) {
610-
if (!$isUnionType) {
616+
if (!$isUnionType && !$isNullable) {
611617
throw $e;
612618
}
613619

614620
if (!$extraAttributesException) {
615621
$extraAttributesException = $e;
616622
}
617623
} catch (MissingConstructorArgumentsException $e) {
618-
if (!$isUnionType) {
624+
if (!$isUnionType && !$isNullable) {
619625
throw $e;
620626
}
621627

@@ -625,6 +631,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
625631
}
626632
}
627633

634+
if ($isNullable) {
635+
return null;
636+
}
637+
628638
if ($extraAttributesException) {
629639
throw $extraAttributesException;
630640
}
@@ -633,6 +643,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
633643
throw $missingConstructorArgumentException;
634644
}
635645

646+
if (!$isUnionType && $e) {
647+
throw $e;
648+
}
649+
636650
if ($context[self::DISABLE_TYPE_ENFORCEMENT] ?? $this->defaultContext[self::DISABLE_TYPE_ENFORCEMENT] ?? false) {
637651
return $data;
638652
}

‎src/Symfony/Component/Serializer/Tests/Fixtures/DummyString.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Serializer\Tests\Fixtures;
13+
14+
use Symfony\Component\Serializer\Normalizer\DenormalizableInterface;
15+
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
16+
17+
/**
18+
* @author Jeroen <github.com/Jeroeny>
19+
*/
20+
class DummyString implements DenormalizableInterface
21+
{
22+
/** @var string $value */
23+
public $value;
24+
25+
public function denormalize(DenormalizerInterface $denormalizer, $data, string $format = null, array $context = [])
26+
{
27+
$this->value = $data;
28+
}
29+
}

‎src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithNotNormalizable.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Serializer\Tests\Fixtures;
13+
14+
/**
15+
* @author Jeroen <github.com/Jeroeny>
16+
*/
17+
class DummyWithNotNormalizable
18+
{
19+
public function __construct(public NotNormalizableDummy|null $value)
20+
{
21+
}
22+
}

‎src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithObjectOrBool.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Serializer\Tests\Fixtures;
13+
14+
/**
15+
* @author Jeroen <github.com/Jeroeny>
16+
*/
17+
class DummyWithObjectOrBool
18+
{
19+
public function __construct(public Php80WithPromotedTypedConstructor|bool $value)
20+
{
21+
}
22+
}

‎src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithObjectOrNull.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Serializer\Tests\Fixtures;
13+
14+
/**
15+
* @author Jeroen <github.com/Jeroeny>
16+
*/
17+
class DummyWithObjectOrNull
18+
{
19+
public function __construct(public Php80WithPromotedTypedConstructor|null $value)
20+
{
21+
}
22+
}

‎src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithStringObject.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Serializer\Tests\Fixtures;
13+
14+
/**
15+
* @author Jeroen <github.com/Jeroeny>
16+
*/
17+
class DummyWithStringObject
18+
{
19+
public function __construct(public DummyString|null $value)
20+
{
21+
}
22+
}

‎src/Symfony/Component/Serializer/Tests/Fixtures/NotNormalizableDummy.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
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\Serializer\Tests\Fixtures;
13+
14+
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
15+
use Symfony\Component\Serializer\Normalizer\DenormalizableInterface;
16+
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
17+
18+
/**
19+
* @author Jeroen <github.com/Jeroeny>
20+
*/
21+
class NotNormalizableDummy implements DenormalizableInterface
22+
{
23+
public function __construct()
24+
{
25+
}
26+
27+
public function denormalize(DenormalizerInterface $denormalizer, $data, string $format = null, array $context = [])
28+
{
29+
throw new NotNormalizableValueException();
30+
}
31+
}

‎src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414
use Doctrine\Common\Annotations\AnnotationReader;
1515
use PHPUnit\Framework\TestCase;
1616
use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
17+
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
18+
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
1719
use Symfony\Component\PropertyInfo\Type;
1820
use Symfony\Component\Serializer\Annotation\Ignore;
1921
use Symfony\Component\Serializer\Exception\ExtraAttributesException;
2022
use Symfony\Component\Serializer\Exception\InvalidArgumentException;
2123
use Symfony\Component\Serializer\Exception\LogicException;
24+
use Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException;
2225
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
2326
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorFromClassMetadata;
2427
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorMapping;
@@ -30,6 +33,7 @@
3033
use Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader;
3134
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
3235
use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;
36+
use Symfony\Component\Serializer\Normalizer\CustomNormalizer;
3337
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
3438
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
3539
use Symfony\Component\Serializer\Serializer;
@@ -40,6 +44,11 @@
4044
use Symfony\Component\Serializer\Tests\Fixtures\Annotations\AbstractDummySecondChild;
4145
use Symfony\Component\Serializer\Tests\Fixtures\DummyFirstChildQuux;
4246
use Symfony\Component\Serializer\Tests\Fixtures\DummySecondChildQuux;
47+
use Symfony\Component\Serializer\Tests\Fixtures\DummyString;
48+
use Symfony\Component\Serializer\Tests\Fixtures\DummyWithNotNormalizable;
49+
use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrBool;
50+
use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrNull;
51+
use Symfony\Component\Serializer\Tests\Fixtures\DummyWithStringObject;
4352

4453
class AbstractObjectNormalizerTest extends TestCase
4554
{
@@ -453,6 +462,60 @@ public function testNormalizeWithIgnoreAnnotationAndPrivateProperties()
453462

454463
$this->assertSame(['foo' => 'foo'], $serializer->normalize(new ObjectDummyWithIgnoreAnnotationAndPrivateProperty()));
455464
}
465+
466+
/**
467+
* @requires PHP 8
468+
*/
469+
public function testDenormalizeUntypedFormat()
470+
{
471+
$serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
472+
$actual = $serializer->denormalize(['value' => ''], DummyWithObjectOrNull::class, 'xml');
473+
474+
$this->assertEquals(new DummyWithObjectOrNull(null), $actual);
475+
}
476+
477+
/**
478+
* @requires PHP 8
479+
*/
480+
public function testDenormalizeUntypedFormatNotNormalizable()
481+
{
482+
$this->expectException(NotNormalizableValueException::class);
483+
$serializer = new Serializer([new CustomNormalizer(), new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
484+
$serializer->denormalize(['value' => 'test'], DummyWithNotNormalizable::class, 'xml');
485+
}
486+
487+
/**
488+
* @requires PHP 8
489+
*/
490+
public function testDenormalizeUntypedFormatMissingArg()
491+
{
492+
$this->expectException(MissingConstructorArgumentsException::class);
493+
$serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
494+
$serializer->denormalize(['value' => 'invalid'], DummyWithObjectOrNull::class, 'xml');
495+
}
496+
497+
/**
498+
* @requires PHP 8
499+
*/
500+
public function testDenormalizeUntypedFormatScalar()
501+
{
502+
$serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
503+
$actual = $serializer->denormalize(['value' => 'false'], DummyWithObjectOrBool::class, 'xml');
504+
505+
$this->assertEquals(new DummyWithObjectOrBool(false), $actual);
506+
}
507+
508+
/**
509+
* @requires PHP 8
510+
*/
511+
public function testDenormalizeUntypedStringObject()
512+
{
513+
$serializer = new Serializer([new CustomNormalizer(), new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
514+
$actual = $serializer->denormalize(['value' => ''], DummyWithStringObject::class, 'xml');
515+
516+
$this->assertEquals(new DummyWithStringObject(new DummyString()), $actual);
517+
$this->assertEquals('', $actual->value->value);
518+
}
456519
}
457520

458521
class AbstractObjectNormalizerDummy extends AbstractObjectNormalizer

‎src/Symfony/Component/Serializer/Tests/SerializerTest.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
1818
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
1919
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
20+
use Symfony\Component\Serializer\Encoder\CsvEncoder;
2021
use Symfony\Component\Serializer\Encoder\DecoderInterface;
2122
use Symfony\Component\Serializer\Encoder\EncoderInterface;
2223
use Symfony\Component\Serializer\Encoder\JsonEncoder;
@@ -62,6 +63,7 @@
6263
use Symfony\Component\Serializer\Tests\Fixtures\DummyMessageNumberTwo;
6364
use Symfony\Component\Serializer\Tests\Fixtures\DummyObjectWithEnumConstructor;
6465
use Symfony\Component\Serializer\Tests\Fixtures\DummyObjectWithEnumProperty;
66+
use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrNull;
6567
use Symfony\Component\Serializer\Tests\Fixtures\FalseBuiltInDummy;
6668
use Symfony\Component\Serializer\Tests\Fixtures\NormalizableTraversableDummy;
6769
use Symfony\Component\Serializer\Tests\Fixtures\Php74Full;
@@ -818,6 +820,17 @@ public function testFalseBuiltInTypes()
818820
$this->assertEquals(new FalseBuiltInDummy(), $actual);
819821
}
820822

823+
/**
824+
* @requires PHP 8
825+
*/
826+
public function testDeserializeUntypedFormat()
827+
{
828+
$serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))], ['csv' => new CsvEncoder()]);
829+
$actual = $serializer->deserialize('value'.\PHP_EOL.',', DummyWithObjectOrNull::class, 'csv', [CsvEncoder::AS_COLLECTION_KEY => false]);
830+
831+
$this->assertEquals(new DummyWithObjectOrNull(null), $actual);
832+
}
833+
821834
private function serializerWithClassDiscriminator()
822835
{
823836
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));

0 commit comments

Comments
 (0)