Skip to content

Commit

Permalink
Fix GH-12870: Creating an xmlns attribute results in a DOMException
Browse files Browse the repository at this point in the history
There were multiple things here since forever, see the GH thread [1]
for discussion.

There were already many fixes to this function previously, and as a
consequence of one of those fixes this started throwing exceptions for a
correct use-case. It turns out that even when reverting to the previous
behaviour there are still bugs. Just fix all of them while we have the
chance.

[1] #12870

Closes GH-12888.
  • Loading branch information
nielsdos committed Dec 7, 2023
1 parent 8524da6 commit e658f80
Show file tree
Hide file tree
Showing 6 changed files with 239 additions and 16 deletions.
47 changes: 40 additions & 7 deletions ext/dom/document.c
Original file line number Diff line number Diff line change
Expand Up @@ -894,29 +894,61 @@ PHP_METHOD(DOMDocument, 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 (nodep != NULL && 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);
}
Expand All @@ -929,6 +961,7 @@ PHP_METHOD(DOMDocument, createAttributeNS)
RETURN_FALSE;
}

error:
xmlFree(localname);
if (prefix != NULL) {
xmlFree(prefix);
Expand Down
25 changes: 16 additions & 9 deletions ext/dom/php_dom.c
Original file line number Diff line number Diff line change
Expand Up @@ -1596,23 +1596,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;
Expand Down
1 change: 1 addition & 0 deletions ext/dom/php_dom.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,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);
Expand Down
26 changes: 26 additions & 0 deletions ext/dom/tests/gh12870.inc
Original file line number Diff line number Diff line change
@@ -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";
}
}
72 changes: 72 additions & 0 deletions ext/dom/tests/gh12870_a.phpt
Original file line number Diff line number Diff line change
@@ -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
84 changes: 84 additions & 0 deletions ext/dom/tests/gh12870_b.phpt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit e658f80

Please sign in to comment.