[Serializer] Fix AbstractObjectNormalizer TypeError on denormalization #44881

JustDylan23 · 2021-12-31T20:40:28Z

Q	A
Branch?	6.1
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #44872
License	MIT
Doc PR

When using dependency injection to get the serializer (instead of manually instantiating it) the object normalizer that is injected into that serializer throws a value exception when doing denormalizing the following:

class Outer {
  public Inner $inner;
}

class Inner {
  public $foo;
}

#[Route('/bug')]
public function test(DenormalizerInterface $denormalizer) {
    $blog = $denormalizer->denormalize([
        'inner' => 'test string',
    ], Outer ::class);
}

This throws

TypeError:
Symfony\Component\Serializer\Normalizer\AbstractNormalizer::prepareForDenormalization(): Argument #1 ($data) must be of type object|array|null, string given, called in /var/www/symfony/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php on line 368

  at vendor/symfony/serializer/Normalizer/AbstractNormalizer.php:299
  at Symfony\Component\Serializer\Normalizer\AbstractNormalizer->prepareForDenormalization('test string')
     (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:368)
  at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize('test string', 'App\\Entity\\User', null, array('cache_key' => 'c93a6d4efa206ea58a62cc6b7fab8dfb', 'deserialization_path' => 'author'))
     (vendor/symfony/serializer/Serializer.php:238)
  at Symfony\Component\Serializer\Serializer->denormalize('test string', 'App\\Entity\\User', null, array('cache_key' => 'c93a6d4efa206ea58a62cc6b7fab8dfb', 'deserialization_path' => 'author'))
     (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:559)
  at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->validateAndDenormalize(array(object(Type)), 'App\\Entity\\Blog', 'author', 'test string', null, array('cache_key' => '44db5a926a1544b1a8585af40107ca3a', 'deserialization_path' => 'author'))
     (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:401)
  at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize(array('author' => 'test string'), 'App\\Entity\\Blog', null, array('cache_key' => '44db5a926a1544b1a8585af40107ca3a'))
     (vendor/symfony/serializer/Serializer.php:238)
  at Symfony\Component\Serializer\Serializer->denormalize(array('author' => 'test string'), 'App\\Entity\\Blog')
     (src/Controller/BugReproductionController.php:18)
  at App\Controller\BugReproductionController->test(object(Serializer))
     (vendor/symfony/http-kernel/HttpKernel.php:152)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:74)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:202)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35)
  at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
     (vendor/autoload_runtime.php:29)
  at require_once('/var/www/symfony/vendor/autoload_runtime.php')
     (public/index.php:5)

I have been going through the code for 8 hours but I have yet to find an optimal solution for this, I did however add a unit test that indicates the bug. Any suggestions on how to solve this are welcome

carsonbot · 2021-12-31T20:40:32Z

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Always add tests
Keep backward compatibility (see https://symfony.com/bc).
Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
Features and deprecations must be submitted against the 6.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

JustDylan23 · 2022-01-03T12:06:50Z

do we want to keep the same behavior as in 5 where an empty object is initialized and no error is thrown or do we want to introduce an exception like I proposed in #44872?

fancyweb · 2022-01-03T12:15:34Z

Firstly, I would just fix it on 6.0 by changing the prepareForDenormalization() argument type to mixed.

… as in Symfony 5

JustDylan23 · 2022-01-03T17:06:59Z

@dunglas can you review this?

dbu · 2022-01-04T09:47:55Z

another way to run into this problem: when de-normalizing an xml response with only the root node and no attributes. the XmlEncoder::decode returns the $rootNode->nodeValue when there are neither children nor attributes.

fancyweb · 2022-01-04T10:07:22Z

src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php

@@ -707,6 +707,17 @@ public function testDenomalizeRecursive()
        $this->assertSame(2, $obj->getInners()[1]->foo);
    }

+    public function testDenomalizeRecursiveWithObjectAttributeWithStringValue()


Suggested change

public function testDenomalizeRecursiveWithObjectAttributeWithStringValue()

public function testDenormalizeRecursiveWithObjectAttributeWithStringValue()

fancyweb · 2022-01-04T10:07:42Z

src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php

@@ -707,6 +707,17 @@ public function testDenomalizeRecursive()
        $this->assertSame(2, $obj->getInners()[1]->foo);
    }

+    public function testDenomalizeRecursiveWithObjectAttributeWithStringValue()
+    {
+        $extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);


Suggested change

$extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);

$extractor = new ReflectionExtractor();

fancyweb · 2022-01-04T10:07:58Z

src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php

+    {
+        $extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
+        $normalizer = new ObjectNormalizer(null, null, null, $extractor);
+        $serializer = new Serializer([new ArrayDenormalizer(), new DateTimeNormalizer(), $normalizer]);


Suggested change

$serializer = new Serializer([new ArrayDenormalizer(), new DateTimeNormalizer(), $normalizer]);

$serializer = new Serializer([$normalizer]);

fancyweb · 2022-01-04T10:08:27Z

src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php

+
+        $obj = $serializer->denormalize(['inner' => 'foo'], ObjectOuter::class);
+
+        self::assertInstanceOf(ObjectInner::class, $obj->getInner());


Suggested change

self::assertInstanceOf(ObjectInner::class, $obj->getInner());

$this->assertInstanceOf(ObjectInner::class, $obj->getInner());

fancyweb · 2022-01-04T10:11:34Z

Also I think it would be better if the test was in AbstractObjectNormalizerTest since what we test is AbstractObjectNormalizer::denormalize.

fancyweb · 2022-01-04T10:15:53Z

Also please target 6.0 instead of 6.1, thanks.

JustDylan23 · 2022-01-04T21:27:12Z

messed up when changing the base branch, my thought process was

get diff between 6.1 and my branch and create a patch
delete branch
push (I should not have done this)
create branch
get diff back
push

apparently the PR is closed now

JustDylan23 · 2022-01-04T21:32:50Z

see #44908 for the new PR

…ormalization (JustDylan23) This PR was squashed before being merged into the 6.0 branch. Discussion ---------- [Serializer] Fix AbstractObjectNormalizer TypeError on denormalization | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44872 | License | MIT | Doc PR | When using dependency injection to get the serializer (instead of manually instantiating it) the object normalizer that is injected into that serializer throws a value exception when doing denormalizing the following: ```php class ObjectOuter { public ObjectInner $inner; } class ObjectInner { public $foo; } public function testDenormalizeRecursiveWithObjectAttributeWithStringValue() { $extractor = new ReflectionExtractor(); $normalizer = new ObjectNormalizer(null, null, null, $extractor); $serializer = new Serializer([$normalizer]); $obj = $serializer->denormalize(['inner' => 'foo'], ObjectOuter::class); $this->assertInstanceOf(ObjectInner::class, $obj->getInner()); } ``` This throws ```php TypeError: Symfony\Component\Serializer\Normalizer\AbstractNormalizer::prepareForDenormalization(): Argument #1 ($data) must be of type object|array|null, string given, called in /var/www/symfony/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php on line 368 at vendor/symfony/serializer/Normalizer/AbstractNormalizer.php:299 at Symfony\Component\Serializer\Normalizer\AbstractNormalizer->prepareForDenormalization('test string') (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:368) at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize('test string', 'App\\Entity\\User', null, array('cache_key' => 'c93a6d4efa206ea58a62cc6b7fab8dfb', 'deserialization_path' => 'author')) (vendor/symfony/serializer/Serializer.php:238) at Symfony\Component\Serializer\Serializer->denormalize('test string', 'App\\Entity\\User', null, array('cache_key' => 'c93a6d4efa206ea58a62cc6b7fab8dfb', 'deserialization_path' => 'author')) (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:559) at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->validateAndDenormalize(array(object(Type)), 'App\\Entity\\Blog', 'author', 'test string', null, array('cache_key' => '44db5a926a1544b1a8585af40107ca3a', 'deserialization_path' => 'author')) (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:401) at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize(array('author' => 'test string'), 'App\\Entity\\Blog', null, array('cache_key' => '44db5a926a1544b1a8585af40107ca3a')) (vendor/symfony/serializer/Serializer.php:238) at Symfony\Component\Serializer\Serializer->denormalize(array('author' => 'test string'), 'App\\Entity\\Blog') (src/Controller/BugReproductionController.php:18) at App\Controller\BugReproductionController->test(object(Serializer)) (vendor/symfony/http-kernel/HttpKernel.php:152) at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1) (vendor/symfony/http-kernel/HttpKernel.php:74) at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true) (vendor/symfony/http-kernel/Kernel.php:202) at Symfony\Component\HttpKernel\Kernel->handle(object(Request)) (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35) at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run() (vendor/autoload_runtime.php:29) at require_once('/var/www/symfony/vendor/autoload_runtime.php') (public/index.php:5) ``` Refer to: #44881 for the description. Was in the middle of changing the base branch and accidentally pushed when the branch was deleted. `@fancyweb` I implemented the requested changes Commits ------- 89092ea [Serializer] Fix AbstractObjectNormalizer TypeError on denormalization

…ormalization (JustDylan23) This PR was squashed before being merged into the 6.0 branch. Discussion ---------- [Serializer] Fix AbstractObjectNormalizer TypeError on denormalization | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44872 | License | MIT | Doc PR | When using dependency injection to get the serializer (instead of manually instantiating it) the object normalizer that is injected into that serializer throws a value exception when doing denormalizing the following: ```php class ObjectOuter { public ObjectInner $inner; } class ObjectInner { public $foo; } public function testDenormalizeRecursiveWithObjectAttributeWithStringValue() { $extractor = new ReflectionExtractor(); $normalizer = new ObjectNormalizer(null, null, null, $extractor); $serializer = new Serializer([$normalizer]); $obj = $serializer->denormalize(['inner' => 'foo'], ObjectOuter::class); $this->assertInstanceOf(ObjectInner::class, $obj->getInner()); } ``` This throws ```php TypeError: Symfony\Component\Serializer\Normalizer\AbstractNormalizer::prepareForDenormalization(): Argument #1 ($data) must be of type object|array|null, string given, called in /var/www/symfony/vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php on line 368 at vendor/symfony/serializer/Normalizer/AbstractNormalizer.php:299 at Symfony\Component\Serializer\Normalizer\AbstractNormalizer->prepareForDenormalization('test string') (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:368) at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize('test string', 'App\\Entity\\User', null, array('cache_key' => 'c93a6d4efa206ea58a62cc6b7fab8dfb', 'deserialization_path' => 'author')) (vendor/symfony/serializer/Serializer.php:238) at Symfony\Component\Serializer\Serializer->denormalize('test string', 'App\\Entity\\User', null, array('cache_key' => 'c93a6d4efa206ea58a62cc6b7fab8dfb', 'deserialization_path' => 'author')) (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:559) at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->validateAndDenormalize(array(object(Type)), 'App\\Entity\\Blog', 'author', 'test string', null, array('cache_key' => '44db5a926a1544b1a8585af40107ca3a', 'deserialization_path' => 'author')) (vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:401) at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize(array('author' => 'test string'), 'App\\Entity\\Blog', null, array('cache_key' => '44db5a926a1544b1a8585af40107ca3a')) (vendor/symfony/serializer/Serializer.php:238) at Symfony\Component\Serializer\Serializer->denormalize(array('author' => 'test string'), 'App\\Entity\\Blog') (src/Controller/BugReproductionController.php:18) at App\Controller\BugReproductionController->test(object(Serializer)) (vendor/symfony/http-kernel/HttpKernel.php:152) at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1) (vendor/symfony/http-kernel/HttpKernel.php:74) at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true) (vendor/symfony/http-kernel/Kernel.php:202) at Symfony\Component\HttpKernel\Kernel->handle(object(Request)) (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35) at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run() (vendor/autoload_runtime.php:29) at require_once('/var/www/symfony/vendor/autoload_runtime.php') (public/index.php:5) ``` Refer to: symfony/symfony#44881 for the description. Was in the middle of changing the base branch and accidentally pushed when the branch was deleted. `@fancyweb` I implemented the requested changes Commits ------- 89092ea279 [Serializer] Fix AbstractObjectNormalizer TypeError on denormalization

JustDylan23 requested a review from dunglas as a code owner December 31, 2021 20:40

carsonbot added Status: Needs Review Bug labels Dec 31, 2021

carsonbot added this to the 6.1 milestone Dec 31, 2021

JustDylan23 changed the title ~~ticket_44872 fixed autoconfigured serializer handling non-array value…~~ [serializer] recursive denormalizar can throw a TypeError Dec 31, 2021

ticket_44872 added unit test to reproduce #44872

8d3da31

ticket_44872 made the behavior of the recursive denormalizer the same…

e7412ad

… as in Symfony 5

fancyweb requested changes Jan 4, 2022

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jan 4, 2022

carsonbot changed the title ~~[serializer] recursive denormalizar can throw a TypeError~~ recursive denormalizar can throw a TypeError Jan 4, 2022

fancyweb modified the milestones: 6.1, 6.0 Jan 4, 2022

fancyweb changed the title ~~recursive denormalizar can throw a TypeError~~ [Serializer] Fix AbstractObjectNormalizer TypeError on denormalization Jan 4, 2022

ticket_44872 implemented PR feedback

e102f02

JustDylan23 closed this Jan 4, 2022

JustDylan23 deleted the ticket_44872 branch January 4, 2022 21:21

JustDylan23 mentioned this pull request Jan 4, 2022

[Serializer] Fix AbstractObjectNormalizer TypeError on denormalization #44908

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serializer] Fix AbstractObjectNormalizer TypeError on denormalization #44881

[Serializer] Fix AbstractObjectNormalizer TypeError on denormalization #44881

JustDylan23 commented Dec 31, 2021 •

edited

carsonbot commented Dec 31, 2021

JustDylan23 commented Jan 3, 2022

fancyweb commented Jan 3, 2022

JustDylan23 commented Jan 3, 2022

dbu commented Jan 4, 2022

fancyweb Jan 4, 2022

fancyweb Jan 4, 2022

fancyweb Jan 4, 2022

fancyweb Jan 4, 2022

fancyweb commented Jan 4, 2022

fancyweb commented Jan 4, 2022

JustDylan23 commented Jan 4, 2022 •

edited

JustDylan23 commented Jan 4, 2022

	public function testDenomalizeRecursiveWithObjectAttributeWithStringValue()
	public function testDenormalizeRecursiveWithObjectAttributeWithStringValue()

	$extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
	$extractor = new ReflectionExtractor();

	$serializer = new Serializer([new ArrayDenormalizer(), new DateTimeNormalizer(), $normalizer]);
	$serializer = new Serializer([$normalizer]);


		$obj = $serializer->denormalize(['inner' => 'foo'], ObjectOuter::class);

		self::assertInstanceOf(ObjectInner::class, $obj->getInner());

	self::assertInstanceOf(ObjectInner::class, $obj->getInner());
	$this->assertInstanceOf(ObjectInner::class, $obj->getInner());

[Serializer] Fix AbstractObjectNormalizer TypeError on denormalization #44881

[Serializer] Fix AbstractObjectNormalizer TypeError on denormalization #44881

Conversation

JustDylan23 commented Dec 31, 2021 • edited

carsonbot commented Dec 31, 2021

JustDylan23 commented Jan 3, 2022

fancyweb commented Jan 3, 2022

JustDylan23 commented Jan 3, 2022

dbu commented Jan 4, 2022

fancyweb Jan 4, 2022

Choose a reason for hiding this comment

fancyweb Jan 4, 2022

Choose a reason for hiding this comment

fancyweb Jan 4, 2022

Choose a reason for hiding this comment

fancyweb Jan 4, 2022

Choose a reason for hiding this comment

fancyweb commented Jan 4, 2022

fancyweb commented Jan 4, 2022

JustDylan23 commented Jan 4, 2022 • edited

JustDylan23 commented Jan 4, 2022

JustDylan23 commented Dec 31, 2021 •

edited

JustDylan23 commented Jan 4, 2022 •

edited