Promote warnings to exceptions in ext/simplexml

Closes GH-6011

Co-authored-by: Nikita Popov <nikita.ppv@gmail.com>

9 files changed, 76 insertions, 48 deletions

diff --git a/Zend/zend_object_handlers.h b/Zend/zend_object_handlers.h
index b401d72ca2..fedc70cc80 100644
--- a/Zend/zend_object_handlers.h
+++ b/Zend/zend_object_handlers.h
@@ -60,7 +60,13 @@ typedef zval *(*zend_object_write_property_t)(zend_object *object, zend_string *
 typedef void (*zend_object_write_dimension_t)(zend_object *object, zval *offset, zval *value);
 
 
-/* Used to create pointer to the property of the object, for future direct r/w access */
+/* Used to create pointer to the property of the object, for future direct r/w access.
+ * May return one of:
+ *  * A zval pointer, without incrementing the reference count.
+ *  * &EG(error_zval), if an exception has been thrown.
+ *  * NULL, if acquiring a direct pointer is not possible.
+ *    In this case, the VM will fall back to using read_property and write_property.
+ */
 typedef zval *(*zend_object_get_property_ptr_ptr_t)(zend_object *object, zend_string *member, int type, void **cache_slot);
 
 /* Used to check if a property of the object exists */
diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
index f788502703..1944ecf797 100644
--- a/ext/simplexml/simplexml.c
+++ b/ext/simplexml/simplexml.c
@@ -411,7 +411,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
 			 * and could also be E_PARSE, but we use this only during parsing
 			 * and this is during runtime.
 			 */
-			zend_throw_error(NULL, "Cannot create unnamed attribute");
+			zend_throw_error(NULL, "Cannot append to an attribute list");
 			return &EG(error_zval);
 		}
 		goto long_dim;
@@ -436,7 +436,7 @@ long_dim:
 			}
 
 			if (!Z_STRLEN_P(member)) {
-				php_error_docref(NULL, E_WARNING, "Cannot write or create unnamed %s", attribs ? "attribute" : "element");
+				zend_value_error("Cannot create %s with an empty name", attribs ? "attribute" : "element");
 				if (member == &tmp_zv) {
 					zval_ptr_dtor_str(&tmp_zv);
 				}
@@ -464,7 +464,7 @@ long_dim:
 			 * and could also be E_PARSE, but we use this only during parsing
 			 * and this is during runtime.
 			 */
-			zend_throw_error(NULL, "Cannot create unnamed attribute");
+			zend_value_error("Cannot append to an attribute list");
 			return &EG(error_zval);
 		}
 		if (attribs && !node && sxe->iter.type == SXE_ITER_ELEMENT) {
@@ -489,8 +489,8 @@ long_dim:
 				if (Z_OBJCE_P(value) == sxe_class_entry) {
 					zval zval_copy;
 					if (sxe_object_cast_ex(Z_OBJ_P(value), &zval_copy, IS_STRING) == FAILURE) {
-						zend_error(E_ERROR, "Unable to cast node to string");
-						/* FIXME: Should not be fatal */
+						zend_throw_error(NULL, "Unable to cast node to string");
+						return &EG(error_zval);
 					}
 
 					value_str = Z_STR(zval_copy);
@@ -501,7 +501,7 @@ long_dim:
 				if (member == &tmp_zv) {
 					zval_ptr_dtor_str(&tmp_zv);
 				}
-				zend_error(E_WARNING, "It is not yet possible to assign complex types to %s", attribs ? "attributes" : "properties");
+				zend_type_error("It's not possible to assign a complex type to %s, %s given", attribs ? "attributes" : "properties", zend_zval_type_name(value));
 				return &EG(error_zval);
 		}
 	}
@@ -656,7 +656,7 @@ static zval *sxe_property_get_adr(zend_object *object, zend_string *zname, int f
 	}
 	ZVAL_STR(&member, zname);
 	if (sxe_prop_dim_write(object, &member, NULL, 1, 0, &node) == &EG(error_zval)) {
-		return NULL;
+		return &EG(error_zval);
 	}
 	type = SXE_ITER_NONE;
 	name = NULL;
@@ -1684,8 +1684,8 @@ SXE_METHOD(addChild)
 	}
 
 	if (qname_len == 0) {
-		php_error_docref(NULL, E_WARNING, "Element name is required");
-		return;
+		zend_argument_value_error(1, "cannot be empty");
+		RETURN_THROWS();
 	}
 
 	sxe = Z_SXEOBJ_P(ZEND_THIS);
@@ -1749,8 +1749,8 @@ SXE_METHOD(addAttribute)
 	}
 
 	if (qname_len == 0) {
-		php_error_docref(NULL, E_WARNING, "Attribute name is required");
-		return;
+		zend_argument_value_error(1, "cannot be empty");
+		RETURN_THROWS();
 	}
 
 	sxe = Z_SXEOBJ_P(ZEND_THIS);
@@ -2274,8 +2274,8 @@ PHP_FUNCTION(simplexml_load_file)
 	}
 
 	if (ZEND_LONG_EXCEEDS_INT(options)) {
-		php_error_docref(NULL, E_WARNING, "Invalid options");
-		RETURN_FALSE;
+		zend_argument_value_error(3, "is too large");
+		RETURN_THROWS();
 	}
 
 	docp = xmlReadFile(filename, NULL, (int)options);
@@ -2319,16 +2319,16 @@ PHP_FUNCTION(simplexml_load_string)
 	}
 
 	if (ZEND_SIZE_T_INT_OVFL(data_len)) {
-		php_error_docref(NULL, E_WARNING, "Data is too long");
-		RETURN_FALSE;
+		zend_argument_value_error(1, "is too long");
+		RETURN_THROWS();
 	}
 	if (ZEND_SIZE_T_INT_OVFL(ns_len)) {
-		php_error_docref(NULL, E_WARNING, "Namespace is too long");
-		RETURN_FALSE;
+		zend_argument_value_error(4, "is too long");
+		RETURN_THROWS();
 	}
 	if (ZEND_LONG_EXCEEDS_INT(options)) {
-		php_error_docref(NULL, E_WARNING, "Invalid options");
-		RETURN_FALSE;
+		zend_argument_value_error(3, "is too large");
+		RETURN_THROWS();
 	}
 
 	docp = xmlReadMemory(data, (int)data_len, NULL, NULL, (int)options);
diff --git a/ext/simplexml/simplexml.stub.php b/ext/simplexml/simplexml.stub.php
index d3e48b0e6a..18cddf2886 100644
--- a/ext/simplexml/simplexml.stub.php
+++ b/ext/simplexml/simplexml.stub.php
@@ -59,7 +59,7 @@ class SimpleXMLElement implements Stringable, Countable, RecursiveIterator
     /** @return bool */
     public function valid() {}
 
-    /** @return ?SimpleXMLElement */
+    /** @return SimpleXMLElement|null */
     public function current() {}
 
     /** @return string|false */
@@ -71,7 +71,7 @@ class SimpleXMLElement implements Stringable, Countable, RecursiveIterator
     /** @return bool */
     public function hasChildren() {}
 
-    /** @return ?SimpleXMLElement */
+    /** @return SimpleXMLElement|null */
     public function getChildren() {}
 }
 
diff --git a/ext/simplexml/simplexml_arginfo.h b/ext/simplexml/simplexml_arginfo.h
index dc369e0321..5aa6d50ba1 100644
--- a/ext/simplexml/simplexml_arginfo.h
+++ b/ext/simplexml/simplexml_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: fbe25d8a7a0a1de0cbd5dc9118e77a2e8d5dbd67 */
+ * Stub hash: 953089f230247acf18b9ac48c0a4c516d144a987 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_simplexml_load_file, 0, 1, SimpleXMLElement, MAY_BE_FALSE)
 	ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
diff --git a/ext/simplexml/tests/012.phpt b/ext/simplexml/tests/012.phpt
index b687b71988..307f8219cd 100644
--- a/ext/simplexml/tests/012.phpt
+++ b/ext/simplexml/tests/012.phpt
@@ -14,8 +14,12 @@ EOF;
 
 $sxe = simplexml_load_string($xml);
 
+try {
+    $sxe[""] = "value";
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-$sxe[""] = "warning";
 $sxe["attr"] = "value";
 
 echo $sxe->asXML();
@@ -24,19 +28,19 @@ $sxe["attr"] = "new value";
 
 echo $sxe->asXML();
 
-$sxe[] = "error";
+try {
+    $sxe[] = "error";
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
 __HALT_COMPILER();
 ?>
 ===DONE===
---EXPECTF--
-Warning: main(): Cannot write or create unnamed attribute in %s012.php on line %d
+--EXPECT--
+Cannot create attribute with an empty name
 <?xml version="1.0" encoding="ISO-8859-1"?>
 <foo attr="value"/>
 <?xml version="1.0" encoding="ISO-8859-1"?>
 <foo attr="new value"/>
-
-Fatal error: Uncaught Error: Cannot create unnamed attribute in %s012.php:%d
-Stack trace:
-#0 {main}
-  thrown in %s012.php on line %d
+Cannot append to an attribute list
diff --git a/ext/simplexml/tests/SimpleXMLElement_addAttribute_required_attribute_name.phpt b/ext/simplexml/tests/SimpleXMLElement_addAttribute_required_attribute_name.phpt
index 34e2e68319..50928ff55e 100644
--- a/ext/simplexml/tests/SimpleXMLElement_addAttribute_required_attribute_name.phpt
+++ b/ext/simplexml/tests/SimpleXMLElement_addAttribute_required_attribute_name.phpt
@@ -8,10 +8,16 @@ Havard Eide <nucleuz@gmail.com>
 --FILE--
 <?php
 $a = new SimpleXMLElement("<php>testfest</php>");
-$a->addAttribute( "", "" );
+
+try {
+    $a->addAttribute( "", "" );
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
 echo $a->asXML();
 ?>
---EXPECTF--
-Warning: SimpleXMLElement::addAttribute(): Attribute name is required in %s on line %d
+--EXPECT--
+SimpleXMLElement::addAttribute(): Argument #1 ($qualifiedName) cannot be empty
 <?xml version="1.0"?>
 <php>testfest</php>
diff --git a/ext/simplexml/tests/SimpleXMLElement_xpath_4.phpt b/ext/simplexml/tests/SimpleXMLElement_xpath_4.phpt
index b389bcfc58..bcff7db2db 100644
--- a/ext/simplexml/tests/SimpleXMLElement_xpath_4.phpt
+++ b/ext/simplexml/tests/SimpleXMLElement_xpath_4.phpt
@@ -7,11 +7,14 @@ if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platforms only");
 ?>
 --FILE--
 <?php
-$xml = simplexml_load_string("XXXXXXX^",$x,0x6000000000000001);
-var_dump($xml);
+
+try {
+    simplexml_load_string("XXXXXXX^", $x, 0x6000000000000001);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
 ?>
 --EXPECTF--
 Warning: Undefined variable $x in %s on line %d
-
-Warning: simplexml_load_string(): Invalid options in %s on line %d
-bool(false)
+simplexml_load_string(): Argument #3 ($options) is too large
diff --git a/ext/simplexml/tests/bug37076_1.phpt b/ext/simplexml/tests/bug37076_1.phpt
index 019915a1b4..d002c93f08 100644
--- a/ext/simplexml/tests/bug37076_1.phpt
+++ b/ext/simplexml/tests/bug37076_1.phpt
@@ -4,13 +4,18 @@ Bug #37076 (SimpleXML ignores .=) (appending to unnamed attribute)
 <?php if (!extension_loaded("simplexml")) print "skip"; ?>
 --FILE--
 <?php
+
 $xml = simplexml_load_string("<root><foo /></root>");
-$xml->{""} .= "bar";
+
+try {
+    $xml->{""} .= "bar";
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
 print $xml->asXML();
 ?>
---EXPECTF--
-Warning: main(): Cannot write or create unnamed element in %s on line %d
-
-Warning: main(): Cannot write or create unnamed element in %s on line %d
+--EXPECT--
+Cannot create element with an empty name
 <?xml version="1.0"?>
 <root><foo/></root>
diff --git a/ext/simplexml/tests/bug38406.phpt b/ext/simplexml/tests/bug38406.phpt
index 51a716cbd3..7e20c7e704 100644
--- a/ext/simplexml/tests/bug38406.phpt
+++ b/ext/simplexml/tests/bug38406.phpt
@@ -13,7 +13,12 @@ $item->otherAttribute = $item->attribute;
 var_dump($item->otherAttribute);
 
 $a = array();
-$item->$a = new stdclass;
+
+try {
+    $item->$a = new stdclass;
+} catch (TypeError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
 echo "Done\n";
 ?>
@@ -28,6 +33,5 @@ object(SimpleXMLElement)#%d (1) {
 }
 
 Warning: Array to string conversion in %s on line %d
-
-Warning: It is not yet possible to assign complex types to properties in %s on line %d
+It's not possible to assign a complex type to properties, stdClass given
 Done