Skip to content

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jan 26, 2019

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

@@ -672,7 +672,7 @@ private function validateAlias(\DOMElement $alias, $file)
}

foreach ($alias->childNodes as $child) {
if (!$child instanceof \DOMElement && self::NS !== $child->namespaceURI) {
if (!$child instanceof \DOMElement || self::NS !== $child->namespaceURI) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally there was no continue statement (this was introduced in #29968). The condition before read like this:

if ($child instanceof \DOMElement && self::NS === $child->namespaceURI) {
    throw new InvalidArgumentException(sprintf('Invalid child element "%s" defined for alias "%s" in "%s".', $child->localName, $alias->getAttribute('id'), $file));
}

This change ensures that we do not access a potentially not existing $namespaceURI property when $child is no DOMElement instance.

@nicolas-grekas
Copy link
Member

Good catch, thanks @xabbuh.

@nicolas-grekas nicolas-grekas merged commit 429bddf into symfony:master Jan 26, 2019
nicolas-grekas added a commit that referenced this pull request Jan 26, 2019
… (xabbuh)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[DependencyInjection] fix DOM element namespace URL access

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

Commits
-------

429bddf fix DOM element namespace URL access
@xabbuh xabbuh deleted the pr-29968 branch January 26, 2019 21:21
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants