Skip to content

Commit b30be40

Browse files
nielsdosnielsdos
committed
Fix bug #55294 and #47530 and #47847: namespace reconciliation issues
We'll use the DOM wrapper version of libxml2 instead of the regular one. It's conforming to the behaviour we expect of DOM. Most of this patch is tests. I based and extended the tests on the code attached with the aforementioned bug reports. Therefore the credits for the tests: Co-authored-by: hilse at web dot de Co-authored-by: robin2008 at altruists dot org Co-authored-by: sgunderson at bigfoot dot com We'll also change the searching point of the internal reconciliation to start at the top of the added tree to avoid redundant work now that the function is changed. Closes GH-11454.
1 parent 9b18466 commit b30be40

File tree

5 files changed

+233
-10
lines changed

5 files changed

+233
-10
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ PHP NEWS
3636
. Fixed bug #78577 (Crash in DOMNameSpace debug info handlers). (nielsdos)
3737
. Fix lifetime issue with getAttributeNodeNS(). (nielsdos)
3838
. Fix "invalid state error" with cloned namespace declarations. (nielsdos)
39+
. Fixed bug #55294 and #47530 and #47847 (various namespace reconciliation
40+
issues). (nielsdos)
3941

4042
- Opcache:
4143
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)

‎ext/dom/php_dom.c

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) {
14411441
}
14421442
/* }}} end dom_set_old_ns */
14431443

1444-
static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep)
1444+
static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr search_parent)
14451445
{
14461446
xmlNsPtr nsptr, nsdftptr, curns, prevns = NULL;
14471447

@@ -1451,7 +1451,7 @@ static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep)
14511451
while (curns) {
14521452
nsdftptr = curns->next;
14531453
if (curns->href != NULL) {
1454-
if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) &&
1454+
if((nsptr = xmlSearchNsByHref(doc, search_parent, curns->href)) &&
14551455
(curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) {
14561456
curns->next = NULL;
14571457
if (prevns == NULL) {
@@ -1469,23 +1469,34 @@ static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep)
14691469
}
14701470
}
14711471

1472+
static void dom_libxml_reconcile_ensure_namespaces_are_declared(xmlNodePtr nodep)
1473+
{
1474+
/* Put on stack to avoid allocation.
1475+
* Although libxml2 currently does not use this for the reconciliation, it still
1476+
* makes sense to do this just in case libxml2's internal change in the future. */
1477+
xmlDOMWrapCtxt dummy_ctxt = {0};
1478+
xmlDOMWrapReconcileNamespaces(&dummy_ctxt, nodep, /* options */ 0);
1479+
}
1480+
14721481
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
14731482
{
1483+
/* Although the node type will be checked by the libxml2 API,
1484+
* we still want to do the internal reconciliation conditionally. */
14741485
if (nodep->type == XML_ELEMENT_NODE) {
1475-
dom_reconcile_ns_internal(doc, nodep);
1476-
xmlReconciliateNs(doc, nodep);
1486+
dom_reconcile_ns_internal(doc, nodep, nodep->parent);
1487+
dom_libxml_reconcile_ensure_namespaces_are_declared(nodep);
14771488
}
14781489
}
14791490
/* }}} */
14801491

1481-
static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
1492+
static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last, xmlNodePtr search_parent)
14821493
{
14831494
ZEND_ASSERT(nodep != NULL);
14841495
while (true) {
14851496
if (nodep->type == XML_ELEMENT_NODE) {
1486-
dom_reconcile_ns_internal(doc, nodep);
1497+
dom_reconcile_ns_internal(doc, nodep, search_parent);
14871498
if (nodep->children) {
1488-
dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */);
1499+
dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */, search_parent);
14891500
}
14901501
}
14911502
if (nodep == last) {
@@ -1497,10 +1508,12 @@ static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlN
14971508

14981509
void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
14991510
{
1500-
dom_reconcile_ns_list_internal(doc, nodep, last);
1501-
/* Outside of the recursion above because xmlReconciliateNs() performs its own recursion. */
1511+
dom_reconcile_ns_list_internal(doc, nodep, last, nodep->parent);
1512+
/* The loop is outside of the recursion in the above call because
1513+
* dom_libxml_reconcile_ensure_namespaces_are_declared() performs its own recursion. */
15021514
while (true) {
1503-
xmlReconciliateNs(doc, nodep);
1515+
/* The internal libxml2 call will already check the node type, no need for us to do it here. */
1516+
dom_libxml_reconcile_ensure_namespaces_are_declared(nodep);
15041517
if (nodep == last) {
15051518
break;
15061519
}

‎ext/dom/tests/bug47530.phpt

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
--TEST--
2+
Bug #47530 (Importing objects into document fragments creates bogus "default" namespace)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function test_document_fragment_with_import() {
9+
$doc = new DOMDocument;
10+
$doc->loadXML('<html xmlns="https://php.net/something" xmlns:ns="https://php.net/whatever"><element ns:foo="https://php.net/bar"/></html>');
11+
$root = $doc->documentElement;
12+
$frag = $doc->createDocumentFragment();
13+
$frag->appendChild($doc->importNode($root->firstChild));
14+
$root->appendChild($frag);
15+
echo $doc->saveXML();
16+
}
17+
18+
function test_document_fragment_without_import() {
19+
$doc = new DOMDocument;
20+
$doc->loadXML('<html xmlns=""><element xmlns:foo="https://php.net/bar"/></html>');
21+
$frag = $doc->createDocumentFragment();
22+
$frag->appendChild($doc->createElementNS('https://php.net/bar', 'bar'));
23+
$frag->appendChild($doc->createElementNS('', 'bar'));
24+
$element = $doc->documentElement->firstChild;
25+
$element->appendChild($frag);
26+
unset($frag); // Free fragment, should not break getting the namespaceURI below
27+
echo $doc->saveXML();
28+
unset($doc);
29+
var_dump($element->firstChild->tagName);
30+
var_dump($element->firstChild->namespaceURI);
31+
}
32+
33+
function test_document_import() {
34+
$xml = <<<XML
35+
<?xml version="1.0" encoding="utf-8"?>
36+
<feed xmlns="http://www.w3.org/2005/Atom">
37+
<div xmlns="http://www.w3.org/1999/xhtml">
38+
<p>Test-Text</p>
39+
</div>
40+
</feed>
41+
XML;
42+
43+
$dom = new DOMDocument();
44+
$dom->loadXML($xml);
45+
46+
$dom2 = new DOMDocument();
47+
$importedNode = $dom2->importNode($dom->documentElement, true);
48+
$dom2->appendChild($importedNode);
49+
50+
echo $dom2->saveXML();
51+
}
52+
53+
function test_partial_document_import() {
54+
$xml = <<<XML
55+
<?xml version="1.0" encoding="utf-8"?>
56+
<feed xmlns="http://www.w3.org/1999/xhtml" xmlns:test="https://php.net/test" xmlns:example="https://php.net/example">
57+
<div>
58+
<p>Test-Text</p>
59+
<example:p>More test text</example:p>
60+
<test:p>Even more test text</test:p>
61+
</div>
62+
</feed>
63+
XML;
64+
65+
$dom = new DOMDocument();
66+
$dom->loadXML($xml);
67+
68+
$dom2 = new DOMDocument();
69+
$dom2->loadXML('<?xml version="1.0"?><container xmlns:test="https://php.net/test" xmlns="https://php.net/example"/>');
70+
$importedNode = $dom2->importNode($dom->documentElement, true);
71+
$dom2->documentElement->appendChild($importedNode);
72+
73+
// Freeing the original document shouldn't break the other document
74+
unset($importedNode);
75+
unset($dom);
76+
77+
echo $dom2->saveXML();
78+
}
79+
80+
function test_document_import_with_attributes() {
81+
$dom = new DOMDocument();
82+
$dom->loadXML('<?xml version="1.0"?><div xmlns="https://php.net/default" xmlns:example="https://php.net/example"><p example:test="test"/><i/></div>');
83+
$dom2 = new DOMDocument();
84+
$dom2->loadXML('<?xml version="1.0"?><div xmlns:example="https://php.net/somethingelse"/>');
85+
$dom2->documentElement->appendChild($dom2->importNode($dom->documentElement->firstChild));
86+
echo $dom2->saveXML(), "\n";
87+
88+
$dom2->documentElement->firstChild->appendChild($dom2->importNode($dom->documentElement->firstChild->nextSibling));
89+
echo $dom2->saveXML(), "\n";
90+
}
91+
92+
function test_appendChild_with_shadowing() {
93+
$dom = new DOMDocument();
94+
$dom->loadXML('<?xml version="1.0"?><container xmlns:default="http://php.net/default"><a xmlns:foo="http://php.net/bar"/><b xmlns:foo="http://php.net/foo"><default:test foo:bar=""/><foo:test2/></b></container>');
95+
96+
$a = $dom->documentElement->firstElementChild;
97+
$b = $a->nextSibling;
98+
$b->remove();
99+
$a->appendChild($b);
100+
101+
echo $dom->saveXML(), "\n";
102+
}
103+
104+
echo "-- Test document fragment with import --\n";
105+
test_document_fragment_with_import();
106+
echo "-- Test document fragment without import --\n";
107+
test_document_fragment_without_import();
108+
echo "-- Test document import --\n";
109+
test_document_import();
110+
echo "-- Test partial document import --\n";
111+
test_partial_document_import();
112+
echo "-- Test document import with attributes --\n";
113+
test_document_import_with_attributes();
114+
echo "-- Test appendChild with shadowing --\n";
115+
test_appendChild_with_shadowing();
116+
117+
?>
118+
--EXPECT--
119+
-- Test document fragment with import --
120+
<?xml version="1.0"?>
121+
<html xmlns="https://php.net/something" xmlns:ns="https://php.net/whatever"><element ns:foo="https://php.net/bar"/></html>
122+
-- Test document fragment without import --
123+
<?xml version="1.0"?>
124+
<html xmlns=""><element xmlns:foo="https://php.net/bar"><foo:bar/><bar xmlns=""/></element></html>
125+
string(7) "foo:bar"
126+
string(19) "https://php.net/bar"
127+
-- Test document import --
128+
<?xml version="1.0"?>
129+
<feed xmlns="http://www.w3.org/2005/Atom">
130+
<div xmlns="http://www.w3.org/1999/xhtml">
131+
<p>Test-Text</p>
132+
</div>
133+
</feed>
134+
-- Test partial document import --
135+
<?xml version="1.0"?>
136+
<container xmlns:test="https://php.net/test" xmlns="https://php.net/example"><feed xmlns="http://www.w3.org/1999/xhtml" xmlns:example="https://php.net/example">
137+
<div>
138+
<p>Test-Text</p>
139+
<example:p>More test text</example:p>
140+
<test:p>Even more test text</test:p>
141+
</div>
142+
</feed></container>
143+
-- Test document import with attributes --
144+
<?xml version="1.0"?>
145+
<div xmlns:example="https://php.net/somethingelse"><p xmlns="https://php.net/default" xmlns:example="https://php.net/example" example:test="test"/></div>
146+
147+
<?xml version="1.0"?>
148+
<div xmlns:example="https://php.net/somethingelse"><p xmlns="https://php.net/default" xmlns:example="https://php.net/example" example:test="test"><i/></p></div>
149+
150+
-- Test appendChild with shadowing --
151+
<?xml version="1.0"?>
152+
<container xmlns:default="http://php.net/default"><a xmlns:foo="http://php.net/bar"><b xmlns:foo="http://php.net/foo"><default:test foo:bar=""/><foo:test2/></b></a></container>

‎ext/dom/tests/bug47847.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Bug #47847 (importNode loses the namespace of an XML element)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$fromdom = new DOMDocument();
8+
$fromdom->loadXML(<<<XML
9+
<?xml version="1.0"?>
10+
<ns:container xmlns:ns="http://php.net">
11+
<ns:inner xmlns="http://php.net">
12+
<ns:WATCH-MY-NAMESPACE xmlns=""/>
13+
</ns:inner>
14+
</ns:container>
15+
XML);
16+
17+
$aDOM = new DOMDocument();
18+
$imported = $aDOM->importNode($fromdom->documentElement->firstElementChild, true);
19+
$aDOM->appendChild($imported);
20+
21+
echo $aDOM->saveXML();
22+
?>
23+
--EXPECT--
24+
<?xml version="1.0"?>
25+
<ns:inner xmlns="http://php.net" xmlns:ns="http://php.net">
26+
<ns:WATCH-MY-NAMESPACE xmlns=""/>
27+
</ns:inner>

‎ext/dom/tests/bug55294.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
Bug #55294 (DOMDocument::importNode shifts namespaces when "default" namespace exists)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$aDOM = new DOMDocument();
9+
$aDOM->loadXML(<<<EOXML
10+
<A xmlns="http://example.com/A">
11+
<B>
12+
<C xmlns="http://example.com/C" xmlns:default="http://example.com/Z" />
13+
</B>
14+
</A>
15+
EOXML
16+
);
17+
18+
$bDOM = new DOMDocument();
19+
$node = $bDOM->importNode($aDOM->getElementsByTagNameNS('http://example.com/A', 'B')->item(0), true);
20+
$bDOM->appendChild($node);
21+
22+
echo $bDOM->saveXML(), "\n";
23+
24+
?>
25+
--EXPECT--
26+
<?xml version="1.0"?>
27+
<B xmlns="http://example.com/A">
28+
<C xmlns="http://example.com/C" xmlns:default="http://example.com/Z"/>
29+
</B>

0 commit comments

Comments
 (0)