From c66221b7ba94417e186960d1069c17f43623f587 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 29 Jun 2024 07:32:36 -0700
Subject: [PATCH] Fix arginfo violation in removeChild() (#14717)

It was possible to return false without throwing an exception.
This is even wrong in "old DOM" because we expect either a NOT_FOUND_ERR
or NO_MODIFICATION_ALLOWED_ERR according to the documentation.
A side effect of this patch is that it prioritises NOT_FOUND_ERR over
NO_MODIFICATION_ALLOWED_ERR but I think that's fine.
---
 ext/dom/node.c                                | 14 +++++--------
 .../xml/Node_removeChild_from_comment.phpt    | 20 +++++++++++++++++++
 2 files changed, 25 insertions(+), 9 deletions(-)
 create mode 100644 ext/dom/tests/modern/xml/Node_removeChild_from_comment.phpt

diff --git a/ext/dom/node.c b/ext/dom/node.c
index 8df57d46800..0a63aa61eb9 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -1229,22 +1229,18 @@ static void dom_node_remove_child(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry
 
 	DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern);
 
-	if (!dom_node_children_valid(nodep)) {
-		RETURN_FALSE;
-	}
-
 	DOM_GET_OBJ(child, node, xmlNodePtr, childobj);
 
 	bool stricterror = dom_get_strict_error(intern->document);
 
-	if (dom_node_is_read_only(nodep) == SUCCESS ||
-		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
-		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
+	if (!nodep->children || child->parent != nodep) {
+		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
 		RETURN_FALSE;
 	}
 
-	if (!nodep->children || child->parent != nodep) {
-		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
+	if (dom_node_is_read_only(nodep) == SUCCESS ||
+		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
+		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
 		RETURN_FALSE;
 	}
 
diff --git a/ext/dom/tests/modern/xml/Node_removeChild_from_comment.phpt b/ext/dom/tests/modern/xml/Node_removeChild_from_comment.phpt
new file mode 100644
index 00000000000..eae3c97dbf0
--- /dev/null
+++ b/ext/dom/tests/modern/xml/Node_removeChild_from_comment.phpt
@@ -0,0 +1,20 @@
+--TEST--
+Removing a child from a comment should result in NOT_FOUND_ERR
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = Dom\XMLDocument::createFromString('<root><!-- comment --><child/></root>');
+$comment = $dom->documentElement->firstChild;
+$child = $comment->nextSibling;
+
+try {
+    $comment->removeChild($child);
+} catch (DOMException $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Not Found Error