Skip to content

Commit

Permalink
feature #54346 [Serializer] Fix: Report Xml warning/error instead of …
Browse files Browse the repository at this point in the history
…silently returning a wrong xml (VincentLanglet)

This PR was squashed before being merged into the 7.1 branch.

Discussion
----------

[Serializer] Fix: Report Xml warning/error instead of silently returning a wrong xml

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

When `DomDocument::saveXML` encounter an error/warning, for example
```
DOMDocument::saveXML(): xmlEscapeEntities : char out of range
```
the method will return false or an empty/incomplete XML.

In case of `false`, since symfony doesn't use strict type, it will be cast into `string` (for 6.4+ version with native typehint) so the `encode` method will return an empty string.
In case of empty/incomplete XML, symfony returns it as if, without any notice about the error/warning.

I think Symfony should not silently ignore such XML error when decoding.

Or should it be an option ?

Commits
-------

70e74dd [Serializer] Fix: Report Xml warning/error instead of silently returning a wrong xml
  • Loading branch information
fabpot committed Apr 5, 2024
2 parents 9ece00c + 70e74dd commit fdedd3b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
23 changes: 21 additions & 2 deletions src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
Expand Up @@ -82,7 +82,7 @@ public function encode(mixed $data, string $format, array $context = []): string
$encoderIgnoredNodeTypes = $context[self::ENCODER_IGNORED_NODE_TYPES] ?? $this->defaultContext[self::ENCODER_IGNORED_NODE_TYPES];
$ignorePiNode = \in_array(\XML_PI_NODE, $encoderIgnoredNodeTypes, true);
if ($data instanceof \DOMDocument) {
return $data->saveXML($ignorePiNode ? $data->documentElement : null);
return $this->saveXml($data, $ignorePiNode ? $data->documentElement : null);
}

$xmlRootNodeName = $context[self::ROOT_NODE_NAME] ?? $this->defaultContext[self::ROOT_NODE_NAME];
Expand All @@ -97,7 +97,7 @@ public function encode(mixed $data, string $format, array $context = []): string
$this->appendNode($dom, $data, $format, $context, $xmlRootNodeName);
}

return $dom->saveXML($ignorePiNode ? $dom->documentElement : null, $context[self::SAVE_OPTIONS] ?? $this->defaultContext[self::SAVE_OPTIONS]);
return $this->saveXml($dom, $ignorePiNode ? $dom->documentElement : null, $context[self::SAVE_OPTIONS] ?? $this->defaultContext[self::SAVE_OPTIONS]);
}

public function decode(string $data, string $format, array $context = []): mixed
Expand Down Expand Up @@ -498,4 +498,23 @@ private function createDomDocument(array $context): \DOMDocument

return $document;
}

/**
* @throws NotEncodableValueException
*/
private function saveXml(\DOMDocument $document, ?\DOMNode $node = null, ?int $options = null): string
{
$prevErrorHandler = set_error_handler(static function ($type, $message, $file, $line, $context = []) use (&$prevErrorHandler) {
if (\E_ERROR === $type || \E_WARNING === $type) {
throw new NotEncodableValueException($message);
}

return $prevErrorHandler ? $prevErrorHandler($type, $message, $file, $line, $context) : false;
});
try {
return $document->saveXML($node, $options);
} finally {
restore_error_handler();
}
}
}
Expand Up @@ -433,6 +433,12 @@ public function testEncodeTraversableWhenNormalizable()
$this->assertEquals($expected, $serializer->serialize(new NormalizableTraversableDummy(), 'xml'));
}

public function testEncodeException()
{
$this->expectException(NotEncodableValueException::class);
$this->encoder->encode('Invalid character: '.\chr(7), 'xml');
}

public function testDecode()
{
$source = $this->getXmlSource();
Expand Down

0 comments on commit fdedd3b

Please sign in to comment.