bug #38534 [Serializer] make XmlEncoder stateless thus reentrant (con…

…norhu) This PR was merged into the 4.4 branch. Discussion ---------- [Serializer] make XmlEncoder stateless thus reentrant | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #37354 | License | MIT Changing dom property of XmlEncoder to stackable. It fixes a DOMException "Wrong document error". When you calling XmlEncoder->encode() in parallel createDomDocument replaces the DomDocument object. Test code: https://github.com/connorhu/symfony-serializer/blob/master/test.php Commits ------- 693f9ab [Serializer] make XmlEncoder stateless thus reentrant
symfony · Jan 31, 2022 · 5c67f40 · 5c67f40
2 parents c569ef1 + 693f9ab
commit 5c67f40
Show file tree

Hide file tree

Showing 6 changed files with 186 additions and 45 deletions.
diff --git a/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php b/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
@@ -17,8 +17,6 @@
 use Symfony\Component\Serializer\SerializerAwareTrait;
 
 /**
- * Encodes XML data.
- *
  * @author Jordi Boggiano <j.boggiano@seld.be>
  * @author John Wards <jwards@whiteoctober.co.uk>
  * @author Fabian Vogler <fabian@equivalence.ch>
@@ -68,13 +66,6 @@ class XmlEncoder implements EncoderInterface, DecoderInterface, NormalizationAwa
         self::TYPE_CAST_ATTRIBUTES => true,
     ];
 
-    /**
-     * @var \DOMDocument
-     */
-    private $dom;
-    private $format;
-    private $context;
-
     /**
      * @param array $defaultContext
      */
@@ -107,19 +98,17 @@ public function encode($data, $format, array $context = [])
 
         $xmlRootNodeName = $context[self::ROOT_NODE_NAME] ?? $this->defaultContext[self::ROOT_NODE_NAME];
 
-        $this->dom = $this->createDomDocument($context);
-        $this->format = $format;
-        $this->context = $context;
+        $dom = $this->createDomDocument($context);
 
         if (null !== $data && !is_scalar($data)) {
-            $root = $this->dom->createElement($xmlRootNodeName);
-            $this->dom->appendChild($root);
-            $this->buildXml($root, $data, $xmlRootNodeName);
+            $root = $dom->createElement($xmlRootNodeName);
+            $dom->appendChild($root);
+            $this->buildXml($root, $data, $format, $context, $xmlRootNodeName);
         } else {
-            $this->appendNode($this->dom, $data, $xmlRootNodeName);
+            $this->appendNode($dom, $data, $format, $context, $xmlRootNodeName);
         }
 
-        return $this->dom->saveXML($ignorePiNode ? $this->dom->documentElement : null);
+        return $dom->saveXML($ignorePiNode ? $dom->documentElement : null);
     }
 
     /**
@@ -242,7 +231,7 @@ public function getRootNodeName()
     final protected function appendXMLString(\DOMNode $node, string $val): bool
     {
         if ('' !== $val) {
-            $frag = $this->dom->createDocumentFragment();
+            $frag = $node->ownerDocument->createDocumentFragment();
             $frag->appendXML($val);
             $node->appendChild($frag);
 
@@ -254,15 +243,15 @@ final protected function appendXMLString(\DOMNode $node, string $val): bool
 
     final protected function appendText(\DOMNode $node, string $val): bool
     {
-        $nodeText = $this->dom->createTextNode($val);
+        $nodeText = $node->ownerDocument->createTextNode($val);
         $node->appendChild($nodeText);
 
         return true;
     }
 
     final protected function appendCData(\DOMNode $node, string $val): bool
     {
-        $nodeText = $this->dom->createCDATASection($val);
+        $nodeText = $node->ownerDocument->createCDATASection($val);
         $node->appendChild($nodeText);
 
         return true;
@@ -284,7 +273,7 @@ final protected function appendDocumentFragment(\DOMNode $node, $fragment): bool
 
     final protected function appendComment(\DOMNode $node, string $data): bool
     {
-        $node->appendChild($this->dom->createComment($data));
+        $node->appendChild($node->ownerDocument->createComment($data));
 
         return true;
     }
@@ -412,22 +401,22 @@ private function parseXmlValue(\DOMNode $node, array $context = [])
      *
      * @throws NotEncodableValueException
      */
-    private function buildXml(\DOMNode $parentNode, $data, string $xmlRootNodeName = null): bool
+    private function buildXml(\DOMNode $parentNode, $data, string $format, array $context, string $xmlRootNodeName = null): bool
     {
         $append = true;
-        $removeEmptyTags = $this->context[self::REMOVE_EMPTY_TAGS] ?? $this->defaultContext[self::REMOVE_EMPTY_TAGS] ?? false;
-        $encoderIgnoredNodeTypes = $this->context[self::ENCODER_IGNORED_NODE_TYPES] ?? $this->defaultContext[self::ENCODER_IGNORED_NODE_TYPES];
+        $removeEmptyTags = $context[self::REMOVE_EMPTY_TAGS] ?? $this->defaultContext[self::REMOVE_EMPTY_TAGS] ?? false;
+        $encoderIgnoredNodeTypes = $context[self::ENCODER_IGNORED_NODE_TYPES] ?? $this->defaultContext[self::ENCODER_IGNORED_NODE_TYPES];
 
-        if (\is_array($data) || ($data instanceof \Traversable && (null === $this->serializer || !$this->serializer->supportsNormalization($data, $this->format)))) {
+        if (\is_array($data) || ($data instanceof \Traversable && (null === $this->serializer || !$this->serializer->supportsNormalization($data, $format)))) {
             foreach ($data as $key => $data) {
                 //Ah this is the magic @ attribute types.
                 if (str_starts_with($key, '@') && $this->isElementNameValid($attributeName = substr($key, 1))) {
                     if (!is_scalar($data)) {
-                        $data = $this->serializer->normalize($data, $this->format, $this->context);
+                        $data = $this->serializer->normalize($data, $format, $context);
                     }
                     $parentNode->setAttribute($attributeName, $data);
                 } elseif ('#' === $key) {
-                    $append = $this->selectNodeType($parentNode, $data);
+                    $append = $this->selectNodeType($parentNode, $data, $format, $context);
                 } elseif ('#comment' === $key) {
                     if (!\in_array(\XML_COMMENT_NODE, $encoderIgnoredNodeTypes, true)) {
                         $append = $this->appendComment($parentNode, $data);
@@ -441,15 +430,15 @@ private function buildXml(\DOMNode $parentNode, $data, string $xmlRootNodeName =
                          * From ["item" => [0,1]];.
                          */
                         foreach ($data as $subData) {
-                            $append = $this->appendNode($parentNode, $subData, $key);
+                            $append = $this->appendNode($parentNode, $subData, $format, $context, $key);
                         }
                     } else {
-                        $append = $this->appendNode($parentNode, $data, $key);
+                        $append = $this->appendNode($parentNode, $data, $format, $context, $key);
                     }
                 } elseif (is_numeric($key) || !$this->isElementNameValid($key)) {
-                    $append = $this->appendNode($parentNode, $data, 'item', $key);
+                    $append = $this->appendNode($parentNode, $data, $format, $context, 'item', $key);
                 } elseif (null !== $data || !$removeEmptyTags) {
-                    $append = $this->appendNode($parentNode, $data, $key);
+                    $append = $this->appendNode($parentNode, $data, $format, $context, $key);
                 }
             }
 
@@ -461,20 +450,20 @@ private function buildXml(\DOMNode $parentNode, $data, string $xmlRootNodeName =
                 throw new BadMethodCallException(sprintf('The serializer needs to be set to allow "%s()" to be used with object data.', __METHOD__));
             }
 
-            $data = $this->serializer->normalize($data, $this->format, $this->context);
+            $data = $this->serializer->normalize($data, $format, $context);
             if (null !== $data && !is_scalar($data)) {
-                return $this->buildXml($parentNode, $data, $xmlRootNodeName);
+                return $this->buildXml($parentNode, $data, $format, $context, $xmlRootNodeName);
             }
 
             // top level data object was normalized into a scalar
             if (!$parentNode->parentNode->parentNode) {
                 $root = $parentNode->parentNode;
                 $root->removeChild($parentNode);
 
-                return $this->appendNode($root, $data, $xmlRootNodeName);
+                return $this->appendNode($root, $data, $format, $context, $xmlRootNodeName);
             }
 
-            return $this->appendNode($parentNode, $data, 'data');
+            return $this->appendNode($parentNode, $data, $format, $context, 'data');
         }
 
         throw new NotEncodableValueException('An unexpected value could not be serialized: '.(!\is_resource($data) ? var_export($data, true) : sprintf('%s resource', get_resource_type($data))));
@@ -485,13 +474,14 @@ private function buildXml(\DOMNode $parentNode, $data, string $xmlRootNodeName =
      *
      * @param array|object $data
      */
-    private function appendNode(\DOMNode $parentNode, $data, string $nodeName, string $key = null): bool
+    private function appendNode(\DOMNode $parentNode, $data, string $format, array $context, string $nodeName, string $key = null): bool
     {
-        $node = $this->dom->createElement($nodeName);
+        $dom = $parentNode instanceof \DomDocument ? $parentNode : $parentNode->ownerDocument;
+        $node = $dom->createElement($nodeName);
         if (null !== $key) {
             $node->setAttribute('key', $key);
         }
-        $appendNode = $this->selectNodeType($node, $data);
+        $appendNode = $this->selectNodeType($node, $data, $format, $context);
         // we may have decided not to append this node, either in error or if its $nodeName is not valid
         if ($appendNode) {
             $parentNode->appendChild($node);
@@ -505,32 +495,32 @@ private function appendNode(\DOMNode $parentNode, $data, string $nodeName, strin
      */
     private function needsCdataWrapping(string $val): bool
     {
-        return 0 < preg_match('/[<>&]/', $val);
+        return preg_match('/[<>&]/', $val);
     }
 
     /**
      * Tests the value being passed and decide what sort of element to create.
      *
      * @throws NotEncodableValueException
      */
-    private function selectNodeType(\DOMNode $node, $val): bool
+    private function selectNodeType(\DOMNode $node, $val, string $format, array $context): bool
     {
         if (\is_array($val)) {
-            return $this->buildXml($node, $val);
+            return $this->buildXml($node, $val, $format, $context);
         } elseif ($val instanceof \SimpleXMLElement) {
-            $child = $this->dom->importNode(dom_import_simplexml($val), true);
+            $child = $node->ownerDocument->importNode(dom_import_simplexml($val), true);
             $node->appendChild($child);
         } elseif ($val instanceof \Traversable) {
-            $this->buildXml($node, $val);
+            $this->buildXml($node, $val, $format, $context);
         } elseif ($val instanceof \DOMNode) {
-            $child = $this->dom->importNode($val, true);
+            $child = $node->ownerDocument->importNode($val, true);
             $node->appendChild($child);
         } elseif (\is_object($val)) {
             if (null === $this->serializer) {
                 throw new BadMethodCallException(sprintf('The serializer needs to be set to allow "%s()" to be used with object data.', __METHOD__));
             }
 
-            return $this->selectNodeType($node, $this->serializer->normalize($val, $this->format, $this->context));
+            return $this->selectNodeType($node, $this->serializer->normalize($val, $format, $context), $format, $context);
         } elseif (is_numeric($val)) {
             return $this->appendText($node, (string) $val);
         } elseif (\is_string($val) && $this->needsCdataWrapping($val)) {

diff --git a/src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php b/src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php
@@ -20,6 +20,10 @@
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 use Symfony\Component\Serializer\Serializer;
 use Symfony\Component\Serializer\Tests\Fixtures\Dummy;
+use Symfony\Component\Serializer\Tests\Fixtures\EnvelopedMessage;
+use Symfony\Component\Serializer\Tests\Fixtures\EnvelopedMessageNormalizer;
+use Symfony\Component\Serializer\Tests\Fixtures\EnvelopeNormalizer;
+use Symfony\Component\Serializer\Tests\Fixtures\EnvelopeObject;
 use Symfony\Component\Serializer\Tests\Fixtures\NormalizableTraversableDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\ScalarDummy;
 
@@ -850,6 +854,23 @@ public function testNotEncodableValueExceptionMessageForAResource()
         (new XmlEncoder())->encode(tmpfile(), 'xml');
     }
 
+    public function testReentrantXmlEncoder()
+    {
+        $envelope = new EnvelopeObject();
+        $message = new EnvelopedMessage();
+        $message->text = 'Symfony is great';
+        $envelope->message = $message;
+
+        $encoder = $this->createXmlEncoderWithEnvelopeNormalizer();
+        $expected = <<<'XML'
+<?xml version="1.0"?>
+<response><message>PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxyZXNwb25zZT48dGV4dD5TeW1mb255IGlzIGdyZWF0PC90ZXh0PjwvcmVzcG9uc2U+Cg==</message></response>
+
+XML;
+
+        $this->assertSame($expected, $encoder->encode($envelope, 'xml'));
+    }
+
     public function testEncodeComment()
     {
         $expected = <<<'XML'
@@ -921,6 +942,21 @@ private function doTestEncodeWithoutComment(bool $legacy = false)
         $this->assertEquals($expected, $encoder->encode($data, 'xml'));
     }
 
+    private function createXmlEncoderWithEnvelopeNormalizer(): XmlEncoder
+    {
+        $normalizers = [
+            $envelopeNormalizer = new EnvelopeNormalizer(),
+            new EnvelopedMessageNormalizer(),
+        ];
+
+        $encoder = new XmlEncoder();
+        $serializer = new Serializer($normalizers, ['xml' => $encoder]);
+        $encoder->setSerializer($serializer);
+        $envelopeNormalizer->setSerializer($serializer);
+
+        return $encoder;
+    }
+
     private function createXmlEncoderWithDateTimeNormalizer(): XmlEncoder
     {
         $encoder = new XmlEncoder();

diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopeNormalizer.php b/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopeNormalizer.php
@@ -0,0 +1,43 @@
+<?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\NormalizerInterface;
+
+/**
+ * @author Karoly Gossler <connor@connor.hu>
+ */
+class EnvelopeNormalizer implements NormalizerInterface
+{
+    private $serializer;
+
+    public function normalize($envelope, $format = null, array $context = [])
+    {
+        $xmlContent = $this->serializer->serialize($envelope->message, 'xml');
+
+        $encodedContent = base64_encode($xmlContent);
+
+        return [
+            'message' => $encodedContent,
+        ];
+    }
+
+    public function supportsNormalization($data, $format = null)
+    {
+        return $data instanceof EnvelopeObject;
+    }
+
+    public function setSerializer($serializer)
+    {
+        $this->serializer = $serializer;
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopeObject.php b/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopeObject.php
@@ -0,0 +1,20 @@
+<?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 Karoly Gossler <connor@connor.hu>
+ */
+class EnvelopeObject
+{
+    public $message;
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopedMessage.php b/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopedMessage.php
@@ -0,0 +1,20 @@
+<?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 Karoly Gossler <connor@connor.hu>
+ */
+class EnvelopedMessage
+{
+    public $text;
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopedMessageNormalizer.php b/src/Symfony/Component/Serializer/Tests/Fixtures/EnvelopedMessageNormalizer.php
@@ -0,0 +1,32 @@
+<?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\NormalizerInterface;
+
+/**
+ * @author Karoly Gossler <connor@connor.hu>
+ */
+class EnvelopedMessageNormalizer implements NormalizerInterface
+{
+    public function normalize($message, $format = null, array $context = [])
+    {
+        return [
+            'text' => $message->text,
+        ];
+    }
+
+    public function supportsNormalization($data, $format = null)
+    {
+        return $data instanceof EnvelopedMessage;
+    }
+}