Fix various namespace prefix conflict resolution bugs

ext/dom/document.c

@@ -805,7 +805,7 @@ PHP_METHOD(DOMDocument, importNode)
 			xmlNodePtr root = xmlDocGetRootElement(docp);
 
 			nsptr = xmlSearchNsByHref (nodep->doc, root, nodep->ns->href);
-			if (nsptr == NULL) {
+			if (nsptr == NULL || nsptr->prefix == NULL) {
 				int errorcode;
 				nsptr = dom_get_ns(root, (char *) nodep->ns->href, &errorcode, (char *) nodep->ns->prefix);
 			}
@@ -910,8 +910,8 @@ PHP_METHOD(DOMDocument, createAttributeNS)
 				nodep = (xmlNodePtr) xmlNewDocProp(docp, (xmlChar *) localname, NULL);
 				if (nodep != NULL && uri_len > 0) {
 					nsptr = xmlSearchNsByHref(nodep->doc, root, (xmlChar *) uri);
-					if (nsptr == NULL) {
-						nsptr = dom_get_ns(root, uri, &errorcode, prefix);
+					if (nsptr == NULL || nsptr->prefix == NULL) {
+						nsptr = dom_get_ns(root, uri, &errorcode, prefix ? prefix : "default");
 					}
 					xmlSetNs(nodep, nsptr);
 				}

ext/dom/element.c

@@ -655,45 +655,6 @@ PHP_METHOD(DOMElement, getAttributeNS)
 }
 /* }}} end dom_element_get_attribute_ns */
 
-static xmlNsPtr _dom_new_reconNs(xmlDocPtr doc, xmlNodePtr tree, xmlNsPtr ns) /* {{{ */
-{
-	xmlNsPtr def;
-	xmlChar prefix[50];
-	int counter = 1;
-
-	if ((tree == NULL) || (ns == NULL) || (ns->type != XML_NAMESPACE_DECL)) {
-		return NULL;
-	}
-
-	/* Code taken from libxml2 (2.6.20) xmlNewReconciliedNs
-	 *
-	 * Find a close prefix which is not already in use.
-	 * Let's strip namespace prefixes longer than 20 chars !
-	 */
-	if (ns->prefix == NULL)
-		snprintf((char *) prefix, sizeof(prefix), "default");
-	else
-		snprintf((char *) prefix, sizeof(prefix), "%.20s", (char *)ns->prefix);
-
-	def = xmlSearchNs(doc, tree, prefix);
-	while (def != NULL) {
-		if (counter > 1000) return(NULL);
-		if (ns->prefix == NULL)
-			snprintf((char *) prefix, sizeof(prefix), "default%d", counter++);
-		else
-			snprintf((char *) prefix, sizeof(prefix), "%.20s%d",
-			(char *)ns->prefix, counter++);
-		def = xmlSearchNs(doc, tree, prefix);
-	}
-
-	/*
-	 * OK, now we are ready to create a new one.
-	 */
-	def = xmlNewNs(tree, ns->href, prefix);
-	return(def);
-}
-/* }}} */
-
 /* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-ElSetAttrNS
 Since: DOM Level 2
 */
@@ -756,27 +717,18 @@ PHP_METHOD(DOMElement, setAttributeNS)
 						tmpnsptr = tmpnsptr->next;
 					}
 					if (tmpnsptr == NULL) {
-						nsptr = _dom_new_reconNs(elemp->doc, elemp, nsptr);
+						nsptr = dom_get_ns_resolve_prefix_conflict(elemp, (const char *) nsptr->href);
 					}
 				}
 			}
 
 			if (nsptr == NULL) {
-				if (prefix == NULL) {
-					if (is_xmlns == 1) {
-						xmlNewNs(elemp, (xmlChar *)value, NULL);
-						xmlReconciliateNs(elemp->doc, elemp);
-					} else {
-						errorcode = NAMESPACE_ERR;
-					}
+				if (is_xmlns == 1) {
+					xmlNewNs(elemp, (xmlChar *)value, prefix == NULL ? NULL : (xmlChar *)localname);
 				} else {
-					if (is_xmlns == 1) {
-						xmlNewNs(elemp, (xmlChar *)value, (xmlChar *)localname);
-					} else {
-						nsptr = dom_get_ns(elemp, uri, &errorcode, prefix);
-					}
-					xmlReconciliateNs(elemp->doc, elemp);
+					nsptr = dom_get_ns(elemp, uri, &errorcode, prefix);
 				}
+				xmlReconciliateNs(elemp->doc, elemp);
 			} else {
 				if (is_xmlns == 1) {
 					if (nsptr->href) {

ext/dom/php_dom.c

@@ -32,10 +32,6 @@
 #define PHP_XPATH 1
 #define PHP_XPTR 2
 
-/* libxml2 doesn't expose this constant as part of their public API.
- * See xmlDOMReconcileNSOptions in tree.c */
-#define PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND (1 << 0)
-
 /* {{{ class entries */
 PHP_DOM_EXPORT zend_class_entry *dom_node_class_entry;
 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
 	 * Although libxml2 currently does not use this for the reconciliation, it still
 	 * makes sense to do this just in case libxml2's internal change in the future. */
 	xmlDOMWrapCtxt dummy_ctxt = {0};
-	bool remove_redundant = nodep->nsDef == NULL && nodep->ns != NULL;
-	xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ remove_redundant ? PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND : 0);
+	xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0);
 }
 
 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
 }
 /* }}} */
 
+/* Creates a new namespace declaration with a random prefix with the given uri on the tree.
+ * This is used to resolve a namespace prefix conflict in cases where spec does not want a
+ * namespace error in case of conflicts, but demands a resolution. */
+xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri)
+{
+	ZEND_ASSERT(tree != NULL);
+	xmlDocPtr doc = tree->doc;
+
+	if (UNEXPECTED(doc == NULL)) {
+		return NULL;
+	}
+
+	/* Code adapted from libxml2 (2.10.4) */
+	char prefix[50];
+	int counter = 1;
+	snprintf(prefix, sizeof(prefix), "default");
+	xmlNsPtr nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
+	while (nsptr != NULL) {
+		if (counter > 1000) {
+			return NULL;
+		}
+		snprintf(prefix, sizeof(prefix), "default%d", counter++);
+		nsptr = xmlSearchNs(doc, tree, (const xmlChar *) prefix);
+	}
+
+	/* Search yielded no conflict */
+	return xmlNewNs(tree, (const xmlChar *) uri, (const xmlChar *) prefix);
+}
+
 /*
 http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS
 
@@ -1574,28 +1598,21 @@ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
 	if (! ((prefix && !strcmp (prefix, "xml") && strcmp(uri, (char *)XML_XML_NAMESPACE)) ||
 		   (prefix && !strcmp (prefix, "xmlns") && strcmp(uri, (char *)DOM_XMLNS_NAMESPACE)) ||
 		   (prefix && !strcmp(uri, (char *)DOM_XMLNS_NAMESPACE) && strcmp (prefix, "xmlns")))) {
-		/* Reuse the old namespaces from doc->oldNs if possible, before creating a new one.
-		 * This will prevent the oldNs list from growing with duplicates. */
-		xmlDocPtr doc = nodep->doc;
-		if (doc && doc->oldNs != NULL) {
-			nsptr = doc->oldNs;
-			do {
-				if (xmlStrEqual(nsptr->prefix, (xmlChar *)prefix) && xmlStrEqual(nsptr->href, (xmlChar *)uri)) {
-					goto out;
-				}
-				nsptr = nsptr->next;
-			} while (nsptr);
-		}
-		/* Couldn't reuse one, create a new one. */
 		nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
 		if (UNEXPECTED(nsptr == NULL)) {
-			goto err;
+			/* Either memory allocation failure, or it's because of a prefix conflict.
+			 * We'll assume a conflict and try again. If it was a memory allocation failure we'll just fail again, whatever.
+			 * 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?".
+			 * This branch will also be taken unlikely anyway as in those cases it'll be for allocation failure. */
+			nsptr = dom_get_ns_resolve_prefix_conflict(nodep, uri);
+			if (UNEXPECTED(nsptr == NULL)) {
+				goto err;
+			}
 		}
 	} else {
 		goto err;
 	}
 
-out:
 	*errorcode = 0;
 	return nsptr;
 err:

ext/dom/php_dom.h

@@ -151,6 +151,7 @@ zend_string *dom_node_concatenated_name_helper(size_t name_len, const char *name
 zend_string *dom_node_get_node_name_attribute_or_element(const xmlNode *nodep);
 bool php_dom_is_node_connected(const xmlNode *node);
 bool php_dom_adopt_node(xmlNodePtr nodep, dom_object *dom_object_new_document, xmlDocPtr new_document);
+xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri);
 
 /* parentnode */
 void dom_parent_node_prepend(dom_object *context, zval *nodes, uint32_t nodesc);

ext/dom/tests/DOMDocument_importNode_attribute_prefix_conflict.phpt

@@ -0,0 +1,65 @@
+--TEST--
+DOMDocument::importNode() with attribute prefix name conflict
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+echo "--- Non-default namespace test case without a default namespace in the destination ---\n";
+
+$dom1 = new DOMDocument();
+$dom2 = new DOMDocument();
+$dom1->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:bar="yes"/>');
+$dom2->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net/2"/>');
+$attribute = $dom1->documentElement->getAttributeNode('foo:bar');
+$imported = $dom2->importNode($attribute);
+$dom2->documentElement->setAttributeNodeNS($imported);
+
+echo $dom1->saveXML();
+echo $dom2->saveXML();
+
+echo "--- Non-default namespace test case with a default namespace in the destination ---\n";
+
+$dom1 = new DOMDocument();
+$dom2 = new DOMDocument();
+$dom1->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:bar="yes"/>');
+$dom2->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" xmlns:foo="http://php.net/2"/>');
+$attribute = $dom1->documentElement->getAttributeNode('foo:bar');
+$imported = $dom2->importNode($attribute);
+$dom2->documentElement->setAttributeNodeNS($imported);
+
+echo $dom1->saveXML();
+echo $dom2->saveXML();
+
+echo "--- Default namespace test case ---\n";
+
+// We don't expect the namespace to be imported because default namespaces on the same element don't apply to attributes
+// but the attribute should be imported
+$dom1 = new DOMDocument();
+$dom2 = new DOMDocument();
+$dom1->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" bar="yes"/>');
+$dom2->loadXML('<?xml version="1.0"?><container xmlns="http://php.net/2"/>');
+$attribute = $dom1->documentElement->getAttributeNode('bar');
+$imported = $dom2->importNode($attribute);
+$dom2->documentElement->setAttributeNodeNS($imported);
+
+echo $dom1->saveXML();
+echo $dom2->saveXML();
+
+?>
+--EXPECT--
+--- Non-default namespace test case without a default namespace in the destination ---
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net" foo:bar="yes"/>
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
+--- Non-default namespace test case with a default namespace in the destination ---
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net" foo:bar="yes"/>
+<?xml version="1.0"?>
+<container xmlns="http://php.net" xmlns:foo="http://php.net/2" xmlns:default="http://php.net" default:bar="yes"/>
+--- Default namespace test case ---
+<?xml version="1.0"?>
+<container xmlns="http://php.net" bar="yes"/>
+<?xml version="1.0"?>
+<container xmlns="http://php.net/2" bar="yes"/>

ext/dom/tests/DOMElement_setAttributeNS_prefix_conflict.phpt

@@ -0,0 +1,35 @@
+--TEST--
+DOMElement::setAttributeNS() with prefix name conflict
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+echo "--- Non-default namespace test case ---\n";
+
+$dom = new DOMDocument();
+$dom->loadXML('<?xml version="1.0"?><container xmlns:foo="http://php.net" foo:bar="yes"/>');
+$dom->documentElement->setAttributeNS('http://php.net/2', 'foo:bar', 'no1');
+echo $dom->saveXML();
+$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no2');
+echo $dom->saveXML();
+
+echo "--- Default namespace test case ---\n";
+
+$dom = new DOMDocument();
+$dom->loadXML('<?xml version="1.0"?><container xmlns="http://php.net" bar="yes"/>');
+$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no1');
+echo $dom->saveXML();
+$dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no2');
+echo $dom->saveXML();
+?>
+--EXPECT--
+--- Non-default namespace test case ---
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net" xmlns:default="http://php.net/2" foo:bar="yes" default:bar="no1"/>
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net" xmlns:default="http://php.net/2" foo:bar="yes" default:bar="no2"/>
+--- Default namespace test case ---
+<?xml version="1.0"?>
+<container xmlns="http://php.net" xmlns:default="http://php.net/2" bar="yes" default:bar="no1"/>
+<?xml version="1.0"?>
+<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

@@ -0,0 +1,36 @@
+--TEST--
+DOMDocument::createAttributeNS() with prefix name conflict - setAttributeNodeNS variation, with prefix
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$doc = new DOMDocument();
+$doc->appendChild($doc->createElement('container'));
+
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns1', 'foo:hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns2', 'foo:hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns3', 'foo:hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns4', 'foo:hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+
+?>
+--EXPECT--
+NULL
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net/ns1" foo:hello=""/>
+
+NULL
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net/ns1" xmlns:default="http://php.net/ns2" foo:hello="" default:hello=""/>
+
+NULL
+<?xml version="1.0"?>
+<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=""/>
+
+NULL
+<?xml version="1.0"?>
+<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

@@ -0,0 +1,36 @@
+--TEST--
+DOMDocument::createAttributeNS() with prefix name conflict - setAttributeNodeNS variation, without prefix
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$doc = new DOMDocument();
+$doc->appendChild($doc->createElement('container'));
+
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns1', 'hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns2', 'hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns3', 'hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http://php.net/ns4', 'hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+
+?>
+--EXPECT--
+NULL
+<?xml version="1.0"?>
+<container xmlns:default="http://php.net/ns1" default:hello=""/>
+
+NULL
+<?xml version="1.0"?>
+<container xmlns:default="http://php.net/ns1" xmlns:default1="http://php.net/ns2" default:hello="" default1:hello=""/>
+
+NULL
+<?xml version="1.0"?>
+<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=""/>
+
+NULL
+<?xml version="1.0"?>
+<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=""/>

ext/dom/tests/createAttributeNS_prefix_conflicts/setAttribute_mixed_prefix.phpt

@@ -0,0 +1,20 @@
+--TEST--
+DOMDocument::createAttributeNS() with prefix name conflict - setAttributeNode variation (DOM Level 3), mixed
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$doc = new DOMDocument();
+$doc->appendChild($doc->createElement('container'));
+
+var_dump($doc->documentElement->setAttributeNode($doc->createAttributeNS('http://php.net/ns1', 'foo:hello'))?->namespaceURI);
+var_dump($doc->documentElement->setAttributeNode($doc->createAttributeNS('http://php.net/ns1', 'hello'))?->namespaceURI);
+echo $doc->saveXML(), "\n";
+
+?>
+--EXPECT--
+NULL
+string(18) "http://php.net/ns1"
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net/ns1" foo:hello=""/>