Fix GH-9628: Implicitly removing nodes from \DOMDocument breaks exist…

…ing references

Change the way lifetime works in ext/libxml and ext/dom

Previously, a node could be freed even when holding a userland reference
to it. This resulted in exceptions when trying to access that node after
it has been implicitly or explicitly removed. After this patch, a node
will only be freed when the last userland reference disappears.

Fixes GH-9628.
Closes GH-11576.

ext/dom/php_dom.c

@@ -310,10 +310,6 @@ zval *dom_read_property(zend_object *object, zend_string *name, int type, void *
 
 	if (obj->prop_handler != NULL) {
 		hnd = zend_hash_find_ptr(obj->prop_handler, name);
-	} else if (instanceof_function(obj->std.ce, dom_node_class_entry)) {
-		zend_throw_error(NULL, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name));
-		retval = &EG(uninitialized_zval);
-		return retval;
 	}
 
 	if (hnd) {

ext/dom/tests/DOMAttr_ownerElement_error_001.phpt

@@ -14,11 +14,7 @@ $document->appendChild($root);
 $attr = $root->setAttribute('category', 'books');
 $document->removeChild($root);
 $root = null;
-try {
-    var_dump($attr->ownerElement);
-} catch (\Error $e) {
-    echo get_class($e) . ': ' . $e->getMessage() . \PHP_EOL;
-}
+var_dump($attr->ownerElement);
 ?>
 --EXPECT--
-Error: Couldn't fetch DOMAttr. Node no longer exists
+NULL

ext/dom/tests/bug36756.phpt

@@ -20,15 +20,11 @@ $node = $xpath->query('//child')->item(0);
 echo $node->nodeName . "\n";
 $GLOBALS['dom']->removeChild($GLOBALS['dom']->firstChild);
 
-try {
-    echo "nodeType: " . $node->nodeType . "\n";
-} catch (\Error $e) {
-    echo get_class($e) . ': ' . $e->getMessage() .\PHP_EOL;
-}
+echo "nodeType: " . $node->nodeType . "\n";
 
 ?>
 --EXPECT--
 root
 nodeType: 1
 child
-Error: Couldn't fetch DOMElement. Node no longer exists
+nodeType: 1

ext/dom/tests/bug70359.phpt

@@ -29,11 +29,7 @@ var_dump($spaceNode->nodeType);
 var_dump($spaceNode->nodeValue);
 
 $dom->documentElement->firstElementChild->remove();
-try {
-    print_r($spaceNode->parentNode);
-} catch (\Error $e) {
-    echo $e->getMessage(), "\n";
-}
+var_dump($spaceNode->parentNode);
 
 echo "-- Test with parent and ns attribute --\n";
 
@@ -67,7 +63,7 @@ DOMNameSpaceNode Object
 -- Test with parent and non-ns attribute --
 int(2)
 string(3) "bar"
-Couldn't fetch DOMAttr. Node no longer exists
+NULL
 -- Test with parent and ns attribute --
 DOMNameSpaceNode Object
 (

ext/dom/tests/delayed_freeing/attribute_declaration.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Delayed freeing attribute declaration
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument;
+$doc->loadXML(<<<'XML'
+<?xml version="1.0"?>
+<!DOCTYPE book [
+<!ELEMENT book (#PCDATA)>
+<!ATTLIST book
+title CDATA #REQUIRED
+>
+]>
+<book title="book title"/>
+XML);
+echo $doc->saveXML(), "\n";
+// Note: no way to query the attribute declaration, but this at least tests destruction order
+$doc->removeChild($doc->doctype);
+echo $doc->saveXML(), "\n";
+?>
+--EXPECT--
+<?xml version="1.0"?>
+<!DOCTYPE book [
+<!ELEMENT book (#PCDATA)>
+<!ATTLIST book title CDATA #REQUIRED>
+]>
+<book title="book title"/>
+
+<?xml version="1.0"?>
+<book title="book title"/>

ext/dom/tests/delayed_freeing/comment_node.phpt

@@ -0,0 +1,23 @@
+--TEST--
+Delayed freeing comment node
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument;
+$comment = $doc->appendChild($doc->createElement('container'))
+    ->appendChild($doc->createComment('my comment'));
+echo $doc->saveXML(), "\n";
+$comment->parentNode->remove();
+echo $doc->saveXML(), "\n";
+echo $doc->saveXML($comment), "\n";
+var_dump($comment->parentNode);
+?>
+--EXPECT--
+<?xml version="1.0"?>
+<container><!--my comment--></container>
+
+<?xml version="1.0"?>
+
+<!--my comment-->
+NULL

ext/dom/tests/delayed_freeing/direct_construction.phpt

@@ -0,0 +1,137 @@
+--TEST--
+Tests with direction construction
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+function node_alike_test($test) {
+    try {
+        var_dump($test->parentNode);
+        var_dump($test->nodeValue);
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+    try {
+        var_dump($test->appendChild($test));
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
+
+echo "-- Test DOMCharacterData --\n";
+$test = new DOMCharacterData("test");
+try {
+    var_dump($test->textContent);
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($test->appendData('test'));
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "-- Test DOMCdataSection --\n";
+$test = new DOMCdataSection("test");
+var_dump($test->textContent);
+var_dump($test->appendData('test'));
+
+echo "-- Test DOMText --\n";
+$test = new DOMText("test");
+try {
+    var_dump($test->wholeText);
+    var_dump($test->parentNode);
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($test->isWhitespaceInElementContent());
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "-- Test DOMComment --\n";
+$test = new DOMComment("my comment");
+var_dump($test->textContent);
+var_dump($test->parentNode);
+var_dump($test->getLineNo());
+
+echo "-- Test DOMElement --\n";
+$test = new DOMElement("qualifiedName", "test");
+var_dump($test->textContent);
+var_dump($test->parentNode);
+try {
+    var_dump($test->appendChild($test));
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "-- Test DOMNode --\n";
+node_alike_test(new DOMNode());
+
+echo "-- Test DOMNotation --\n";
+node_alike_test(new DOMNotation());
+
+echo "-- Test DOMProcessingInstruction --\n";
+node_alike_test(new DOMProcessingInstruction("name", "value"));
+
+echo "-- Test DOMEntity --\n";
+$test = new DOMEntity();
+try {
+    var_dump($test->nodeValue);
+    var_dump($test->parentNode);
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($test->appendChild($test));
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "-- Test DOMAttr --\n";
+$test = new DOMAttr("attr", "value");
+var_dump($test->nodeValue);
+var_dump($test->parentNode);
+try {
+    var_dump($test->appendChild($test));
+} catch (Throwable $e) {
+    echo $e->getMessage(), "\n";
+}
+?>
+--EXPECT--
+-- Test DOMCharacterData --
+Invalid State Error
+Couldn't fetch DOMCharacterData
+-- Test DOMCdataSection --
+string(4) "test"
+bool(true)
+-- Test DOMText --
+string(4) "test"
+NULL
+bool(false)
+-- Test DOMComment --
+string(10) "my comment"
+NULL
+int(0)
+-- Test DOMElement --
+string(4) "test"
+NULL
+No Modification Allowed Error
+-- Test DOMNode --
+Invalid State Error
+Couldn't fetch DOMNode
+-- Test DOMNotation --
+Invalid State Error
+Couldn't fetch DOMNotation
+-- Test DOMProcessingInstruction --
+NULL
+string(5) "value"
+bool(false)
+-- Test DOMEntity --
+Invalid State Error
+Couldn't fetch DOMEntity
+-- Test DOMAttr --
+string(5) "value"
+NULL
+No Modification Allowed Error

ext/dom/tests/delayed_freeing/document_fragment.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Delayed freeing document fragment
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument;
+$frag = $doc->createDocumentFragment();
+$frag->appendChild($doc->createElementNS('some:ns', 'child', 'text content'));
+$child = $doc->appendChild($doc->createElement('root'))->appendChild($frag);
+var_dump($doc->textContent);
+$doc->documentElement->remove();
+var_dump($doc->textContent);
+unset($doc);
+var_dump($child->textContent);
+
+$doc = new DOMDocument;
+$doc->appendChild($doc->createElement('container'));
+$doc->documentElement->appendChild($doc->importNode($frag));
+unset($frag);
+var_dump($doc->textContent);
+
+var_dump($child->parentNode);
+?>
+--EXPECTF--
+string(12) "text content"
+string(0) ""
+string(12) "text content"
+
+Warning: DOMNode::appendChild(): Document Fragment is empty in %s on line %d
+string(0) ""
+NULL

ext/dom/tests/delayed_freeing/dom_character_data.phpt

@@ -0,0 +1,23 @@
+--TEST--
+Delayed freeing character data
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument;
+$doc->loadXML(<<<'XML'
+<?xml version="1.0"?>
+<container><![CDATA[This is a CDATA section<p>test</p>]]></container>
+XML);
+$cdata = $doc->documentElement->firstChild;
+var_dump($cdata->wholeText, $cdata->parentNode->tagName);
+$cdata->parentNode->remove();
+var_dump($cdata->wholeText, $cdata->parentNode->tagName);
+?>
+--EXPECTF--
+string(34) "This is a CDATA section<p>test</p>"
+string(9) "container"
+
+Warning: Attempt to read property "tagName" on null in %s on line %d
+string(34) "This is a CDATA section<p>test</p>"
+NULL

ext/dom/tests/delayed_freeing/dtd_node.phpt

@@ -0,0 +1,22 @@
+--TEST--
+Delayed freeing dtd node
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument;
+$dtd = $doc->implementation->createDocumentType('qualified name', 'public', 'system');
+$doc = $doc->implementation->createDocument('', '', $dtd);
+echo $doc->saveXML(), "\n";
+unset($doc);
+echo $dtd->ownerDocument->saveXML();
+$dtd->ownerDocument->removeChild($dtd);
+var_dump($dtd->ownerDocument->nodeName);
+?>
+--EXPECT--
+<?xml version="1.0"?>
+<!DOCTYPE qualified name PUBLIC "public" "system">
+
+<?xml version="1.0"?>
+<!DOCTYPE qualified name PUBLIC "public" "system">
+string(9) "#document"

ext/dom/tests/delayed_freeing/element_declaration.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Delayed freeing element declaration
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument;
+$doc->loadXML(<<<'XML'
+<?xml version="1.0"?>
+<!DOCTYPE books [
+<!ELEMENT parent (child1, child2)>
+<!ELEMENT child1 (#PCDATA)>
+<!ELEMENT child2 (#PCDATA)>
+]>
+<container><parent/></container>
+XML, LIBXML_NOENT);
+$element = $doc->documentElement->firstElementChild;
+echo $doc->saveXML(), "\n";
+var_dump($element->tagName);
+var_dump($element->textContent);
+
+$doc->removeChild($doc->doctype);
+echo $doc->saveXML(), "\n";
+var_dump($element->tagName);
+var_dump($element->textContent);
+?>
+--EXPECT--
+<?xml version="1.0"?>
+<!DOCTYPE books [
+<!ELEMENT parent (child1 , child2)>
+<!ELEMENT child1 (#PCDATA)>
+<!ELEMENT child2 (#PCDATA)>
+]>
+<container><parent/></container>
+
+string(6) "parent"
+string(0) ""
+<?xml version="1.0"?>
+<container><parent/></container>
+
+string(6) "parent"
+string(0) ""