Hi everyone,

While looking for things to work on in php-src my friend Thomas pointed me
to a peculiar special case in ext/dom that leads to massive inconsistency
problems in the API.

There is an undocumented class DOMNameSpaceNode that gets returned from
DOMElement::getAttributeNode(NS) if you select an attribute that represents
a namespace (xmlns). This special case is intentionally handled in the
code, contrary to Pythons DOM Extension which doesn't do this and contrary
to the DOM Specification, which does not have a special DOMNameSpaceNode.
Its all DOMAttr.

Problematically DOMNameSpaceNode doesn't extend from DOMAttr, you cannot
pass this class to DOMElement::removeAttributeNode(DOMAttr $attr):

    Fatal error: Uncaught TypeError: Argument 1 passed to
DOMElement::removeAttributeNode() must be an instance of DOMAttr, instance
of DOMNameSpaceNode given

Code example here: https://3v4l.org/jkC5s

In addition the DOMNameSpaceNode renames all properties compared to
DOMAttr, clearly violating the interface documented here
http://php.net/manual/de/domelement.getattributenode.php

Two potential fixes come to mind:

1. Have DOMNameSpaceNode extend DOMAttr - maybe this was the originally
intended behavior, becuase the properties are all named differently?
2. Have DOMElement::removeAttributeNode accept also a DOMNameSpaceNode
(3.) Remove the non standard DOMNameSpaceNode class and use a DOMAttr
instead

I think approach #1 is the right one from a BC perspective and also fixing
the DOM API to be more consistent with the standard.

But I am also not opposed to #3 in the longer term, looking at Github
DOMNamespaceNode in the open source world is only used for one apc test, in
PHP Compat and when dumping it in Symfony Var Dumper. I imagine its more
deeply used in closed source code working deeply with XML.

The second problem that this inconsistency creates is
DOMElement::removeAttributeNS($namespaceUri, $localName). When you want to
use it to remove a namespace attribute (xmlns). By default the xmlns
namespace has the uri http://www.w3.org/2000/xmlns/. Hence removing
xmlns:$name namespace would expected to be:

DOMElement::removeAttributeNS("http://www.w3.org/2000/xmlns/";, "foo") //
removes xmlns:foo

But thats not how it works, there is special code implemented that requires
you to pick the URI from xmlns:foo value:

<root xmlns:foo="urn:foo" />

DOMElement::removeAttributeNS("urn:foo", "foo");

But if your node has an attribute in this namespace with the same name, it
gets removed as well:

<root xmlns:foo="urn:foo" foo:foo="bar" />

Would incorrectly become this, removing 2 attributes:

<root />

This is a bug that cannot be fixed in a BC way. I have no clue how to fix
this completly broken behavior in a good way. I propose to break BC here
and requiring "http://www.w3.org/2000/xmlns/"; as namespaceURI from PHP 8.0
to delete a namespace (xmlns) attribute.

Should I put both issues into an RFC or are these rather bugs that can be
fixed without an RFC?

greetings
Benjamin

Reply via email to