Fix denormalizing empty string into object|null parameter

symfony · Nov 21, 2023 · 85ca4e0 · 85ca4e0
1 parent 5a062e9
commit 85ca4e0
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 50 deletions.
diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
@@ -290,6 +290,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 = []);
 
@@ -303,6 +305,9 @@ 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'])) {
@@ -429,16 +434,11 @@ 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
@@ -463,7 +463,11 @@ 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)) {
+                        if (Type::BUILTIN_TYPE_STRING === $builtinType) {
+                            return '';
+                        }
+
+                        if ($type->isNullable()) {
                             return null;
                         }
                     }
@@ -565,28 +569,24 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
                     return $data;
                 }
             } catch (NotNormalizableValueException|InvalidArgumentException $e) {
-                if (!$isUnionTypeOrNullable) {
+                if (!$isUnionType) {
                     throw $e;
                 }
             } catch (ExtraAttributesException $e) {
-                if (!$isUnionTypeOrNullable) {
+                if (!$isUnionType) {
                     throw $e;
                 }
 
                 $extraAttributesException ??= $e;
             } catch (MissingConstructorArgumentsException $e) {
-                if (!$isUnionTypeOrNullable) {
+                if (!$isUnionType) {
                     throw $e;
                 }
 
                 $missingConstructorArgumentsException ??= $e;
             }
         }
 
-        if ('' === $data && $isUnionTypeOrNullable && !$hasNonObjectType && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) {
-            return null;
-        }
-
         if ($extraAttributesException) {
             throw $extraAttributesException;
         }
@@ -595,10 +595,6 @@ 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/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php
@@ -24,6 +24,7 @@
 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;
@@ -38,6 +39,7 @@
 use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
 use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;
 use Symfony\Component\Serializer\Normalizer\BackedEnumNormalizer;
+use Symfony\Component\Serializer\Normalizer\CustomNormalizer;
 use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;
 use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
 use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
@@ -49,6 +51,9 @@
 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\DummyWithNotNormalizable;
+use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrBool;
+use Symfony\Component\Serializer\Tests\Fixtures\DummyWithObjectOrNull;
 use Symfony\Component\Serializer\Tests\Normalizer\Features\ObjectDummyWithContextAttribute;
 
 class AbstractObjectNormalizerTest extends TestCase
@@ -835,6 +840,36 @@ public function testDenormalizeWithCorrectOrderOfAttributeAndProperty()
         $test = $normalizer->denormalize($data, $obj::class);
         $this->assertSame('nested-id', $test->id);
     }
+
+    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);
+    }
 }
 
 class AbstractObjectNormalizerDummy extends AbstractObjectNormalizer

diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.php
@@ -22,7 +22,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;
@@ -60,8 +59,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\FooImplementationDummy;
@@ -856,36 +853,6 @@ public function testDeserializeUntypedFormat()
         $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()));