Skip to content

Commit 6c8fb12

Browse files
kocsismatenikic
kocsismate
and
nikic
committed
Promote warnings to exceptions in ext/simplexml
Closes GH-6011 Co-authored-by: Nikita Popov <[email protected]>
1 parent f77200f commit 6c8fb12

9 files changed

+76
-48
lines changed

‎Zend/zend_object_handlers.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,13 @@ typedef zval *(*zend_object_write_property_t)(zend_object *object, zend_string *
6060
typedef void (*zend_object_write_dimension_t)(zend_object *object, zval *offset, zval *value);
6161

6262

63-
/* Used to create pointer to the property of the object, for future direct r/w access */
63+
/* Used to create pointer to the property of the object, for future direct r/w access.
64+
* May return one of:
65+
* * A zval pointer, without incrementing the reference count.
66+
* * &EG(error_zval), if an exception has been thrown.
67+
* * NULL, if acquiring a direct pointer is not possible.
68+
* In this case, the VM will fall back to using read_property and write_property.
69+
*/
6470
typedef zval *(*zend_object_get_property_ptr_ptr_t)(zend_object *object, zend_string *member, int type, void **cache_slot);
6571

6672
/* Used to check if a property of the object exists */

‎ext/simplexml/simplexml.c

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
411411
* and could also be E_PARSE, but we use this only during parsing
412412
* and this is during runtime.
413413
*/
414-
zend_throw_error(NULL, "Cannot create unnamed attribute");
414+
zend_throw_error(NULL, "Cannot append to an attribute list");
415415
return &EG(error_zval);
416416
}
417417
goto long_dim;
@@ -436,7 +436,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
436436
}
437437

438438
if (!Z_STRLEN_P(member)) {
439-
php_error_docref(NULL, E_WARNING, "Cannot write or create unnamed %s", attribs ? "attribute" : "element");
439+
zend_value_error("Cannot create %s with an empty name", attribs ? "attribute" : "element");
440440
if (member == &tmp_zv) {
441441
zval_ptr_dtor_str(&tmp_zv);
442442
}
@@ -464,7 +464,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
464464
* and could also be E_PARSE, but we use this only during parsing
465465
* and this is during runtime.
466466
*/
467-
zend_throw_error(NULL, "Cannot create unnamed attribute");
467+
zend_value_error("Cannot append to an attribute list");
468468
return &EG(error_zval);
469469
}
470470
if (attribs && !node && sxe->iter.type == SXE_ITER_ELEMENT) {
@@ -489,8 +489,8 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
489489
if (Z_OBJCE_P(value) == sxe_class_entry) {
490490
zval zval_copy;
491491
if (sxe_object_cast_ex(Z_OBJ_P(value), &zval_copy, IS_STRING) == FAILURE) {
492-
zend_error(E_ERROR, "Unable to cast node to string");
493-
/* FIXME: Should not be fatal */
492+
zend_throw_error(NULL, "Unable to cast node to string");
493+
return &EG(error_zval);
494494
}
495495

496496
value_str = Z_STR(zval_copy);
@@ -501,7 +501,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
501501
if (member == &tmp_zv) {
502502
zval_ptr_dtor_str(&tmp_zv);
503503
}
504-
zend_error(E_WARNING, "It is not yet possible to assign complex types to %s", attribs ? "attributes" : "properties");
504+
zend_type_error("It's not possible to assign a complex type to %s, %s given", attribs ? "attributes" : "properties", zend_zval_type_name(value));
505505
return &EG(error_zval);
506506
}
507507
}
@@ -656,7 +656,7 @@ static zval *sxe_property_get_adr(zend_object *object, zend_string *zname, int f
656656
}
657657
ZVAL_STR(&member, zname);
658658
if (sxe_prop_dim_write(object, &member, NULL, 1, 0, &node) == &EG(error_zval)) {
659-
return NULL;
659+
return &EG(error_zval);
660660
}
661661
type = SXE_ITER_NONE;
662662
name = NULL;
@@ -1684,8 +1684,8 @@ SXE_METHOD(addChild)
16841684
}
16851685

16861686
if (qname_len == 0) {
1687-
php_error_docref(NULL, E_WARNING, "Element name is required");
1688-
return;
1687+
zend_argument_value_error(1, "cannot be empty");
1688+
RETURN_THROWS();
16891689
}
16901690

16911691
sxe = Z_SXEOBJ_P(ZEND_THIS);
@@ -1749,8 +1749,8 @@ SXE_METHOD(addAttribute)
17491749
}
17501750

17511751
if (qname_len == 0) {
1752-
php_error_docref(NULL, E_WARNING, "Attribute name is required");
1753-
return;
1752+
zend_argument_value_error(1, "cannot be empty");
1753+
RETURN_THROWS();
17541754
}
17551755

17561756
sxe = Z_SXEOBJ_P(ZEND_THIS);
@@ -2274,8 +2274,8 @@ PHP_FUNCTION(simplexml_load_file)
22742274
}
22752275

22762276
if (ZEND_LONG_EXCEEDS_INT(options)) {
2277-
php_error_docref(NULL, E_WARNING, "Invalid options");
2278-
RETURN_FALSE;
2277+
zend_argument_value_error(3, "is too large");
2278+
RETURN_THROWS();
22792279
}
22802280

22812281
docp = xmlReadFile(filename, NULL, (int)options);
@@ -2319,16 +2319,16 @@ PHP_FUNCTION(simplexml_load_string)
23192319
}
23202320

23212321
if (ZEND_SIZE_T_INT_OVFL(data_len)) {
2322-
php_error_docref(NULL, E_WARNING, "Data is too long");
2323-
RETURN_FALSE;
2322+
zend_argument_value_error(1, "is too long");
2323+
RETURN_THROWS();
23242324
}
23252325
if (ZEND_SIZE_T_INT_OVFL(ns_len)) {
2326-
php_error_docref(NULL, E_WARNING, "Namespace is too long");
2327-
RETURN_FALSE;
2326+
zend_argument_value_error(4, "is too long");
2327+
RETURN_THROWS();
23282328
}
23292329
if (ZEND_LONG_EXCEEDS_INT(options)) {
2330-
php_error_docref(NULL, E_WARNING, "Invalid options");
2331-
RETURN_FALSE;
2330+
zend_argument_value_error(3, "is too large");
2331+
RETURN_THROWS();
23322332
}
23332333

23342334
docp = xmlReadMemory(data, (int)data_len, NULL, NULL, (int)options);

‎ext/simplexml/simplexml.stub.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function rewind() {}
5959
/** @return bool */
6060
public function valid() {}
6161

62-
/** @return ?SimpleXMLElement */
62+
/** @return SimpleXMLElement|null */
6363
public function current() {}
6464

6565
/** @return string|false */
@@ -71,7 +71,7 @@ public function next() {}
7171
/** @return bool */
7272
public function hasChildren() {}
7373

74-
/** @return ?SimpleXMLElement */
74+
/** @return SimpleXMLElement|null */
7575
public function getChildren() {}
7676
}
7777

‎ext/simplexml/simplexml_arginfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: fbe25d8a7a0a1de0cbd5dc9118e77a2e8d5dbd67 */
2+
* Stub hash: 953089f230247acf18b9ac48c0a4c516d144a987 */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_simplexml_load_file, 0, 1, SimpleXMLElement, MAY_BE_FALSE)
55
ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)

‎ext/simplexml/tests/012.phpt

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@ EOF;
1414

1515
$sxe = simplexml_load_string($xml);
1616

17+
try {
18+
$sxe[""] = "value";
19+
} catch (ValueError $exception) {
20+
echo $exception->getMessage() . "\n";
21+
}
1722

18-
$sxe[""] = "warning";
1923
$sxe["attr"] = "value";
2024

2125
echo $sxe->asXML();
@@ -24,19 +28,19 @@ $sxe["attr"] = "new value";
2428

2529
echo $sxe->asXML();
2630

27-
$sxe[] = "error";
31+
try {
32+
$sxe[] = "error";
33+
} catch (ValueError $exception) {
34+
echo $exception->getMessage() . "\n";
35+
}
2836

2937
__HALT_COMPILER();
3038
?>
3139
===DONE===
32-
--EXPECTF--
33-
Warning: main(): Cannot write or create unnamed attribute in %s012.php on line %d
40+
--EXPECT--
41+
Cannot create attribute with an empty name
3442
<?xml version="1.0" encoding="ISO-8859-1"?>
3543
<foo attr="value"/>
3644
<?xml version="1.0" encoding="ISO-8859-1"?>
3745
<foo attr="new value"/>
38-
39-
Fatal error: Uncaught Error: Cannot create unnamed attribute in %s012.php:%d
40-
Stack trace:
41-
#0 {main}
42-
thrown in %s012.php on line %d
46+
Cannot append to an attribute list

‎ext/simplexml/tests/SimpleXMLElement_addAttribute_required_attribute_name.phpt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,16 @@ Havard Eide <[email protected]>
88
--FILE--
99
<?php
1010
$a = new SimpleXMLElement("<php>testfest</php>");
11-
$a->addAttribute( "", "" );
11+
12+
try {
13+
$a->addAttribute( "", "" );
14+
} catch (ValueError $exception) {
15+
echo $exception->getMessage() . "\n";
16+
}
17+
1218
echo $a->asXML();
1319
?>
14-
--EXPECTF--
15-
Warning: SimpleXMLElement::addAttribute(): Attribute name is required in %s on line %d
20+
--EXPECT--
21+
SimpleXMLElement::addAttribute(): Argument #1 ($qualifiedName) cannot be empty
1622
<?xml version="1.0"?>
1723
<php>testfest</php>

‎ext/simplexml/tests/SimpleXMLElement_xpath_4.phpt

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@ if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platforms only");
77
?>
88
--FILE--
99
<?php
10-
$xml = simplexml_load_string("XXXXXXX^",$x,0x6000000000000001);
11-
var_dump($xml);
10+
11+
try {
12+
simplexml_load_string("XXXXXXX^", $x, 0x6000000000000001);
13+
} catch (ValueError $exception) {
14+
echo $exception->getMessage() . "\n";
15+
}
16+
1217
?>
1318
--EXPECTF--
1419
Warning: Undefined variable $x in %s on line %d
15-
16-
Warning: simplexml_load_string(): Invalid options in %s on line %d
17-
bool(false)
20+
simplexml_load_string(): Argument #3 ($options) is too large

‎ext/simplexml/tests/bug37076_1.phpt

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ Bug #37076 (SimpleXML ignores .=) (appending to unnamed attribute)
44
<?php if (!extension_loaded("simplexml")) print "skip"; ?>
55
--FILE--
66
<?php
7+
78
$xml = simplexml_load_string("<root><foo /></root>");
8-
$xml->{""} .= "bar";
9+
10+
try {
11+
$xml->{""} .= "bar";
12+
} catch (ValueError $exception) {
13+
echo $exception->getMessage() . "\n";
14+
}
15+
916
print $xml->asXML();
1017
?>
11-
--EXPECTF--
12-
Warning: main(): Cannot write or create unnamed element in %s on line %d
13-
14-
Warning: main(): Cannot write or create unnamed element in %s on line %d
18+
--EXPECT--
19+
Cannot create element with an empty name
1520
<?xml version="1.0"?>
1621
<root><foo/></root>

‎ext/simplexml/tests/bug38406.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ $item->otherAttribute = $item->attribute;
1313
var_dump($item->otherAttribute);
1414

1515
$a = array();
16-
$item->$a = new stdclass;
16+
17+
try {
18+
$item->$a = new stdclass;
19+
} catch (TypeError $exception) {
20+
echo $exception->getMessage() . "\n";
21+
}
1722

1823
echo "Done\n";
1924
?>
@@ -28,6 +33,5 @@ object(SimpleXMLElement)#%d (1) {
2833
}
2934

3035
Warning: Array to string conversion in %s on line %d
31-
32-
Warning: It is not yet possible to assign complex types to properties in %s on line %d
36+
It's not possible to assign a complex type to properties, stdClass given
3337
Done

0 commit comments

Comments
 (0)