Fix denormalizing empty string into object|null parameter

symfony · Nov 23, 2023 · 1a725d3 · 1a725d3
1 parent dad8af3
commit 1a725d3
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 43 deletions.
diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
@@ -334,6 +334,8 @@ 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 = []);
 
@@ -457,13 +459,12 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
         $isUnionType = \count($types) > 1;
         $extraAttributesException = null;
         $missingConstructorArgumentException = null;
+        $isNullable = false;
         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
@@ -488,9 +489,12 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
                             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) {
@@ -594,19 +598,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 (!$isUnionTypeOrNullable) {
+                if (!$isUnionType && !$isNullable) {
                     throw $e;
                 }
 
                 if (!$extraAttributesException) {
                     $extraAttributesException = $e;
                 }
             } catch (MissingConstructorArgumentsException $e) {
-                if (!$isUnionTypeOrNullable) {
+                if (!$isUnionType && !$isNullable) {
                     throw $e;
                 }
 
@@ -616,7 +620,7 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
             }
         }
 
-        if ('' === $data && $isUnionTypeOrNullable && !$hasNonObjectType && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) {
+        if ($isNullable) {
             return null;
         }
 

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/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
@@ -24,7 +24,7 @@ public function __construct()
     {
     }
 
-    public function denormalize(DenormalizerInterface $denormalizer, float|int|bool|array|string $data, string $format = null, array $context = [])
+    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
@@ -24,7 +24,6 @@
 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;
@@ -64,8 +63,6 @@
 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\NormalizableTraversableDummy;
@@ -823,44 +820,17 @@ public function testFalseBuiltInTypes()
         $this->assertEquals(new FalseBuiltInDummy(), $actual);
     }
 
+    /**
+     * @requires PHP 8
+     */
     public function testDeserializeUntypedFormat()
     {
-        $serializer = new Serializer([new ObjectNormalizer(propertyTypeExtractor: new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]))], ['csv' => new CsvEncoder()]);
+        $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);
     }
 
-    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 AnnotationLoader(new AnnotationReader()));