Skip to content

Commit bb3e5a8

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.1' into PHP-8.2
* PHP-8.1: Fix phpGH-11404: DOMDocument::savexml and friends ommit xmlns="" declaration for null namespace, creating incorrect xml representation of the DOM
2 parents 2086740 + 7eb3e9c commit bb3e5a8

File tree

7 files changed

+216
-10
lines changed

7 files changed

+216
-10
lines changed

‎NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ PHP NEWS
3838
. Fix "invalid state error" with cloned namespace declarations. (nielsdos)
3939
. Fixed bug #55294 and #47530 and #47847 (various namespace reconciliation
4040
issues). (nielsdos)
41+
. Fixed bug GH-11404 (DOMDocument::saveXML and friends omit xmlns=""
42+
declaration for null namespace). (nielsdos)
4143

4244
- Opcache:
4345
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)

‎ext/dom/document.c

+4
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,10 @@ PHP_METHOD(DOMDocument, createElementNS)
878878

879879
if (errorcode == 0) {
880880
if (xmlValidateName((xmlChar *) localname, 0) == 0) {
881+
/* https://dom.spec.whatwg.org/#validate-and-extract: demands us to set an empty string uri to NULL */
882+
if (uri_len == 0) {
883+
uri = NULL;
884+
}
881885
nodep = xmlNewDocNode(docp, NULL, (xmlChar *) localname, (xmlChar *) value);
882886
if (nodep != NULL && uri != NULL) {
883887
nsptr = xmlSearchNsByHref(nodep->doc, nodep, (xmlChar *) uri);

‎ext/dom/element.c

+4
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ PHP_METHOD(DOMElement, __construct)
5656
if (uri_len > 0) {
5757
errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len);
5858
if (errorcode == 0) {
59+
/* https://dom.spec.whatwg.org/#validate-and-extract: demands us to set an empty string uri to NULL */
60+
if (uri_len == 0) {
61+
uri = NULL;
62+
}
5963
nodep = xmlNewNode (NULL, (xmlChar *)localname);
6064
if (nodep != NULL && uri != NULL) {
6165
nsptr = dom_get_ns(nodep, uri, &errorcode, prefix);

‎ext/dom/node.c

+7-9
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,6 @@ Since: DOM Level 2
531531
int dom_node_namespace_uri_read(dom_object *obj, zval *retval)
532532
{
533533
xmlNode *nodep = dom_object_get_node(obj);
534-
char *str = NULL;
535534

536535
if (nodep == NULL) {
537536
php_dom_throw_error(INVALID_STATE_ERR, 1);
@@ -543,20 +542,19 @@ int dom_node_namespace_uri_read(dom_object *obj, zval *retval)
543542
case XML_ATTRIBUTE_NODE:
544543
case XML_NAMESPACE_DECL:
545544
if (nodep->ns != NULL) {
546-
str = (char *) nodep->ns->href;
545+
char *str = (char *) nodep->ns->href;
546+
/* https://dom.spec.whatwg.org/#concept-attribute: namespaceUri is "null or a non-empty string" */
547+
if (str != NULL && str[0] != '\0') {
548+
ZVAL_STRING(retval, str);
549+
return SUCCESS;
550+
}
547551
}
548552
break;
549553
default:
550-
str = NULL;
551554
break;
552555
}
553556

554-
if (str != NULL) {
555-
ZVAL_STRING(retval, str);
556-
} else {
557-
ZVAL_NULL(retval);
558-
}
559-
557+
ZVAL_NULL(retval);
560558
return SUCCESS;
561559
}
562560

‎ext/dom/php_dom.c

+38
Original file line numberDiff line numberDiff line change
@@ -1429,13 +1429,33 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep
14291429
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0);
14301430
}
14311431

1432+
static bool dom_must_replace_namespace_by_empty_default(xmlDocPtr doc, xmlNodePtr nodep)
1433+
{
1434+
xmlNsPtr default_ns = xmlSearchNs(doc, nodep->parent, NULL);
1435+
return default_ns != NULL && default_ns->href != NULL && default_ns->href[0] != '\0';
1436+
}
1437+
1438+
static void dom_replace_namespace_by_empty_default(xmlDocPtr doc, xmlNodePtr nodep)
1439+
{
1440+
ZEND_ASSERT(nodep->ns == NULL);
1441+
/* The node uses the default empty namespace, but the current default namespace is non-empty.
1442+
* We can't unconditionally do this because otherwise libxml2 creates an xmlns="" declaration.
1443+
* Note: there's no point searching the oldNs list, because we haven't found it in the tree anyway.
1444+
* Ideally this would be pre-allocated but unfortunately libxml2 doesn't offer such a functionality. */
1445+
xmlSetNs(nodep, xmlNewNs(nodep, (const xmlChar *) "", NULL));
1446+
}
1447+
14321448
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
14331449
{
14341450
/* Although the node type will be checked by the libxml2 API,
14351451
* we still want to do the internal reconciliation conditionally. */
14361452
if (nodep->type == XML_ELEMENT_NODE) {
14371453
dom_reconcile_ns_internal(doc, nodep, nodep->parent);
14381454
dom_libxml_reconcile_ensure_namespaces_are_declared(nodep);
1455+
/* Check nodep->ns first to avoid an expensive lookup. */
1456+
if (nodep->ns == NULL && dom_must_replace_namespace_by_empty_default(doc, nodep)) {
1457+
dom_replace_namespace_by_empty_default(doc, nodep);
1458+
}
14391459
}
14401460
}
14411461
/* }}} */
@@ -1459,12 +1479,30 @@ static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlN
14591479

14601480
void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
14611481
{
1482+
bool did_compute_must_replace_namespace_by_empty_default = false;
1483+
bool must_replace_namespace_by_empty_default = false;
1484+
14621485
dom_reconcile_ns_list_internal(doc, nodep, last, nodep->parent);
1486+
14631487
/* The loop is outside of the recursion in the above call because
14641488
* dom_libxml_reconcile_ensure_namespaces_are_declared() performs its own recursion. */
14651489
while (true) {
14661490
/* The internal libxml2 call will already check the node type, no need for us to do it here. */
14671491
dom_libxml_reconcile_ensure_namespaces_are_declared(nodep);
1492+
1493+
/* We don't have to handle the children, because if their ns's are NULL they'll just take on the default
1494+
* which should've been reconciled before. */
1495+
if (nodep->ns == NULL) {
1496+
/* This is an optimistic approach: we assume that most of the time we don't need the result of the computation. */
1497+
if (!did_compute_must_replace_namespace_by_empty_default) {
1498+
did_compute_must_replace_namespace_by_empty_default = true;
1499+
must_replace_namespace_by_empty_default = dom_must_replace_namespace_by_empty_default(doc, nodep);
1500+
}
1501+
if (must_replace_namespace_by_empty_default) {
1502+
dom_replace_namespace_by_empty_default(doc, nodep);
1503+
}
1504+
}
1505+
14681506
if (nodep == last) {
14691507
break;
14701508
}

‎ext/dom/tests/bug47530.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ test_appendChild_with_shadowing();
121121
<html xmlns="https://php.net/something" xmlns:ns="https://php.net/whatever"><element ns:foo="https://php.net/bar"/></html>
122122
-- Test document fragment without import --
123123
<?xml version="1.0"?>
124-
<html xmlns=""><element xmlns:foo="https://php.net/bar"><foo:bar/><bar xmlns=""/></element></html>
124+
<html xmlns=""><element xmlns:foo="https://php.net/bar"><foo:bar/><bar/></element></html>
125125
string(7) "foo:bar"
126126
string(19) "https://php.net/bar"
127127
-- Test document import --

‎ext/dom/tests/gh11404.phpt

+160
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
--TEST--
2+
GH-11404: DOMDocument::savexml and friends ommit xmlns="" declaration for null namespace, creating incorrect xml representation of the DOM
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
echo "-- Test append and attributes: with default namespace variation --\n";
9+
10+
function testAppendAndAttributes($dom) {
11+
$nodeA = $dom->createElement('a');
12+
$nodeB = $dom->createElementNS(null, 'b');
13+
$nodeC = $dom->createElementNS('', 'c');
14+
$nodeD = $dom->createElement('d');
15+
$nodeD->setAttributeNS('some:ns', 'x:attrib', 'val');
16+
$nodeE = $dom->createElementNS('some:ns', 'e');
17+
// And these two respect the default ns.
18+
$nodeE->setAttributeNS(null, 'attrib1', 'val');
19+
$nodeE->setAttributeNS('', 'attrib2', 'val');
20+
21+
$dom->documentElement->appendChild($nodeA);
22+
$dom->documentElement->appendChild($nodeB);
23+
$dom->documentElement->appendChild($nodeC);
24+
$dom->documentElement->appendChild($nodeD);
25+
$dom->documentElement->appendChild($nodeE);
26+
27+
var_dump($nodeA->namespaceURI);
28+
var_dump($nodeB->namespaceURI);
29+
var_dump($nodeC->namespaceURI);
30+
var_dump($nodeD->namespaceURI);
31+
var_dump($nodeE->namespaceURI);
32+
33+
echo $dom->saveXML();
34+
35+
// Create a subtree without using a fragment
36+
$subtree = $dom->createElement('subtree');
37+
$subtree->appendChild($dom->createElementNS('some:ns', 'subtreechild1'));
38+
$subtree->firstElementChild->appendChild($dom->createElement('subtreechild2'));
39+
$dom->documentElement->appendChild($subtree);
40+
41+
echo $dom->saveXML();
42+
43+
// Create a subtree with the use of a fragment
44+
$subtree = $dom->createDocumentFragment();
45+
$subtree->appendChild($child3 = $dom->createElement('child3'));
46+
$child3->appendChild($dom->createElement('child4'));
47+
$subtree->appendChild($dom->createElement('child5'));
48+
$dom->documentElement->appendChild($subtree);
49+
50+
echo $dom->saveXML();
51+
}
52+
53+
$dom1 = new DOMDocument;
54+
$dom1->loadXML('<?xml version="1.0" ?><with xmlns="some:ns" />');
55+
testAppendAndAttributes($dom1);
56+
57+
echo "-- Test append and attributes: without default namespace variation --\n";
58+
59+
$dom1 = new DOMDocument;
60+
$dom1->loadXML('<?xml version="1.0" ?><with/>');
61+
testAppendAndAttributes($dom1);
62+
63+
echo "-- Test import --\n";
64+
65+
function testImport(?string $href, string $toBeImported) {
66+
$dom1 = new DOMDocument;
67+
$decl = $href === NULL ? '' : "xmlns=\"$href\"";
68+
$dom1->loadXML('<?xml version="1.0" ?><with ' . $decl . '/>');
69+
70+
$dom2 = new DOMDocument;
71+
$dom2->loadXML('<?xml version="1.0" ?>' . $toBeImported);
72+
73+
$dom1->documentElement->append(
74+
$imported = $dom1->importNode($dom2->documentElement, true)
75+
);
76+
77+
var_dump($imported->namespaceURI);
78+
79+
echo $dom1->saveXML();
80+
}
81+
82+
testImport(null, '<none/>');
83+
testImport('', '<none/>');
84+
testImport('some:ns', '<none/>');
85+
testImport('', '<none><div xmlns="some:ns"/></none>');
86+
testImport('some:ns', '<none xmlns="some:ns"><div xmlns=""/></none>');
87+
88+
echo "-- Namespace URI comparison --\n";
89+
90+
$dom1 = new DOMDocument;
91+
$dom1->loadXML('<?xml version="1.0"?><test xmlns="a:b"><div/></test>');
92+
var_dump($dom1->firstElementChild->namespaceURI);
93+
var_dump($dom1->firstElementChild->firstElementChild->namespaceURI);
94+
95+
$dom1 = new DOMDocument;
96+
$dom1->appendChild($dom1->createElementNS('a:b', 'parent'));
97+
$dom1->firstElementChild->appendChild($dom1->createElementNS('a:b', 'child1'));
98+
$dom1->firstElementChild->appendChild($second = $dom1->createElement('child2'));
99+
var_dump($dom1->firstElementChild->namespaceURI);
100+
var_dump($dom1->firstElementChild->firstElementChild->namespaceURI);
101+
var_dump($second->namespaceURI);
102+
echo $dom1->saveXML();
103+
104+
$dom1 = new DOMDocument;
105+
$dom1->loadXML('<?xml version="1.0"?><test xmlns="a:b"/>');
106+
var_dump($dom1->firstElementChild->namespaceURI);
107+
$dom1->firstElementChild->appendChild($dom1->createElementNS('a:b', 'tag'));
108+
var_dump($dom1->firstElementChild->firstElementChild->namespaceURI);
109+
?>
110+
--EXPECT--
111+
-- Test append and attributes: with default namespace variation --
112+
NULL
113+
NULL
114+
NULL
115+
NULL
116+
string(7) "some:ns"
117+
<?xml version="1.0"?>
118+
<with xmlns="some:ns"><a xmlns=""/><b xmlns=""/><c xmlns=""/><d xmlns:x="some:ns" xmlns="" x:attrib="val"/><e attrib1="val" attrib2="val"/></with>
119+
<?xml version="1.0"?>
120+
<with xmlns="some:ns"><a xmlns=""/><b xmlns=""/><c xmlns=""/><d xmlns:x="some:ns" xmlns="" x:attrib="val"/><e attrib1="val" attrib2="val"/><subtree xmlns=""><subtreechild1 xmlns="some:ns"><subtreechild2 xmlns=""/></subtreechild1></subtree></with>
121+
<?xml version="1.0"?>
122+
<with xmlns="some:ns"><a xmlns=""/><b xmlns=""/><c xmlns=""/><d xmlns:x="some:ns" xmlns="" x:attrib="val"/><e attrib1="val" attrib2="val"/><subtree xmlns=""><subtreechild1 xmlns="some:ns"><subtreechild2 xmlns=""/></subtreechild1></subtree><child3 xmlns=""><child4/></child3><child5 xmlns=""/></with>
123+
-- Test append and attributes: without default namespace variation --
124+
NULL
125+
NULL
126+
NULL
127+
NULL
128+
string(7) "some:ns"
129+
<?xml version="1.0"?>
130+
<with><a/><b/><c/><d xmlns:x="some:ns" x:attrib="val"/><e xmlns="some:ns" attrib1="val" attrib2="val"/></with>
131+
<?xml version="1.0"?>
132+
<with><a/><b/><c/><d xmlns:x="some:ns" x:attrib="val"/><e xmlns="some:ns" attrib1="val" attrib2="val"/><subtree><subtreechild1 xmlns="some:ns"><subtreechild2 xmlns=""/></subtreechild1></subtree></with>
133+
<?xml version="1.0"?>
134+
<with><a/><b/><c/><d xmlns:x="some:ns" x:attrib="val"/><e xmlns="some:ns" attrib1="val" attrib2="val"/><subtree><subtreechild1 xmlns="some:ns"><subtreechild2 xmlns=""/></subtreechild1></subtree><child3><child4/></child3><child5/></with>
135+
-- Test import --
136+
NULL
137+
<?xml version="1.0"?>
138+
<with><none/></with>
139+
NULL
140+
<?xml version="1.0"?>
141+
<with xmlns=""><none/></with>
142+
NULL
143+
<?xml version="1.0"?>
144+
<with xmlns="some:ns"><none xmlns=""/></with>
145+
NULL
146+
<?xml version="1.0"?>
147+
<with xmlns=""><none><div xmlns="some:ns"/></none></with>
148+
string(7) "some:ns"
149+
<?xml version="1.0"?>
150+
<with xmlns="some:ns"><none><div xmlns=""/></none></with>
151+
-- Namespace URI comparison --
152+
string(3) "a:b"
153+
string(3) "a:b"
154+
string(3) "a:b"
155+
string(3) "a:b"
156+
NULL
157+
<?xml version="1.0"?>
158+
<parent xmlns="a:b"><child1/><child2 xmlns=""/></parent>
159+
string(3) "a:b"
160+
string(3) "a:b"

0 commit comments

Comments
 (0)