Skip to content

Commit d975b79

Browse files
nielsdosnielsdos
committed
Fix various namespace prefix conflict resolution bugs
The output could still be improved by removing redundant namespace declarations, but that's another issue. At least the output is correct now.
1 parent e02dd27 commit d975b79

7 files changed

+88
-70
lines changed

‎ext/dom/document.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ PHP_METHOD(DOMDocument, importNode)
839839
xmlNodePtr root = xmlDocGetRootElement(docp);
840840

841841
nsptr = xmlSearchNsByHref (nodep->doc, root, nodep->ns->href);
842-
if (nsptr == NULL) {
842+
if (nsptr == NULL || nsptr->prefix == NULL) {
843843
int errorcode;
844844
nsptr = dom_get_ns(root, (char *) nodep->ns->href, &errorcode, (char *) nodep->ns->prefix);
845845
}
@@ -946,7 +946,7 @@ PHP_METHOD(DOMDocument, createAttributeNS)
946946
nodep = (xmlNodePtr) xmlNewDocProp(docp, (xmlChar *) localname, NULL);
947947
if (nodep != NULL && uri_len > 0) {
948948
nsptr = xmlSearchNsByHref(nodep->doc, root, (xmlChar *) uri);
949-
if (nsptr == NULL) {
949+
if (nsptr == NULL || nsptr->prefix == NULL) {
950950
nsptr = dom_get_ns(root, uri, &errorcode, prefix);
951951
}
952952
xmlSetNs(nodep, nsptr);

‎ext/dom/element.c

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -557,45 +557,6 @@ PHP_METHOD(DOMElement, getAttributeNS)
557557
}
558558
/* }}} end dom_element_get_attribute_ns */
559559

560-
static xmlNsPtr _dom_new_reconNs(xmlDocPtr doc, xmlNodePtr tree, xmlNsPtr ns) /* {{{ */
561-
{
562-
xmlNsPtr def;
563-
xmlChar prefix[50];
564-
int counter = 1;
565-
566-
if ((tree == NULL) || (ns == NULL) || (ns->type != XML_NAMESPACE_DECL)) {
567-
return NULL;
568-
}
569-
570-
/* Code taken from libxml2 (2.6.20) xmlNewReconciliedNs
571-
*
572-
* Find a close prefix which is not already in use.
573-
* Let's strip namespace prefixes longer than 20 chars !
574-
*/
575-
if (ns->prefix == NULL)
576-
snprintf((char *) prefix, sizeof(prefix), "default");
577-
else
578-
snprintf((char *) prefix, sizeof(prefix), "%.20s", (char *)ns->prefix);
579-
580-
def = xmlSearchNs(doc, tree, prefix);
581-
while (def != NULL) {
582-
if (counter > 1000) return(NULL);
583-
if (ns->prefix == NULL)
584-
snprintf((char *) prefix, sizeof(prefix), "default%d", counter++);
585-
else
586-
snprintf((char *) prefix, sizeof(prefix), "%.20s%d",
587-
(char *)ns->prefix, counter++);
588-
def = xmlSearchNs(doc, tree, prefix);
589-
}
590-
591-
/*
592-
* OK, now we are ready to create a new one.
593-
*/
594-
def = xmlNewNs(tree, ns->href, prefix);
595-
return(def);
596-
}
597-
/* }}} */
598-
599560
/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-ElSetAttrNS
600561
Since: DOM Level 2
601562
*/
@@ -663,27 +624,18 @@ PHP_METHOD(DOMElement, setAttributeNS)
663624
tmpnsptr = tmpnsptr->next;
664625
}
665626
if (tmpnsptr == NULL) {
666-
nsptr = _dom_new_reconNs(elemp->doc, elemp, nsptr);
627+
nsptr = dom_get_ns_resolve_prefix_conflict(elemp, (const char *) nsptr->href);
667628
}
668629
}
669630
}
670631

671632
if (nsptr == NULL) {
672-
if (prefix == NULL) {
673-
if (is_xmlns == 1) {
674-
xmlNewNs(elemp, (xmlChar *)value, NULL);
675-
xmlReconciliateNs(elemp->doc, elemp);
676-
} else {
677-
errorcode = NAMESPACE_ERR;
678-
}
633+
if (is_xmlns == 1) {
634+
xmlNewNs(elemp, (xmlChar *)value, prefix == NULL ? NULL : (xmlChar *)localname);
679635
} else {
680-
if (is_xmlns == 1) {
681-
xmlNewNs(elemp, (xmlChar *)value, (xmlChar *)localname);
682-
} else {
683-
nsptr = dom_get_ns(elemp, uri, &errorcode, prefix);
684-
}
685-
xmlReconciliateNs(elemp->doc, elemp);
636+
nsptr = dom_get_ns(elemp, uri, &errorcode, prefix);
686637
}
638+
xmlReconciliateNs(elemp->doc, elemp);
687639
} else {
688640
if (is_xmlns == 1) {
689641
if (nsptr->href) {

‎ext/dom/php_dom.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,6 +1576,35 @@ int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, i
15761576
}
15771577
/* }}} */
15781578

1579+
/* Creates a new namespace declaration with a random prefix with the given uri on the tree.
1580+
* This is used to resolve a namespace prefix conflict in cases where spec does not want a
1581+
* namespace error in case of conflicts, but demands a resolution. */
1582+
xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri)
1583+
{
1584+
ZEND_ASSERT(tree != NULL);
1585+
xmlDocPtr doc = tree->doc;
1586+
1587+
if (UNEXPECTED(doc == NULL)) {
1588+
return NULL;
1589+
}
1590+
1591+
/* Code adapted from libxml2 (2.10.4) */
1592+
char prefix[50];
1593+
int counter = 1;
1594+
snprintf(prefix, sizeof(prefix), "default");
1595+
xmlNsPtr nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
1596+
while (nsptr != NULL) {
1597+
if (counter > 1000) {
1598+
return NULL;
1599+
}
1600+
snprintf(prefix, sizeof(prefix), "default%d", counter++);
1601+
nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
1602+
}
1603+
1604+
/* Search yielded no conflict */
1605+
return xmlNewNs(tree, (const xmlChar *) uri, (const xmlChar *) prefix);
1606+
}
1607+
15791608
/*
15801609
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS
15811610
@@ -1596,6 +1625,13 @@ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
15961625
(prefix && !strcmp (prefix, "xmlns") && strcmp(uri, (char *)DOM_XMLNS_NAMESPACE)) ||
15971626
(prefix && !strcmp(uri, (char *)DOM_XMLNS_NAMESPACE) && strcmp (prefix, "xmlns")))) {
15981627
nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
1628+
if (UNEXPECTED(nsptr == NULL)) {
1629+
/* Either memory allocation failure, or it's because of a prefix conflict.
1630+
* We'll assume a conflict and try again. If it was a memory allocation failure we'll just fail again, whatever.
1631+
* 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?".
1632+
* This branch will also be taken unlikely anyway as in those cases it'll be for allocation failure. */
1633+
nsptr = dom_get_ns_resolve_prefix_conflict(nodep, uri);
1634+
}
15991635
}
16001636

16011637
if (nsptr == NULL) {

‎ext/dom/php_dom.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ xmlNode *php_dom_libxml_notation_iter(xmlHashTable *ht, int index);
139139
zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref);
140140
void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce);
141141
xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern);
142+
xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri);
142143

143144
void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc);
144145
void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc);

‎ext/dom/tests/DOMDocument_createAttributeNS_prefix_conflict.phpt

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,38 @@ var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http
4545
echo $doc->saveXML(), "\n";
4646

4747
?>
48-
--EXPECTF--
48+
--EXPECT--
4949
--- setAttributeNode variation (you shouldn't do this, but we test this anyway) ---
5050
NULL
51+
string(18) "http://php.net/ns1"
52+
string(18) "http://php.net/ns2"
53+
string(18) "http://php.net/ns3"
54+
<?xml version="1.0"?>
55+
<container xmlns="http://php.net/ns1" xmlns:default="http://php.net/ns2" xmlns:default1="http://php.net/ns3" xmlns:default2="http://php.net/ns4" default2:hello=""/>
5156

52-
Fatal error: Uncaught DOMException: Namespace Error in %s:%d
53-
Stack trace:
54-
#0 %s(%d): DOMDocument->createAttributeNS('http://php.net/...', 'hello')
55-
#1 {main}
56-
thrown in %s on line %d
57+
string(18) "http://php.net/ns4"
58+
string(18) "http://php.net/ns1"
59+
string(18) "http://php.net/ns2"
60+
string(18) "http://php.net/ns3"
61+
<?xml version="1.0"?>
62+
<container xmlns="http://php.net/ns1" xmlns:default="http://php.net/ns2" xmlns:default1="http://php.net/ns3" xmlns:default2="http://php.net/ns4" xmlns:foo="http://php.net/ns1" default2:hello=""/>
63+
64+
NULL
65+
string(18) "http://php.net/ns1"
66+
<?xml version="1.0"?>
67+
<container xmlns:foo="http://php.net/ns1" foo:hello=""/>
68+
69+
--- setAttributeNodeNS variation ---
70+
string(18) "http://php.net/ns1"
71+
NULL
72+
NULL
73+
NULL
74+
<?xml version="1.0"?>
75+
<container xmlns:foo="http://php.net/ns1" xmlns="http://php.net/ns2" xmlns:default="http://php.net/ns3" xmlns:default1="http://php.net/ns4" foo:hello="" hello="" default:hello="" default1:hello=""/>
76+
77+
string(18) "http://php.net/ns1"
78+
string(18) "http://php.net/ns2"
79+
string(18) "http://php.net/ns3"
80+
string(18) "http://php.net/ns4"
81+
<?xml version="1.0"?>
82+
<container xmlns:foo="http://php.net/ns1" xmlns="http://php.net/ns2" xmlns:default="http://php.net/ns3" xmlns:default1="http://php.net/ns4" xmlns:default2="http://php.net/ns2" foo:hello="" default2:hello="" default:hello="" default1:hello=""/>

‎ext/dom/tests/DOMDocument_importNode_attribute_prefix_conflict.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ echo $dom2->saveXML();
5252
<?xml version="1.0"?>
5353
<container xmlns:foo="http://php.net" foo:bar="yes"/>
5454
<?xml version="1.0"?>
55-
<container xmlns:foo="http://php.net/2" bar="yes"/>
55+
<container xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
5656
--- Non-default namespace test case with a default namespace in the destination ---
5757
<?xml version="1.0"?>
5858
<container xmlns:foo="http://php.net" foo:bar="yes"/>
5959
<?xml version="1.0"?>
60-
<container xmlns="http://php.net" xmlns:foo="http://php.net/2" bar="yes"/>
60+
<container xmlns="http://php.net" xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
6161
--- Default namespace test case ---
6262
<?xml version="1.0"?>
6363
<container xmlns="http://php.net" bar="yes"/>

‎ext/dom/tests/DOMElement_setAttributeNS_prefix_conflict.phpt

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@ echo $dom->saveXML();
2222
$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no2');
2323
echo $dom->saveXML();
2424
?>
25-
--EXPECTF--
25+
--EXPECT--
2626
--- Non-default namespace test case ---
27-
28-
Fatal error: Uncaught DOMException: Namespace Error in %s:%d
29-
Stack trace:
30-
#0 %s(%d): DOMElement->setAttributeNS('http://php.net/...', 'foo:bar', 'no1')
31-
#1 {main}
32-
thrown in %s on line %d
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"/>

0 commit comments

Comments
 (0)