Skip to content

Commit 961e57e

Browse files
nielsdosnielsdos
committed
Fix phpGH-11500: Namespace reuse in createElementNS() generates wrong output
When you construct a DOM tree containing subtrees which are constructed top-down, this won't remove the redundant namespaces. That's because the following conditions hold: 1) The namespace are reused from the doc->oldNs list. 2) Therefore during reconciliation no nsDef field is set, so no redundant namespaces are removed by our reconciliation code. Furthermore, it would only be fixed up automatically if the tree wasn't added in bottom-up way, or if it had been constructed bottom-up from the start. Fix it by setting a flag to remove redundant namespaces in the libxml2 reconciliation call. Since removing redundant namespaces may have a performance cost, we only do this after performing a simple check. Closes phpGH-11528.
1 parent c0147a0 commit 961e57e

File tree

3 files changed

+166
-1
lines changed

3 files changed

+166
-1
lines changed

‎NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ PHP NEWS
66
. Fixed bug GH-11507 (String concatenation performance regression in 8.3).
77
(nielsdos)
88

9+
- DOM:
10+
. Fixed bug GH-11500 (Namespace reuse in createElementNS() generates wrong
11+
output). (nielsdos)
12+
913
- Fileinfo:
1014
. Fix GH-11408 (Unable to build PHP 8.3.0 alpha 1 / fileinfo extension).
1115
(nielsdos)

‎ext/dom/php_dom.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
#define PHP_XPATH 1
3333
#define PHP_XPTR 2
3434

35+
/* libxml2 doesn't expose this constant as part of their public API.
36+
* See xmlDOMReconcileNSOptions in tree.c */
37+
#define PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND (1 << 0)
38+
3539
/* {{{ class entries */
3640
PHP_DOM_EXPORT zend_class_entry *dom_node_class_entry;
3741
PHP_DOM_EXPORT zend_class_entry *dom_domexception_class_entry;
@@ -1494,7 +1498,8 @@ static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep
14941498
* Although libxml2 currently does not use this for the reconciliation, it still
14951499
* makes sense to do this just in case libxml2's internal change in the future. */
14961500
xmlDOMWrapCtxt dummy_ctxt = {0};
1497-
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0);
1501+
bool remove_redundant = nodep->nsDef == NULL && nodep->ns != NULL;
1502+
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ remove_redundant ? PHP_LIBXML2_DOM_RECONNS_REMOVEREDUND : 0);
14981503
}
14991504

15001505
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */

‎ext/dom/tests/gh11500.phpt

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
--TEST--
2+
GH-11500 (Namespace reuse in createElementNS() generates wrong output)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
function api_test_depth2($root_ns) {
8+
$dom = new DOMDocument();
9+
$root = $dom->createElementNS($root_ns, 'root');
10+
$dom->appendChild($root);
11+
12+
$a1 = $dom->createElementNS('http://example.com', 'a1');
13+
$b1 = $a1->appendChild($dom->createElementNS('http://example.com', 'b1'));
14+
$root->appendChild($a1);
15+
16+
$a2 = $dom->createElementNS('http://example.com', 'a2');
17+
$b2 = $a2->appendChild($dom->createElementNS('http://example.com', 'b2'));
18+
$root->appendChild($a2);
19+
20+
echo $dom->saveXML();
21+
22+
var_dump($root->namespaceURI);
23+
var_dump($a1->namespaceURI);
24+
var_dump($b1->namespaceURI);
25+
var_dump($a2->namespaceURI);
26+
var_dump($b2->namespaceURI);
27+
}
28+
29+
function api_test_depth3($root_ns, $swapped) {
30+
$dom = new DOMDocument();
31+
$root = $dom->createElementNS($root_ns, 'root');
32+
$dom->appendChild($root);
33+
34+
$a1 = $dom->createElementNS('http://example.com', 'a1');
35+
$b1 = $a1->appendChild($dom->createElementNS('http://example.com', 'b1'));
36+
$c1 = $b1->appendChild($dom->createElementNS('http://example.com', 'c1'));
37+
$root->appendChild($a1);
38+
39+
$a2 = $dom->createElementNS('http://example.com', 'a2');
40+
if ($swapped) {
41+
$b2 = $dom->createElementNS('http://example.com', 'b2');
42+
$c2 = $b2->appendChild($dom->createElementNS('http://example.com', 'c2'));
43+
$a2->appendChild($b2);
44+
} else {
45+
$b2 = $a2->appendChild($dom->createElementNS('http://example.com', 'b2'));
46+
$c2 = $b2->appendChild($dom->createElementNS('http://example.com', 'c2'));
47+
}
48+
$root->appendChild($a2);
49+
50+
echo $dom->saveXML();
51+
52+
var_dump($root->namespaceURI);
53+
var_dump($a1->namespaceURI);
54+
var_dump($b1->namespaceURI);
55+
var_dump($c1->namespaceURI);
56+
var_dump($a2->namespaceURI);
57+
var_dump($b2->namespaceURI);
58+
var_dump($c2->namespaceURI);
59+
}
60+
61+
echo "-- Constructed from API (depth 2, mismatched root variation) --\n";
62+
api_test_depth2('http://example2.com');
63+
64+
echo "-- Constructed from API (depth 2, matching root variation) --\n";
65+
api_test_depth2('http://example.com');
66+
67+
echo "-- Constructed from API (depth 3, mismatched root variation, non-swapped) --\n";
68+
api_test_depth3('http://example2.com', false);
69+
70+
echo "-- Constructed from API (depth 3, matching root variation, non-swapped) --\n";
71+
api_test_depth3('http://example.com', false);
72+
73+
echo "-- Constructed from API (depth 3, mismatched root variation, swapped) --\n";
74+
api_test_depth3('http://example2.com', true);
75+
76+
echo "-- Constructed from API (depth 3, matching root variation, swapped) --\n";
77+
api_test_depth3('http://example.com', true);
78+
79+
echo "-- Constructed depth 2 from string --\n";
80+
$xml = '<?xml version="1.0"?><root xmlns="http://example2.com"><a1 xmlns="http://example.com"><b1/></a1><a2 xmlns="http://example.com"><b2/></a2></root>';
81+
$dom = new DOMDocument;
82+
$dom->loadXML($xml);
83+
echo $dom->saveXML(), "\n";
84+
85+
var_dump($dom->documentElement->namespaceURI); // root
86+
var_dump($dom->documentElement->firstChild->namespaceURI); // a1
87+
var_dump($dom->documentElement->firstChild->firstChild->namespaceURI); // b1
88+
var_dump($dom->documentElement->firstChild->nextSibling->namespaceURI); // a2
89+
var_dump($dom->documentElement->firstChild->nextSibling->firstChild->namespaceURI); // b2
90+
?>
91+
--EXPECT--
92+
-- Constructed from API (depth 2, mismatched root variation) --
93+
<?xml version="1.0"?>
94+
<root xmlns="http://example2.com"><a1 xmlns="http://example.com"><b1/></a1><a2 xmlns="http://example.com"><b2/></a2></root>
95+
string(19) "http://example2.com"
96+
string(18) "http://example.com"
97+
string(18) "http://example.com"
98+
string(18) "http://example.com"
99+
string(18) "http://example.com"
100+
-- Constructed from API (depth 2, matching root variation) --
101+
<?xml version="1.0"?>
102+
<root xmlns="http://example.com"><a1><b1/></a1><a2><b2/></a2></root>
103+
string(18) "http://example.com"
104+
string(18) "http://example.com"
105+
string(18) "http://example.com"
106+
string(18) "http://example.com"
107+
string(18) "http://example.com"
108+
-- Constructed from API (depth 3, mismatched root variation, non-swapped) --
109+
<?xml version="1.0"?>
110+
<root xmlns="http://example2.com"><a1 xmlns="http://example.com"><b1><c1/></b1></a1><a2 xmlns="http://example.com"><b2><c2/></b2></a2></root>
111+
string(19) "http://example2.com"
112+
string(18) "http://example.com"
113+
string(18) "http://example.com"
114+
string(18) "http://example.com"
115+
string(18) "http://example.com"
116+
string(18) "http://example.com"
117+
string(18) "http://example.com"
118+
-- Constructed from API (depth 3, matching root variation, non-swapped) --
119+
<?xml version="1.0"?>
120+
<root xmlns="http://example.com"><a1><b1><c1/></b1></a1><a2><b2><c2/></b2></a2></root>
121+
string(18) "http://example.com"
122+
string(18) "http://example.com"
123+
string(18) "http://example.com"
124+
string(18) "http://example.com"
125+
string(18) "http://example.com"
126+
string(18) "http://example.com"
127+
string(18) "http://example.com"
128+
-- Constructed from API (depth 3, mismatched root variation, swapped) --
129+
<?xml version="1.0"?>
130+
<root xmlns="http://example2.com"><a1 xmlns="http://example.com"><b1><c1/></b1></a1><a2 xmlns="http://example.com"><b2><c2/></b2></a2></root>
131+
string(19) "http://example2.com"
132+
string(18) "http://example.com"
133+
string(18) "http://example.com"
134+
string(18) "http://example.com"
135+
string(18) "http://example.com"
136+
string(18) "http://example.com"
137+
string(18) "http://example.com"
138+
-- Constructed from API (depth 3, matching root variation, swapped) --
139+
<?xml version="1.0"?>
140+
<root xmlns="http://example.com"><a1><b1><c1/></b1></a1><a2><b2><c2/></b2></a2></root>
141+
string(18) "http://example.com"
142+
string(18) "http://example.com"
143+
string(18) "http://example.com"
144+
string(18) "http://example.com"
145+
string(18) "http://example.com"
146+
string(18) "http://example.com"
147+
string(18) "http://example.com"
148+
-- Constructed depth 2 from string --
149+
<?xml version="1.0"?>
150+
<root xmlns="http://example2.com"><a1 xmlns="http://example.com"><b1/></a1><a2 xmlns="http://example.com"><b2/></a2></root>
151+
152+
string(19) "http://example2.com"
153+
string(18) "http://example.com"
154+
string(18) "http://example.com"
155+
string(18) "http://example.com"
156+
string(18) "http://example.com"

0 commit comments

Comments
 (0)