Skip to content

Throw directly instead of replacing error handler in ext/date #6954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 7, 2021
Merged

Throw directly instead of replacing error handler in ext/date #6954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4fb14c3
Throw directly instead of replacing error handler in ext/date
Girgias May 6, 2021
dc6703c
Address review comments
Girgias May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Zend/tests/bug54043.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ var_dump(error_get_last());

?>
--EXPECT--
string(105) "DateTime::__construct(): Failed to parse time string (9999-11-33) at position 9 (3): Unexpected character"
string(80) "Failed to parse time string (9999-11-33) at position 9 (3): Unexpected character"
NULL
65 changes: 27 additions & 38 deletions ext/date/php_date.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2215,10 +2215,10 @@ PHPAPI int php_date_initialize(php_date_obj *dateobj, const char *time_str, size
/* update last errors and warnings */
update_errors_warnings(err);


/* If called from a constructor throw an exception */
if ((flags & PHP_DATE_INIT_CTOR) && err && err->error_count) {
/* spit out the first library error message, at least */
php_error_docref(NULL, E_WARNING, "Failed to parse time string (%s) at position %d (%c): %s", time_str,
zend_throw_exception_ex(NULL, 0, "Failed to parse time string (%s) at position %d (%c): %s", time_str,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I keep E_WARNING as the Exception code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't use exception codes.

err->error_messages[0].position, err->error_messages[0].character, err->error_messages[0].message);
}
if (err && err->error_count) {
Expand Down Expand Up @@ -2386,17 +2386,14 @@ PHP_METHOD(DateTime, __construct)
zval *timezone_object = NULL;
char *time_str = NULL;
size_t time_str_len = 0;
zend_error_handling error_handling;

ZEND_PARSE_PARAMETERS_START(0, 2)
Z_PARAM_OPTIONAL
Z_PARAM_STRING(time_str, time_str_len)
Z_PARAM_OBJECT_OF_CLASS_OR_NULL(timezone_object, date_ce_timezone)
ZEND_PARSE_PARAMETERS_END();

zend_replace_error_handling(EH_THROW, NULL, &error_handling);
php_date_initialize(Z_PHPDATE_P(ZEND_THIS), time_str, time_str_len, NULL, timezone_object, PHP_DATE_INIT_CTOR);
zend_restore_error_handling(&error_handling);
}
/* }}} */

Expand All @@ -2406,17 +2403,14 @@ PHP_METHOD(DateTimeImmutable, __construct)
zval *timezone_object = NULL;
char *time_str = NULL;
size_t time_str_len = 0;
zend_error_handling error_handling;

ZEND_PARSE_PARAMETERS_START(0, 2)
Z_PARAM_OPTIONAL
Z_PARAM_STRING(time_str, time_str_len)
Z_PARAM_OBJECT_OF_CLASS_OR_NULL(timezone_object, date_ce_timezone)
ZEND_PARSE_PARAMETERS_END();

zend_replace_error_handling(EH_THROW, NULL, &error_handling);
php_date_initialize(Z_PHPDATE_P(ZEND_THIS), time_str, time_str_len, NULL, timezone_object, PHP_DATE_INIT_CTOR);
zend_restore_error_handling(&error_handling);
}
/* }}} */

Expand Down Expand Up @@ -3686,35 +3680,35 @@ PHP_FUNCTION(timezone_location_get)
}
/* }}} */

static int date_interval_initialize(timelib_rel_time **rt, /*const*/ char *format, size_t format_length) /* {{{ */
static bool date_interval_initialize(timelib_rel_time **rt, /*const*/ char *format, size_t format_length) /* {{{ */
{
timelib_time *b = NULL, *e = NULL;
timelib_rel_time *p = NULL;
int r = 0;
int retval = 0;
bool retval = false;
timelib_error_container *errors;

timelib_strtointerval(format, format_length, &b, &e, &p, &r, &errors);

if (errors->error_count > 0) {
php_error_docref(NULL, E_WARNING, "Unknown or bad format (%s)", format);
retval = FAILURE;
zend_throw_exception_ex(NULL, 0, "Unknown or bad format (%s)", format);
retval = false;
if (p) {
timelib_rel_time_dtor(p);
}
} else {
if(p) {
*rt = p;
retval = SUCCESS;
retval = true;
} else {
if(b && e) {
timelib_update_ts(b, NULL);
timelib_update_ts(e, NULL);
*rt = timelib_diff(b, e);
retval = SUCCESS;
retval = true;
} else {
php_error_docref(NULL, E_WARNING, "Failed to parse interval (%s)", format);
retval = FAILURE;
zend_throw_exception_ex(NULL, 0, "Failed to parse interval (%s)", format);
retval = false;
}
}
}
Expand Down Expand Up @@ -3853,20 +3847,19 @@ PHP_METHOD(DateInterval, __construct)
{
zend_string *interval_string = NULL;
timelib_rel_time *reltime;
zend_error_handling error_handling;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(interval_string)
ZEND_PARSE_PARAMETERS_END();

zend_replace_error_handling(EH_THROW, NULL, &error_handling);
if (date_interval_initialize(&reltime, ZSTR_VAL(interval_string), ZSTR_LEN(interval_string)) == SUCCESS) {
php_interval_obj *diobj = Z_PHPINTERVAL_P(ZEND_THIS);
diobj->diff = reltime;
diobj->initialized = 1;
diobj->civil_or_wall = PHP_DATE_WALL;
if (!date_interval_initialize(&reltime, ZSTR_VAL(interval_string), ZSTR_LEN(interval_string))) {
RETURN_THROWS();
}
zend_restore_error_handling(&error_handling);

php_interval_obj *diobj = Z_PHPINTERVAL_P(ZEND_THIS);
diobj->diff = reltime;
diobj->initialized = 1;
diobj->civil_or_wall = PHP_DATE_WALL;
}
/* }}} */

Expand Down Expand Up @@ -4117,19 +4110,19 @@ PHP_FUNCTION(date_interval_format)
}
/* }}} */

static int date_period_initialize(timelib_time **st, timelib_time **et, timelib_rel_time **d, zend_long *recurrences, /*const*/ char *format, size_t format_length) /* {{{ */
static bool date_period_initialize(timelib_time **st, timelib_time **et, timelib_rel_time **d, zend_long *recurrences, /*const*/ char *format, size_t format_length) /* {{{ */
{
timelib_time *b = NULL, *e = NULL;
timelib_rel_time *p = NULL;
int r = 0;
int retval = 0;
timelib_error_container *errors;
bool retval = false;

timelib_strtointerval(format, format_length, &b, &e, &p, &r, &errors);

if (errors->error_count > 0) {
php_error_docref(NULL, E_WARNING, "Unknown or bad format (%s)", format);
retval = FAILURE;
retval = false;
zend_throw_exception_ex(NULL, 0, "Unknown or bad format (%s)", format);
if (b) {
timelib_time_dtor(b);
}
Expand All @@ -4144,7 +4137,7 @@ static int date_period_initialize(timelib_time **st, timelib_time **et, timelib_
*et = e;
*d = p;
*recurrences = r;
retval = SUCCESS;
retval = true;
}
timelib_error_container_dtor(errors);
return retval;
Expand All @@ -4160,7 +4153,6 @@ PHP_METHOD(DatePeriod, __construct)
char *isostr = NULL;
size_t isostr_len = 0;
timelib_time *clone;
zend_error_handling error_handling;

if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OOl|l", &start, date_ce_interface, &interval, date_ce_interval, &recurrences, &options) == FAILURE) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "OOO|l", &start, date_ce_interface, &interval, date_ce_interval, &end, date_ce_interface, &options) == FAILURE) {
Expand All @@ -4175,28 +4167,25 @@ PHP_METHOD(DatePeriod, __construct)
dpobj->current = NULL;

if (isostr) {
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
date_period_initialize(&(dpobj->start), &(dpobj->end), &(dpobj->interval), &recurrences, isostr, isostr_len);
zend_restore_error_handling(&error_handling);
if (EG(exception)) {
if (!date_period_initialize(&(dpobj->start), &(dpobj->end), &(dpobj->interval), &recurrences, isostr, isostr_len)) {
RETURN_THROWS();
}

if (dpobj->start == NULL) {
zend_string *func = get_active_function_or_method_name();
zend_throw_error(zend_ce_exception, "%s(): ISO interval must contain a start date, \"%s\" given", ZSTR_VAL(func), isostr);
zend_throw_exception_ex(NULL, 0, "%s(): ISO interval must contain a start date, \"%s\" given", ZSTR_VAL(func), isostr);
zend_string_release(func);
RETURN_THROWS();
}
if (dpobj->interval == NULL) {
zend_string *func = get_active_function_or_method_name();
zend_throw_error(zend_ce_exception, "%s(): ISO interval must contain an interval, \"%s\" given", ZSTR_VAL(func), isostr);
zend_throw_exception_ex(NULL, 0, "%s(): ISO interval must contain an interval, \"%s\" given", ZSTR_VAL(func), isostr);
zend_string_release(func);
RETURN_THROWS();
}
if (dpobj->end == NULL && recurrences == 0) {
zend_string *func = get_active_function_or_method_name();
zend_throw_error(zend_ce_exception, "%s(): ISO interval must contain an end date or a recurrence count, \"%s\" given", ZSTR_VAL(func), isostr);
zend_throw_exception_ex(NULL, 0, "%s(): ISO interval must contain an end date or a recurrence count, \"%s\" given", ZSTR_VAL(func), isostr);
zend_string_release(func);
RETURN_THROWS();
}
Expand Down Expand Up @@ -4238,7 +4227,7 @@ PHP_METHOD(DatePeriod, __construct)

if (dpobj->end == NULL && recurrences < 1) {
zend_string *func = get_active_function_or_method_name();
zend_throw_error(zend_ce_exception, "%s(): Recurrence count must be greater than 0", ZSTR_VAL(func));
zend_throw_exception_ex(NULL, 0, "%s(): Recurrence count must be greater than 0", ZSTR_VAL(func));
zend_string_release(func);
RETURN_THROWS();
}
Expand Down
2 changes: 1 addition & 1 deletion ext/date/tests/bug44562.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ foreach ( $dp as $d )

?>
--EXPECT--
DatePeriod::__construct(): Unknown or bad format (2D)
Unknown or bad format (2D)
string(24) "2008-07-20T22:44:53+0200"
string(24) "2008-07-21T22:44:53+0200"
string(24) "2008-07-22T22:44:53+0200"
Expand Down
6 changes: 3 additions & 3 deletions ext/date/tests/bug52808.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ object(DateInterval)#%d (16) {
["have_special_relative"]=>
int(0)
}
DateInterval::__construct(): Failed to parse interval (2007-05-11T15:30:00Z/)
DateInterval::__construct(): Failed to parse interval (2007-05-11T15:30:00Z)
DateInterval::__construct(): Unknown or bad format (2007-05-11T15:30:00Z/:00Z)
Failed to parse interval (2007-05-11T15:30:00Z/)
Failed to parse interval (2007-05-11T15:30:00Z)
Unknown or bad format (2007-05-11T15:30:00Z/:00Z)
==DONE==
2 changes: 1 addition & 1 deletion ext/date/tests/bug54283.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ try {
?>
--EXPECTF--
Deprecated: DatePeriod::__construct(): Passing null to parameter #1 ($start) of type string is deprecated in %s on line %d
string(51) "DatePeriod::__construct(): Unknown or bad format ()"
string(24) "Unknown or bad format ()"
2 changes: 1 addition & 1 deletion ext/date/tests/bug62500.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ int(3)

Warning: Undefined property: Crasher::$2 in %s on line %d
NULL
string(%s) "DateInterval::__construct(): Unknown or bad format (blah)"
string(28) "Unknown or bad format (blah)"
6 changes: 3 additions & 3 deletions ext/date/tests/date_interval_bad_format_leak.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ try {

?>
--EXPECT--
DateInterval::__construct(): Unknown or bad format (P3"D)
DatePeriod::__construct(): Unknown or bad format (P3"D)
DatePeriod::__construct(): Unknown or bad format (2008-03-01T12:00:00Z1)
Unknown or bad format (P3"D)
Unknown or bad format (P3"D)
Unknown or bad format (2008-03-01T12:00:00Z1)
2 changes: 1 addition & 1 deletion ext/date/tests/oo_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ echo "DONE\n";
--EXPECTF--
string(19) "%d-%d-%d %d:%d:%d"
The DateTime object has not been correctly initialized by its constructor
DateTime::__construct(): Failed to parse time string (1am todax) at position 4 (t): The timezone could not be found in the database
Failed to parse time string (1am todax) at position 4 (t): The timezone could not be found in the database
string(3) "UTC"
The DateTimeZone object has not been correctly initialized by its constructor
DateTimeZone::__construct(): Unknown or bad timezone (GottaFindThisOne)
Expand Down