Skip to content

[Serializer] Fix denormalizing empty string into object|null parameter #52172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,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;
Expand All @@ -491,18 +493,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) {
Expand Down Expand Up @@ -603,19 +609,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;
}

Expand All @@ -625,6 +631,10 @@ private function validateAndDenormalize(array $types, string $currentClass, stri
}
}

if ($isNullable) {
return null;
}

if ($extraAttributesException) {
throw $extraAttributesException;
}
Expand All @@ -633,6 +643,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;
}
Expand Down
29 changes: 29 additions & 0 deletions src/Symfony/Component/Serializer/Tests/Fixtures/DummyString.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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)
{
}
}
Original file line number Diff line number Diff line change
@@ -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)
{
}
}
Original file line number Diff line number Diff line change
@@ -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)
{
}
}
Original file line number Diff line number Diff line change
@@ -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)
{
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@
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\Annotation\Ignore;
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;
Expand All @@ -30,6 +33,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;
Expand All @@ -40,6 +44,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
{
Expand Down Expand Up @@ -453,6 +462,60 @@ public function testNormalizeWithIgnoreAnnotationAndPrivateProperties()

$this->assertSame(['foo' => 'foo'], $serializer->normalize(new ObjectDummyWithIgnoreAnnotationAndPrivateProperty()));
}

/**
* @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
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Serializer/Tests/SerializerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down