Skip to content

Commit

Permalink
feature #26445 [Serializer] Ignore comments when decoding XML (q0rban)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 4.1-dev branch (closes #26445).

Discussion
----------

[Serializer] Ignore comments when decoding XML

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

Previously, if the first line of XML was a comment, that would be used as the root node of the decoded XML. This work strips comments from decoded XML by default, but also allows for customizing which XML node types are ignored during decoding. The first two commits in this PR contain tests only to prove the existence of this "bug".

Commits
-------

f6760d3 [Serializer] Ignore comments when decoding XML
  • Loading branch information
fabpot committed Mar 22, 2018
2 parents 7262c59 + f6760d3 commit bbeca51
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 4 deletions.
5 changes: 5 additions & 0 deletions UPGRADE-4.1.md
Expand Up @@ -80,6 +80,11 @@ SecurityBundle
* The `SecurityUserValueResolver` class is deprecated, use
`Symfony\Component\Security\Http\Controller\UserValueResolver` instead.

Serializer
----------

* Decoding XML with `XmlEncoder` now ignores comment node types by default.

Translation
-----------

Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Serializer/CHANGELOG.md
Expand Up @@ -11,6 +11,8 @@ CHANGELOG
* added optional `bool $escapeFormulas = false` argument to `CsvEncoder::__construct`
* added `AbstractObjectNormalizer::setMaxDepthHandler` to set a handler to call when the configured
maximum depth is reached
* added optional `int[] $ignoredNodeTypes` argument to `XmlEncoder::__construct`. XML decoding now
ignores comment node types by default.

4.0.0
-----
Expand Down
11 changes: 7 additions & 4 deletions src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
Expand Up @@ -37,16 +37,19 @@ class XmlEncoder implements EncoderInterface, DecoderInterface, NormalizationAwa
private $context;
private $rootNodeName = 'response';
private $loadOptions;
private $ignoredNodeTypes;

/**
* Construct new XmlEncoder and allow to change the root node element name.
*
* @param int|null $loadOptions A bit field of LIBXML_* constants
* @param int|null $loadOptions A bit field of LIBXML_* constants
* @param int[] $ignoredNodeTypes an array of ignored XML node types, each one of the DOM Predefined XML_* Constants
*/
public function __construct(string $rootNodeName = 'response', int $loadOptions = null)
public function __construct(string $rootNodeName = 'response', int $loadOptions = null, array $ignoredNodeTypes = array(XML_PI_NODE, XML_COMMENT_NODE))
{
$this->rootNodeName = $rootNodeName;
$this->loadOptions = null !== $loadOptions ? $loadOptions : LIBXML_NONET | LIBXML_NOBLANKS;
$this->ignoredNodeTypes = $ignoredNodeTypes;
}

/**
Expand Down Expand Up @@ -105,7 +108,7 @@ public function decode($data, $format, array $context = array())
if (XML_DOCUMENT_TYPE_NODE === $child->nodeType) {
throw new NotEncodableValueException('Document types are not allowed.');
}
if (!$rootNode && XML_PI_NODE !== $child->nodeType) {
if (!$rootNode && !\in_array($child->nodeType, $this->ignoredNodeTypes, true)) {
$rootNode = $child;
}
}
Expand Down Expand Up @@ -316,7 +319,7 @@ private function parseXmlValue(\DOMNode $node, array $context = array())
$value = array();

foreach ($node->childNodes as $subnode) {
if (XML_PI_NODE === $subnode->nodeType) {
if (\in_array($subnode->nodeType, $this->ignoredNodeTypes, true)) {
continue;
}

Expand Down
56 changes: 56 additions & 0 deletions src/Symfony/Component/Serializer/Tests/Encoder/XmlEncoderTest.php
Expand Up @@ -515,6 +515,62 @@ public function testDecodeIgnoreWhiteSpace()
$this->assertEquals($expected, $this->encoder->decode($source, 'xml'));
}

public function testDecodeIgnoreComments()
{
$source = <<<'XML'
<?xml version="1.0"?>
<!-- This comment should not become the root node. -->
<people>
<person>
<!-- Even if the first comment didn't become the root node, we don't
want this comment either. -->
<firstname>Benjamin</firstname>
<lastname>Alexandre</lastname>
</person>
<person>
<firstname>Damien</firstname>
<lastname>Clay</lastname>
</person>
</people>
XML;

$expected = array('person' => array(
array('firstname' => 'Benjamin', 'lastname' => 'Alexandre'),
array('firstname' => 'Damien', 'lastname' => 'Clay'),
));

$this->assertEquals($expected, $this->encoder->decode($source, 'xml'));
}

public function testDecodePreserveComments()
{
$source = <<<'XML'
<?xml version="1.0"?>
<people>
<person>
<!-- This comment should be decoded. -->
<firstname>Benjamin</firstname>
<lastname>Alexandre</lastname>
</person>
<person>
<firstname>Damien</firstname>
<lastname>Clay</lastname>
</person>
</people>
XML;

$this->encoder = new XmlEncoder('people', null, array(XML_PI_NODE));
$serializer = new Serializer(array(new CustomNormalizer()), array('xml' => new XmlEncoder()));
$this->encoder->setSerializer($serializer);

$expected = array('person' => array(
array('firstname' => 'Benjamin', 'lastname' => 'Alexandre', '#comment' => ' This comment should be decoded. '),
array('firstname' => 'Damien', 'lastname' => 'Clay'),
));

$this->assertEquals($expected, $this->encoder->decode($source, 'xml'));
}

public function testDecodeAlwaysAsCollection()
{
$this->encoder = new XmlEncoder('response', null);
Expand Down

0 comments on commit bbeca51

Please sign in to comment.