diff --git a/ext/dom/node.c b/ext/dom/node.c
index e061b13e0137b..48e3c5cc2a747 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -820,11 +820,62 @@ int dom_node_text_content_write(dom_object *obj, zval *newval)
/* }}} */
-static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlNodePtr nextsib, xmlNodePtr fragment, dom_object *intern, dom_object *childobj) /* {{{ */
+/* Returns true if the node was changed, false otherwise. */
+static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
{
- xmlNodePtr newchild, node;
+ dom_object *childobj = php_dom_object_get_data(node);
+ if (childobj && !childobj->document) {
+ childobj->document = document;
+ document->refcount++;
+ return true;
+ }
+ return false;
+}
+
+/* TODO: on 8.4 replace the loop with the tree walk helper function. */
+static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
+{
+ /* Applies the document to the entire subtree. */
+ xmlSetTreeDoc(node, doc);
+
+ if (!dom_set_document_ref_obj_single(node, doc, document)) {
+ return;
+ }
+
+ xmlNodePtr base = node;
+ node = node->children;
+ while (node != NULL) {
+ ZEND_ASSERT(node != base);
+
+ if (!dom_set_document_ref_obj_single(node, doc, document)) {
+ break;
+ }
+
+ if (node->type == XML_ELEMENT_NODE) {
+ if (node->children) {
+ node = node->children;
+ continue;
+ }
+ }
+
+ if (node->next) {
+ node = node->next;
+ } else {
+ /* Go upwards, until we find a parent node with a next sibling, or until we hit the base. */
+ do {
+ node = node->parent;
+ if (node == base) {
+ return;
+ }
+ } while (node->next == NULL);
+ node = node->next;
+ }
+ }
+}
- newchild = fragment->children;
+static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlNodePtr nextsib, xmlNodePtr fragment, dom_object *intern, dom_object *childobj) /* {{{ */
+{
+ xmlNodePtr newchild = fragment->children;
if (newchild) {
if (prevsib == NULL) {
@@ -840,17 +891,10 @@ static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib,
nextsib->prev = fragment->last;
}
- node = newchild;
+ /* Assign parent node pointer */
+ xmlNodePtr node = newchild;
while (node != NULL) {
node->parent = nodep;
- if (node->doc != nodep->doc) {
- xmlSetTreeDoc(node, nodep->doc);
- if (node->_private != NULL) {
- childobj = node->_private;
- childobj->document = intern->document;
- php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
- }
- }
if (node == fragment->last) {
break;
}
@@ -930,8 +974,7 @@ PHP_METHOD(DOMNode, insertBefore)
}
if (child->doc == NULL && parentp->doc != NULL) {
- childobj->document = intern->document;
- php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
+ dom_set_document_pointers(child, parentp->doc, intern->document);
}
php_libxml_invalidate_node_list_cache(intern->document);
@@ -949,9 +992,6 @@ PHP_METHOD(DOMNode, insertBefore)
if (child->type == XML_TEXT_NODE && (refp->type == XML_TEXT_NODE ||
(refp->prev != NULL && refp->prev->type == XML_TEXT_NODE))) {
- if (child->doc == NULL) {
- xmlSetTreeDoc(child, parentp->doc);
- }
new_child = child;
new_child->parent = refp->parent;
new_child->next = refp;
@@ -1003,9 +1043,6 @@ PHP_METHOD(DOMNode, insertBefore)
}
if (child->type == XML_TEXT_NODE && parentp->last != NULL && parentp->last->type == XML_TEXT_NODE) {
child->parent = parentp;
- if (child->doc == NULL) {
- xmlSetTreeDoc(child, parentp->doc);
- }
new_child = child;
if (parentp->children == NULL) {
parentp->children = child;
@@ -1111,6 +1148,10 @@ PHP_METHOD(DOMNode, replaceChild)
RETURN_FALSE;
}
+ if (newchild->doc == NULL && nodep->doc != NULL) {
+ dom_set_document_pointers(newchild, nodep->doc, intern->document);
+ }
+
if (newchild->type == XML_DOCUMENT_FRAG_NODE) {
xmlNodePtr prevsib, nextsib;
prevsib = oldchild->prev;
@@ -1127,11 +1168,6 @@ PHP_METHOD(DOMNode, replaceChild)
xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
replacedoctype = (intSubset == (xmlDtd *) oldchild);
- if (newchild->doc == NULL && nodep->doc != NULL) {
- xmlSetTreeDoc(newchild, nodep->doc);
- newchildobj->document = intern->document;
- php_libxml_increment_doc_ref((php_libxml_node_object *)newchildobj, NULL);
- }
xmlReplaceNode(oldchild, newchild);
dom_reconcile_ns(nodep->doc, newchild);
@@ -1216,8 +1252,7 @@ PHP_METHOD(DOMNode, appendChild)
}
if (child->doc == NULL && nodep->doc != NULL) {
- childobj->document = intern->document;
- php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
+ dom_set_document_pointers(child, nodep->doc, intern->document);
}
if (child->parent != NULL){
@@ -1226,9 +1261,6 @@ PHP_METHOD(DOMNode, appendChild)
if (child->type == XML_TEXT_NODE && nodep->last != NULL && nodep->last->type == XML_TEXT_NODE) {
child->parent = nodep;
- if (child->doc == NULL) {
- xmlSetTreeDoc(child, nodep->doc);
- }
new_child = child;
if (nodep->children == NULL) {
nodep->children = child;
diff --git a/ext/dom/tests/gh16150.phpt b/ext/dom/tests/gh16150.phpt
new file mode 100644
index 0000000000000..dd1096d6812cc
--- /dev/null
+++ b/ext/dom/tests/gh16150.phpt
@@ -0,0 +1,27 @@
+--TEST--
+GH-16150 (Use after free in php_dom.c)
+--EXTENSIONS--
+dom
+--FILE--
+{$fname}($e3);
+ $e2->append($e1);
+ $e3->{$fname}($e2);
+ echo $doc->saveXML();
+}
+
+test('appendChild');
+test('insertBefore');
+
+?>
+--EXPECT--
+
+
+
+
diff --git a/ext/dom/tests/gh16152.phpt b/ext/dom/tests/gh16152.phpt
new file mode 100644
index 0000000000000..604980b855e2f
--- /dev/null
+++ b/ext/dom/tests/gh16152.phpt
@@ -0,0 +1,27 @@
+--TEST--
+GH-16152 (Memory leak in DOMProcessingInstruction/DOMDocument)
+--EXTENSIONS--
+dom
+--FILE--
+append($instr);
+ $frag->append($frag2);
+ $doc->{$fname}($frag);
+ echo $doc->saveXML();
+}
+
+test('insertBefore');
+test('appendChild');
+
+?>
+--EXPECT--
+
+
+
+