Skip to content

[Serializer] Handle invalid mapping type property type #60296

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: 7.2
Choose a base branch
from
Open

[Serializer] Handle invalid mapping type property type #60296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebad77f
Handle invalid mapping type property type
KorvinSzanto Apr 28, 2025
6b84342
Add tests for invalid mapped class type
KorvinSzanto Apr 29, 2025
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
4 changes: 4 additions & 0 deletions src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);

maybe?

}

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);
}
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Serializer/Tests/Fixtures/Attributes/AbstractDummyFirstChild.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
class AbstractDummyFirstChild extends AbstractDummy
{
public $bar;
public $baz;

/** @var DummyFirstChildQuux|null */
public $quux;
Expand Down
69 changes: 69 additions & 0 deletions src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure you need the PhpDocExtractor here

$serializer = new Serializer([$normalizer]);
$normalizer->setSerializer($serializer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed?

$normalizedData = $normalizer->denormalize(['foo' => 'foo', 'baz' => 'baz', 'quux' => ['value' => 'quux'], 'type' => $typeValue], AbstractDummy::class);

if ($shouldFail) {
$this->fail('Expected exception not thrown.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed because of the exceptException above

}

$this->assertInstanceOf(DummyFirstChildQuux::class, $normalizedData->quux);
}

public function provideInvalidDiscriminatorTypes()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function provideInvalidDiscriminatorTypes()
/**
* @return iterable<array{0: mixed, 1: bool}>
*/
public function provideInvalidDiscriminatorTypes(): iterable

{
$toStringObject = new class() {
public function __toString()
{
return 'first';
}
};

return [
[[], true],
[new StdClass(), true],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[new StdClass(), true],
[new \stdClass(), true],

[123, true],
[false, true],
['first', false],
[$toStringObject, false],
];
}

public function testDenormalizeWithDiscriminatorMapUsesCorrectClassname()
{
$factory = new ClassMetadataFactory(new AttributeLoader());
Expand Down
Loading