From 417c4d0fc1b1416f2168db9feb8987612389a4b5 Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Sun, 19 Apr 2020 14:35:25 +0200
Subject: [PATCH 1/7] Promote warnings to error in DOM ext

---
 ext/dom/attr.c                  |  4 +--
 ext/dom/cdatasection.c          |  2 +-
 ext/dom/comment.c               |  2 +-
 ext/dom/document.c              |  1 +
 ext/dom/documentfragment.c      |  3 +-
 ext/dom/domimplementation.c     | 18 +++++-----
 ext/dom/element.c               | 23 ++++++++-----
 ext/dom/entityreference.c       |  4 +--
 ext/dom/namednodemap.c          | 58 ++++++++++++++++-----------------
 ext/dom/node.c                  | 13 +++++++-
 ext/dom/parentnode.c            |  2 ++
 ext/dom/php_dom.c               |  3 ++
 ext/dom/php_dom.h               |  1 +
 ext/dom/processinginstruction.c |  4 +--
 ext/dom/text.c                  |  6 +++-
 ext/dom/xpath.c                 | 11 ++++++-
 16 files changed, 95 insertions(+), 60 deletions(-)

diff --git a/ext/dom/attr.c b/ext/dom/attr.c
index 99178bfe3e9ba..e348f8fc5abaf 100644
--- a/ext/dom/attr.c
+++ b/ext/dom/attr.c
@@ -50,14 +50,14 @@ PHP_METHOD(DOMAttr, __construct)
 	name_valid = xmlValidateName((xmlChar *) name, 0);
 	if (name_valid != 0) {
 		php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	nodep = xmlNewProp(NULL, (xmlChar *) name, (xmlChar *) value);
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	oldnode = dom_object_get_node(intern);
diff --git a/ext/dom/cdatasection.c b/ext/dom/cdatasection.c
index ce62377917745..2d6fd8dc05c6c 100644
--- a/ext/dom/cdatasection.c
+++ b/ext/dom/cdatasection.c
@@ -46,7 +46,7 @@ PHP_METHOD(DOMCdataSection, __construct)
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		return;
+		RETURN_THROWS();
 	}
 
 	intern = Z_DOMOBJ_P(ZEND_THIS);
diff --git a/ext/dom/comment.c b/ext/dom/comment.c
index db2341b72584f..79588f0c87abb 100644
--- a/ext/dom/comment.c
+++ b/ext/dom/comment.c
@@ -46,7 +46,7 @@ PHP_METHOD(DOMComment, __construct)
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		return;
+		RETURN_THROWS();
 	}
 
 	intern = Z_DOMOBJ_P(ZEND_THIS);
diff --git a/ext/dom/document.c b/ext/dom/document.c
index 6ff1750137187..2e0fde32dd786 100644
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -161,6 +161,7 @@ int dom_document_encoding_write(dom_object *obj, zval *newval)
 		}
 		docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str));
     } else {
+    	// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Invalid Document Encoding");
     }
 
diff --git a/ext/dom/documentfragment.c b/ext/dom/documentfragment.c
index 641c3f5b34d45..a5ae898f86cf7 100644
--- a/ext/dom/documentfragment.c
+++ b/ext/dom/documentfragment.c
@@ -44,7 +44,7 @@ PHP_METHOD(DOMDocumentFragment, __construct)
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		return;
+		RETURN_THROWS();
 	}
 
 	intern = Z_DOMOBJ_P(ZEND_THIS);
@@ -109,6 +109,7 @@ PHP_METHOD(DOMDocumentFragment, appendXML) {
 	DOM_GET_OBJ(nodep, id, xmlNodePtr, intern);
 
 	if (dom_node_is_read_only(nodep) == SUCCESS) {
+		// Should always be in strict mode?
 		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(intern->document));
 		RETURN_FALSE;
 	}
diff --git a/ext/dom/domimplementation.c b/ext/dom/domimplementation.c
index 86ec4a23684fe..b195af5549c66 100644
--- a/ext/dom/domimplementation.c
+++ b/ext/dom/domimplementation.c
@@ -67,8 +67,8 @@ PHP_METHOD(DOMImplementation, createDocumentType)
 	}
 
 	if (name_len == 0) {
-		php_error_docref(NULL, E_WARNING, "qualifiedName is required");
-		RETURN_FALSE;
+		zend_argument_value_error(1, "cannot be empty");
+		RETURN_THROWS();
 	}
 
 	if (publicid_len > 0) {
@@ -134,12 +134,12 @@ PHP_METHOD(DOMImplementation, createDocument)
 	if (node != NULL) {
 		DOM_GET_OBJ(doctype, node, xmlDtdPtr, doctobj);
 		if (doctype->type == XML_DOCUMENT_TYPE_NODE) {
-			php_error_docref(NULL, E_WARNING, "Invalid DocumentType object");
-			RETURN_FALSE;
+			zend_argument_value_error(3, "is an invalid DocumentType object");
+			RETURN_THROWS();
 		}
 		if (doctype->doc != NULL) {
 			php_dom_throw_error(WRONG_DOCUMENT_ERR, 1);
-			RETURN_FALSE;
+			RETURN_THROWS();
 		}
 	} else {
 		doctobj = NULL;
@@ -163,7 +163,7 @@ PHP_METHOD(DOMImplementation, createDocument)
 			xmlFree(localname);
 		}
 		php_dom_throw_error(errorcode, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	/* currently letting libxml2 set the version string */
@@ -195,9 +195,9 @@ PHP_METHOD(DOMImplementation, createDocument)
 			}
 			xmlFreeDoc(docp);
 			xmlFree(localname);
-			/* Need some type of error here */
-			php_error_docref(NULL, E_WARNING, "Unexpected Error");
-			RETURN_FALSE;
+			/* Need some better type of error here */
+			php_dom_throw_error(PHP_ERR, 1);
+			RETURN_THROWS();
 		}
 
 		nodep->nsDef = nsptr;
diff --git a/ext/dom/element.c b/ext/dom/element.c
index 41bc7296ca875..75f735ce689b4 100644
--- a/ext/dom/element.c
+++ b/ext/dom/element.c
@@ -49,7 +49,7 @@ PHP_METHOD(DOMElement, __construct)
 	name_valid = xmlValidateName((xmlChar *) name, 0);
 	if (name_valid != 0) {
 		php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
-		return;
+		RETURN_THROWS();
 	}
 
 	/* Namespace logic is separate and only when uri passed in to insure no BC breakage */
@@ -71,7 +71,7 @@ PHP_METHOD(DOMElement, __construct)
 				xmlFreeNode(nodep);
 			}
 			php_dom_throw_error(errorcode, 1);
-			return;
+			RETURN_THROWS();
 		}
 	} else {
 	    /* If you don't pass a namespace uri, then you can't set a prefix */
@@ -80,14 +80,14 @@ PHP_METHOD(DOMElement, __construct)
 			xmlFree(localname);
 			xmlFree(prefix);
 	        php_dom_throw_error(NAMESPACE_ERR, 1);
-	        return;
+	        RETURN_THROWS();
 	    }
 		nodep = xmlNewNode(NULL, (xmlChar *) name);
 	}
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		return;
+		RETURN_THROWS();
 	}
 
 	if (value_len > 0) {
@@ -255,19 +255,20 @@ PHP_METHOD(DOMElement, setAttribute)
 	}
 
 	if (name_len == 0) {
-		php_error_docref(NULL, E_WARNING, "Attribute Name is required");
-		RETURN_FALSE;
+		zend_argument_value_error(1, "cannot be empty");
+		RETURN_THROWS();
 	}
 
 	name_valid = xmlValidateName((xmlChar *) name, 0);
 	if (name_valid != 0) {
 		php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	DOM_GET_OBJ(nodep, id, xmlNodePtr, intern);
 
 	if (dom_node_is_read_only(nodep) == SUCCESS) {
+		// Todo make it always strict?
 		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(intern->document));
 		RETURN_FALSE;
 	}
@@ -294,6 +295,7 @@ PHP_METHOD(DOMElement, setAttribute)
 		attr = (xmlNodePtr)xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value);
 	}
 	if (!attr) {
+		// TODO Convert to error?
 		php_error_docref(NULL, E_WARNING, "No such attribute '%s'", name);
 		RETURN_FALSE;
 	}
@@ -424,6 +426,7 @@ PHP_METHOD(DOMElement, setAttributeNode)
 	DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
 
 	if (attrp->type != XML_ATTRIBUTE_NODE) {
+		// Todo convert to a ValueError?
 		php_error_docref(NULL, E_WARNING, "Attribute node is required");
 		RETURN_FALSE;
 	}
@@ -628,8 +631,8 @@ PHP_METHOD(DOMElement, setAttributeNS)
 	}
 
 	if (name_len == 0) {
-		php_error_docref(NULL, E_WARNING, "Attribute Name is required");
-		RETURN_FALSE;
+		zend_argument_value_error(2, "cannot be empty");
+		RETURN_THROWS();
 	}
 
 	DOM_GET_OBJ(elemp, id, xmlNodePtr, intern);
@@ -641,6 +644,7 @@ PHP_METHOD(DOMElement, setAttributeNS)
 		RETURN_NULL();
 	}
 
+	// Todo should always be strict?
 	errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len);
 
 	if (errorcode == 0) {
@@ -874,6 +878,7 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
 	DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
 
 	if (attrp->type != XML_ATTRIBUTE_NODE) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Attribute node is required");
 		RETURN_FALSE;
 	}
diff --git a/ext/dom/entityreference.c b/ext/dom/entityreference.c
index 4e45119b48f06..635cfe7653c43 100644
--- a/ext/dom/entityreference.c
+++ b/ext/dom/entityreference.c
@@ -46,14 +46,14 @@ PHP_METHOD(DOMEntityReference, __construct)
 	name_valid = xmlValidateName((xmlChar *) name, 0);
 	if (name_valid != 0) {
 		php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	node = xmlNewReference(NULL, (xmlChar *) name);
 
 	if (!node) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	intern = Z_DOMOBJ_P(ZEND_THIS);
diff --git a/ext/dom/namednodemap.c b/ext/dom/namednodemap.c
index fd96003e565f8..26e0551f3bc87 100644
--- a/ext/dom/namednodemap.c
+++ b/ext/dom/namednodemap.c
@@ -146,44 +146,42 @@ PHP_METHOD(DOMNamedNodeMap, item)
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
 		RETURN_THROWS();
 	}
-	if (index >= 0) {
-		if (ZEND_LONG_INT_OVFL(index)) {
-			php_error_docref(NULL, E_WARNING, "Invalid index");
-			RETURN_NULL();
-		}
+	if (index < 0 || ZEND_LONG_INT_OVFL(index)) {
+		zend_argument_value_error(1, "must be between 0 and %d", INT_MAX);
+		RETURN_THROWS();
+	}
 
-		intern = Z_DOMOBJ_P(id);
+	intern = Z_DOMOBJ_P(id);
 
-		objmap = (dom_nnodemap_object *)intern->ptr;
+	objmap = (dom_nnodemap_object *)intern->ptr;
 
-		if (objmap != NULL) {
-			if ((objmap->nodetype == XML_NOTATION_NODE) ||
-				objmap->nodetype == XML_ENTITY_NODE) {
-				if (objmap->ht) {
-					if (objmap->nodetype == XML_ENTITY_NODE) {
-						itemnode = php_dom_libxml_hash_iter(objmap->ht, index);
-					} else {
-						itemnode = php_dom_libxml_notation_iter(objmap->ht, index);
-					}
+	if (objmap != NULL) {
+		if ((objmap->nodetype == XML_NOTATION_NODE) ||
+			objmap->nodetype == XML_ENTITY_NODE) {
+			if (objmap->ht) {
+				if (objmap->nodetype == XML_ENTITY_NODE) {
+					itemnode = php_dom_libxml_hash_iter(objmap->ht, index);
+				} else {
+					itemnode = php_dom_libxml_notation_iter(objmap->ht, index);
 				}
-			} else {
-				nodep = dom_object_get_node(objmap->baseobj);
-				if (nodep) {
-					curnode = (xmlNodePtr)nodep->properties;
-					count = 0;
-					while (count < index && curnode != NULL) {
-						count++;
-						curnode = (xmlNodePtr)curnode->next;
-					}
-					itemnode = curnode;
+			}
+		} else {
+			nodep = dom_object_get_node(objmap->baseobj);
+			if (nodep) {
+				curnode = (xmlNodePtr)nodep->properties;
+				count = 0;
+				while (count < index && curnode != NULL) {
+					count++;
+					curnode = (xmlNodePtr)curnode->next;
 				}
+				itemnode = curnode;
 			}
 		}
+	}
 
-		if (itemnode) {
-			DOM_RET_OBJ(itemnode, &ret, objmap->baseobj);
-			return;
-		}
+	if (itemnode) {
+		DOM_RET_OBJ(itemnode, &ret, objmap->baseobj);
+		return;
 	}
 
 	RETVAL_NULL();
diff --git a/ext/dom/node.c b/ext/dom/node.c
index 16a6c6cc54e1e..b0cf580ce1616 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -98,6 +98,7 @@ int dom_node_node_name_read(dom_object *obj, zval *retval)
 			str = "#text";
 			break;
 		default:
+			// Todo convert to error?
 			php_error_docref(NULL, E_WARNING, "Invalid Node Type");
 	}
 
@@ -856,6 +857,7 @@ PHP_METHOD(DOMNode, insertBefore)
 
 	new_child = NULL;
 
+	// Todo should always be strict?
 	stricterror = dom_get_strict_error(intern->document);
 
 	if (dom_node_is_read_only(parentp) == SUCCESS ||
@@ -875,6 +877,7 @@ PHP_METHOD(DOMNode, insertBefore)
 	}
 
 	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
+		// Todo convert to Error?
 		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
 		RETURN_FALSE;
 	}
@@ -981,6 +984,7 @@ PHP_METHOD(DOMNode, insertBefore)
 	}
 
 	if (NULL == new_child) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Couldn't add newnode as the previous sibling of refnode");
 		RETURN_FALSE;
 	}
@@ -1023,6 +1027,7 @@ PHP_METHOD(DOMNode, replaceChild)
 		RETURN_FALSE;
 	}
 
+	// Todo make alway strict?
 	stricterror = dom_get_strict_error(intern->document);
 
 	if (dom_node_is_read_only(nodep) == SUCCESS ||
@@ -1074,7 +1079,7 @@ PHP_METHOD(DOMNode, replaceChild)
 		DOM_RET_OBJ(oldchild, &ret, intern);
 		return;
 	} else {
-		php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document));
+		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
 		RETURN_FALSE;
 	}
 }
@@ -1173,6 +1178,7 @@ PHP_METHOD(DOMNode, appendChild)
 	}
 
 	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
 		RETURN_FALSE;
 	}
@@ -1462,6 +1468,7 @@ PHP_METHOD(DOMNode, lookupPrefix)
 		}
 	}
 
+	// Todo return empty string?
 	RETURN_NULL();
 }
 /* }}} end dom_node_lookup_prefix */
@@ -1570,6 +1577,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 	docp = nodep->doc;
 
 	if (! docp) {
+		// TOdo convert to error?
 		php_error_docref(NULL, E_WARNING, "Node must be associated with a document");
 		RETURN_FALSE;
 	}
@@ -1587,6 +1595,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 					xmlXPathFreeObject(xpathobjp);
 				}
 				xmlXPathFreeContext(ctxp);
+				// Todo convert to error?
 				php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset.");
 				RETURN_FALSE;
 			}
@@ -1601,6 +1610,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 		if (tmp && Z_TYPE_P(tmp) == IS_STRING) {
 			xquery = Z_STRVAL_P(tmp);
 		} else {
+			// Todo convert to error?
 			php_error_docref(NULL, E_WARNING, "'query' missing from xpath array or is not a string");
 			RETURN_FALSE;
 		}
@@ -1738,6 +1748,7 @@ PHP_METHOD(DOMNode, getNodePath)
 
 	value = (char *) xmlGetNodePath(nodep);
 	if (value == NULL) {
+		// Todo return empty string?
 		RETURN_NULL();
 	} else {
 		RETVAL_STRING(value);
diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c
index f47416edff3dd..8650354763767 100644
--- a/ext/dom/parentnode.c
+++ b/ext/dom/parentnode.c
@@ -151,6 +151,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
 		return NULL;
 	}
 
+	// Todo always strict mode?
 	stricterror = dom_get_strict_error(document);
 
 	for (i = 0; i < nodesc; i++) {
@@ -378,6 +379,7 @@ void dom_child_node_remove(dom_object *context)
 		return;
 	}
 
+	// Todo always strict?
 	stricterror = dom_get_strict_error(context->document);
 
 	if (dom_node_is_read_only(child) == SUCCESS ||
diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c
index 8eae00fc03bbe..49e265f5953e7 100644
--- a/ext/dom/php_dom.c
+++ b/ext/dom/php_dom.c
@@ -317,6 +317,7 @@ 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)) {
+		// Should this be an error?
 		php_error(E_WARNING, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name));
 	}
 
@@ -466,6 +467,7 @@ PHP_FUNCTION(dom_import_simplexml)
 	if (nodep && nodeobj && (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE)) {
 		DOM_RET_OBJ((xmlNodePtr) nodep, &ret, (dom_object *)nodeobj);
 	} else {
+		// Should this be an error?
 		php_error_docref(NULL, E_WARNING, "Invalid Nodetype to import");
 		RETURN_NULL();
 	}
@@ -1208,6 +1210,7 @@ PHP_DOM_EXPORT zend_bool php_dom_create_object(xmlNodePtr obj, zval *return_valu
 			break;
 		}
 		default:
+			// Todo convert to value error?
 			php_error_docref(NULL, E_WARNING, "Unsupported node type: %d", obj->type);
 			ZVAL_NULL(return_value);
 			return 0;
diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
index 285980b64644b..03b616ad4f4c4 100644
--- a/ext/dom/php_dom.h
+++ b/ext/dom/php_dom.h
@@ -150,6 +150,7 @@ entry = zend_register_internal_class_ex(&ce, parent_ce);
 		RETURN_THROWS(); \
 	}
 
+// Make this an error?
 #define DOM_NOT_IMPLEMENTED() \
 	php_error_docref(NULL, E_WARNING, "Not yet implemented"); \
 	return;
diff --git a/ext/dom/processinginstruction.c b/ext/dom/processinginstruction.c
index 9255213e7162a..73367442eb1c7 100644
--- a/ext/dom/processinginstruction.c
+++ b/ext/dom/processinginstruction.c
@@ -46,14 +46,14 @@ PHP_METHOD(DOMProcessingInstruction, __construct)
 	name_valid = xmlValidateName((xmlChar *) name, 0);
 	if (name_valid != 0) {
 		php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	nodep = xmlNewPI((xmlChar *) name, (xmlChar *) value);
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	intern = Z_DOMOBJ_P(ZEND_THIS);
diff --git a/ext/dom/text.c b/ext/dom/text.c
index b1053b2039bfc..76c66a898f88c 100644
--- a/ext/dom/text.c
+++ b/ext/dom/text.c
@@ -47,7 +47,7 @@ PHP_METHOD(DOMText, __construct)
 
 	if (!nodep) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	intern = Z_DOMOBJ_P(ZEND_THIS);
@@ -123,16 +123,19 @@ PHP_METHOD(DOMText, splitText)
 	DOM_GET_OBJ(node, id, xmlNodePtr, intern);
 
 	if (node->type != XML_TEXT_NODE && node->type != XML_CDATA_SECTION_NODE) {
+		// Todo make this throw an error?
 		RETURN_FALSE;
 	}
 
 	cur = xmlNodeGetContent(node);
 	if (cur == NULL) {
+		// Todo make this throw an error?
 		RETURN_FALSE;
 	}
 	length = xmlUTF8Strlen(cur);
 
 	if (ZEND_LONG_INT_OVFL(offset) || (int)offset > length || offset < 0) {
+		// Todo make this throw an error?
 		xmlFree(cur);
 		RETURN_FALSE;
 	}
@@ -149,6 +152,7 @@ PHP_METHOD(DOMText, splitText)
 	xmlFree(second);
 
 	if (nnode == NULL) {
+		// Todo make this throw an error?
 		RETURN_FALSE;
 	}
 
diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index cc3cbf8433655..0d8246e81b40e 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -136,6 +136,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 
 	obj = valuePop(ctxt);
 	if (obj->stringval == NULL) {
+		// Todo convert to type error?
 		php_error_docref(NULL, E_WARNING, "Handler name must be a string");
 		xmlXPathFreeObject(obj);
 		if (fci.param_count > 0) {
@@ -154,8 +155,10 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 	fci.retval = &retval;
 
 	if (!zend_make_callable(&fci.function_name, &callable)) {
+		// TODO make this an error?
 		php_error_docref(NULL, E_WARNING, "Unable to call handler %s()", ZSTR_VAL(callable));
 	} else if (intern->registerPhpFunctions == 2 && zend_hash_exists(intern->registered_phpfunctions, callable) == 0) {
+		// TODO make this an error?
 		php_error_docref(NULL, E_WARNING, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable));
 		/* Push an empty string, so that we at least have an xslt result... */
 		valuePush(ctxt, xmlXPathNewString((xmlChar *)""));
@@ -176,6 +179,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 			} else if (Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE) {
 				valuePush(ctxt, xmlXPathNewBoolean(Z_TYPE(retval) == IS_TRUE));
 			} else if (Z_TYPE(retval) == IS_OBJECT) {
+				// TODO make this an error?
 				php_error_docref(NULL, E_WARNING, "A PHP Object cannot be converted to a XPath-string");
 				valuePush(ctxt, xmlXPathNewString((xmlChar *)""));
 			} else {
@@ -228,7 +232,7 @@ PHP_METHOD(DOMXPath, __construct)
 	ctx = xmlXPathNewContext(docp);
 	if (ctx == NULL) {
 		php_dom_throw_error(INVALID_STATE_ERR, 1);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	intern = Z_XPATHOBJ_P(ZEND_THIS);
@@ -308,6 +312,7 @@ PHP_METHOD(DOMXPath, registerNamespace)
 
 	ctxp = (xmlXPathContextPtr) intern->dom.ptr;
 	if (ctxp == NULL) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Invalid XPath Context");
 		RETURN_FALSE;
 	}
@@ -352,12 +357,14 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 
 	ctxp = (xmlXPathContextPtr) intern->dom.ptr;
 	if (ctxp == NULL) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Invalid XPath Context");
 		RETURN_FALSE;
 	}
 
 	docp = (xmlDocPtr) ctxp->doc;
 	if (docp == NULL) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Invalid XPath Document Pointer");
 		RETURN_FALSE;
 	}
@@ -371,6 +378,7 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 	}
 
 	if (nodep && docp != nodep->doc) {
+		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Node From Wrong Document");
 		RETURN_FALSE;
 	}
@@ -401,6 +409,7 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 	}
 
 	if (! xpathobjp) {
+		// Make this an error state?
 		RETURN_FALSE;
 	}
 

From e21c667a7315978ee58a96fcec875eab8dcbe89f Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Wed, 9 Sep 2020 17:23:51 +0200
Subject: [PATCH 2/7] Reviews

---
 ext/dom/document.c                            |  6 +--
 ext/dom/element.c                             | 15 +++-----
 ext/dom/node.c                                | 37 ++++++++++---------
 ext/dom/php_dom.c                             | 14 +++----
 ext/dom/php_dom.h                             |  5 +--
 .../tests/DOMAttr_ownerElement_error_001.phpt | 13 ++++---
 ext/dom/tests/DOMDocument_adoptNode.phpt      | 10 +++--
 ext/dom/tests/DOMDocument_encoding_basic.phpt | 18 +++++----
 ext/dom/tests/bug36756.phpt                   | 15 ++++----
 ext/dom/tests/bug77569.phpt                   | 10 +++--
 ext/dom/xpath.c                               | 33 +++++++----------
 11 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/ext/dom/document.c b/ext/dom/document.c
index 2e0fde32dd786..5295f2f0857de 100644
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -136,7 +136,7 @@ int dom_document_encoding_read(dom_object *obj, zval *retval)
 	return SUCCESS;
 }
 
-int dom_document_encoding_write(dom_object *obj, zval *newval)
+zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
 {
 	xmlDoc *docp = (xmlDocPtr) dom_object_get_node(obj);
 	zend_string *str;
@@ -161,8 +161,8 @@ int dom_document_encoding_write(dom_object *obj, zval *newval)
 		}
 		docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str));
     } else {
-    	// Todo convert to error?
-		php_error_docref(NULL, E_WARNING, "Invalid Document Encoding");
+		zend_value_error("Invalid Document Encoding");
+		return FAILURE;
     }
 
 	zend_string_release_ex(str, 0);
diff --git a/ext/dom/element.c b/ext/dom/element.c
index 75f735ce689b4..9a3693b301fdc 100644
--- a/ext/dom/element.c
+++ b/ext/dom/element.c
@@ -295,9 +295,8 @@ PHP_METHOD(DOMElement, setAttribute)
 		attr = (xmlNodePtr)xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value);
 	}
 	if (!attr) {
-		// TODO Convert to error?
-		php_error_docref(NULL, E_WARNING, "No such attribute '%s'", name);
-		RETURN_FALSE;
+		zend_argument_value_error(1, "must be a valid XML attribute");
+		RETURN_THROWS();
 	}
 
 	DOM_RET_OBJ(attr, &ret, intern);
@@ -426,9 +425,8 @@ PHP_METHOD(DOMElement, setAttributeNode)
 	DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
 
 	if (attrp->type != XML_ATTRIBUTE_NODE) {
-		// Todo convert to a ValueError?
-		php_error_docref(NULL, E_WARNING, "Attribute node is required");
-		RETURN_FALSE;
+		zend_argument_value_error(1, "must have the node attribute");
+		RETURN_THROWS();
 	}
 
 	if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) {
@@ -878,9 +876,8 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
 	DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
 
 	if (attrp->type != XML_ATTRIBUTE_NODE) {
-		// Todo convert to error?
-		php_error_docref(NULL, E_WARNING, "Attribute node is required");
-		RETURN_FALSE;
+		zend_argument_value_error(1, "must have node attribute");
+		RETURN_THROWS();
 	}
 
 	if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) {
diff --git a/ext/dom/node.c b/ext/dom/node.c
index b0cf580ce1616..50580368ea4af 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -35,7 +35,7 @@ readonly=yes
 URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-F68D095
 Since:
 */
-int dom_node_node_name_read(dom_object *obj, zval *retval)
+zend_result dom_node_node_name_read(dom_object *obj, zval *retval)
 {
 	xmlNode *nodep;
 	xmlNsPtr ns;
@@ -98,8 +98,8 @@ int dom_node_node_name_read(dom_object *obj, zval *retval)
 			str = "#text";
 			break;
 		default:
-			// Todo convert to error?
-			php_error_docref(NULL, E_WARNING, "Invalid Node Type");
+			zend_value_error("XML Node type must be one of XML_*_NODE");
+			return FAILURE;
 	}
 
 	if (str != NULL) {
@@ -984,9 +984,8 @@ PHP_METHOD(DOMNode, insertBefore)
 	}
 
 	if (NULL == new_child) {
-		// Todo convert to error?
-		php_error_docref(NULL, E_WARNING, "Couldn't add newnode as the previous sibling of refnode");
-		RETURN_FALSE;
+		zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
+		RETURN_THROWS();
 	}
 
 	dom_reconcile_ns(parentp->doc, new_child);
@@ -1227,6 +1226,7 @@ PHP_METHOD(DOMNode, appendChild)
 	if (new_child == NULL) {
 		new_child = xmlAddChild(nodep, child);
 		if (new_child == NULL) {
+			// TODO Convert to Error?
 			php_error_docref(NULL, E_WARNING, "Couldn't append node");
 			RETURN_FALSE;
 		}
@@ -1577,9 +1577,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 	docp = nodep->doc;
 
 	if (! docp) {
-		// TOdo convert to error?
-		php_error_docref(NULL, E_WARNING, "Node must be associated with a document");
-		RETURN_FALSE;
+		zend_throw_error(NULL, "Node must be associated with a document");
+		RETURN_THROWS();
 	}
 
 	if (xpath_array == NULL) {
@@ -1595,9 +1594,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 					xmlXPathFreeObject(xpathobjp);
 				}
 				xmlXPathFreeContext(ctxp);
-				// Todo convert to error?
-				php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset.");
-				RETURN_FALSE;
+				zend_throw_error(NULL, "XPath query did not return a nodeset.");
+				RETURN_THROWS();
 			}
 		}
 	} else {
@@ -1607,13 +1605,16 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 		char *xquery;
 
 		tmp = zend_hash_str_find(ht, "query", sizeof("query")-1);
-		if (tmp && Z_TYPE_P(tmp) == IS_STRING) {
-			xquery = Z_STRVAL_P(tmp);
-		} else {
-			// Todo convert to error?
-			php_error_docref(NULL, E_WARNING, "'query' missing from xpath array or is not a string");
-			RETURN_FALSE;
+		if (!tmp) {
+			// TODO Use argument version after checking which num arg it is
+			zend_value_error("\"query\" option must be in XPath array");
+			RETURN_THROWS();
+		}
+		if (Z_TYPE_P(tmp) != IS_STRING) {
+			zend_type_error("\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
+			RETURN_THROWS();
 		}
+		xquery = Z_STRVAL_P(tmp);
 
 		ctxp = xmlXPathNewContext(docp);
 		ctxp->node = nodep;
diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c
index 49e265f5953e7..3aa0962530294 100644
--- a/ext/dom/php_dom.c
+++ b/ext/dom/php_dom.c
@@ -317,8 +317,9 @@ 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)) {
-		// Should this be an error?
-		php_error(E_WARNING, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name));
+		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) {
@@ -467,9 +468,8 @@ PHP_FUNCTION(dom_import_simplexml)
 	if (nodep && nodeobj && (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE)) {
 		DOM_RET_OBJ((xmlNodePtr) nodep, &ret, (dom_object *)nodeobj);
 	} else {
-		// Should this be an error?
-		php_error_docref(NULL, E_WARNING, "Invalid Nodetype to import");
-		RETURN_NULL();
+		zend_argument_value_error(1, "cannot import invalid node type");
+		RETURN_THROWS();
 	}
 }
 /* }}} */
@@ -1210,8 +1210,8 @@ PHP_DOM_EXPORT zend_bool php_dom_create_object(xmlNodePtr obj, zval *return_valu
 			break;
 		}
 		default:
-			// Todo convert to value error?
-			php_error_docref(NULL, E_WARNING, "Unsupported node type: %d", obj->type);
+			/* TODO Convert to a ZEND assertion? */
+			zend_throw_error(NULL, "Unsupported node type: %d", obj->type);
 			ZVAL_NULL(return_value);
 			return 0;
 	}
diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
index 03b616ad4f4c4..24e1ea646a05d 100644
--- a/ext/dom/php_dom.h
+++ b/ext/dom/php_dom.h
@@ -150,10 +150,9 @@ entry = zend_register_internal_class_ex(&ce, parent_ce);
 		RETURN_THROWS(); \
 	}
 
-// Make this an error?
 #define DOM_NOT_IMPLEMENTED() \
-	php_error_docref(NULL, E_WARNING, "Not yet implemented"); \
-	return;
+	zend_throw_error(NULL, "Not yet implemented"); \
+	RETURN_THROWS();
 
 #define DOM_NODELIST 0
 #define DOM_NAMEDNODEMAP 1
diff --git a/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt b/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt
index e57c3da913c1e..67a66ae16ea35 100644
--- a/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt
+++ b/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt
@@ -14,10 +14,11 @@ $document->appendChild($root);
 $attr = $root->setAttribute('category', 'books');
 $document->removeChild($root);
 $root = null;
-var_dump($attr->ownerElement);
+try {
+    var_dump($attr->ownerElement);
+} catch (\Error $e) {
+    echo get_class($e) . ': ' . $e->getMessage() . \PHP_EOL;
+}
 ?>
---EXPECTF--
-Warning: Couldn't fetch DOMAttr. Node no longer exists in %s on line %d
-
-Warning: Undefined property: DOMAttr::$ownerElement in %s on line %d
-NULL
+--EXPECT--
+Error: Couldn't fetch DOMAttr. Node no longer exists
diff --git a/ext/dom/tests/DOMDocument_adoptNode.phpt b/ext/dom/tests/DOMDocument_adoptNode.phpt
index 5fc8e79d9dc31..0f0a32ae498ff 100644
--- a/ext/dom/tests/DOMDocument_adoptNode.phpt
+++ b/ext/dom/tests/DOMDocument_adoptNode.phpt
@@ -10,7 +10,11 @@ require_once('skipif.inc');
 $dom = new DOMDocument();
 $dom->loadXML("<root />");
 
-$dom->adoptNode($dom->documentElement);
+try {
+    $dom->adoptNode($dom->documentElement);
+} catch (\Error $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 ?>
---EXPECTF--
-Warning: DOMDocument::adoptNode(): Not yet implemented in %s
+--EXPECT--
+Not yet implemented
diff --git a/ext/dom/tests/DOMDocument_encoding_basic.phpt b/ext/dom/tests/DOMDocument_encoding_basic.phpt
index 6ac405da69859..f05a74014bf82 100644
--- a/ext/dom/tests/DOMDocument_encoding_basic.phpt
+++ b/ext/dom/tests/DOMDocument_encoding_basic.phpt
@@ -19,10 +19,14 @@ if( !$dom )
     exit;
 }
 
-echo "Empty Encoding Read: {$dom->encoding}\n";
+echo "Empty Encoding Read: '{$dom->encoding}'\n";
 
-$ret = $dom->encoding = 'NYPHP DOMinatrix';
-echo "Adding invalid encoding: $ret\n";
+try {
+    $ret = $dom->encoding = 'NYPHP DOMinatrix';
+    echo "Adding invalid encoding: $ret\n";
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 
 $ret = $dom->encoding = 'ISO-8859-1';
 echo "Adding ISO-8859-1 encoding: $ret\n";
@@ -38,11 +42,9 @@ echo "UTF-16 Encoding Read: {$dom->encoding}\n";
 
 
 ?>
---EXPECTF--
-Empty Encoding Read: 
-
-Warning: main(): Invalid Document Encoding in %s on line %d
-Adding invalid encoding: NYPHP DOMinatrix
+--EXPECT--
+Empty Encoding Read: ''
+Invalid Document Encoding
 Adding ISO-8859-1 encoding: ISO-8859-1
 ISO-8859-1 Encoding Read: ISO-8859-1
 Adding UTF-8 encoding: UTF-8
diff --git a/ext/dom/tests/bug36756.phpt b/ext/dom/tests/bug36756.phpt
index ccf9ba5049bb1..bf7aa20276bf0 100644
--- a/ext/dom/tests/bug36756.phpt
+++ b/ext/dom/tests/bug36756.phpt
@@ -13,7 +13,6 @@ $node = $xpath->query('/root')->item(0);
 echo $node->nodeName . "\n";
 $dom->removeChild($GLOBALS['dom']->firstChild);
 echo "nodeType: " . $node->nodeType . "\n";
-
 /* Node gets destroyed during removeChild */
 $dom->loadXML('<root><child/></root>');
 $xpath = new DOMXpath($dom);
@@ -21,15 +20,15 @@ $node = $xpath->query('//child')->item(0);
 echo $node->nodeName . "\n";
 $GLOBALS['dom']->removeChild($GLOBALS['dom']->firstChild);
 
-echo "nodeType: " . $node->nodeType . "\n";
+try {
+    echo "nodeType: " . $node->nodeType . "\n";
+} catch (\Error $e) {
+    echo get_class($e) . ': ' . $e->getMessage() .\PHP_EOL;
+}
 
 ?>
---EXPECTF--
+--EXPECT--
 root
 nodeType: 1
 child
-
-Warning: Couldn't fetch DOMElement. Node no longer exists in %sbug36756.php on line %d
-
-Warning: Undefined property: DOMElement::$nodeType in %s on line %d
-nodeType:
+Error: Couldn't fetch DOMElement. Node no longer exists
diff --git a/ext/dom/tests/bug77569.phpt b/ext/dom/tests/bug77569.phpt
index 9eef2af65a788..8e25d7b6b57dc 100644
--- a/ext/dom/tests/bug77569.phpt
+++ b/ext/dom/tests/bug77569.phpt
@@ -8,7 +8,11 @@ if (!extension_loaded('dom')) die('skip dom extension not available');
 <?php
 $imp = new DOMImplementation;
 $dom = $imp->createDocument("", "");
-$dom->encoding = null;
+try {
+    $dom->encoding = null;
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 ?>
---EXPECTF--
-Warning: main(): Invalid Document Encoding in %s on line %d
+--EXPECT--
+Invalid Document Encoding
diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index 0d8246e81b40e..3fa91ec2bb549 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -136,8 +136,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 
 	obj = valuePop(ctxt);
 	if (obj->stringval == NULL) {
-		// Todo convert to type error?
-		php_error_docref(NULL, E_WARNING, "Handler name must be a string");
+		zend_type_error("Handler name must be a string");
 		xmlXPathFreeObject(obj);
 		if (fci.param_count > 0) {
 			for (i = 0; i < nargs - 1; i++) {
@@ -155,13 +154,11 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 	fci.retval = &retval;
 
 	if (!zend_make_callable(&fci.function_name, &callable)) {
-		// TODO make this an error?
-		php_error_docref(NULL, E_WARNING, "Unable to call handler %s()", ZSTR_VAL(callable));
+		zend_throw_error(NULL, "Unable to call handler %s()", ZSTR_VAL(callable));
+		return;
 	} else if (intern->registerPhpFunctions == 2 && zend_hash_exists(intern->registered_phpfunctions, callable) == 0) {
-		// TODO make this an error?
-		php_error_docref(NULL, E_WARNING, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable));
-		/* Push an empty string, so that we at least have an xslt result... */
-		valuePush(ctxt, xmlXPathNewString((xmlChar *)""));
+		zend_throw_error(NULL, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable));
+		return;
 	} else {
 		result = zend_call_function(&fci, NULL);
 		if (result == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
@@ -179,9 +176,8 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 			} else if (Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE) {
 				valuePush(ctxt, xmlXPathNewBoolean(Z_TYPE(retval) == IS_TRUE));
 			} else if (Z_TYPE(retval) == IS_OBJECT) {
-				// TODO make this an error?
-				php_error_docref(NULL, E_WARNING, "A PHP Object cannot be converted to a XPath-string");
-				valuePush(ctxt, xmlXPathNewString((xmlChar *)""));
+				zend_type_error("A PHP Object cannot be converted to a XPath-string");
+				return;
 			} else {
 				zend_string *str = zval_get_string(&retval);
 				valuePush(ctxt, xmlXPathNewString((xmlChar *) ZSTR_VAL(str)));
@@ -312,9 +308,8 @@ PHP_METHOD(DOMXPath, registerNamespace)
 
 	ctxp = (xmlXPathContextPtr) intern->dom.ptr;
 	if (ctxp == NULL) {
-		// Todo convert to error?
-		php_error_docref(NULL, E_WARNING, "Invalid XPath Context");
-		RETURN_FALSE;
+		zend_throw_error(NULL, "Invalid XPath Context");
+		RETURN_THROWS();
 	}
 
 	if (xmlXPathRegisterNs(ctxp, prefix, ns_uri) != 0) {
@@ -357,9 +352,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 
 	ctxp = (xmlXPathContextPtr) intern->dom.ptr;
 	if (ctxp == NULL) {
-		// Todo convert to error?
-		php_error_docref(NULL, E_WARNING, "Invalid XPath Context");
-		RETURN_FALSE;
+		zend_throw_error(NULL, "Invalid XPath Context");
+		RETURN_THROWS();
 	}
 
 	docp = (xmlDocPtr) ctxp->doc;
@@ -378,9 +372,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 	}
 
 	if (nodep && docp != nodep->doc) {
-		// Todo convert to error?
-		php_error_docref(NULL, E_WARNING, "Node From Wrong Document");
-		RETURN_FALSE;
+		zend_throw_error(NULL, "Node From Wrong Document");
+		RETURN_THROWS();
 	}
 
 	ctxp->node = nodep;

From 702e0623385a03ea92a77f98c64356b3e12dc75a Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Fri, 11 Sep 2020 16:26:35 +0200
Subject: [PATCH 3/7] Review

---
 ext/dom/document.c                            |  2 +-
 ext/dom/node.c                                |  7 +++----
 ext/dom/php_dom.c                             |  2 +-
 ext/dom/tests/DOMDocument_encoding_basic.phpt |  2 +-
 ext/dom/tests/bug77569.phpt                   |  2 +-
 ext/dom/tests/domxpath.phpt                   |  8 ++++++++
 ext/dom/xpath.c                               | 15 +++++----------
 7 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/ext/dom/document.c b/ext/dom/document.c
index 5295f2f0857de..b580842667d3a 100644
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -161,7 +161,7 @@ zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
 		}
 		docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str));
     } else {
-		zend_value_error("Invalid Document Encoding");
+		zend_value_error("Invalid document encoding");
 		return FAILURE;
     }
 
diff --git a/ext/dom/node.c b/ext/dom/node.c
index 50580368ea4af..cf4f73f416e1a 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -1594,7 +1594,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 					xmlXPathFreeObject(xpathobjp);
 				}
 				xmlXPathFreeContext(ctxp);
-				zend_throw_error(NULL, "XPath query did not return a nodeset.");
+				zend_throw_error(NULL, "XPath query did not return a nodeset");
 				RETURN_THROWS();
 			}
 		}
@@ -1606,12 +1606,11 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 
 		tmp = zend_hash_str_find(ht, "query", sizeof("query")-1);
 		if (!tmp) {
-			// TODO Use argument version after checking which num arg it is
-			zend_value_error("\"query\" option must be in XPath array");
+			zend_argument_value_error(3, "\"query\" option must be in XPath array");
 			RETURN_THROWS();
 		}
 		if (Z_TYPE_P(tmp) != IS_STRING) {
-			zend_type_error("\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
+			zend_argument_type_error(3, "\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
 			RETURN_THROWS();
 		}
 		xquery = Z_STRVAL_P(tmp);
diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c
index 3aa0962530294..78d22f6a5e05c 100644
--- a/ext/dom/php_dom.c
+++ b/ext/dom/php_dom.c
@@ -468,7 +468,7 @@ PHP_FUNCTION(dom_import_simplexml)
 	if (nodep && nodeobj && (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE)) {
 		DOM_RET_OBJ((xmlNodePtr) nodep, &ret, (dom_object *)nodeobj);
 	} else {
-		zend_argument_value_error(1, "cannot import invalid node type");
+		zend_argument_value_error(1, "is not a valid node type");
 		RETURN_THROWS();
 	}
 }
diff --git a/ext/dom/tests/DOMDocument_encoding_basic.phpt b/ext/dom/tests/DOMDocument_encoding_basic.phpt
index f05a74014bf82..925c84d5f185e 100644
--- a/ext/dom/tests/DOMDocument_encoding_basic.phpt
+++ b/ext/dom/tests/DOMDocument_encoding_basic.phpt
@@ -44,7 +44,7 @@ echo "UTF-16 Encoding Read: {$dom->encoding}\n";
 ?>
 --EXPECT--
 Empty Encoding Read: ''
-Invalid Document Encoding
+Invalid document encoding
 Adding ISO-8859-1 encoding: ISO-8859-1
 ISO-8859-1 Encoding Read: ISO-8859-1
 Adding UTF-8 encoding: UTF-8
diff --git a/ext/dom/tests/bug77569.phpt b/ext/dom/tests/bug77569.phpt
index 8e25d7b6b57dc..4c63e263e47d6 100644
--- a/ext/dom/tests/bug77569.phpt
+++ b/ext/dom/tests/bug77569.phpt
@@ -15,4 +15,4 @@ try {
 }
 ?>
 --EXPECT--
-Invalid Document Encoding
+Invalid document encoding
diff --git a/ext/dom/tests/domxpath.phpt b/ext/dom/tests/domxpath.phpt
index f67b2584aacba..ad4fc8a8ba1c5 100644
--- a/ext/dom/tests/domxpath.phpt
+++ b/ext/dom/tests/domxpath.phpt
@@ -50,9 +50,17 @@ $root->appendChild($dom->createElementNS("urn::default", "testnode", 5));
 
 $avg = $xpath->evaluate('number(php:function("MyAverage", //def:testnode))');
 var_dump($avg);
+
+try {
+    $xpath->registerPHPFunctions('non_existent');
+    $avg = $xpath->evaluate('number(php:function("non_existent", //def:testnode))');
+} catch (\Error $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 ?>
 --EXPECT--
 myval
 float(1)
 bool(true)
 float(4)
+Unable to call handler non_existent()
diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index 3fa91ec2bb549..da24e259d23a5 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -138,13 +138,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 	if (obj->stringval == NULL) {
 		zend_type_error("Handler name must be a string");
 		xmlXPathFreeObject(obj);
-		if (fci.param_count > 0) {
-			for (i = 0; i < nargs - 1; i++) {
-				zval_ptr_dtor(&fci.params[i]);
-			}
-			efree(fci.params);
-		}
-		return;
+		goto cleanup;
 	}
 	ZVAL_STRING(&fci.function_name, (char *) obj->stringval);
 	xmlXPathFreeObject(obj);
@@ -155,10 +149,10 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 
 	if (!zend_make_callable(&fci.function_name, &callable)) {
 		zend_throw_error(NULL, "Unable to call handler %s()", ZSTR_VAL(callable));
-		return;
+		goto cleanup;
 	} else if (intern->registerPhpFunctions == 2 && zend_hash_exists(intern->registered_phpfunctions, callable) == 0) {
 		zend_throw_error(NULL, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable));
-		return;
+		goto cleanup;
 	} else {
 		result = zend_call_function(&fci, NULL);
 		if (result == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
@@ -186,6 +180,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
 			zval_ptr_dtor(&retval);
 		}
 	}
+cleanup:
 	zend_string_release_ex(callable, 0);
 	zval_ptr_dtor_str(&fci.function_name);
 	if (fci.param_count > 0) {
@@ -372,7 +367,7 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 	}
 
 	if (nodep && docp != nodep->doc) {
-		zend_throw_error(NULL, "Node From Wrong Document");
+		zend_throw_error(NULL, "Node from wrong document");
 		RETURN_THROWS();
 	}
 

From a626b0315ab6cbee902740e5e9277f0e04095988 Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Sun, 13 Sep 2020 15:13:13 +0200
Subject: [PATCH 4/7] XPath test addition

---
 ext/dom/tests/domxpath.phpt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/ext/dom/tests/domxpath.phpt b/ext/dom/tests/domxpath.phpt
index ad4fc8a8ba1c5..32aaffee01869 100644
--- a/ext/dom/tests/domxpath.phpt
+++ b/ext/dom/tests/domxpath.phpt
@@ -57,6 +57,12 @@ try {
 } catch (\Error $e) {
     echo $e->getMessage() . \PHP_EOL;
 }
+try {
+    $xpath->registerPhpFunctions(['non_existant']);
+    $avg = $xpath->evaluate('number(php:function("non_existent", //def:testnode))');
+} catch (\Error $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 ?>
 --EXPECT--
 myval
@@ -64,3 +70,4 @@ float(1)
 bool(true)
 float(4)
 Unable to call handler non_existent()
+Unable to call handler non_existent()

From 81022bceaea780887e37a2c34cb912c19004c9fd Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Sun, 13 Sep 2020 15:28:01 +0200
Subject: [PATCH 5/7] Drop comments and split a warning case

---
 ext/dom/documentfragment.c |  1 -
 ext/dom/element.c          |  2 --
 ext/dom/node.c             |  9 +++------
 ext/dom/parentnode.c       |  2 --
 ext/dom/text.c             | 15 ++++++++++-----
 ext/dom/xpath.c            |  3 +--
 6 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/ext/dom/documentfragment.c b/ext/dom/documentfragment.c
index a5ae898f86cf7..051416792d349 100644
--- a/ext/dom/documentfragment.c
+++ b/ext/dom/documentfragment.c
@@ -109,7 +109,6 @@ PHP_METHOD(DOMDocumentFragment, appendXML) {
 	DOM_GET_OBJ(nodep, id, xmlNodePtr, intern);
 
 	if (dom_node_is_read_only(nodep) == SUCCESS) {
-		// Should always be in strict mode?
 		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(intern->document));
 		RETURN_FALSE;
 	}
diff --git a/ext/dom/element.c b/ext/dom/element.c
index 9a3693b301fdc..58cf20672143e 100644
--- a/ext/dom/element.c
+++ b/ext/dom/element.c
@@ -268,7 +268,6 @@ PHP_METHOD(DOMElement, setAttribute)
 	DOM_GET_OBJ(nodep, id, xmlNodePtr, intern);
 
 	if (dom_node_is_read_only(nodep) == SUCCESS) {
-		// Todo make it always strict?
 		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(intern->document));
 		RETURN_FALSE;
 	}
@@ -642,7 +641,6 @@ PHP_METHOD(DOMElement, setAttributeNS)
 		RETURN_NULL();
 	}
 
-	// Todo should always be strict?
 	errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len);
 
 	if (errorcode == 0) {
diff --git a/ext/dom/node.c b/ext/dom/node.c
index cf4f73f416e1a..12246b6cdbd13 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -857,7 +857,6 @@ PHP_METHOD(DOMNode, insertBefore)
 
 	new_child = NULL;
 
-	// Todo should always be strict?
 	stricterror = dom_get_strict_error(intern->document);
 
 	if (dom_node_is_read_only(parentp) == SUCCESS ||
@@ -877,7 +876,7 @@ PHP_METHOD(DOMNode, insertBefore)
 	}
 
 	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
-		// Todo convert to Error?
+		/* TODO Drop Warning? */
 		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
 		RETURN_FALSE;
 	}
@@ -1026,7 +1025,6 @@ PHP_METHOD(DOMNode, replaceChild)
 		RETURN_FALSE;
 	}
 
-	// Todo make alway strict?
 	stricterror = dom_get_strict_error(intern->document);
 
 	if (dom_node_is_read_only(nodep) == SUCCESS ||
@@ -1177,7 +1175,7 @@ PHP_METHOD(DOMNode, appendChild)
 	}
 
 	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
-		// Todo convert to error?
+		/* TODO Drop Warning? */
 		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
 		RETURN_FALSE;
 	}
@@ -1468,7 +1466,6 @@ PHP_METHOD(DOMNode, lookupPrefix)
 		}
 	}
 
-	// Todo return empty string?
 	RETURN_NULL();
 }
 /* }}} end dom_node_lookup_prefix */
@@ -1748,7 +1745,7 @@ PHP_METHOD(DOMNode, getNodePath)
 
 	value = (char *) xmlGetNodePath(nodep);
 	if (value == NULL) {
-		// Todo return empty string?
+		/* TODO Research if can return empty string */
 		RETURN_NULL();
 	} else {
 		RETVAL_STRING(value);
diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c
index 8650354763767..f47416edff3dd 100644
--- a/ext/dom/parentnode.c
+++ b/ext/dom/parentnode.c
@@ -151,7 +151,6 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
 		return NULL;
 	}
 
-	// Todo always strict mode?
 	stricterror = dom_get_strict_error(document);
 
 	for (i = 0; i < nodesc; i++) {
@@ -379,7 +378,6 @@ void dom_child_node_remove(dom_object *context)
 		return;
 	}
 
-	// Todo always strict?
 	stricterror = dom_get_strict_error(context->document);
 
 	if (dom_node_is_read_only(child) == SUCCESS ||
diff --git a/ext/dom/text.c b/ext/dom/text.c
index 76c66a898f88c..7a45be246360e 100644
--- a/ext/dom/text.c
+++ b/ext/dom/text.c
@@ -122,20 +122,25 @@ PHP_METHOD(DOMText, splitText)
 	}
 	DOM_GET_OBJ(node, id, xmlNodePtr, intern);
 
+	if (offset < 0) {
+		zend_argument_value_error(1, "must be greater than or equal to 0");
+		RETURN_THROWS();
+	}
+
 	if (node->type != XML_TEXT_NODE && node->type != XML_CDATA_SECTION_NODE) {
-		// Todo make this throw an error?
+		/* TODO Add warning? */
 		RETURN_FALSE;
 	}
 
 	cur = xmlNodeGetContent(node);
 	if (cur == NULL) {
-		// Todo make this throw an error?
+		/* TODO Add warning? */
 		RETURN_FALSE;
 	}
 	length = xmlUTF8Strlen(cur);
 
-	if (ZEND_LONG_INT_OVFL(offset) || (int)offset > length || offset < 0) {
-		// Todo make this throw an error?
+	if (ZEND_LONG_INT_OVFL(offset) || (int)offset > length) {
+		/* TODO Add warning? */
 		xmlFree(cur);
 		RETURN_FALSE;
 	}
@@ -152,7 +157,7 @@ PHP_METHOD(DOMText, splitText)
 	xmlFree(second);
 
 	if (nnode == NULL) {
-		// Todo make this throw an error?
+		/* TODO Add warning? */
 		RETURN_FALSE;
 	}
 
diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c
index da24e259d23a5..1f97601c426d1 100644
--- a/ext/dom/xpath.c
+++ b/ext/dom/xpath.c
@@ -353,7 +353,6 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 
 	docp = (xmlDocPtr) ctxp->doc;
 	if (docp == NULL) {
-		// Todo convert to error?
 		php_error_docref(NULL, E_WARNING, "Invalid XPath Document Pointer");
 		RETURN_FALSE;
 	}
@@ -397,7 +396,7 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 	}
 
 	if (! xpathobjp) {
-		// Make this an error state?
+		/* TODO Add Warning? */
 		RETURN_FALSE;
 	}
 

From 59ac05e4fce594042ca3a1f117ab07026d4cb06b Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Tue, 15 Sep 2020 02:51:11 +0200
Subject: [PATCH 6/7] Address review

assertion for C programming error, normalize another warning case to error + test
---
 ext/dom/element.c                         |  8 ++++----
 ext/dom/node.c                            | 16 ++++++++--------
 ext/dom/tests/DOMNode_C14NFile_basic.phpt | 12 ++++++++++++
 ext/dom/tests/DOMNode_C14N_basic.phpt     | 13 +++++++++++++
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/ext/dom/element.c b/ext/dom/element.c
index 58cf20672143e..317619001beea 100644
--- a/ext/dom/element.c
+++ b/ext/dom/element.c
@@ -873,10 +873,10 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
 
 	DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
 
-	if (attrp->type != XML_ATTRIBUTE_NODE) {
-		zend_argument_value_error(1, "must have node attribute");
-		RETURN_THROWS();
-	}
+	/* ZPP Guarantees that a DOMAttr class is given, as it is converted to a xmlAttr
+	 * to pass to libxml (see http://www.xmlsoft.org/html/libxml-tree.html#xmlAttr)
+	 * if it is not of type XML_ATTRIBUTE_NODE it indicates a bug somewhere */
+	ZEND_ASSERT(attrp->type == XML_ATTRIBUTE_NODE);
 
 	if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) {
 		php_dom_throw_error(WRONG_DOCUMENT_ERR, dom_get_strict_error(intern->document));
diff --git a/ext/dom/node.c b/ext/dom/node.c
index 12246b6cdbd13..a6f88b5c0f1bb 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -35,7 +35,7 @@ readonly=yes
 URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-F68D095
 Since:
 */
-zend_result dom_node_node_name_read(dom_object *obj, zval *retval)
+int dom_node_node_name_read(dom_object *obj, zval *retval)
 {
 	xmlNode *nodep;
 	xmlNsPtr ns;
@@ -97,9 +97,7 @@ zend_result dom_node_node_name_read(dom_object *obj, zval *retval)
 		case XML_TEXT_NODE:
 			str = "#text";
 			break;
-		default:
-			zend_value_error("XML Node type must be one of XML_*_NODE");
-			return FAILURE;
+		EMPTY_SWITCH_DEFAULT_CASE();
 	}
 
 	if (str != NULL) {
@@ -1603,11 +1601,13 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 
 		tmp = zend_hash_str_find(ht, "query", sizeof("query")-1);
 		if (!tmp) {
-			zend_argument_value_error(3, "\"query\" option must be in XPath array");
+			/* if mode == 0 then $xpath arg is 3, if mode == 1 then $xpath is 4 */
+			zend_argument_value_error(3 + mode, "must have a \"query\" key");
 			RETURN_THROWS();
 		}
 		if (Z_TYPE_P(tmp) != IS_STRING) {
-			zend_argument_type_error(3, "\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
+			/* if mode == 0 then $xpath arg is 3, if mode == 1 then $xpath is 4 */
+			zend_argument_type_error(3 + mode, "\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
 			RETURN_THROWS();
 		}
 		xquery = Z_STRVAL_P(tmp);
@@ -1638,8 +1638,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
 				xmlXPathFreeObject(xpathobjp);
 			}
 			xmlXPathFreeContext(ctxp);
-			php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset.");
-			RETURN_FALSE;
+			zend_throw_error(NULL, "XPath query did not return a nodeset");
+			RETURN_THROWS();
 		}
 	}
 
diff --git a/ext/dom/tests/DOMNode_C14NFile_basic.phpt b/ext/dom/tests/DOMNode_C14NFile_basic.phpt
index b006e47deb7ad..85a8b6f1a51bd 100644
--- a/ext/dom/tests/DOMNode_C14NFile_basic.phpt
+++ b/ext/dom/tests/DOMNode_C14NFile_basic.phpt
@@ -27,6 +27,16 @@ $node = $doc->getElementsByTagName('title')->item(0);
 var_dump($node->C14NFile($output));
 $content = file_get_contents($output);
 var_dump($content);
+try {
+    var_dump($node->C14NFile($output, false, false, []));
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+try {
+    var_dump($node->C14NFile($output, false, false, ['query' => []]));
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 ?>
 --CLEAN--
 <?php
@@ -36,3 +46,5 @@ unlink($output);
 --EXPECT--
 int(34)
 string(34) "<title>The Grapes of Wrath</title>"
+DOMNode::C14NFile(): Argument #4 ($xpath) must have a "query" key
+DOMNode::C14NFile(): Argument #4 ($xpath) "query" option must be a string, array given
diff --git a/ext/dom/tests/DOMNode_C14N_basic.phpt b/ext/dom/tests/DOMNode_C14N_basic.phpt
index bbfd7bcefb298..79a5366e222f5 100644
--- a/ext/dom/tests/DOMNode_C14N_basic.phpt
+++ b/ext/dom/tests/DOMNode_C14N_basic.phpt
@@ -24,6 +24,19 @@ $doc = new DOMDocument();
 $doc->loadXML($xml);
 $node = $doc->getElementsByTagName('title')->item(0);
 var_dump($node->C14N());
+
+try {
+    var_dump($node->C14N(false, false, []));
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+try {
+    var_dump($node->C14N(false, false, ['query' => []]));
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 ?>
 --EXPECT--
 string(34) "<title>The Grapes of Wrath</title>"
+DOMNode::C14N(): Argument #3 ($xpath) must have a "query" key
+DOMNode::C14N(): Argument #3 ($xpath) "query" option must be a string, array given

From bff98683ae5eb8ed61c7c36508a261442744a627 Mon Sep 17 00:00:00 2001
From: George Peter Banyard <girgias@php.net>
Date: Tue, 22 Sep 2020 15:53:56 +0100
Subject: [PATCH 7/7] Adjust stubs

---
 ext/dom/php_dom.stub.php  | 4 ++--
 ext/dom/php_dom_arginfo.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ext/dom/php_dom.stub.php b/ext/dom/php_dom.stub.php
index 5f15f99e174e5..bce79c9177d61 100644
--- a/ext/dom/php_dom.stub.php
+++ b/ext/dom/php_dom.stub.php
@@ -201,7 +201,7 @@ public function hasAttributeNS(?string $namespace, string $localName) {}
     /** @return bool */
     public function removeAttribute(string $qualifiedName) {}
 
-    /** @return DOMAttr|false */
+    /** @return void */
     public function removeAttributeNS(?string $namespace, string $localName) {}
 
     /** @return DOMAttr|false */
@@ -210,7 +210,7 @@ public function removeAttributeNode(string $qualifiedName) {}
     /** @return DOMAttr|bool */
     public function setAttribute(string $qualifiedName, string $value) {}
 
-    /** @return bool|null */
+    /** @return void */
     public function setAttributeNS(?string $namespace, string $qualifiedName, string $value) {}
 
     /** @return DOMAttr|null|false */
diff --git a/ext/dom/php_dom_arginfo.h b/ext/dom/php_dom_arginfo.h
index d5da40e46393f..fb8aaf59f23d9 100644
--- a/ext/dom/php_dom_arginfo.h
+++ b/ext/dom/php_dom_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 8ac9356f9b19b84e98d335bc9d091b022a0f549d */
+ * Stub hash: 128108b08807ce0b125fc7b963bf3c5b77e6987a */
 
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_dom_import_simplexml, 0, 1, DOMElement, 1)
 	ZEND_ARG_TYPE_INFO(0, node, IS_OBJECT, 0)