Fatal error: Call to a member function normalize() on a non-object in XmlEncoder when encoding into xml and there are embedded objects in the response

Working on reproducing this issue in the 8.2-x branch.

@davidwbarratt or @cilefen, what's the best way to reproduce the issue? Are you using a REST view with an XML serializer? If so what kind of fields are you exporting?

I see where the "Symfony\Component\Serializer\Encoder\XmlEncoder" object is being instantiated without a serializer. I also see a serializer should have an instance (or multiple) of a normalizer in order for it to be able to normalize an object.

I haven't dug around this code at all, and I see that there's definitely a bug, but it will help if I can reproduce it. Thanks!

Come find me in the sprint room.

Sorry, mentoring room...

Sucks I missed you earlier @cilefen. Had to leave at 3pm.

This is likely still the same problem it was, this is the fix. I'm pretty sure I can reproduce but some steps in the issue description here would be good. There is a unit test for this, but the base encoder is set manually for the test so it can be mocked. So we might want to think about that. We could probably expose this in \Drupal\serialization\Tests\EntitySerializationTest but likely there is not an object getting serialized in that test so the serializer on the base xml encoder is not called.

I think maybe this is not exposed in the regular entity tests also, because the entity is first normalized, which converts everything to an array. If you did the following, it would expose the bug:

$container->get('serializer')->encode(['a' => User::load(123)], 'xml')

Something simple like this can test it. A test only patch won't really show much though as the patch is required to have the setSerializer() method on the encoder.

The only way I can reproduce this is if I add xml to one of the REST resource config entities (i.e. to make it support XML), but then fail to rebuild the router.

That's being fixed in #2815845: Importing (deploying) REST resource config entities should automatically do the necessary route rebuilding.

If you think this issue still needs to be resolved: this issue will need an IS update to clarify how to reproduce this, and test to prove that it's fixed.

Finally: in #2800873: Add XML GET REST test coverage, work around XML encoder quirks, we're considering to remove XML support.

Also, this is normal at best if this has received zero confirmations since it was reported: it doesn't seem to affect many sites. Also only 7 followers. Probably because almost no sites use the XML serializer.

Downgrading to normal.

This can be reproduced quite easily IIRC, if you are using the serializer outside of rest. Not sure how not rebuilding the router would reproduce this issue? REST endpoints are not the only user of the serializer :) If you see my comment in #11 you can still reproduce this easily. As we already have the test and the fix for this, it still seems worth us just committing this.

Patch in #11 is still good for 8.4.x

Rerolled the patch against latest 8.4.x, please review.

Sweet, another commit mining attempt.

The reroll was not good and broke things with a duplicate use statement.

I'm tempted to close this because of #2800873: Add XML GET REST test coverage, work around XML encoder quirks.

Hmm, well I think we should still support bugs of this nature. You can serialize anything using XML - not necessarily entities, or custom stuff. Anything really. Plus this already has the test coverage and the work is done :)

Every minute spent on improving XML support is a minute not spent on improving everything else, which is what the 99.99% use. But, sure.

+++ b/core/modules/serialization/tests/src/Unit/Encoder/XmlEncoderTest.php
@@ -76,4 +79,23 @@ public function testDecode() {
+     // The serializer should being set on the Drupal encoder should then set it

Grammatical problems here.

And can we please get a failing test-only patch and a passing complete patch? That'll help the core committer that looks at this.

Here are those changes quickly, I actually hit this issue when doing something for work yesterday! :)

I actually hit this issue when doing something for work yesterday! :)

So you're using the xml normalizer?

Aren't you also running into #2800873: Add XML GET REST test coverage, work around XML encoder quirks then?

Ha, well, it wasn't being used for a fieldable entity - so it was ok.

Makes sense :)

Committed and pushed 081132d to 8.4.x and 009a535 to 8.3.x. Thanks!

I've committed this to 8.3.x because essentially the XmlEncoder is broken and useless without this.

@ec0g thanks for trying to reproduce the issue. I thought about crediting you on this issue because of comment #5 which pinpointed where things were going wrong. But this was also included in the issue summary.

@Pavan B S I've not credited you on the issue because the reroll was not a good one. Given the patch contained a unit test that could have easily been run prior to uploading the patch to confirm the re-roll worked.

diff --git a/core/modules/serialization/tests/src/Unit/Encoder/XmlEncoderTest.php b/core/modules/serialization/tests/src/Unit/Encoder/XmlEncoderTest.php
index b0ee6b3..2a5bf3e 100644
--- a/core/modules/serialization/tests/src/Unit/Encoder/XmlEncoderTest.php
+++ b/core/modules/serialization/tests/src/Unit/Encoder/XmlEncoderTest.php
@@ -83,19 +83,21 @@ public function testDecode() {
     * @covers ::getBaseEncoder
     */
   public function testDefaultEncoderHasSerializer() {
-     // The serializer should be set on the Drupal encoder, which should then
-     // set it on our default encoder.
-     $encoder = new XmlEncoder();
-     $serialzer = new Serializer([new GetSetMethodNormalizer()]);
-     $encoder->setSerializer($serialzer);
-     $base_encoder = $encoder->getBaseEncoder();
-     $this->assertInstanceOf(BaseXmlEncoder::class, $base_encoder);
-     // Test the encoder.
-     $base_encoder->encode(['a' => new TestObject()], 'xml');
-   }
- }
- class TestObject {
+    // The serializer should be set on the Drupal encoder, which should then
+    // set it on our default encoder.
+    $encoder = new XmlEncoder();
+    $serialzer = new Serializer([new GetSetMethodNormalizer()]);
+    $encoder->setSerializer($serialzer);
+    $base_encoder = $encoder->getBaseEncoder();
+    $this->assertInstanceOf(BaseXmlEncoder::class, $base_encoder);
+    // Test the encoder.
+    $base_encoder->encode(['a' => new TestObject()], 'xml');
+  }
+
+}
+
+class TestObject {
   public function getA() {
-     return 'A';
-   }
+   return 'A';
+  }
 }

Sorted out all the spacing on commit.

Thanks for your impressive diligence, Alex!

This fix actually breaks the serializer.
Line

$this->baseEncoder->setSerializer($this->serializer);

Using the service 'serializer.encoder.xml' no longer works, as $this->serializer is NULL.

That would suggest you're trying to use the XmlEncoder directly, not through the serializer. This is a standard pattern from Symfony, SerializerAwareEncoders get setSerializer() called on them when the serializer instance is created. You can't have been using this too well before due to this bug anyway. The XmlEncoder needs the serializer to do anything useful.

#37 + #38: somebody reported the same at #2891850: Fatal error on serialization module.