Skip to content

Commit 15ff830

Browse files
ndosschendossche
committed
Fix GH-11625: DOMElement::replaceWith() doesn't replace node with DOMDocumentFragment but just deletes node or causes wrapping <></> depending on libxml2 version
Depending on the libxml2 version, the behaviour is either to not render the fragment correctly, or to wrap it inside <></>. Fix it by unpacking fragments manually. This has the side effect that we need to move the unlinking check in the replacement function to earlier because the empty child list is now possible in non-error cases. Also fixes a mistake in the linked list management. Closes GH-11627.
1 parent 0d07b6d commit 15ff830

File tree

3 files changed

+92
-7
lines changed

3 files changed

+92
-7
lines changed

‎NEWS‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ PHP NEWS
2020
- Date:
2121
. Fixed bug GH-11368 (Date modify returns invalid datetime). (Derick)
2222

23+
- DOM:
24+
. Fixed bug GH-11625 (DOMElement::replaceWith() doesn't replace node with
25+
DOMDocumentFragment but just deletes node or causes wrapping <></>
26+
depending on libxml2 version). (nielsdos)
27+
2328
- Fileinfo:
2429
. Fixed bug GH-11298 (finfo returns wrong mime type for xz files). (Anatol)
2530

‎ext/dom/parentnode.c‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,15 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
183183
goto err;
184184
}
185185

186-
if (newNode->parent != NULL) {
186+
if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
187+
/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
188+
newNode = newNode->children;
189+
if (UNEXPECTED(newNode == NULL)) {
190+
/* No nodes to add, nothing to do here */
191+
continue;
192+
}
193+
xmlUnlinkNode(newNode);
194+
} else if (newNode->parent != NULL) {
187195
xmlUnlinkNode(newNode);
188196
}
189197

@@ -370,7 +378,7 @@ static void dom_pre_insert(xmlNodePtr insertion_point, xmlNodePtr parentNode, xm
370378
insertion_point->prev->next = newchild;
371379
newchild->prev = insertion_point->prev;
372380
}
373-
insertion_point->prev = newchild;
381+
insertion_point->prev = fragment->last;
374382
if (parentNode->children == insertion_point) {
375383
parentNode->children = newchild;
376384
}
@@ -555,14 +563,14 @@ void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc)
555563
xmlNodePtr newchild = fragment->children;
556564
xmlDocPtr doc = parentNode->doc;
557565

566+
/* Unlink and free it unless it became a part of the fragment. */
567+
if (child->parent != fragment) {
568+
xmlUnlinkNode(child);
569+
}
570+
558571
if (newchild) {
559572
xmlNodePtr last = fragment->last;
560573

561-
/* Unlink and free it unless it became a part of the fragment. */
562-
if (child->parent != fragment) {
563-
xmlUnlinkNode(child);
564-
}
565-
566574
dom_pre_insert(insertion_point, parentNode, newchild, fragment);
567575

568576
dom_fragment_assign_parent_node(parentNode, fragment);

‎ext/dom/tests/gh11625.phpt‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
--TEST--
2+
GH-11625 (DOMElement::replaceWith() doesn't replace node with DOMDocumentFragment but just deletes node or causes wrapping <></> depending on libxml2 version)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function test($mutator) {
9+
$html = <<<XML
10+
<body>
11+
<div></div><div></div>
12+
</body>
13+
XML;
14+
15+
$dom = new DOMDocument();
16+
$dom->loadXML($html);
17+
18+
$divs = iterator_to_array($dom->getElementsByTagName('div')->getIterator());
19+
$i = 0;
20+
foreach ($divs as $div) {
21+
$mutator($dom, $div, $i);
22+
echo $dom->saveHTML();
23+
$i++;
24+
}
25+
}
26+
27+
echo "--- Single replacement ---\n";
28+
29+
test(function($dom, $div, $i) {
30+
$fragment = $dom->createDocumentFragment();
31+
$fragment->appendXML("<p>Hi $i!</p>");
32+
$div->replaceWith($fragment);
33+
});
34+
35+
echo "--- Multiple replacement ---\n";
36+
37+
test(function($dom, $div, $i) {
38+
$fragment = $dom->createDocumentFragment();
39+
$fragment->appendXML("<p>Hi $i!</p>");
40+
$div->replaceWith($fragment, $dom->createElement('x'), "hello");
41+
});
42+
43+
echo "--- Empty fragment replacement ---\n";
44+
45+
test(function($dom, $div, $i) {
46+
$fragment = $dom->createDocumentFragment();
47+
$div->replaceWith($fragment);
48+
});
49+
50+
?>
51+
--EXPECT--
52+
--- Single replacement ---
53+
<body>
54+
<p>Hi 0!</p><div></div>
55+
</body>
56+
<body>
57+
<p>Hi 0!</p><p>Hi 1!</p>
58+
</body>
59+
--- Multiple replacement ---
60+
<body>
61+
<p>Hi 0!</p><x></x>hello<div></div>
62+
</body>
63+
<body>
64+
<p>Hi 0!</p><x></x>hello<p>Hi 1!</p><x></x>hello
65+
</body>
66+
--- Empty fragment replacement ---
67+
<body>
68+
<div></div>
69+
</body>
70+
<body>
71+
72+
</body>

0 commit comments

Comments
 (0)