Promote warnings to exceptions in ext/simplexml

Closes GH-6011

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

diff --git a/Zend/zend_object_handlers.h b/Zend/zend_object_handlers.h
index b401d72ca294abf6e50f69f907ac5f5b24cd527b..fedc70cc8099727da57e7dab7ab5469d17ad0b37 100644 (file)
--- 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 f788502703b081757255ecf6951e3b7e80163ec6..1944ecf797819ce98a0ac79083299a52a6bbf65a 100644 (file)
--- 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 d3e48b0e6aaf77092d2468f0122238936e116028..18cddf288616225cf9f6101a32de6546e8ca3feb 100644 (file)
--- 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 dc369e03212d8a5963859bca9f8d743a4b980fea..5aa6d50ba1d034345b2b74dd90cfa7c0051cfed2 100644 (file)
--- 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 b687b7198819843037c309a595ed5c282a7ea997..307f8219cd70e35a4b80159cf419b8387e8a3fc0 100644 (file)
--- 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 34e2e683195b0e4cb30cb452635079adbc0f36dc..50928ff55e553557eebd7aa9fff26d38ea11ed2a 100644 (file)
--- 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 b389bcfc582f0636d05a18aa9fbfd6884c7ba1bf..bcff7db2db06af7b544e12f5ef0c430fd030a304 100644 (file)
--- 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 019915a1b4ee535575a47a8648d89fbbe36cc17f..d002c93f0856213f0c342ef2277c13c39e8ad032 100644 (file)
--- 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 51a716cbd352ee5314b5edfa39169004c411dff8..7e20c7e70447ffbde14d7e582ae38d063275ec31 100644 (file)
--- 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