Skip to content

Commit 3c7e791

Browse files
GirgiasGirgias
committed
Deal with error cases
1 parent ca40da4 commit 3c7e791

File tree

3 files changed

+197
-16
lines changed

3 files changed

+197
-16
lines changed

‎ext/xml/tests/set_handler_errors.phpt

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
--TEST--
2+
Error conditions when setting invalid handler callables
3+
--EXTENSIONS--
4+
xml
5+
--FILE--
6+
<?php
7+
declare(strict_types=1);
8+
9+
/* Use xml_set_processing_instruction_handler() for generic implementation */
10+
echo 'Invalid $parser:', PHP_EOL;
11+
$obj = new stdClass();
12+
try {
13+
xml_set_processing_instruction_handler($obj, null);
14+
} catch (\Throwable $e) {
15+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
16+
}
17+
18+
/* Create valid parser */
19+
$parser = xml_parser_create();
20+
21+
echo 'Invalid callable type true:', PHP_EOL;
22+
try {
23+
xml_set_processing_instruction_handler($parser, true);
24+
} catch (\Throwable $e) {
25+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
26+
}
27+
28+
echo 'Invalid callable type int:', PHP_EOL;
29+
try {
30+
xml_set_processing_instruction_handler($parser, 10);
31+
} catch (\Throwable $e) {
32+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
33+
}
34+
35+
echo 'String not callable and no object set:', PHP_EOL;
36+
try {
37+
xml_set_processing_instruction_handler($parser, "nonexistent_method");
38+
} catch (\Throwable $e) {
39+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
40+
}
41+
42+
echo 'String non existent method on set object:', PHP_EOL;
43+
xml_set_object($parser, $obj);
44+
try {
45+
xml_set_processing_instruction_handler($parser, "nonexistent_method");
46+
} catch (\Throwable $e) {
47+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
48+
}
49+
50+
?>
51+
--EXPECT--
52+
Invalid $parser:
53+
TypeError: xml_set_processing_instruction_handler(): Argument #1 ($parser) must be of type XMLParser, stdClass given
54+
Invalid callable type true:
55+
TypeError: xml_set_processing_instruction_handler(): Argument #2 ($handler) must be of type callable|string|null|false
56+
Invalid callable type int:
57+
TypeError: xml_set_processing_instruction_handler(): Argument #2 ($handler) must be of type callable|string|null|false
58+
String not callable and no object set:
59+
ValueError: xml_set_processing_instruction_handler(): Argument #2 ($handler) an object must be set via xml_set_object() to be able to lookup method
60+
String non existent method on set object:
61+
ValueError: xml_set_processing_instruction_handler(): Argument #2 ($handler) method stdClass::nonexistent_method() does not exist

‎ext/xml/tests/xml_set_element_handler_errors.phpt

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
--TEST--
2+
Error conditions when setting invalid handler callables for xml_set_element_handler()
3+
--EXTENSIONS--
4+
xml
5+
--FILE--
6+
<?php
7+
declare(strict_types=1);
8+
9+
function dummy() {}
10+
11+
echo 'Invalid $parser:', PHP_EOL;
12+
$obj = new stdClass();
13+
try {
14+
xml_set_element_handler($obj, null, null);
15+
} catch (\Throwable $e) {
16+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
17+
}
18+
19+
/* Create valid parser */
20+
$parser = xml_parser_create();
21+
22+
echo 'Invalid start callable type true:', PHP_EOL;
23+
try {
24+
xml_set_element_handler($parser, true, null);
25+
} catch (\Throwable $e) {
26+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
27+
}
28+
echo 'Invalid end callable type true:', PHP_EOL;
29+
try {
30+
xml_set_element_handler($parser, null, true);
31+
} catch (\Throwable $e) {
32+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
33+
}
34+
35+
echo 'Invalid start callable type int:', PHP_EOL;
36+
try {
37+
xml_set_element_handler($parser, 10, null);
38+
} catch (\Throwable $e) {
39+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
40+
}
41+
echo 'Invalid end callable type int:', PHP_EOL;
42+
try {
43+
xml_set_element_handler($parser, null, 10);
44+
} catch (\Throwable $e) {
45+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
46+
}
47+
48+
echo 'Invalid start callable, no object set and string not callable:', PHP_EOL;
49+
try {
50+
xml_set_element_handler($parser, "nonexistent_method", null);
51+
} catch (\Throwable $e) {
52+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
53+
}
54+
echo 'Invalid end callable, no object set and string not callable:', PHP_EOL;
55+
try {
56+
xml_set_element_handler($parser, null, "nonexistent_method");
57+
} catch (\Throwable $e) {
58+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
59+
}
60+
61+
echo 'Invalid start callable, string non existent method on set object:', PHP_EOL;
62+
xml_set_object($parser, $obj);
63+
try {
64+
xml_set_element_handler($parser, "nonexistent_method", null);
65+
} catch (\Throwable $e) {
66+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
67+
}
68+
echo 'Invalid end callable, string non existent method on set object:', PHP_EOL;
69+
xml_set_object($parser, $obj);
70+
try {
71+
xml_set_element_handler($parser, null, "nonexistent_method");
72+
} catch (\Throwable $e) {
73+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
74+
}
75+
76+
?>
77+
--EXPECT--
78+
Invalid $parser:
79+
TypeError: xml_set_element_handler(): Argument #1 ($parser) must be of type XMLParser, stdClass given
80+
Invalid start callable type true:
81+
TypeError: xml_set_element_handler(): Argument #2 ($start_handler) must be of type callable|string|null|false
82+
Invalid end callable type true:
83+
TypeError: xml_set_element_handler(): Argument #3 ($end_handler) must be of type callable|string|null|false
84+
Invalid start callable type int:
85+
TypeError: xml_set_element_handler(): Argument #2 ($start_handler) must be of type callable|string|null|false
86+
Invalid end callable type int:
87+
TypeError: xml_set_element_handler(): Argument #3 ($end_handler) must be of type callable|string|null|false
88+
Invalid start callable, no object set and string not callable:
89+
ValueError: xml_set_element_handler(): Argument #2 ($start_handler) an object must be set via xml_set_object() to be able to lookup method
90+
Invalid end callable, no object set and string not callable:
91+
ValueError: xml_set_element_handler(): Argument #3 ($end_handler) an object must be set via xml_set_object() to be able to lookup method
92+
Invalid start callable, string non existent method on set object:
93+
ValueError: xml_set_element_handler(): Argument #2 ($start_handler) method stdClass::nonexistent_method() does not exist
94+
Invalid end callable, string non existent method on set object:
95+
ValueError: xml_set_element_handler(): Argument #3 ($end_handler) method stdClass::nonexistent_method() does not exist

‎ext/xml/xml.c

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,23 +1075,22 @@ static bool php_xml_check_string_method_arg(
10751075
zend_string *method_name,
10761076
zend_fcall_info_cache *const parser_handler_fcc
10771077
) {
1078-
1079-
if (Z_ISUNDEF(parser->object)) {
1080-
// TODO Error
1081-
return false;
1082-
}
1083-
10841078
if (ZSTR_LEN(method_name) == 0) {
10851079
/* Unset handler */
10861080
return true;
10871081
}
10881082

1083+
if (Z_ISUNDEF(parser->object)) {
1084+
zend_argument_value_error(arg_num, "an object must be set via xml_set_object() to be able to lookup method");
1085+
return false;
1086+
}
1087+
10891088
zend_class_entry *ce = Z_OBJCE(parser->object);
10901089
zend_string *lc_name = zend_string_tolower(method_name);
10911090
zend_function *method_ptr = zend_hash_find_ptr(&ce->function_table, lc_name);
10921091
zend_string_release_ex(lc_name, 0);
10931092
if (!method_ptr) {
1094-
// TODO Error
1093+
zend_argument_value_error(arg_num, "method %s::%s() does not exist", ZSTR_VAL(ce->name), ZSTR_VAL(method_name));
10951094
return false;
10961095
}
10971096

@@ -1169,13 +1168,33 @@ PHP_FUNCTION(xml_set_element_handler)
11691168
RETURN_FALSE;
11701169
}
11711170
} else {
1172-
zval *dummy;
1173-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ozz", &pind, xml_parser_ce, &dummy) == FAILURE) {
1171+
zval *dummy_start;
1172+
zval *dummy_end;
1173+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ozz", &pind, xml_parser_ce, &dummy_start, &dummy_end) == FAILURE) {
11741174
RETURN_THROWS();
11751175
} else {
1176-
// TODO Deal with types
1177-
zend_argument_type_error(2, "must be of type callable|string|null|false");
1178-
RETURN_THROWS();
1176+
switch (Z_TYPE_P(dummy_start)) {
1177+
case IS_FALSE:
1178+
case IS_NULL:
1179+
break;
1180+
case IS_STRING:
1181+
// TODO ... how to deal with callable on one arg and false on another?
1182+
default:
1183+
zend_argument_type_error(2, "must be of type callable|string|null|false");
1184+
RETURN_THROWS();
1185+
}
1186+
switch (Z_TYPE_P(dummy_end)) {
1187+
case IS_FALSE:
1188+
case IS_NULL:
1189+
break;
1190+
case IS_STRING:
1191+
// TODO ... how to deal with callable on one arg and false on another?
1192+
default:
1193+
zend_argument_type_error(3, "must be of type callable|string|null|false");
1194+
RETURN_THROWS();
1195+
}
1196+
1197+
parser = Z_XMLPARSER_P(pind);
11791198
}
11801199
}
11811200

@@ -1227,11 +1246,16 @@ static void php_xml_set_handler_parse_callable(
12271246
zval *dummy;
12281247
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oz", &pind, xml_parser_ce, &dummy) == FAILURE) {
12291248
RETURN_THROWS();
1230-
} else {
1231-
// TODO Deal with types
1232-
zend_argument_type_error(2, "must be of type callable|string|null|false");
1233-
RETURN_THROWS();
12341249
}
1250+
switch (Z_TYPE_P(dummy)) {
1251+
case IS_FALSE:
1252+
break;
1253+
default:
1254+
zend_argument_type_error(2, "must be of type callable|string|null|false");
1255+
RETURN_THROWS();
1256+
}
1257+
1258+
*parser = Z_XMLPARSER_P(pind);
12351259
}
12361260
}
12371261

@@ -1241,6 +1265,7 @@ static void php_xml_set_handler_parse_callable(
12411265
xml_parser *parser = NULL; \
12421266
zend_fcall_info_cache handler_fcc = {0}; \
12431267
php_xml_set_handler_parse_callable(INTERNAL_FUNCTION_PARAM_PASSTHRU, &parser, &handler_fcc); \
1268+
if (EG(exception)) { return; } \
12441269
ZEND_ASSERT(parser); \
12451270
xml_set_handler(&parser->parser_handler_name, &handler_fcc); \
12461271
parse_function(parser->parser, c_function); \

0 commit comments

Comments
 (0)