From ebad77f722a20dbed3abb2b22a0f68c09cf2bd48 Mon Sep 17 00:00:00 2001 From: Korvin Szanto Date: Mon, 28 Apr 2025 14:49:34 -0700 Subject: [PATCH 1/5] Handle invalid mapping type property type --- .../Serializer/Normalizer/AbstractObjectNormalizer.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index 00d2e3b00ef83..d3036da0c6c0d 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -1159,6 +1159,10 @@ private function getMappedClass(array $data, string $class, array $context): str throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('Type property "%s" not found for the abstract object "%s".', $mapping->getTypeProperty(), $class), null, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false); } + if (!is_string($type) && (!is_object($type) || !method_exists($type, '__toString'))) { + throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('The type property "%s" for the abstract object "%s" must be a string.', $mapping->getTypeProperty(), $class), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false); + } + if (null === $mappedClass = $mapping->getClassForType($type)) { throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('The type "%s" is not a valid value.', $type), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), true); } From 6b84342a6aa7d31363a5f09a306b21e04bcb25d7 Mon Sep 17 00:00:00 2001 From: Korvin Szanto Date: Tue, 29 Apr 2025 14:28:35 -0700 Subject: [PATCH 2/5] Add tests for invalid mapped class type --- .../Attributes/AbstractDummyFirstChild.php | 1 + .../AbstractObjectNormalizerTest.php | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/Attributes/AbstractDummyFirstChild.php b/src/Symfony/Component/Serializer/Tests/Fixtures/Attributes/AbstractDummyFirstChild.php index b5cd2faba24ee..cb9cb0abbe347 100644 --- a/src/Symfony/Component/Serializer/Tests/Fixtures/Attributes/AbstractDummyFirstChild.php +++ b/src/Symfony/Component/Serializer/Tests/Fixtures/Attributes/AbstractDummyFirstChild.php @@ -16,6 +16,7 @@ class AbstractDummyFirstChild extends AbstractDummy { public $bar; + public $baz; /** @var DummyFirstChildQuux|null */ public $quux; diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php index 270b65f33ef66..bc9a956e4781d 100644 --- a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php +++ b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Serializer\Tests\Normalizer; use PHPUnit\Framework\TestCase; +use stdClass; use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor; use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor; use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor; @@ -525,6 +526,74 @@ private function getDenormalizerForStringCollection() return $denormalizer; } + /** + * @dataProvider provideInvalidDiscriminatorTypes + */ + public function testDenormalizeWithDiscriminatorMapHandlesInvalidTypeValue(mixed $typeValue, bool $shouldFail) + { + if ($shouldFail) { + $this->expectException(NotNormalizableValueException::class); + $this->expectExceptionMessage( + 'The type property "type" for the abstract object "Symfony\Component\Serializer\Tests\Fixtures\Attributes\AbstractDummy" must be a string.' + ); + } + + $factory = new ClassMetadataFactory(new AttributeLoader()); + + $loaderMock = new class implements ClassMetadataFactoryInterface { + public function getMetadataFor($value): ClassMetadataInterface + { + if (AbstractDummy::class === $value) { + return new ClassMetadata( + AbstractDummy::class, + new ClassDiscriminatorMapping('type', [ + 'first' => AbstractDummyFirstChild::class, + 'second' => AbstractDummySecondChild::class, + ]) + ); + } + + throw new InvalidArgumentException(); + } + + public function hasMetadataFor($value): bool + { + return AbstractDummy::class === $value; + } + }; + + $discriminatorResolver = new ClassDiscriminatorFromClassMetadata($loaderMock); + $normalizer = new AbstractObjectNormalizerDummy($factory, null, new PhpDocExtractor(), $discriminatorResolver); + $serializer = new Serializer([$normalizer]); + $normalizer->setSerializer($serializer); + $normalizedData = $normalizer->denormalize(['foo' => 'foo', 'baz' => 'baz', 'quux' => ['value' => 'quux'], 'type' => $typeValue], AbstractDummy::class); + + if ($shouldFail) { + $this->fail('Expected exception not thrown.'); + } + + $this->assertInstanceOf(DummyFirstChildQuux::class, $normalizedData->quux); + } + + public function provideInvalidDiscriminatorTypes() + { + $toStringObject = new class() { + public function __toString() + { + return 'first'; + } + }; + + return [ + [[], true], + [new StdClass(), true], + [123, true], + [false, true], + ['first', false], + [$toStringObject, false], + ]; + } + public function testDenormalizeWithDiscriminatorMapUsesCorrectClassname() { $factory = new ClassMetadataFactory(new AttributeLoader()); From b866a9af8a2769185ed8ccfe5a42b9656df561a4 Mon Sep 17 00:00:00 2001 From: Korvin Szanto Date: Sat, 28 Jun 2025 08:20:16 -0700 Subject: [PATCH 3/5] Update test after feedback --- .../Normalizer/AbstractObjectNormalizer.php | 2 +- .../AbstractObjectNormalizerTest.php | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index d3036da0c6c0d..982a8815f21d2 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -1160,7 +1160,7 @@ private function getMappedClass(array $data, string $class, array $context): str } if (!is_string($type) && (!is_object($type) || !method_exists($type, '__toString'))) { - throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('The type property "%s" for the abstract object "%s" must be a string.', $mapping->getTypeProperty(), $class), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false); + throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('The type property "%s" for the abstract object "%s" must be a string or a stringable object.', $mapping->getTypeProperty(), $class), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false); } if (null === $mappedClass = $mapping->getClassForType($type)) { diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php index bc9a956e4781d..9e61e47d7c2eb 100644 --- a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php +++ b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Serializer\Tests\Normalizer; use PHPUnit\Framework\TestCase; -use stdClass; use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor; use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor; use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor; @@ -529,12 +528,12 @@ private function getDenormalizerForStringCollection() /** * @dataProvider provideInvalidDiscriminatorTypes */ - public function testDenormalizeWithDiscriminatorMapHandlesInvalidTypeValue(mixed $typeValue, bool $shouldFail) + public function testDenormalizeWithDiscriminatorMapHandlesInvalidTypeValue(mixed $typeValue, bool $shouldFail): void { if ($shouldFail) { $this->expectException(NotNormalizableValueException::class); $this->expectExceptionMessage( - 'The type property "type" for the abstract object "Symfony\Component\Serializer\Tests\Fixtures\Attributes\AbstractDummy" must be a string.' + 'The type property "type" for the abstract object "Symfony\Component\Serializer\Tests\Fixtures\Attributes\AbstractDummy" must be a string or a stringable object.' ); } @@ -563,19 +562,18 @@ public function hasMetadataFor($value): bool }; $discriminatorResolver = new ClassDiscriminatorFromClassMetadata($loaderMock); - $normalizer = new AbstractObjectNormalizerDummy($factory, null, new PhpDocExtractor(), $discriminatorResolver); + $normalizer = new AbstractObjectNormalizerDummy($factory, null, new ReflectionExtractor(), $discriminatorResolver); $serializer = new Serializer([$normalizer]); $normalizer->setSerializer($serializer); $normalizedData = $normalizer->denormalize(['foo' => 'foo', 'baz' => 'baz', 'quux' => ['value' => 'quux'], 'type' => $typeValue], AbstractDummy::class); - if ($shouldFail) { - $this->fail('Expected exception not thrown.'); - } - $this->assertInstanceOf(DummyFirstChildQuux::class, $normalizedData->quux); } - public function provideInvalidDiscriminatorTypes() + /** + * @return iterable + */ + public static function provideInvalidDiscriminatorTypes(): array { $toStringObject = new class() { public function __toString() @@ -586,7 +584,7 @@ public function __toString() return [ [[], true], - [new StdClass(), true], + [new \stdClass(), true], [123, true], [false, true], ['first', false], From d87082f37dfd2402768a6e708d4edb52b825374d Mon Sep 17 00:00:00 2001 From: Korvin Szanto Date: Sat, 28 Jun 2025 08:24:44 -0700 Subject: [PATCH 4/5] Apply fabpot patches --- .../Serializer/Normalizer/AbstractObjectNormalizer.php | 2 +- .../Tests/Normalizer/AbstractObjectNormalizerTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index 982a8815f21d2..2129e4aac8fed 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -1159,7 +1159,7 @@ private function getMappedClass(array $data, string $class, array $context): str throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('Type property "%s" not found for the abstract object "%s".', $mapping->getTypeProperty(), $class), null, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false); } - if (!is_string($type) && (!is_object($type) || !method_exists($type, '__toString'))) { + if (!\is_string($type) && (!\is_object($type) || !method_exists($type, '__toString'))) { throw NotNormalizableValueException::createForUnexpectedDataType(\sprintf('The type property "%s" for the abstract object "%s" must be a string or a stringable object.', $mapping->getTypeProperty(), $class), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false); } diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php index 9e61e47d7c2eb..cae7da48ba93e 100644 --- a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php +++ b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php @@ -528,7 +528,7 @@ private function getDenormalizerForStringCollection() /** * @dataProvider provideInvalidDiscriminatorTypes */ - public function testDenormalizeWithDiscriminatorMapHandlesInvalidTypeValue(mixed $typeValue, bool $shouldFail): void + public function testDenormalizeWithDiscriminatorMapHandlesInvalidTypeValue(mixed $typeValue, bool $shouldFail) { if ($shouldFail) { $this->expectException(NotNormalizableValueException::class); @@ -575,7 +575,7 @@ public function hasMetadataFor($value): bool */ public static function provideInvalidDiscriminatorTypes(): array { - $toStringObject = new class() { + $toStringObject = new class { public function __toString() { return 'first'; From 0caf0813a28cde777f5f7ce28e3cd1f53528ada3 Mon Sep 17 00:00:00 2001 From: Korvin Szanto Date: Sat, 28 Jun 2025 10:35:55 -0700 Subject: [PATCH 5/5] Fix unrelated test --- src/Symfony/Component/Serializer/Tests/SerializerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.php index 8fd0ff8850f63..776a26fa033bc 100644 --- a/src/Symfony/Component/Serializer/Tests/SerializerTest.php +++ b/src/Symfony/Component/Serializer/Tests/SerializerTest.php @@ -452,7 +452,7 @@ public function hasMetadataFor($value): bool $discriminatorResolver = new ClassDiscriminatorFromClassMetadata($loaderMock); $serializer = new Serializer([new ObjectNormalizer(null, null, null, new PhpDocExtractor(), $discriminatorResolver)], ['json' => new JsonEncoder()]); - $jsonData = '{"type":"first","quux":{"value":"quux"},"bar":"bar-value","foo":"foo-value"}'; + $jsonData = '{"type":"first","quux":{"value":"quux"},"bar":"bar-value","baz":null,"foo":"foo-value"}'; $deserialized = $serializer->deserialize($jsonData, AbstractDummy::class, 'json'); $this->assertEquals($example, $deserialized);