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.

Author: nielsdos

ext/dom/document.c

@@ -839,7 +839,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);
 			}
@@ -946,7 +946,7 @@ 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) {
+					if (nsptr == NULL || nsptr->prefix == NULL) {
 						nsptr = dom_get_ns(root, uri, &errorcode, prefix);
 					}
 					xmlSetNs(nodep, nsptr);

ext/dom/element.c

@@ -557,45 +557,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
 */
@@ -663,27 +624,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

@@ -1576,6 +1576,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
 
@@ -1596,6 +1625,13 @@ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
 		   (prefix && !strcmp (prefix, "xmlns") && strcmp(uri, (char *)DOM_XMLNS_NAMESPACE)) ||
 		   (prefix && !strcmp(uri, (char *)DOM_XMLNS_NAMESPACE) && strcmp (prefix, "xmlns")))) {
 		nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
+		if (UNEXPECTED(nsptr == NULL)) {
+			/* 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 (nsptr == NULL) {

ext/dom/php_dom.h

@@ -139,6 +139,7 @@ xmlNode *php_dom_libxml_notation_iter(xmlHashTable *ht, int index);
 zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref);
 void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce);
 xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern);
+xmlNsPtr dom_get_ns_resolve_prefix_conflict(xmlNodePtr tree, const char *uri);
 
 void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc);
 void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc);

ext/dom/tests/DOMDocument_createAttributeNS_prefix_conflict.phpt

@@ -45,12 +45,38 @@ var_dump($doc->documentElement->setAttributeNodeNS($doc->createAttributeNS('http
 echo $doc->saveXML(), "\n";
 
 ?>
---EXPECTF--
+--EXPECT--
 --- setAttributeNode variation (you shouldn't do this, but we test this anyway) ---
 NULL
+string(18) "http://php.net/ns1"
+string(18) "http://php.net/ns2"
+string(18) "http://php.net/ns3"
+<?xml version="1.0"?>
+<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=""/>
 
-Fatal error: Uncaught DOMException: Namespace Error in %s:%d
-Stack trace:
-#0 %s(%d): DOMDocument->createAttributeNS('http://php.net/...', 'hello')
-#1 {main}
-  thrown in %s on line %d
+string(18) "http://php.net/ns4"
+string(18) "http://php.net/ns1"
+string(18) "http://php.net/ns2"
+string(18) "http://php.net/ns3"
+<?xml version="1.0"?>
+<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=""/>
+
+NULL
+string(18) "http://php.net/ns1"
+<?xml version="1.0"?>
+<container xmlns:foo="http://php.net/ns1" foo:hello=""/>
+
+--- setAttributeNodeNS variation ---
+string(18) "http://php.net/ns1"
+NULL
+NULL
+NULL
+<?xml version="1.0"?>
+<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=""/>
+
+string(18) "http://php.net/ns1"
+string(18) "http://php.net/ns2"
+string(18) "http://php.net/ns3"
+string(18) "http://php.net/ns4"
+<?xml version="1.0"?>
+<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

@@ -39,7 +39,7 @@ echo $dom2->saveXML();
 <?xml version="1.0"?>
 <container xmlns:foo="http://php.net" foo:bar="yes"/>
 <?xml version="1.0"?>
-<container xmlns:foo="http://php.net/2" bar="yes"/>
+<container 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"/>

ext/dom/tests/DOMElement_setAttributeNS_prefix_conflict.phpt

@@ -22,11 +22,14 @@ echo $dom->saveXML();
 $dom->documentElement->setAttributeNS('http://php.net/2', 'bar', 'no2');
 echo $dom->saveXML();
 ?>
---EXPECTF--
+--EXPECT--
 --- Non-default namespace test case ---
-
-Fatal error: Uncaught DOMException: Namespace Error in %s:%d
-Stack trace:
-#0 %s(%d): DOMElement->setAttributeNS('http://php.net/...', 'foo:bar', 'no1')
-#1 {main}
-  thrown in %s on line %d
+<?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"/>