Merge branch 'PHP-8.3'

* PHP-8.3: Fix GH-12870: Creating an xmlns attribute results in a DOMException
php · Dec 7, 2023 · ec79fc9 · ec79fc9
2 parents 7684a3d + e658f80
commit ec79fc9
Show file tree

Hide file tree

Showing 6 changed files with 242 additions and 17 deletions.
diff --git a/ext/dom/document.c b/ext/dom/document.c
@@ -884,34 +884,66 @@ PHP_METHOD(DOM_Document, createAttributeNS)
 	xmlNodePtr nodep = NULL, root;
 	xmlNsPtr nsptr;
 	int ret;
-	size_t uri_len = 0, name_len = 0;
-	char *uri, *name;
+	zend_string *name, *uri;
 	char *localname = NULL, *prefix = NULL;
 	dom_object *intern;
 	int errorcode;
 
 	id = ZEND_THIS;
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s!s", &uri, &uri_len, &name, &name_len) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "S!S", &uri, &name) == FAILURE) {
 		RETURN_THROWS();
 	}
 
 	DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
 
+	if (UNEXPECTED(uri == NULL)) {
+		uri = zend_empty_string;
+	}
+	size_t uri_len = ZSTR_LEN(uri);
+
 	root = xmlDocGetRootElement(docp);
 	if (root != NULL) {
-		errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len);
+		errorcode = dom_check_qname(ZSTR_VAL(name), &localname, &prefix, uri_len, ZSTR_LEN(name));
+		/* TODO: switch to early goto-out style error-checking */
 		if (errorcode == 0) {
 			if (xmlValidateName((xmlChar *) localname, 0) == 0) {
+				/* If prefix is "xml" and namespace is not the XML namespace, then throw a "NamespaceError" DOMException. */
+				if (UNEXPECTED(!zend_string_equals_literal(uri, "http://www.w3.org/XML/1998/namespace") && xmlStrEqual(BAD_CAST prefix, BAD_CAST "xml"))) {
+					errorcode = NAMESPACE_ERR;
+					goto error;
+				}
+				/* If either qualifiedName or prefix is "xmlns" and namespace is not the XMLNS namespace, then throw a "NamespaceError" DOMException. */
+				if (UNEXPECTED((zend_string_equals_literal(name, "xmlns") || xmlStrEqual(BAD_CAST prefix, BAD_CAST "xmlns")) && !zend_string_equals_literal(uri, "http://www.w3.org/2000/xmlns/"))) {
+					errorcode = NAMESPACE_ERR;
+					goto error;
+				}
+				/* If namespace is the XMLNS namespace and neither qualifiedName nor prefix is "xmlns", then throw a "NamespaceError" DOMException. */
+				if (UNEXPECTED(zend_string_equals_literal(uri, "http://www.w3.org/2000/xmlns/") && !zend_string_equals_literal(name, "xmlns") && !xmlStrEqual(BAD_CAST prefix, BAD_CAST "xmlns"))) {
+					errorcode = NAMESPACE_ERR;
+					goto error;
+				}
+
 				nodep = (xmlNodePtr) xmlNewDocProp(docp, (xmlChar *) localname, NULL);
 				if (UNEXPECTED(nodep == NULL)) {
 					php_dom_throw_error(INVALID_STATE_ERR, /* strict */ true);
 					RETURN_THROWS();
 				}
 
 				if (uri_len > 0) {
-					nsptr = xmlSearchNsByHref(nodep->doc, root, (xmlChar *) uri);
-					if (nsptr == NULL || nsptr->prefix == NULL) {
-						nsptr = dom_get_ns(root, uri, &errorcode, prefix ? prefix : "default");
+					nsptr = xmlSearchNsByHref(docp, root, BAD_CAST ZSTR_VAL(uri));
+
+					if (zend_string_equals_literal(name, "xmlns") || xmlStrEqual(BAD_CAST prefix, BAD_CAST "xml")) {
+						if (nsptr == NULL) {
+							nsptr = xmlNewNs(NULL, BAD_CAST ZSTR_VAL(uri), BAD_CAST prefix);
+							php_libxml_set_old_ns(docp, nsptr);
+						}
+					} else {
+						if (nsptr == NULL || nsptr->prefix == NULL) {
+							nsptr = dom_get_ns_unchecked(root, ZSTR_VAL(uri), prefix ? prefix : "default");
+							if (UNEXPECTED(nsptr == NULL)) {
+								errorcode = NAMESPACE_ERR;
+							}
+						}
 					}
 					xmlSetNs(nodep, nsptr);
 				}
@@ -924,6 +956,7 @@ PHP_METHOD(DOM_Document, createAttributeNS)
 		RETURN_FALSE;
 	}
 
+error:
 	xmlFree(localname);
 	if (prefix != NULL) {
 		xmlFree(prefix);

diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c
@@ -1551,7 +1551,9 @@ void php_dom_reconcile_attribute_namespace_after_insertion(xmlAttrPtr attrp)
 		if (matching_ns && xmlStrEqual(matching_ns->href, attrp->ns->href)) {
 			attrp->ns = matching_ns;
 		} else {
-			xmlReconciliateNs(nodep->doc, nodep);
+			if (attrp->ns->prefix != NULL) {
+				xmlReconciliateNs(nodep->doc, nodep);
+			}
 		}
 	}
 }
@@ -1691,23 +1693,30 @@ NAMESPACE_ERR: Raised if
 5. the namespaceURI is "http://www.w3.org/2000/xmlns/" and neither the	qualifiedName nor its prefix is "xmlns".
 */
 
+xmlNsPtr dom_get_ns_unchecked(xmlNodePtr nodep, char *uri, char *prefix)
+{
+	xmlNsPtr 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. */
+		return dom_get_ns_resolve_prefix_conflict(nodep, uri);
+	}
+
+	return nsptr;
+}
+
 /* {{{ 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;
 
 	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")))) {
-		nsptr = xmlNewNs(nodep, (xmlChar *)uri, (xmlChar *)prefix);
+		nsptr = dom_get_ns_unchecked(nodep, uri, 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 (UNEXPECTED(nsptr == NULL)) {
-				goto err;
-			}
+			goto err;
 		}
 	} else {
 		goto err;

diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
@@ -130,6 +130,7 @@ void php_dom_throw_error_with_message(int error_code, char *error_message, int s
 void node_list_unlink(xmlNodePtr node);
 int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, int name_len);
 xmlNsPtr dom_get_ns(xmlNodePtr node, char *uri, int *errorcode, char *prefix);
+xmlNsPtr dom_get_ns_unchecked(xmlNodePtr nodep, char *uri, char *prefix);
 void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep);
 void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last);
 xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName);

diff --git a/ext/dom/tests/gh12870.inc b/ext/dom/tests/gh12870.inc
@@ -0,0 +1,26 @@
+<?php
+
+function test(?string $uri, string $qualifiedName) {
+    $uri_readable = is_null($uri) ? 'NULL' : "\"$uri\"";
+    echo "--- Testing $uri_readable, \"$qualifiedName\" ---\n";
+    $d = new DOMDocument();
+    $d->appendChild($d->createElement('root'));
+    try {
+        $attr = $d->createAttributeNS($uri, $qualifiedName);
+        $d->documentElement->setAttributeNodeNS($attr);
+        echo "Attr prefix: ";
+        var_dump($attr->prefix);
+        echo "Attr namespaceURI: ";
+        var_dump($attr->namespaceURI);
+        echo "Attr value: ";
+        var_dump($attr->value);
+        echo "root namespaceURI: ";
+        var_dump($d->documentElement->namespaceURI);
+        echo "Equality check: ";
+        $parts = explode(':', $qualifiedName);
+        var_dump($attr === $d->documentElement->getAttributeNodeNS($uri, $parts[count($parts) - 1]));
+        echo $d->saveXML(), "\n";
+    } catch (DOMException $e) {
+        echo "Exception: ", $e->getMessage(), "\n\n";
+    }
+}
diff --git a/ext/dom/tests/gh12870_a.phpt b/ext/dom/tests/gh12870_a.phpt
@@ -0,0 +1,72 @@
+--TEST--
+GH-12870 (Creating an xmlns attribute results in a DOMException) - xmlns variations
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+require __DIR__ . '/gh12870.inc';
+
+echo "=== NORMAL CASES ===\n";
+
+test('http://www.w3.org/2000/xmlns/qx', 'foo:xmlns');
+test('http://www.w3.org/2000/xmlns/', 'xmlns');
+test('http://www.w3.org/2000/xmlns/', 'xmlns:xmlns');
+
+echo "=== ERROR CASES ===\n";
+
+test('http://www.w3.org/2000/xmlns/', 'bar:xmlns');
+test('http://www.w3.org/2000/xmlns/a', 'xmlns');
+test('http://www.w3.org/2000/xmlns/a', 'xmlns:bar');
+test(null, 'xmlns:bar');
+test('', 'xmlns');
+test('http://www.w3.org/2000/xmlns/', '');
+
+?>
+--EXPECT--
+=== NORMAL CASES ===
+--- Testing "http://www.w3.org/2000/xmlns/qx", "foo:xmlns" ---
+Attr prefix: string(3) "foo"
+Attr namespaceURI: string(31) "http://www.w3.org/2000/xmlns/qx"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xmlns:foo="http://www.w3.org/2000/xmlns/qx" foo:xmlns=""/>
+
+--- Testing "http://www.w3.org/2000/xmlns/", "xmlns" ---
+Attr prefix: string(0) ""
+Attr namespaceURI: string(29) "http://www.w3.org/2000/xmlns/"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xmlns=""/>
+
+--- Testing "http://www.w3.org/2000/xmlns/", "xmlns:xmlns" ---
+Attr prefix: string(5) "xmlns"
+Attr namespaceURI: string(29) "http://www.w3.org/2000/xmlns/"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xmlns:xmlns="http://www.w3.org/2000/xmlns/" xmlns:xmlns=""/>
+
+=== ERROR CASES ===
+--- Testing "http://www.w3.org/2000/xmlns/", "bar:xmlns" ---
+Exception: Namespace Error
+
+--- Testing "http://www.w3.org/2000/xmlns/a", "xmlns" ---
+Exception: Namespace Error
+
+--- Testing "http://www.w3.org/2000/xmlns/a", "xmlns:bar" ---
+Exception: Namespace Error
+
+--- Testing NULL, "xmlns:bar" ---
+Exception: Namespace Error
+
+--- Testing "", "xmlns" ---
+Exception: Namespace Error
+
+--- Testing "http://www.w3.org/2000/xmlns/", "" ---
+Exception: Namespace Error
diff --git a/ext/dom/tests/gh12870_b.phpt b/ext/dom/tests/gh12870_b.phpt
@@ -0,0 +1,84 @@
+--TEST--
+GH-12870 (Creating an xmlns attribute results in a DOMException) - xml variations
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+require __DIR__ . '/gh12870.inc';
+
+echo "=== NORMAL CASES ===\n";
+
+test('http://www.w3.org/XML/1998/namespaceqx', 'foo:xml');
+test('http://www.w3.org/XML/1998/namespace', 'xml');
+test('http://www.w3.org/XML/1998/namespace', 'bar:xml');
+test('', 'xml');
+test('http://www.w3.org/XML/1998/namespacea', 'xml');
+
+echo "=== ERROR CASES ===\n";
+
+test('http://www.w3.org/XML/1998/namespace', 'xmlns:xml');
+test('http://www.w3.org/XML/1998/namespace', '');
+test('http://www.w3.org/XML/1998/namespacea', 'xml:foo');
+test(NULL, 'xml:foo');
+
+?>
+--EXPECT--
+=== NORMAL CASES ===
+--- Testing "http://www.w3.org/XML/1998/namespaceqx", "foo:xml" ---
+Attr prefix: string(3) "foo"
+Attr namespaceURI: string(38) "http://www.w3.org/XML/1998/namespaceqx"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xmlns:foo="http://www.w3.org/XML/1998/namespaceqx" foo:xml=""/>
+
+--- Testing "http://www.w3.org/XML/1998/namespace", "xml" ---
+Attr prefix: string(3) "xml"
+Attr namespaceURI: string(36) "http://www.w3.org/XML/1998/namespace"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xml:xml=""/>
+
+--- Testing "http://www.w3.org/XML/1998/namespace", "bar:xml" ---
+Attr prefix: string(3) "xml"
+Attr namespaceURI: string(36) "http://www.w3.org/XML/1998/namespace"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xml:xml=""/>
+
+--- Testing "", "xml" ---
+Attr prefix: string(0) ""
+Attr namespaceURI: NULL
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(false)
+<?xml version="1.0"?>
+<root xml=""/>
+
+--- Testing "http://www.w3.org/XML/1998/namespacea", "xml" ---
+Attr prefix: string(7) "default"
+Attr namespaceURI: string(37) "http://www.w3.org/XML/1998/namespacea"
+Attr value: string(0) ""
+root namespaceURI: NULL
+Equality check: bool(true)
+<?xml version="1.0"?>
+<root xmlns:default="http://www.w3.org/XML/1998/namespacea" default:xml=""/>
+
+=== ERROR CASES ===
+--- Testing "http://www.w3.org/XML/1998/namespace", "xmlns:xml" ---
+Exception: Namespace Error
+
+--- Testing "http://www.w3.org/XML/1998/namespace", "" ---
+Exception: Namespace Error
+
+--- Testing "http://www.w3.org/XML/1998/namespacea", "xml:foo" ---
+Exception: Namespace Error
+
+--- Testing NULL, "xml:foo" ---
+Exception: Namespace Error