Fix denormalizing empty string into object|null parameter

symfony · Nov 24, 2023 · 64eaf96 · 64eaf96
1 parent 48be4b3
commit 64eaf96
Show file tree

Hide file tree

Showing 9 changed files with 245 additions and 7 deletions.
diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
@@ -457,8 +457,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
     {
         $expectedTypes = [];
         $isUnionType = \count($types) > 1;
+        $e = null;
         $extraAttributesException = null;
         $missingConstructorArgumentException = null;
+        $isNullable = false;
         foreach ($types as $type) {
             if (null === $data && $type->isNullable()) {
                 return null;
@@ -481,18 +483,22 @@ 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 [];
                         }
 
-                        if ($type->isNullable() && \in_array($builtinType, [Type::BUILTIN_TYPE_BOOL, Type::BUILTIN_TYPE_INT, Type::BUILTIN_TYPE_FLOAT], true)) {
-                            return null;
+                        if (Type::BUILTIN_TYPE_STRING === $builtinType) {
+                            return '';
                         }
+
+                        // Don't return null yet because Object-types that come first may accept empty-string too
+                        $isNullable = $isNullable ?: $type->isNullable();
                     }
 
-                    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) {
@@ -593,19 +599,19 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
                     return $data;
                 }
             } catch (NotNormalizableValueException $e) {
-                if (!$isUnionType) {
+                if (!$isUnionType && !$isNullable) {
                     throw $e;
                 }
             } catch (ExtraAttributesException $e) {
-                if (!$isUnionType) {
+                if (!$isUnionType && !$isNullable) {
                     throw $e;
                 }
 
                 if (!$extraAttributesException) {
                     $extraAttributesException = $e;
                 }
             } catch (MissingConstructorArgumentsException $e) {
-                if (!$isUnionType) {
+                if (!$isUnionType && !$isNullable) {
                     throw $e;
                 }
 
@@ -615,6 +621,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
             }
         }
 
+        if ($isNullable) {
+            return null;
+        }
+
         if ($extraAttributesException) {
             throw $extraAttributesException;
         }
@@ -623,6 +633,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
             throw $missingConstructorArgumentException;
         }
 
+        if (!$isUnionType && $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/DummyString.php b/src/Symfony/Component/Serializer/Tests/Fixtures/DummyString.php
@@ -0,0 +1,29 @@
+<?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\Normalizer\DenormalizableInterface;
+use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
+
+/**
+ * @author Jeroen <github.com/Jeroeny>
+ */
+class DummyString implements DenormalizableInterface
+{
+    /** @var string $value */
+    public $value;
+
+    public function denormalize(DenormalizerInterface $denormalizer, $data, string $format = null, array $context = [])
+    {
+        $this->value = $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/DummyWithStringObject.php b/src/Symfony/Component/Serializer/Tests/Fixtures/DummyWithStringObject.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 DummyWithStringObject
+{
+    public function __construct(public DummyString|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, $data, string $format = null, array $context = [])
+    {
+        throw new NotNormalizableValueException();
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php
@@ -14,10 +14,13 @@
 use Doctrine\Common\Annotations\AnnotationReader;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
+use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
+use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
 use Symfony\Component\PropertyInfo\Type;
 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\Mapping\ClassDiscriminatorFromClassMetadata;
 use Symfony\Component\Serializer\Mapping\ClassDiscriminatorMapping;
@@ -29,6 +32,7 @@
 use Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader;
 use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
 use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;
+use Symfony\Component\Serializer\Normalizer\CustomNormalizer;
 use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
 use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
 use Symfony\Component\Serializer\Serializer;
@@ -39,6 +43,11 @@
 use Symfony\Component\Serializer\Tests\Fixtures\Annotations\AbstractDummySecondChild;
 use Symfony\Component\Serializer\Tests\Fixtures\DummyFirstChildQuux;
 use Symfony\Component\Serializer\Tests\Fixtures\DummySecondChildQuux;
+use Symfony\Component\Serializer\Tests\Fixtures\DummyString;
+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\DummyWithStringObject;
 
 class AbstractObjectNormalizerTest extends TestCase
 {
@@ -444,6 +453,60 @@ public function testNormalizeEmptyObject()
         $normalizedData = $normalizer->normalize(new EmptyDummy(), 'any', ['preserve_empty_objects' => true]);
         $this->assertEquals(new \ArrayObject(), $normalizedData);
     }
+
+    /**
+     * @requires PHP 8
+     */
+    public function testDenormalizeUntypedFormat()
+    {
+        $serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $actual = $serializer->denormalize(['value' => ''], DummyWithObjectOrNull::class, 'xml');
+
+        $this->assertEquals(new DummyWithObjectOrNull(null), $actual);
+    }
+
+    /**
+     * @requires PHP 8
+     */
+    public function testDenormalizeUntypedFormatNotNormalizable()
+    {
+        $this->expectException(NotNormalizableValueException::class);
+        $serializer = new Serializer([new CustomNormalizer(), new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $serializer->denormalize(['value' => 'test'], DummyWithNotNormalizable::class, 'xml');
+    }
+
+    /**
+     * @requires PHP 8
+     */
+    public function testDenormalizeUntypedFormatMissingArg()
+    {
+        $this->expectException(MissingConstructorArgumentsException::class);
+        $serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $serializer->denormalize(['value' => 'invalid'], DummyWithObjectOrNull::class, 'xml');
+    }
+
+    /**
+     * @requires PHP 8
+     */
+    public function testDenormalizeUntypedFormatScalar()
+    {
+        $serializer = new Serializer([new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $actual = $serializer->denormalize(['value' => 'false'], DummyWithObjectOrBool::class, 'xml');
+
+        $this->assertEquals(new DummyWithObjectOrBool(false), $actual);
+    }
+
+    /**
+     * @requires PHP 8
+     */
+    public function testDenormalizeUntypedStringObject()
+    {
+        $serializer = new Serializer([new CustomNormalizer(), new ObjectNormalizer(null, null, null, new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))]);
+        $actual = $serializer->denormalize(['value' => ''], DummyWithStringObject::class, 'xml');
+
+        $this->assertEquals(new DummyWithStringObject(new DummyString()), $actual);
+        $this->assertEquals('', $actual->value->value);
+    }
 }
 
 class AbstractObjectNormalizerDummy extends AbstractObjectNormalizer

diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.php
@@ -17,6 +17,7 @@
 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\DecoderInterface;
 use Symfony\Component\Serializer\Encoder\EncoderInterface;
 use Symfony\Component\Serializer\Encoder\JsonEncoder;
@@ -62,6 +63,7 @@
 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\DummyWithObjectOrNull;
 use Symfony\Component\Serializer\Tests\Fixtures\FalseBuiltInDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\NormalizableTraversableDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\Php74Full;
@@ -818,6 +820,17 @@ public function testFalseBuiltInTypes()
         $this->assertEquals(new FalseBuiltInDummy(), $actual);
     }
 
+    /**
+     * @requires PHP 8
+     */
+    public function testDeserializeUntypedFormat()
+    {
+        $serializer = new Serializer([new ObjectNormalizer(null, null, null, 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);
+    }
+
     private function serializerWithClassDiscriminator()
     {
         $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));