Skip to content

Commit 73a1e48

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 5e601e6 commit 73a1e48

7 files changed

+90
-91
lines changed

‎ext/dom/document.c

Lines changed: 2 additions & 2 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,7 +910,7 @@ 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) {
913+
if (nsptr == NULL || nsptr->prefix == NULL) {
914914
nsptr = dom_get_ns(root, uri, &errorcode, prefix);
915915
}
916916
xmlSetNs(nodep, nsptr);

‎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;
@@ -1502,8 +1498,7 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep
15021498
* Although libxml2 currently does not use this for the reconciliation, it still
15031499
* makes sense to do this just in case libxml2's internal change in the future. */
15041500
xmlDOMWrapCtxt dummy_ctxt = {0};
1505-
bool remove_redundant = nodep->nsDef == NULL && nodep->ns != NULL;
1506-
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ remove_redundant ? PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND : 0);
1501+
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0);
15071502
}
15081503

15091504
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
@@ -1586,6 +1581,35 @@ int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, i
15861581
}
15871582
/* }}} */
15881583

1584+
/* Creates a new namespace declaration with a random prefix with the given uri on the tree.
1585+
* This is used to resolve a namespace prefix conflict in cases where spec does not want a
1586+
* namespace error in case of conflicts, but demands a resolution. */
1587+
xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri)
1588+
{
1589+
ZEND_ASSERT(tree != NULL);
1590+
xmlDocPtr doc = tree->doc;
1591+
1592+
if (UNEXPECTED(doc == NULL)) {
1593+
return NULL;
1594+
}
1595+
1596+
/* Code adapted from libxml2 (2.10.4) */
1597+
char prefix[50];
1598+
int counter = 1;
1599+
snprintf(prefix, sizeof(prefix), "default");
1600+
xmlNsPtr nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
1601+
while (nsptr != NULL) {
1602+
if (counter > 1000) {
1603+
return NULL;
1604+
}
1605+
snprintf(prefix, sizeof(prefix), "default%d", counter++);
1606+
nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
1607+
}
1608+
1609+
/* Search yielded no conflict */
1610+
return xmlNewNs(tree, (const xmlChar *) uri, (const xmlChar *) prefix);
1611+
}
1612+
15891613
/*
15901614
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS
15911615
@@ -1603,28 +1627,21 @@ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
16031627
if (! ((prefix && !strcmp (prefix, "xml") && strcmp(uri, (char *)XML_XML_NAMESPACE)) ||
16041628
(prefix && !strcmp (prefix, "xmlns") && strcmp(uri, (char *)DOM_XMLNS_NAMESPACE)) ||
16051629
(prefix && !strcmp(uri, (char *)DOM_XMLNS_NAMESPACE) && strcmp (prefix, "xmlns")))) {
1606-
/* Reuse the old namespaces from doc->oldNs if possible, before creating a new one.
1607-
* This will prevent the oldNs list from growing with duplicates. */
1608-
xmlDocPtr doc = nodep->doc;
1609-
if (doc && doc->oldNs != NULL) {
1610-
nsptr = doc->oldNs;
1611-
do {
1612-
if (xmlStrEqual(nsptr->prefix, (xmlChar *)prefix) && xmlStrEqual(nsptr->href, (xmlChar *)uri)) {
1613-
goto out;
1614-
}
1615-
nsptr = nsptr->next;
1616-
} while (nsptr);
1617-
}
1618-
/* Couldn't reuse one, create a new one. */
16191630
nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
16201631
if (UNEXPECTED(nsptr == NULL)) {
1621-
goto err;
1632+
/* Either memory allocation failure, or it's because of a prefix conflict.
1633+
* We'll assume a conflict and try again. If it was a memory allocation failure we'll just fail again, whatever.
1634+
* 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?".
1635+
* This branch will also be taken unlikely anyway as in those cases it'll be for allocation failure. */
1636+
nsptr = dom_get_ns_resolve_prefix_conflict(nodep, uri);
1637+
if (UNEXPECTED(nsptr == NULL)) {
1638+
goto err;
1639+
}
16221640
}
16231641
} else {
16241642
goto err;
16251643
}
16261644

1627-
out:
16281645
*errorcode = 0;
16291646
return nsptr;
16301647
err:

‎ext/dom/php_dom.h

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

156157
/* parentnode */
157158
void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t 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)