Skip to content

Commit 122d759

Browse files
nikicnikic
committed
Always throw TypeException on throwing zpp failures
Introduces a ZEND_PARSE_PARAMS_THROW flag for zpp, which forces to report FAILURE errors using a TypeException instead of a Warning, like it would happen in strict mode. Adds a zend_parse_parameters_throw() convenience function, which invokes zpp with this flag. Converts all cases I could identify, where we currently have throwing zpp usage in constructors and replaces them with this API. Error handling is still replaced to EH_THROW in some cases to handle other, domain-specific errors in constructors.
1 parent 884b036 commit 122d759

File tree

88 files changed

+424
-640
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

88 files changed

+424
-640
lines changed

Diff for: ‎Zend/zend.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1316,14 +1316,14 @@ ZEND_API void zend_type_error(const char *format, ...) /* {{{ */
13161316
va_end(va);
13171317
} /* }}} */
13181318

1319-
ZEND_API void zend_internal_type_error(zend_bool strict, const char *format, ...) /* {{{ */
1319+
ZEND_API void zend_internal_type_error(zend_bool throw_exception, const char *format, ...) /* {{{ */
13201320
{
13211321
va_list va;
13221322
char *message = NULL;
13231323

13241324
va_start(va, format);
13251325
zend_vspprintf(&message, 0, format, va);
1326-
if (strict) {
1326+
if (throw_exception) {
13271327
zend_throw_exception(zend_get_type_exception(), message, E_ERROR);
13281328
} else {
13291329
zend_error(E_WARNING, message);

Diff for: ‎Zend/zend.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ extern ZEND_API zend_string *(*zend_resolve_path)(const char *filename, int file
286286

287287
ZEND_API void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
288288
ZEND_API void zend_type_error(const char *format, ...);
289-
ZEND_API void zend_internal_type_error(zend_bool strict, const char *format, ...);
289+
ZEND_API void zend_internal_type_error(zend_bool throw_exception, const char *format, ...);
290290

291291
void zenderror(const char *error);
292292

Diff for: ‎Zend/zend_API.c

+33-11
Original file line numberDiff line numberDiff line change
@@ -761,13 +761,16 @@ static int zend_parse_arg(int arg_num, zval *arg, va_list *va, const char **spec
761761
if (!(flags & ZEND_PARSE_PARAMS_QUIET) && (*expected_type || error)) {
762762
const char *space;
763763
const char *class_name = get_active_class_name(&space);
764+
zend_bool throw_exception =
765+
ZEND_ARG_USES_STRICT_TYPES() || (flags & ZEND_PARSE_PARAMS_THROW);
764766

765767
if (error) {
766-
zend_internal_type_error(ZEND_ARG_USES_STRICT_TYPES(), "%s%s%s() expects parameter %d %s",
767-
class_name, space, get_active_function_name(), arg_num, error);
768+
zend_internal_type_error(throw_exception, "%s%s%s() expects parameter %d %s",
769+
class_name, space, get_active_function_name(), arg_num, error);
768770
efree(error);
769771
} else {
770-
zend_internal_type_error(ZEND_ARG_USES_STRICT_TYPES(), "%s%s%s() expects parameter %d to be %s, %s given",
772+
zend_internal_type_error(throw_exception,
773+
"%s%s%s() expects parameter %d to be %s, %s given",
771774
class_name, space, get_active_function_name(), arg_num, expected_type,
772775
zend_zval_type_name(arg));
773776
}
@@ -876,7 +879,9 @@ static int zend_parse_va_args(int num_args, const char *type_spec, va_list *va,
876879
if (!(flags & ZEND_PARSE_PARAMS_QUIET)) {
877880
zend_function *active_function = EG(current_execute_data)->func;
878881
const char *class_name = active_function->common.scope ? active_function->common.scope->name->val : "";
879-
zend_internal_type_error(ZEND_ARG_USES_STRICT_TYPES(), "%s%s%s() expects %s %d parameter%s, %d given",
882+
zend_bool throw_exception =
883+
ZEND_ARG_USES_STRICT_TYPES() || (flags & ZEND_PARSE_PARAMS_THROW);
884+
zend_internal_type_error(throw_exception, "%s%s%s() expects %s %d parameter%s, %d given",
880885
class_name,
881886
class_name[0] ? "::" : "",
882887
active_function->common.function_name->val,
@@ -938,18 +943,19 @@ static int zend_parse_va_args(int num_args, const char *type_spec, va_list *va,
938943
}
939944
/* }}} */
940945

941-
#define RETURN_IF_ZERO_ARGS(num_args, type_spec, flags) { \
946+
#define RETURN_IF_ZERO_ARGS(num_args, type_spec, flags) do { \
942947
int __num_args = (num_args); \
943-
\
944948
if (0 == (type_spec)[0] && 0 != __num_args && !(flags & ZEND_PARSE_PARAMS_QUIET)) { \
945949
const char *__space; \
946950
const char * __class_name = get_active_class_name(&__space); \
947-
zend_internal_type_error(ZEND_ARG_USES_STRICT_TYPES(), "%s%s%s() expects exactly 0 parameters, %d given", \
948-
__class_name, __space, \
949-
get_active_function_name(), __num_args); \
951+
zend_bool throw_exception = \
952+
ZEND_ARG_USES_STRICT_TYPES() || (flags & ZEND_PARSE_PARAMS_THROW); \
953+
zend_internal_type_error(throw_exception, \
954+
"%s%s%s() expects exactly 0 parameters, %d given", \
955+
__class_name, __space, get_active_function_name(), __num_args); \
950956
return FAILURE; \
951-
}\
952-
}
957+
} \
958+
} while(0)
953959

954960
ZEND_API int zend_parse_parameters_ex(int flags, int num_args, const char *type_spec, ...) /* {{{ */
955961
{
@@ -982,6 +988,22 @@ ZEND_API int zend_parse_parameters(int num_args, const char *type_spec, ...) /*
982988
}
983989
/* }}} */
984990

991+
ZEND_API int zend_parse_parameters_throw(int num_args, const char *type_spec, ...) /* {{{ */
992+
{
993+
va_list va;
994+
int retval;
995+
int flags = ZEND_PARSE_PARAMS_THROW;
996+
997+
RETURN_IF_ZERO_ARGS(num_args, type_spec, flags);
998+
999+
va_start(va, type_spec);
1000+
retval = zend_parse_va_args(num_args, type_spec, &va, flags);
1001+
va_end(va);
1002+
1003+
return retval;
1004+
}
1005+
/* }}} */
1006+
9851007
ZEND_API int zend_parse_method_parameters(int num_args, zval *this_ptr, const char *type_spec, ...) /* {{{ */
9861008
{
9871009
va_list va;

Diff for: ‎Zend/zend_API.h

+2
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,10 @@ ZEND_API int zend_copy_parameters_array(int param_count, zval *argument_array);
257257
/* Parameter parsing API -- andrei */
258258

259259
#define ZEND_PARSE_PARAMS_QUIET (1<<1)
260+
#define ZEND_PARSE_PARAMS_THROW (1<<2)
260261
ZEND_API int zend_parse_parameters(int num_args, const char *type_spec, ...);
261262
ZEND_API int zend_parse_parameters_ex(int flags, int num_args, const char *type_spec, ...);
263+
ZEND_API int zend_parse_parameters_throw(int num_args, const char *type_spec, ...);
262264
ZEND_API char *zend_zval_type_name(const zval *arg);
263265

264266
ZEND_API int zend_parse_method_parameters(int num_args, zval *this_ptr, const char *type_spec, ...);

Diff for: ‎ext/date/php_date.c

+24-16
Original file line numberDiff line numberDiff line change
@@ -2663,10 +2663,12 @@ PHP_METHOD(DateTime, __construct)
26632663
size_t time_str_len = 0;
26642664
zend_error_handling error_handling;
26652665

2666-
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
2667-
if (SUCCESS == zend_parse_parameters(ZEND_NUM_ARGS(), "|sO!", &time_str, &time_str_len, &timezone_object, date_ce_timezone)) {
2668-
php_date_initialize(Z_PHPDATE_P(getThis()), time_str, time_str_len, NULL, timezone_object, 1);
2666+
if (FAILURE == zend_parse_parameters_throw(ZEND_NUM_ARGS(), "|sO!", &time_str, &time_str_len, &timezone_object, date_ce_timezone)) {
2667+
return;
26692668
}
2669+
2670+
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
2671+
php_date_initialize(Z_PHPDATE_P(getThis()), time_str, time_str_len, NULL, timezone_object, 1);
26702672
zend_restore_error_handling(&error_handling);
26712673
}
26722674
/* }}} */
@@ -2681,10 +2683,12 @@ PHP_METHOD(DateTimeImmutable, __construct)
26812683
size_t time_str_len = 0;
26822684
zend_error_handling error_handling;
26832685

2684-
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
2685-
if (SUCCESS == zend_parse_parameters(ZEND_NUM_ARGS(), "|sO!", &time_str, &time_str_len, &timezone_object, date_ce_timezone)) {
2686-
php_date_initialize(Z_PHPDATE_P(getThis()), time_str, time_str_len, NULL, timezone_object, 1);
2686+
if (FAILURE == zend_parse_parameters_throw(ZEND_NUM_ARGS(), "|sO!", &time_str, &time_str_len, &timezone_object, date_ce_timezone)) {
2687+
return;
26872688
}
2689+
2690+
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
2691+
php_date_initialize(Z_PHPDATE_P(getThis()), time_str, time_str_len, NULL, timezone_object, 1);
26882692
zend_restore_error_handling(&error_handling);
26892693
}
26902694
/* }}} */
@@ -3641,11 +3645,13 @@ PHP_METHOD(DateTimeZone, __construct)
36413645
php_timezone_obj *tzobj;
36423646
zend_error_handling error_handling;
36433647

3644-
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
3645-
if (SUCCESS == zend_parse_parameters(ZEND_NUM_ARGS(), "s", &tz, &tz_len)) {
3646-
tzobj = Z_PHPTIMEZONE_P(getThis());
3647-
timezone_initialize(tzobj, tz);
3648+
if (FAILURE == zend_parse_parameters_throw(ZEND_NUM_ARGS(), "s", &tz, &tz_len)) {
3649+
return;
36483650
}
3651+
3652+
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
3653+
tzobj = Z_PHPTIMEZONE_P(getThis());
3654+
timezone_initialize(tzobj, tz);
36493655
zend_restore_error_handling(&error_handling);
36503656
}
36513657
/* }}} */
@@ -4070,13 +4076,15 @@ PHP_METHOD(DateInterval, __construct)
40704076
timelib_rel_time *reltime;
40714077
zend_error_handling error_handling;
40724078

4079+
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "s", &interval_string, &interval_string_length) == FAILURE) {
4080+
return;
4081+
}
4082+
40734083
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
4074-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &interval_string, &interval_string_length) == SUCCESS) {
4075-
if (date_interval_initialize(&reltime, interval_string, interval_string_length) == SUCCESS) {
4076-
diobj = Z_PHPINTERVAL_P(getThis());
4077-
diobj->diff = reltime;
4078-
diobj->initialized = 1;
4079-
}
4084+
if (date_interval_initialize(&reltime, interval_string, interval_string_length) == SUCCESS) {
4085+
diobj = Z_PHPINTERVAL_P(getThis());
4086+
diobj->diff = reltime;
4087+
diobj->initialized = 1;
40804088
}
40814089
zend_restore_error_handling(&error_handling);
40824090
}

Diff for: ‎ext/date/tests/DateTimeZone_construct_error.phpt

+7-8
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,17 @@ echo "*** Testing DateTimeZone() : error conditions ***\n";
1515
echo "\n-- Testing new DateTimeZone() with more than expected no. of arguments --\n";
1616
$timezone = "GMT";
1717
$extra_arg = 99;
18-
var_dump( new DateTimeZone($timezone, $extra_arg) );
18+
try {
19+
new DateTimeZone($timezone, $extra_arg);
20+
} catch (TypeException $e) {
21+
echo $e->getMessage(), "\n";
22+
}
1923

2024
?>
2125
===DONE===
2226
--EXPECTF--
2327
*** Testing DateTimeZone() : error conditions ***
2428

2529
-- Testing new DateTimeZone() with more than expected no. of arguments --
26-
27-
Fatal error: Uncaught exception 'Exception' with message 'DateTimeZone::__construct() expects exactly 1 parameter, 2 given' in %s:%d
28-
Stack trace:
29-
#0 %s(%d): DateTimeZone->__construct('GMT', 99)
30-
#1 {main}
31-
thrown in %s on line %d
32-
30+
DateTimeZone::__construct() expects exactly 1 parameter, 2 given
31+
===DONE===

Diff for: ‎ext/date/tests/DateTimeZone_construct_variation1.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ foreach($inputs as $variation =>$timezone) {
9797
echo "\n-- $variation --\n";
9898
try {
9999
var_dump( new DateTimezone($timezone) );
100-
} catch(Exception $e) {
100+
} catch (BaseException $e) {
101101
$msg = $e->getMessage();
102102
echo "FAILED: " . $msg . "\n";
103103
}

Diff for: ‎ext/date/tests/DateTime_construct_error.phpt

+7-7
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@ echo "\n-- Testing new DateTime() with more than expected no. of arguments --\n"
1616
$time = "GMT";
1717
$timezone = timezone_open("GMT");
1818
$extra_arg = 99;
19-
var_dump( new DateTime($time, $timezone, $extra_arg) );
19+
try {
20+
var_dump( new DateTime($time, $timezone, $extra_arg) );
21+
} catch (TypeException $e) {
22+
echo $e->getMessage(), "\n";
23+
}
2024

2125
?>
2226
===DONE===
2327
--EXPECTF--
2428
*** Testing date_create() : error conditions ***
2529

2630
-- Testing new DateTime() with more than expected no. of arguments --
27-
28-
Fatal error: Uncaught exception 'Exception' with message 'DateTime::__construct() expects at most 2 parameters, 3 given' in %s:%d
29-
Stack trace:
30-
#0 %s(%d): DateTime->__construct('GMT', Object(DateTimeZone), 99)
31-
#1 {main}
32-
thrown in %s on line %d
31+
DateTime::__construct() expects at most 2 parameters, 3 given
32+
===DONE===

Diff for: ‎ext/date/tests/DateTime_construct_variation1.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,14 @@ foreach($inputs as $variation =>$time) {
102102

103103
try {
104104
var_dump( new DateTime($time) );
105-
} catch(Exception $e) {
105+
} catch (BaseException $e) {
106106
$msg = $e->getMessage();
107107
echo "FAILED: " . $msg . "\n";
108108
}
109109

110110
try {
111111
var_dump( new DateTime($time, $timezone) );
112-
} catch(Exception$e) {
112+
} catch (BaseException$e) {
113113
$msg = $e->getMessage();
114114
echo "FAILED: " . $msg . "\n";
115115
}

Diff for: ‎ext/date/tests/DateTime_construct_variation2.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ foreach($inputs as $variation =>$timezone) {
102102

103103
try {
104104
var_dump( new DateTime($time, $timezone) );
105-
} catch(Exception $e) {
105+
} catch (BaseException $e) {
106106
$msg = $e->getMessage();
107107
echo "FAILED: " . $msg . "\n";
108108
}

Diff for: ‎ext/dom/attr.c

+2-6
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,17 @@ const zend_function_entry php_dom_attr_class_functions[] = {
5555
/* {{{ proto void DOMAttr::__construct(string name, [string value]); */
5656
PHP_METHOD(domattr, __construct)
5757
{
58-
zval *id;
58+
zval *id = getThis();
5959
xmlAttrPtr nodep = NULL;
6060
xmlNodePtr oldnode = NULL;
6161
dom_object *intern;
6262
char *name, *value = NULL;
6363
size_t name_len, value_len, name_valid;
64-
zend_error_handling error_handling;
6564

66-
zend_replace_error_handling(EH_THROW, dom_domexception_class_entry, &error_handling);
67-
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Os|s", &id, dom_attr_class_entry, &name, &name_len, &value, &value_len) == FAILURE) {
68-
zend_restore_error_handling(&error_handling);
65+
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "s|s", &name, &name_len, &value, &value_len) == FAILURE) {
6966
return;
7067
}
7168

72-
zend_restore_error_handling(&error_handling);
7369
intern = Z_DOMOBJ_P(id);
7470

7571
name_valid = xmlValidateName((xmlChar *) name, 0);

Diff for: ‎ext/dom/cdatasection.c

+2-6
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,16 @@ const zend_function_entry php_dom_cdatasection_class_functions[] = {
5050
PHP_METHOD(domcdatasection, __construct)
5151
{
5252

53-
zval *id;
53+
zval *id = getThis();
5454
xmlNodePtr nodep = NULL, oldnode = NULL;
5555
dom_object *intern;
5656
char *value = NULL;
5757
size_t value_len;
58-
zend_error_handling error_handling;
5958

60-
zend_replace_error_handling(EH_THROW, dom_domexception_class_entry, &error_handling);
61-
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Os", &id, dom_cdatasection_class_entry, &value, &value_len) == FAILURE) {
62-
zend_restore_error_handling(&error_handling);
59+
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "s", &value, &value_len) == FAILURE) {
6360
return;
6461
}
6562

66-
zend_restore_error_handling(&error_handling);
6763
nodep = xmlNewCDataBlock(NULL, (xmlChar *) value, value_len);
6864

6965
if (!nodep) {

Diff for: ‎ext/dom/comment.c

+2-6
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,16 @@ const zend_function_entry php_dom_comment_class_functions[] = {
5050
PHP_METHOD(domcomment, __construct)
5151
{
5252

53-
zval *id;
53+
zval *id = getThis();
5454
xmlNodePtr nodep = NULL, oldnode = NULL;
5555
dom_object *intern;
5656
char *value = NULL;
5757
size_t value_len;
58-
zend_error_handling error_handling;
5958

60-
zend_replace_error_handling(EH_THROW, dom_domexception_class_entry, &error_handling);
61-
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "O|s", &id, dom_comment_class_entry, &value, &value_len) == FAILURE) {
62-
zend_restore_error_handling(&error_handling);
59+
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "|s", &value, &value_len) == FAILURE) {
6360
return;
6461
}
6562

66-
zend_restore_error_handling(&error_handling);
6763
nodep = xmlNewComment((xmlChar *) value);
6864

6965
if (!nodep) {

Diff for: ‎ext/dom/document.c

+2-6
Original file line numberDiff line numberDiff line change
@@ -1253,21 +1253,17 @@ PHP_FUNCTION(dom_document_rename_node)
12531253
PHP_METHOD(domdocument, __construct)
12541254
{
12551255

1256-
zval *id;
1256+
zval *id = getThis();
12571257
xmlDoc *docp = NULL, *olddoc;
12581258
dom_object *intern;
12591259
char *encoding, *version = NULL;
12601260
size_t encoding_len = 0, version_len = 0;
12611261
int refcount;
1262-
zend_error_handling error_handling;
12631262

1264-
zend_replace_error_handling(EH_THROW, dom_domexception_class_entry, &error_handling);
1265-
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "O|ss", &id, dom_document_class_entry, &version, &version_len, &encoding, &encoding_len) == FAILURE) {
1266-
zend_restore_error_handling(&error_handling);
1263+
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "|ss", &version, &version_len, &encoding, &encoding_len) == FAILURE) {
12671264
return;
12681265
}
12691266

1270-
zend_restore_error_handling(&error_handling);
12711267
docp = xmlNewDoc((xmlChar *) version);
12721268

12731269
if (!docp) {

Diff for: ‎ext/dom/documentfragment.c

+2-6
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,14 @@ const zend_function_entry php_dom_documentfragment_class_functions[] = {
5353
PHP_METHOD(domdocumentfragment, __construct)
5454
{
5555

56-
zval *id;
56+
zval *id = getThis();
5757
xmlNodePtr nodep = NULL, oldnode = NULL;
5858
dom_object *intern;
59-
zend_error_handling error_handling;
6059

61-
zend_replace_error_handling(EH_THROW, dom_domexception_class_entry, &error_handling);
62-
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "O", &id, dom_documentfragment_class_entry) == FAILURE) {
63-
zend_restore_error_handling(&error_handling);
60+
if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "") == FAILURE) {
6461
return;
6562
}
6663

67-
zend_restore_error_handling(&error_handling);
6864
nodep = xmlNewDocFragment(NULL);
6965

7066
if (!nodep) {

0 commit comments

Comments
 (0)