Skip to content

Commit a7bdb8b

Browse files
committed
Fix GH-23365: DOMNode::insertBefore() drops the node used as its own reference
insertBefore($n, $n) unlinked the node, then rebuilt its position from the pointers the unlink had just cleared, leaving it out of the document with a self-referencing sibling list, freed twice at teardown. Retarget the reference to the node's next sibling, as the modern DOM already does. Close GH-23379
1 parent 293f22e commit a7bdb8b

3 files changed

Lines changed: 59 additions & 1 deletion

File tree

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ PHP NEWS
66
. Fixed use-after-free when re-constructing a DOMXPath whose php:function
77
registrations are freed while still reachable from the cycle collector.
88
(Ilia Alshanetsky)
9+
. Fixed bug GH-23365 (DOMNode::insertBefore($n, $n) drops the node and
10+
leaves a self-referencing sibling list). (David Carlier)
911

1012
- Intl:
1113
. Fixed cloning IntlDateFormatter and MessageFormatter losing PHP-side state

ext/dom/node.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,9 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
931931
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
932932
RETURN_FALSE;
933933
}
934+
if (refp == child) {
935+
refp = child->next;
936+
}
934937
}
935938

936939
if (child->doc == NULL && parentp->doc != NULL) {
@@ -940,7 +943,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
940943

941944
php_libxml_invalidate_node_list_cache(intern->document);
942945

943-
if (ref != NULL) {
946+
if (refp != NULL) {
944947
if (child->parent != NULL) {
945948
xmlUnlinkNode(child);
946949
}

ext/dom/tests/gh23365.phpt

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--TEST--
2+
GH-23365 (DOMNode::insertBefore($n, $n) drops the node and leaves a self-referencing sibling list)
3+
--CREDITS--
4+
Alexandre Daubois
5+
--EXTENSIONS--
6+
dom
7+
--FILE--
8+
<?php
9+
$doc = new DOMDocument();
10+
$doc->loadXML('<root>text<child/></root>');
11+
$root = $doc->documentElement;
12+
13+
$text = $root->firstChild;
14+
var_dump($root->insertBefore($text, $text) === $text);
15+
var_dump($root->childNodes->length);
16+
var_dump($text->parentNode === $root, $text->nextSibling === $text, $text->previousSibling === $text);
17+
18+
$child = $root->lastChild;
19+
var_dump($root->insertBefore($child, $child) === $child);
20+
var_dump($root->childNodes->length);
21+
22+
echo $doc->saveXML($root), PHP_EOL;
23+
24+
$doc2 = new DOMDocument();
25+
$doc2->loadXML('<root a="1" b="2"/>');
26+
$el = $doc2->documentElement;
27+
$attr = $el->getAttributeNode('a');
28+
var_dump($el->insertBefore($attr, $attr) === $attr);
29+
echo $doc2->saveXML($el), PHP_EOL;
30+
31+
$doc3 = new DOMDocument();
32+
$root3 = $doc3->appendChild($doc3->createElement('root'));
33+
$root3->appendChild($doc3->createTextNode('A'));
34+
$t = $root3->appendChild($doc3->createTextNode('B'));
35+
$root3->insertBefore($t, $t);
36+
$root3->appendChild($t);
37+
echo $doc3->saveXML($root3), PHP_EOL;
38+
unset($t, $root3, $doc3);
39+
echo "done", PHP_EOL;
40+
?>
41+
--EXPECT--
42+
bool(true)
43+
int(2)
44+
bool(true)
45+
bool(false)
46+
bool(false)
47+
bool(true)
48+
int(2)
49+
<root>text<child/></root>
50+
bool(true)
51+
<root a="1" b="2"/>
52+
<root>AB</root>
53+
done

0 commit comments

Comments
 (0)