Skip to content

Commit 866bd04

Browse files
committed
Fix GH-17040: SimpleXML's unset can break DOM objects
Don't free the underlying nodes if we still have objects pointing to them, otherwise the objects are left with a NULL node pointer.
1 parent e50cf7a commit 866bd04

File tree

2 files changed

+34
-10
lines changed

2 files changed

+34
-10
lines changed

ext/simplexml/simplexml.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ static void php_sxe_iterator_move_forward(zend_object_iterator *iter);
5454
static void php_sxe_iterator_rewind(zend_object_iterator *iter);
5555
static zend_result sxe_object_cast_ex(zend_object *readobj, zval *writeobj, int type);
5656

57+
static void sxe_unlink_node(xmlNodePtr node)
58+
{
59+
xmlUnlinkNode(node);
60+
/* Only destroy the nodes if we have no objects using them anymore.
61+
* Don't assume simplexml owns these. */
62+
if (!node->_private) {
63+
php_libxml_node_free_resource(node);
64+
}
65+
}
66+
5767
/* {{{ _node_as_zval() */
5868
static void _node_as_zval(php_sxe_object *sxe, xmlNodePtr node, zval *value, SXE_ITER itertype, char *name, const xmlChar *nsprefix, int isprefix)
5969
{
@@ -554,8 +564,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
554564
}
555565
if (value_str) {
556566
while ((tempnode = (xmlNodePtr) newnode->children)) {
557-
xmlUnlinkNode(tempnode);
558-
php_libxml_node_free_resource((xmlNodePtr) tempnode);
567+
sxe_unlink_node(tempnode);
559568
}
560569
change_node_zval(newnode, value_str);
561570
}
@@ -829,8 +838,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
829838
while (attr && nodendx <= Z_LVAL_P(member)) {
830839
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
831840
if (nodendx == Z_LVAL_P(member)) {
832-
xmlUnlinkNode((xmlNodePtr) attr);
833-
php_libxml_node_free_resource((xmlNodePtr) attr);
841+
sxe_unlink_node((xmlNodePtr) attr);
834842
break;
835843
}
836844
nodendx++;
@@ -841,8 +849,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
841849
while (attr) {
842850
anext = attr->next;
843851
if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && xmlStrEqual(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
844-
xmlUnlinkNode((xmlNodePtr) attr);
845-
php_libxml_node_free_resource((xmlNodePtr) attr);
852+
sxe_unlink_node((xmlNodePtr) attr);
846853
break;
847854
}
848855
attr = anext;
@@ -857,8 +864,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
857864
}
858865
node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL);
859866
if (node) {
860-
xmlUnlinkNode(node);
861-
php_libxml_node_free_resource(node);
867+
sxe_unlink_node(node);
862868
}
863869
} else {
864870
node = node->children;
@@ -868,8 +874,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
868874
SKIP_TEXT(node);
869875

870876
if (xmlStrEqual(node->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
871-
xmlUnlinkNode(node);
872-
php_libxml_node_free_resource(node);
877+
sxe_unlink_node(node);
873878
}
874879

875880
next_iter:

ext/simplexml/tests/gh17040.phpt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
GH-17040 (SimpleXML's unset can break DOM objects)
3+
--EXTENSIONS--
4+
dom
5+
simplexml
6+
--FILE--
7+
<?php
8+
$dom = new DOMDocument;
9+
$tag = $dom->appendChild($dom->createElement("style"));
10+
$html = simplexml_import_dom($tag);
11+
unset($html[0]);
12+
$tag->append("foo");
13+
echo $dom->saveXML(), "\n";
14+
echo $dom->saveXML($tag), "\n";
15+
?>
16+
--EXPECT--
17+
<?xml version="1.0"?>
18+
19+
<style>foo</style>

0 commit comments

Comments
 (0)