Skip to content

Commit d46dc56

Browse files
nielsdosnielsdos
committed
Fix various namespace prefix conflict resolution bugs and namespace shift bugs
There are two linked issues: - Conflicts couldn't be resolved by changing the prefix name. - Lacking a prefix would shift the namespace as the default namespace, causing elements to suddenly become part of the namespace instead of the attributes. The output could still be improved by removing redundant namespace declarations, but that's another issue. At least the output is correct now. Closes GH-11777.
1 parent 82972f4 commit d46dc56

14 files changed

+323
-80
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ PHP NEWS
2020
. Align DOMChildNode parent checks with spec. (nielsdos)
2121
. Fixed bug #80927 (Removing documentElement after creating attribute node:
2222
possible use-after-free). (nielsdos)
23+
. Fix various namespace prefix conflict resolution bugs. (nielsdos)
24+
. Fix calling createAttributeNS() without prefix causing the default
25+
namespace of the element to change. (nielsdos)
2326

2427
- Opcache:
2528
. Avoid resetting JIT counter handlers from multiple processes/threads.

‎UPGRADING

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ PHP 8.3 UPGRADE NOTES
5353
. Using the DOMParentNode and DOMChildNode methods without a document now works
5454
instead of throwing a HIERARCHY_REQUEST_ERR DOMException. This is in line with
5555
the behaviour spec demands.
56+
. createAttributeNS() without specifying a prefix would incorrectly create a default
57+
namespace, placing the element inside the namespace instead of the attribute.
58+
This bug is now fixed.
59+
. createAttributeNS() would previously incorrectly throw a NAMESPACE_ERR when the
60+
prefix was already used for a different uri. It now correctly chooses a
61+
different prefix when there's a prefix name conflict.
5662

5763
- FFI:
5864
. C functions that have a return type of void now return null instead of

‎ext/dom/document.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,7 @@ PHP_METHOD(DOMDocument, importNode)
805805
xmlNodePtr root = xmlDocGetRootElement(docp);
806806

807807
nsptr = xmlSearchNsByHref (nodep->doc, root, nodep->ns->href);
808-
if (nsptr == NULL) {
808+
if (nsptr == NULL || nsptr->prefix == NULL) {
809809
int errorcode;
810810
nsptr = dom_get_ns(root, (char *) nodep->ns->href, &errorcode, (char *) nodep->ns->prefix);
811811
}
@@ -910,8 +910,8 @@ PHP_METHOD(DOMDocument, createAttributeNS)
910910
nodep = (xmlNodePtr) xmlNewDocProp(docp, (xmlChar *) localname, NULL);
911911
if (nodep != NULL && uri_len > 0) {
912912
nsptr = xmlSearchNsByHref(nodep->doc, root, (xmlChar *) uri);
913-
if (nsptr == NULL) {
914-
nsptr = dom_get_ns(root, uri, &errorcode, prefix);
913+
if (nsptr == NULL || nsptr->prefix == NULL) {
914+
nsptr = dom_get_ns(root, uri, &errorcode, prefix ? prefix : "default");
915915
}
916916
xmlSetNs(nodep, nsptr);
917917
}

‎ext/dom/element.c

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -655,45 +655,6 @@ PHP_METHOD(DOMElement, getAttributeNS)
655655
}
656656
/* }}} end dom_element_get_attribute_ns */
657657

658-
static xmlNsPtr _dom_new_reconNs(xmlDocPtr doc, xmlNodePtr tree, xmlNsPtr ns) /* {{{ */
659-
{
660-
xmlNsPtr def;
661-
xmlChar prefix[50];
662-
int counter = 1;
663-
664-
if ((tree == NULL) || (ns == NULL) || (ns->type != XML_NAMESPACE_DECL)) {
665-
return NULL;
666-
}
667-
668-
/* Code taken from libxml2 (2.6.20) xmlNewReconciliedNs
669-
*
670-
* Find a close prefix which is not already in use.
671-
* Let's strip namespace prefixes longer than 20 chars !
672-
*/
673-
if (ns->prefix == NULL)
674-
snprintf((char *) prefix, sizeof(prefix), "default");
675-
else
676-
snprintf((char *) prefix, sizeof(prefix), "%.20s", (char *)ns->prefix);
677-
678-
def = xmlSearchNs(doc, tree, prefix);
679-
while (def != NULL) {
680-
if (counter > 1000) return(NULL);
681-
if (ns->prefix == NULL)
682-
snprintf((char *) prefix, sizeof(prefix), "default%d", counter++);
683-
else
684-
snprintf((char *) prefix, sizeof(prefix), "%.20s%d",
685-
(char *)ns->prefix, counter++);
686-
def = xmlSearchNs(doc, tree, prefix);
687-
}
688-
689-
/*
690-
* OK, now we are ready to create a new one.
691-
*/
692-
def = xmlNewNs(tree, ns->href, prefix);
693-
return(def);
694-
}
695-
/* }}} */
696-
697658
/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-ElSetAttrNS
698659
Since: DOM Level 2
699660
*/
@@ -756,27 +717,18 @@ PHP_METHOD(DOMElement, setAttributeNS)
756717
tmpnsptr = tmpnsptr->next;
757718
}
758719
if (tmpnsptr == NULL) {
759-
nsptr = _dom_new_reconNs(elemp->doc, elemp, nsptr);
720+
nsptr = dom_get_ns_resolve_prefix_conflict(elemp, (const char *) nsptr->href);
760721
}
761722
}
762723
}
763724

764725
if (nsptr == NULL) {
765-
if (prefix == NULL) {
766-
if (is_xmlns == 1) {
767-
xmlNewNs(elemp, (xmlChar *)value, NULL);
768-
xmlReconciliateNs(elemp->doc, elemp);
769-
} else {
770-
errorcode = NAMESPACE_ERR;
771-
}
726+
if (is_xmlns == 1) {
727+
xmlNewNs(elemp, (xmlChar *)value, prefix == NULL ? NULL : (xmlChar *)localname);
772728
} else {
773-
if (is_xmlns == 1) {
774-
xmlNewNs(elemp, (xmlChar *)value, (xmlChar *)localname);
775-
} else {
776-
nsptr = dom_get_ns(elemp, uri, &errorcode, prefix);
777-
}
778-
xmlReconciliateNs(elemp->doc, elemp);
729+
nsptr = dom_get_ns(elemp, uri, &errorcode, prefix);
779730
}
731+
xmlReconciliateNs(elemp->doc, elemp);
780732
} else {
781733
if (is_xmlns == 1) {
782734
if (nsptr->href) {

‎ext/dom/php_dom.c

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@
3232
#define PHP_XPATH 1
3333
#define PHP_XPTR 2
3434

35-
/* libxml2 doesn't expose this constant as part of their public API.
36-
* See xmlDOMReconcileNSOptions in tree.c */
37-
#define PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND (1 << 0)
38-
3935
/* {{{ class entries */
4036
PHP_DOM_EXPORT zend_class_entry *dom_node_class_entry;
4137
PHP_DOM_EXPORT zend_class_entry *dom_domexception_class_entry;
@@ -1473,8 +1469,7 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep
14731469
* Although libxml2 currently does not use this for the reconciliation, it still
14741470
* makes sense to do this just in case libxml2's internal change in the future. */
14751471
xmlDOMWrapCtxt dummy_ctxt = {0};
1476-
bool remove_redundant = nodep->nsDef == NULL && nodep->ns != NULL;
1477-
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ remove_redundant ? PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND : 0);
1472+
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0);
14781473
}
14791474

14801475
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
@@ -1557,6 +1552,35 @@ int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, i
15571552
}
15581553
/* }}} */
15591554

1555+
/* Creates a new namespace declaration with a random prefix with the given uri on the tree.
1556+
* This is used to resolve a namespace prefix conflict in cases where spec does not want a
1557+
* namespace error in case of conflicts, but demands a resolution. */
1558+
xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri)
1559+
{
1560+
ZEND_ASSERT(tree != NULL);
1561+
xmlDocPtr doc = tree->doc;
1562+
1563+
if (UNEXPECTED(doc == NULL)) {
1564+
return NULL;
1565+
}
1566+
1567+
/* Code adapted from libxml2 (2.10.4) */
1568+
char prefix[50];
1569+
int counter = 1;
1570+
snprintf(prefix, sizeof(prefix), "default");
1571+
xmlNsPtr nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
1572+
while (nsptr != NULL) {
1573+
if (counter > 1000) {
1574+
return NULL;
1575+
}
1576+
snprintf(prefix, sizeof(prefix), "default%d", counter++);
1577+
nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
1578+
}
1579+
1580+
/* Search yielded no conflict */
1581+
return xmlNewNs(tree, (const xmlChar *) uri, (const xmlChar *) prefix);
1582+
}
1583+
15601584
/*
15611585
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS
15621586
@@ -1574,28 +1598,21 @@ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
15741598
if (! ((prefix && !strcmp (prefix, "xml") && strcmp(uri, (char *)XML_XML_NAMESPACE)) ||
15751599
(prefix && !strcmp (prefix, "xmlns") && strcmp(uri, (char *)DOM_XMLNS_NAMESPACE)) ||
15761600
(prefix && !strcmp(uri, (char *)DOM_XMLNS_NAMESPACE) && strcmp (prefix, "xmlns")))) {
1577-
/* Reuse the old namespaces from doc->oldNs if possible, before creating a new one.
1578-
* This will prevent the oldNs list from growing with duplicates. */
1579-
xmlDocPtr doc = nodep->doc;
1580-
if (doc && doc->oldNs != NULL) {
1581-
nsptr = doc->oldNs;
1582-
do {
1583-
if (xmlStrEqual(nsptr->prefix, (xmlChar *)prefix) && xmlStrEqual(nsptr->href, (xmlChar *)uri)) {
1584-
goto out;
1585-
}
1586-
nsptr = nsptr->next;
1587-
} while (nsptr);
1588-
}
1589-
/* Couldn't reuse one, create a new one. */
15901601
nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
15911602
if (UNEXPECTED(nsptr == NULL)) {
1592-
goto err;
1603+
/* Either memory allocation failure, or it's because of a prefix conflict.
1604+
* We'll assume a conflict and try again. If it was a memory allocation failure we'll just fail again, whatever.
1605+
* This isn't needed for every caller (such as createElementNS & DOMElement::__construct), but isn't harmful and simplifies the mental model "when do I use which function?".
1606+
* This branch will also be taken unlikely anyway as in those cases it'll be for allocation failure. */
1607+
nsptr = dom_get_ns_resolve_prefix_conflict(nodep, uri);
1608+
if (UNEXPECTED(nsptr == NULL)) {
1609+
goto err;
1610+
}
15931611
}
15941612
} else {
15951613
goto err;
15961614
}
15971615

1598-
out:
15991616
*errorcode = 0;
16001617
return nsptr;
16011618
err:

‎ext/dom/php_dom.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ zend_string *dom_node_concatenated_name_helper(size_t name_len, const char *name
151151
zend_string *dom_node_get_node_name_attribute_or_element(const xmlNode *nodep);
152152
bool php_dom_is_node_connected(const xmlNode *node);
153153
bool php_dom_adopt_node(xmlNodePtr nodep, dom_object *dom_object_new_document, xmlDocPtr new_document);
154+
xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri);
154155

155156
/* parentnode */
156157
void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc);

‎ext/dom/tests/DOMDocument_importNode_attribute_prefix_conflict.phpt

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
DOMDocument::importNode() with attribute prefix name conflict
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
echo "--- Non-default namespace test case without a default namespace in the destination ---\n";
9+
10+
$dom1 = new DOMDocument();
11+
$dom2 = new DOMDocument();
12+
$dom1->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:bar="yes"/>');
13+
$dom2->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net/2"/>');
14+
$attribute = $dom1->documentElement->getAttributeNode('foo:bar');
15+
$imported = $dom2->importNode($attribute);
16+
$dom2->documentElement->setAttributeNodeNS($imported);
17+
18+
echo $dom1->saveXML();
19+
echo $dom2->saveXML();
20+
21+
echo "--- Non-default namespace test case with a default namespace in the destination ---\n";
22+
23+
$dom1 = new DOMDocument();
24+
$dom2 = new DOMDocument();
25+
$dom1->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:bar="yes"/>');
26+
$dom2->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" xmlns:foo="http://php.net/2"/>');
27+
$attribute = $dom1->documentElement->getAttributeNode('foo:bar');
28+
$imported = $dom2->importNode($attribute);
29+
$dom2->documentElement->setAttributeNodeNS($imported);
30+
31+
echo $dom1->saveXML();
32+
echo $dom2->saveXML();
33+
34+
echo "--- Default namespace test case ---\n";
35+
36+
// We don't expect the namespace to be imported because default namespaces on the same element don't apply to attributes
37+
// but the attribute should be imported
38+
$dom1 = new DOMDocument();
39+
$dom2 = new DOMDocument();
40+
$dom1->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" bar="yes"/>');
41+
$dom2->loadXML('<?xml version="1.0"?><container xmlns="http://php.net/2"/>');
42+
$attribute = $dom1->documentElement->getAttributeNode('bar');
43+
$imported = $dom2->importNode($attribute);
44+
$dom2->documentElement->setAttributeNodeNS($imported);
45+
46+
echo $dom1->saveXML();
47+
echo $dom2->saveXML();
48+
49+
?>
50+
--EXPECT--
51+
--- Non-default namespace test case without a default namespace in the destination ---
52+
<?xml version="1.0"?>
53+
<container xmlns:foo="http://php.net" foo:bar="yes"/>
54+
<?xml version="1.0"?>
55+
<container xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
56+
--- Non-default namespace test case with a default namespace in the destination ---
57+
<?xml version="1.0"?>
58+
<container xmlns:foo="http://php.net" foo:bar="yes"/>
59+
<?xml version="1.0"?>
60+
<container xmlns="http://php.net" xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
61+
--- Default namespace test case ---
62+
<?xml version="1.0"?>
63+
<container xmlns="http://php.net" bar="yes"/>
64+
<?xml version="1.0"?>
65+
<container xmlns="http://php.net/2" bar="yes"/>

‎ext/dom/tests/DOMElement_setAttributeNS_prefix_conflict.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
DOMElement::setAttributeNS() with prefix name conflict
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
echo "--- Non-default namespace test case ---\n";
8+
9+
$dom = new DOMDocument();
10+
$dom->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:bar="yes"/>');
11+
$dom->documentElement->setAttributeNS('http://php.net/2', 'foo:bar', 'no1');
12+
echo $dom->saveXML();
13+
$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no2');
14+
echo $dom->saveXML();
15+
16+
echo "--- Default namespace test case ---\n";
17+
18+
$dom = new DOMDocument();
19+
$dom->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" bar="yes"/>');
20+
$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no1');
21+
echo $dom->saveXML();
22+
$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no2');
23+
echo $dom->saveXML();
24+
?>
25+
--EXPECT--
26+
--- Non-default namespace test case ---
27+
<?xml version="1.0"?>
28+
<container xmlns:foo="http://php.net" xmlns:default="http://php.net/2" foo:bar="yes" default:bar="no1"/>
29+
<?xml version="1.0"?>
30+
<container xmlns:foo="http://php.net" xmlns:default="http://php.net/2" foo:bar="yes" default:bar="no2"/>
31+
--- Default namespace test case ---
32+
<?xml version="1.0"?>
33+
<container xmlns="http://php.net" xmlns:default="http://php.net/2" bar="yes" default:bar="no1"/>
34+
<?xml version="1.0"?>
35+
<container xmlns="http://php.net" xmlns:default="http://php.net/2" bar="yes" default:bar="no2"/>

‎ext/dom/tests/createAttributeNS_prefix_conflicts/setAttributeNS_with_prefix.phpt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
DOMDocument::createAttributeNS() with prefix name conflict - setAttributeNodeNS variation, with prefix
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$doc = new DOMDocument();
9+
$doc->appendChild($doc->createElement('container'));
10+
11+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns1', 'foo:hello'))?->namespaceURI);
12+
echo $doc->saveXML(), "\n";
13+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns2', 'foo:hello'))?->namespaceURI);
14+
echo $doc->saveXML(), "\n";
15+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns3', 'foo:hello'))?->namespaceURI);
16+
echo $doc->saveXML(), "\n";
17+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns4', 'foo:hello'))?->namespaceURI);
18+
echo $doc->saveXML(), "\n";
19+
20+
?>
21+
--EXPECT--
22+
NULL
23+
<?xml version="1.0"?>
24+
<container xmlns:foo="http://php.net/ns1" foo:hello=""/>
25+
26+
NULL
27+
<?xml version="1.0"?>
28+
<container xmlns:foo="http://php.net/ns1" xmlns:default="http://php.net/ns2" foo:hello="" default:hello=""/>
29+
30+
NULL
31+
<?xml version="1.0"?>
32+
<container xmlns:foo="http://php.net/ns1" xmlns:default="http://php.net/ns2" xmlns:default1="http://php.net/ns3" foo:hello="" default:hello="" default1:hello=""/>
33+
34+
NULL
35+
<?xml version="1.0"?>
36+
<container xmlns:foo="http://php.net/ns1" xmlns:default="http://php.net/ns2" xmlns:default1="http://php.net/ns3" xmlns:default2="http://php.net/ns4" foo:hello="" default:hello="" default1:hello="" default2:hello=""/>

‎ext/dom/tests/createAttributeNS_prefix_conflicts/setAttributeNS_without_prefix.phpt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
DOMDocument::createAttributeNS() with prefix name conflict - setAttributeNodeNS variation, without prefix
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$doc = new DOMDocument();
9+
$doc->appendChild($doc->createElement('container'));
10+
11+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns1', 'hello'))?->namespaceURI);
12+
echo $doc->saveXML(), "\n";
13+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns2', 'hello'))?->namespaceURI);
14+
echo $doc->saveXML(), "\n";
15+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns3', 'hello'))?->namespaceURI);
16+
echo $doc->saveXML(), "\n";
17+
var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns4', 'hello'))?->namespaceURI);
18+
echo $doc->saveXML(), "\n";
19+
20+
?>
21+
--EXPECT--
22+
NULL
23+
<?xml version="1.0"?>
24+
<container xmlns:default="http://php.net/ns1" default:hello=""/>
25+
26+
NULL
27+
<?xml version="1.0"?>
28+
<container xmlns:default="http://php.net/ns1" xmlns:default1="http://php.net/ns2" default:hello="" default1:hello=""/>
29+
30+
NULL
31+
<?xml version="1.0"?>
32+
<container xmlns:default="http://php.net/ns1" xmlns:default1="http://php.net/ns2" xmlns:default2="http://php.net/ns3" default:hello="" default1:hello="" default2:hello=""/>
33+
34+
NULL
35+
<?xml version="1.0"?>
36+
<container xmlns:default="http://php.net/ns1" xmlns:default1="http://php.net/ns2" xmlns:default2="http://php.net/ns3" xmlns:default3="http://php.net/ns4" default:hello="" default1:hello="" default2:hello="" default3:hello=""/>

0 commit comments

Comments
 (0)