Fix #79700: Bad performance with namespaced nodes due to wrong libxml assumption

* Use a prepending strategy instead of appending in dom_set_old_ns()

Looping to the end of the list is wasteful. We can just put the new
nodes at the front of the list. I don't believe we can fully prepend,
because libxml2 may assume that the xml namespace is the first one, so
we'll put the new ones as the second one.

* Reuse namespaces from doc->oldNs if possible in dom_get_ns()

* Add a test for reconciling a reused namespace

* Explain why there can't be a cycle between oldNs and nsDef

Closes GH-11376.

Also fixes #77894.

Author: nielsdos

NEWS

@@ -6,6 +6,12 @@ PHP                                                                        NEWS
   . Fix GH-11388 (Allow "final" modifier when importing a method from a trait).
     (nielsdos)
 
+- DOM:
+  . Fix #79700 (wrong use of libxml oldNs leads to performance problem).
+    (nielsdos)
+  . Fix #77894 (DOMNode::C14N() very slow on generated DOMDocuments even after
+    normalisation). (nielsdos)
+
 08 Jun 2023, PHP 8.3.0alpha1
 
 - CLI:

ext/dom/php_dom.c

@@ -1369,11 +1369,16 @@ void dom_normalize (xmlNodePtr nodep)
 
 /* {{{ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) */
 void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) {
-	xmlNs *cur;
-
 	if (doc == NULL)
 		return;
 
+	ZEND_ASSERT(ns->next == NULL);
+
+	/* Note: we'll use a prepend strategy instead of append to
+	 * make sure we don't lose performance when the list is long.
+	 * As libxml2 could assume the xml node is the first one, we'll place our
+	 * new entries after the first one. */
+
 	if (doc->oldNs == NULL) {
 		doc->oldNs = (xmlNsPtr) xmlMalloc(sizeof(xmlNs));
 		if (doc->oldNs == NULL) {
@@ -1383,13 +1388,10 @@ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) {
 		doc->oldNs->type = XML_LOCAL_NAMESPACE;
 		doc->oldNs->href = xmlStrdup(XML_XML_NAMESPACE);
 		doc->oldNs->prefix = xmlStrdup((const xmlChar *)"xml");
+	} else {
+		ns->next = doc->oldNs->next;
 	}
-
-	cur = doc->oldNs;
-	while (cur->next != NULL) {
-		cur = cur->next;
-	}
-	cur->next = ns;
+	doc->oldNs->next = ns;
 }
 /* }}} end dom_set_old_ns */
 
@@ -1411,6 +1413,9 @@ static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep)
 					} else {
 						prevns->next = nsdftptr;
 					}
+					/* Note: we can't get here if the ns is already on the oldNs list.
+					 * This is because in that case the definition won't be on the node, and
+					 * therefore won't be in the nodep->nsDef list. */
 					dom_set_old_ns(doc, curns);
 					curns = prevns;
 				}
@@ -1509,22 +1514,38 @@ NAMESPACE_ERR: Raised if
 
 /* {{{ xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) */
 xmlNsPtr dom_get_ns(xmlNodePtr nodep, char *uri, int *errorcode, char *prefix) {
-	xmlNsPtr nsptr = NULL;
-
-	*errorcode = 0;
+	xmlNsPtr nsptr;
 
 	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;
+		}
+	} else {
+		goto err;
 	}
 
-	if (nsptr == NULL) {
-		*errorcode = NAMESPACE_ERR;
-	}
-
+out:
+	*errorcode = 0;
 	return nsptr;
-
+err:
+	*errorcode = NAMESPACE_ERR;
+	return NULL;
 }
 /* }}} end dom_get_ns */
 

ext/dom/tests/reconcile_reused_namespace.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Reconcile a reused namespace from doc->oldNs
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = new DOMDocument();
+$root = $dom->createElementNS('http://www.w3.org/2000/xhtml', 'html');
+
+$dom->loadXML(<<<XML
+<?xml version="1.0"?>
+<html
+    xmlns="http://www.w3.org/2000/xhtml"
+    xmlns:a="http://example.com/A"
+    xmlns:b="http://example.com/B"
+/>
+XML);
+$root = $dom->firstElementChild;
+
+echo "Add first\n";
+$element = $dom->createElementNS('http://example.com/B', 'p', 'Hello World');
+$root->appendChild($element);
+
+echo "Add second\n";
+$element = $dom->createElementNS('http://example.com/A', 'p', 'Hello World');
+$root->appendChild($element);
+
+echo "Add third\n";
+$element = $dom->createElementNS('http://example.com/A', 'p', 'Hello World');
+$root->appendChild($element);
+
+var_dump($dom->saveXML());
+
+?>
+--EXPECT--
+Add first
+Add second
+Add third
+string(201) "<?xml version="1.0"?>
+<html xmlns="http://www.w3.org/2000/xhtml" xmlns:a="http://example.com/A" xmlns:b="http://example.com/B"><b:p>Hello World</b:p><a:p>Hello World</a:p><a:p>Hello World</a:p></html>
+"