Fix denormalizing empty string into object|null parameter

symfony · Oct 23, 2023 · 1c5b764 · 1c5b764
1 parent eee7e8f
commit 1c5b764
Show file tree

Hide file tree

Showing 6 changed files with 159 additions and 10 deletions.
diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
@@ -290,8 +290,6 @@ abstract protected function extractAttributes(object $object, string $format = n
 
     /**
      * Gets the attribute value.
-     *
-     * @return mixed
      */
     abstract protected function getAttributeValue(object $object, string $attribute, string $format = null, array $context = []);
 
@@ -305,9 +303,6 @@ public function supportsDenormalization(mixed $data, string $type, string $forma
         return class_exists($type) || (interface_exists($type, false) && null !== $this->classDiscriminatorResolver?->getMappingForClass($type));
     }
 
-    /**
-     * @return mixed
-     */
     public function denormalize(mixed $data, string $type, string $format = null, array $context = [])
     {
         if (!isset($context['cache_key'])) {
@@ -434,11 +429,16 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
         $isUnionType = \count($types) > 1;
         $extraAttributesException = null;
         $missingConstructorArgumentsException = null;
+        $hasNonObjectType = false;
+        $isUnionTypeOrNullable = $isUnionType;
+
         foreach ($types as $type) {
             if (null === $data && $type->isNullable()) {
                 return null;
             }
 
+            $isUnionTypeOrNullable = $isUnionTypeOrNullable ?: $type->isNullable();
+            $hasNonObjectType = $hasNonObjectType ?: Type::BUILTIN_TYPE_OBJECT !== $type->getBuiltinType();
             $collectionValueType = $type->isCollection() ? $type->getCollectionValueTypes()[0] ?? null : null;
 
             // Fix a collection that contains the only one element
@@ -456,9 +456,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
                 // In XML and CSV all basic datatypes are represented as strings, it is e.g. not possible to determine,
                 // if a value is meant to be a string, float, int or a boolean value from the serialized representation.
                 // That's why we have to transform the values, if one of these non-string basic datatypes is expected.
+                $builtinType = $type->getBuiltinType();
                 if (\is_string($data) && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) {
                     if ('' === $data) {
-                        if (Type::BUILTIN_TYPE_ARRAY === $builtinType = $type->getBuiltinType()) {
+                        if (Type::BUILTIN_TYPE_ARRAY === $builtinType) {
                             return [];
                         }
 
@@ -467,7 +468,7 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
                         }
                     }
 
-                    switch ($builtinType ?? $type->getBuiltinType()) {
+                    switch ($builtinType) {
                         case Type::BUILTIN_TYPE_BOOL:
                             // according to https://www.w3.org/TR/xmlschema-2/#boolean, valid representations are "false", "true", "0" and "1"
                             if ('false' === $data || '0' === $data) {
@@ -564,24 +565,28 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
                     return $data;
                 }
             } catch (NotNormalizableValueException|InvalidArgumentException $e) {
-                if (!$isUnionType) {
+                if (!$isUnionTypeOrNullable) {
                     throw $e;
                 }
             } catch (ExtraAttributesException $e) {
-                if (!$isUnionType) {
+                if (!$isUnionTypeOrNullable) {
                     throw $e;
                 }
 
                 $extraAttributesException ??= $e;
             } catch (MissingConstructorArgumentsException $e) {
-                if (!$isUnionType) {
+                if (!$isUnionTypeOrNullable) {
                     throw $e;
                 }
 
                 $missingConstructorArgumentsException ??= $e;
             }
         }
 
+        if ('' === $data && $isUnionTypeOrNullable && !$hasNonObjectType && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) {
+            return null;
+        }
+
         if ($extraAttributesException) {
             throw $extraAttributesException;
         }
@@ -590,6 +595,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
             throw $missingConstructorArgumentsException;
         }
 
+        if (!$isUnionType && isset($e)) {
+            throw $e;
+        }
+
         if ($context[self::DISABLE_TYPE_ENFORCEMENT] ?? $this->defaultContext[self::DISABLE_TYPE_ENFORCEMENT] ?? false) {
             return $data;
         }

diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithNotNormalizable.php b/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithNotNormalizable.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Serializer\Tests\Fixtures;
+
+/**
+ * @author Jeroen <github.com/Jeroeny>
+ */
+class DummyWithNotNormalizable
+{
+    public function __construct(public NotNormalizableDummy|null $value)
+    {
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithObjectOrBool.php b/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithObjectOrBool.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Serializer\Tests\Fixtures;
+
+/**
+ * @author Jeroen <github.com/Jeroeny>
+ */
+class DummyWithObjectOrBool
+{
+    public function __construct(public Php80WithPromotedTypedConstructor|bool $value)
+    {
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithObjectOrNull.php b/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithObjectOrNull.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Serializer\Tests\Fixtures;
+
+/**
+ * @author Jeroen <github.com/Jeroeny>
+ */
+class DummyWithObjectOrNull
+{
+    public function __construct(public Php80WithPromotedTypedConstructor|null $value)
+    {
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/NotNormalizableDummy.php b/src/Symfony/Component/Serializer/Tests/Fixtures/NotNormalizableDummy.php
@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Serializer\Tests\Fixtures;
+
+use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
+use Symfony\Component\Serializer\Normalizer\DenormalizableInterface;
+use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
+
+/**
+ * @author Jeroen <github.com/Jeroeny>
+ */
+class NotNormalizableDummy implements DenormalizableInterface
+{
+    public function __construct()
+    {
+    }
+
+    public function denormalize(DenormalizerInterface $denormalizer, float|int|bool|array|string $data, string $format = null, array $context = [])
+    {
+        throw new NotNormalizableValueException();
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.php
@@ -16,10 +16,12 @@
 use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
 use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
 use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
+use Symfony\Component\Serializer\Encoder\CsvEncoder;
 use Symfony\Component\Serializer\Encoder\JsonEncoder;
 use Symfony\Component\Serializer\Exception\ExtraAttributesException;
 use Symfony\Component\Serializer\Exception\InvalidArgumentException;
 use Symfony\Component\Serializer\Exception\LogicException;
+use Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException;
 use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
 use Symfony\Component\Serializer\Exception\PartialDenormalizationException;
 use Symfony\Component\Serializer\Exception\UnexpectedValueException;
@@ -57,6 +59,9 @@
 use Symfony\Component\Serializer\Tests\Fixtures\DummyMessageNumberTwo;
 use Symfony\Component\Serializer\Tests\Fixtures\DummyObjectWithEnumConstructor;
 use Symfony\Component\Serializer\Tests\Fixtures\DummyObjectWithEnumProperty;
+use Symfony\Component\Serializer\Tests\Fixtures\DummyWithNotNormalizable;
+use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrBool;
+use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrNull;
 use Symfony\Component\Serializer\Tests\Fixtures\FalseBuiltInDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\FooImplementationDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\FooInterfaceDummyDenormalizer;
@@ -842,6 +847,44 @@ public function testTrueBuiltInTypes()
         $this->assertEquals(new TrueBuiltInDummy(), $actual);
     }
 
+    public function testDeserializeUntypedFormat()
+    {
+        $serializer = new Serializer([new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))], ['csv' => new CsvEncoder()]);
+        $actual = $serializer->deserialize('value'.\PHP_EOL.',',DummyWithObjectOrNull::class, 'csv', [CsvEncoder::AS_COLLECTION_KEY => false]);
+
+        $this->assertEquals(new DummyWithObjectOrNull(null), $actual);
+    }
+
+    public function testDenormalizeUntypedFormat()
+    {
+        $serializer = new Serializer([new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $actual = $serializer->denormalize(['value' => ''], DummyWithObjectOrNull::class, 'xml');
+
+        $this->assertEquals(new DummyWithObjectOrNull(null), $actual);
+    }
+
+    public function testDenormalizeUntypedFormatNotNormalizable()
+    {
+        $this->expectException(NotNormalizableValueException::class);
+        $serializer = new Serializer([new CustomNormalizer(), new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $serializer->denormalize(['value' => 'test'], DummyWithNotNormalizable::class, 'xml');
+    }
+
+    public function testDenormalizeUntypedFormatMissingArg()
+    {
+        $this->expectException(MissingConstructorArgumentsException::class);
+        $serializer = new Serializer([new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $serializer->denormalize(['value' => 'invalid'], DummyWithObjectOrNull::class, 'xml');
+    }
+
+    public function testDenormalizeUntypedFormatScalar()
+    {
+        $serializer = new Serializer([new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $actual = $serializer->denormalize(['value' => 'false'], DummyWithObjectOrBool::class, 'xml');
+
+        $this->assertEquals(new DummyWithObjectOrBool(false), $actual);
+    }
+
     private function serializerWithClassDiscriminator()
     {
         $classMetadataFactory = new ClassMetadataFactory(new AttributeLoader());