From 38c634e74adf8a0312055ee3e56c44b20226e175 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 29 May 2024 13:26:15 +0100 Subject: [PATCH 1/4] ext/intl: Small extension cleanup --- ext/intl/collator/collator_class.c | 9 --------- ext/intl/resourcebundle/resourcebundle_iterator.c | 9 +++++---- ext/intl/tests/collator_get_error_code.phpt | 3 --- ext/intl/tests/intl_get_error_code.phpt | 3 --- ext/intl/tests/intl_get_error_message.phpt | 3 --- ext/intl/tests/locale_compose_locale.phpt | 2 -- 6 files changed, 5 insertions(+), 24 deletions(-) diff --git a/ext/intl/collator/collator_class.c b/ext/intl/collator/collator_class.c index db974b8748ec6..3a883c9e2e645 100644 --- a/ext/intl/collator/collator_class.c +++ b/ext/intl/collator/collator_class.c @@ -77,15 +77,6 @@ void collator_register_Collator_symbols(int module_number) Collator_handlers.offset = XtOffsetOf(Collator_object, zo); Collator_handlers.clone_obj = NULL; Collator_handlers.free_obj = Collator_objects_free; - - /* Declare 'Collator' class properties. */ - if( !Collator_ce_ptr ) - { - zend_error( E_ERROR, - "Collator: attempt to create properties " - "on a non-registered class." ); - return; - } } /* }}} */ diff --git a/ext/intl/resourcebundle/resourcebundle_iterator.c b/ext/intl/resourcebundle/resourcebundle_iterator.c index 9cd82d29bbeb4..8e405f0a5868e 100644 --- a/ext/intl/resourcebundle/resourcebundle_iterator.c +++ b/ext/intl/resourcebundle/resourcebundle_iterator.c @@ -148,13 +148,14 @@ static const zend_object_iterator_funcs resourcebundle_iterator_funcs = { /* {{{ resourcebundle_get_iterator */ zend_object_iterator *resourcebundle_get_iterator( zend_class_entry *ce, zval *object, int byref ) { - ResourceBundle_object *rb = Z_INTL_RESOURCEBUNDLE_P(object ); - ResourceBundle_iterator *iterator = emalloc( sizeof( ResourceBundle_iterator ) ); - if (byref) { - php_error( E_ERROR, "ResourceBundle does not support writable iterators" ); + zend_throw_error(NULL, "An iterator cannot be used with foreach by reference"); + return NULL; } + ResourceBundle_object *rb = Z_INTL_RESOURCEBUNDLE_P(object ); + ResourceBundle_iterator *iterator = emalloc( sizeof( ResourceBundle_iterator ) ); + zend_iterator_init(&iterator->intern); Z_ADDREF_P(object); ZVAL_OBJ(&iterator->intern.data, Z_OBJ_P(object)); diff --git a/ext/intl/tests/collator_get_error_code.phpt b/ext/intl/tests/collator_get_error_code.phpt index 605dc9a3d3651..7c4ab22ae98ad 100644 --- a/ext/intl/tests/collator_get_error_code.phpt +++ b/ext/intl/tests/collator_get_error_code.phpt @@ -36,9 +36,6 @@ function ut_main() return $res; } -# Suppress warning messages. -error_reporting( E_ERROR ); - include_once( 'ut_common.inc' ); ut_run(); ?> diff --git a/ext/intl/tests/intl_get_error_code.phpt b/ext/intl/tests/intl_get_error_code.phpt index 473911e0c365d..161e327d4ac7e 100644 --- a/ext/intl/tests/intl_get_error_code.phpt +++ b/ext/intl/tests/intl_get_error_code.phpt @@ -8,9 +8,6 @@ intl * Check getting global error code. */ -// Suppress warning messages. -error_reporting( E_ERROR ); - if( collator_get_locale(new Collator('en_US'), -1) !== false ) echo "failed\n"; else diff --git a/ext/intl/tests/intl_get_error_message.phpt b/ext/intl/tests/intl_get_error_message.phpt index 4fde4b1d50129..93d2e5c653afa 100644 --- a/ext/intl/tests/intl_get_error_message.phpt +++ b/ext/intl/tests/intl_get_error_message.phpt @@ -8,9 +8,6 @@ intl * Check getting global error message. */ -// Suppress warning messages. -error_reporting( E_ERROR ); - if( collator_get_locale(new Collator('en_US'), -1) !== false ) echo "failed\n"; else diff --git a/ext/intl/tests/locale_compose_locale.phpt b/ext/intl/tests/locale_compose_locale.phpt index 18641872da8ca..0a2a501b9baf9 100644 --- a/ext/intl/tests/locale_compose_locale.phpt +++ b/ext/intl/tests/locale_compose_locale.phpt @@ -101,8 +101,6 @@ function ut_main() 'loc12' => $loc_parts_arr12 ); - error_reporting( E_ERROR ); - $cnt = 0; $res_str = ''; foreach($loc_parts_arr as $key => $value ){ From c29eed4a5ad5eaf43fba07027bd8da6ba717ad37 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 29 May 2024 14:51:30 +0100 Subject: [PATCH 2/4] ext/intl: Normalize cloning error handling behaviour Always throw a Error exception as we cannot progress from here --- .../breakiterator/breakiterator_class.cpp | 27 +++-------- ext/intl/calendar/calendar_class.cpp | 24 ++-------- ext/intl/converter/converter.c | 11 +---- ext/intl/dateformat/dateformat_class.c | 26 +++++----- .../dateformat/datepatterngenerator_class.cpp | 19 ++------ ext/intl/formatter/formatter_class.c | 24 ++++------ ext/intl/msgformat/msgformat_class.c | 22 ++++----- ext/intl/spoofchecker/spoofchecker_class.c | 35 +++++++------- ext/intl/tests/bug62915.phpt | 12 ++--- ext/intl/tests/dateformat_clone_bad_obj.phpt | 11 +++-- ext/intl/tests/formatter_clone_bad_obj.phpt | 9 ++-- ext/intl/tests/msgfmt_clone_bad_obj.phpt | 11 +++-- ext/intl/tests/timezone_clone_error.phpt | 18 +++---- ext/intl/timezone/timezone_class.cpp | 32 ++++--------- .../transliterator/transliterator_class.c | 47 ++++++------------- 15 files changed, 115 insertions(+), 213 deletions(-) diff --git a/ext/intl/breakiterator/breakiterator_class.cpp b/ext/intl/breakiterator/breakiterator_class.cpp index d4dec4f31d7a5..4976d4ff675bc 100644 --- a/ext/intl/breakiterator/breakiterator_class.cpp +++ b/ext/intl/breakiterator/breakiterator_class.cpp @@ -96,37 +96,22 @@ static int BreakIterator_compare_objects(zval *object1, /* {{{ clone handler for BreakIterator */ static zend_object *BreakIterator_clone_obj(zend_object *object) { - BreakIterator_object *bio_orig, - *bio_new; - zend_object *ret_val; - - bio_orig = php_intl_breakiterator_fetch_object(object); - intl_errors_reset(INTL_DATA_ERROR_P(bio_orig)); - - ret_val = BreakIterator_ce_ptr->create_object(object->ce); - bio_new = php_intl_breakiterator_fetch_object(ret_val); + BreakIterator_object *bio_orig = php_intl_breakiterator_fetch_object(object); + zend_object *ret_val = BreakIterator_ce_ptr->create_object(object->ce); + BreakIterator_object *bio_new = php_intl_breakiterator_fetch_object(ret_val); zend_objects_clone_members(&bio_new->zo, &bio_orig->zo); if (bio_orig->biter != NULL) { - BreakIterator *new_biter; - - new_biter = bio_orig->biter->clone(); + BreakIterator *new_biter = bio_orig->biter->clone(); if (!new_biter) { - zend_string *err_msg; - intl_errors_set_code(BREAKITER_ERROR_P(bio_orig), - U_MEMORY_ALLOCATION_ERROR); - intl_errors_set_custom_msg(BREAKITER_ERROR_P(bio_orig), - "Could not clone BreakIterator", 0); - err_msg = intl_error_get_message(BREAKITER_ERROR_P(bio_orig)); - zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0); - zend_string_free(err_msg); + zend_throw_error(NULL, "Failed to clone BreakIterator"); } else { bio_new->biter = new_biter; ZVAL_COPY(&bio_new->text, &bio_orig->text); } } else { - zend_throw_exception(NULL, "Cannot clone unconstructed BreakIterator", 0); + zend_throw_error(NULL, "Cannot clone uninitialized BreakIterator"); } return ret_val; diff --git a/ext/intl/calendar/calendar_class.cpp b/ext/intl/calendar/calendar_class.cpp index 556b5df208fd4..97b21ff8f965a 100644 --- a/ext/intl/calendar/calendar_class.cpp +++ b/ext/intl/calendar/calendar_class.cpp @@ -77,16 +77,9 @@ U_CFUNC void calendar_object_construct(zval *object, /* {{{ clone handler for Calendar */ static zend_object *Calendar_clone_obj(zend_object *object) { - Calendar_object *co_orig, - *co_new; - zend_object *ret_val; - intl_error_reset(NULL); - - co_orig = php_intl_calendar_fetch_object(object); - intl_error_reset(INTL_DATA_ERROR_P(co_orig)); - - ret_val = Calendar_ce_ptr->create_object(object->ce); - co_new = php_intl_calendar_fetch_object(ret_val); + Calendar_object *co_orig = php_intl_calendar_fetch_object(object); + zend_object *ret_val = Calendar_ce_ptr->create_object(object->ce); + Calendar_object *co_new = php_intl_calendar_fetch_object(ret_val); zend_objects_clone_members(&co_new->zo, &co_orig->zo); @@ -95,19 +88,12 @@ static zend_object *Calendar_clone_obj(zend_object *object) newCalendar = co_orig->ucal->clone(); if (UNEXPECTED(!newCalendar)) { - zend_string *err_msg; - intl_errors_set_code(CALENDAR_ERROR_P(co_orig), - U_MEMORY_ALLOCATION_ERROR); - intl_errors_set_custom_msg(CALENDAR_ERROR_P(co_orig), - "Could not clone IntlCalendar", 0); - err_msg = intl_error_get_message(CALENDAR_ERROR_P(co_orig)); - zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0); - zend_string_free(err_msg); + zend_throw_error(NULL, "Failed to clone IntlCalendar"); } else { co_new->ucal = newCalendar; } } else { - zend_throw_exception(NULL, "Cannot clone unconstructed IntlCalendar", 0); + zend_throw_error(NULL, "Cannot clone uninitialized IntlCalendar"); } return ret_val; diff --git a/ext/intl/converter/converter.c b/ext/intl/converter/converter.c index 31d6c6ab4c132..135495643898d 100644 --- a/ext/intl/converter/converter.c +++ b/ext/intl/converter/converter.c @@ -929,8 +929,6 @@ static zend_object *php_converter_clone_object(zend_object *object) { zend_object *retval = php_converter_object_ctor(object->ce, &objval); UErrorCode error = U_ZERO_ERROR; - intl_errors_reset(&oldobj->error); - #if U_ICU_VERSION_MAJOR_NUM > 70 objval->src = ucnv_clone(oldobj->src, &error); #else @@ -944,14 +942,9 @@ static zend_object *php_converter_clone_object(zend_object *object) { objval->dest = ucnv_safeClone(oldobj->dest, NULL, NULL, &error); #endif } - if (U_FAILURE(error)) { - zend_string *err_msg; - THROW_UFAILURE(oldobj, "ucnv_safeClone", error); - - err_msg = intl_error_get_message(&oldobj->error); - zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0); - zend_string_release_ex(err_msg, 0); + if (U_FAILURE(error)) { + zend_throw_error(NULL, "Failed to clone UConverter"); return retval; } diff --git a/ext/intl/dateformat/dateformat_class.c b/ext/intl/dateformat/dateformat_class.c index f547640efbb0c..15bf5bf23ce57 100644 --- a/ext/intl/dateformat/dateformat_class.c +++ b/ext/intl/dateformat/dateformat_class.c @@ -64,27 +64,23 @@ zend_object *IntlDateFormatter_object_create(zend_class_entry *ce) /* {{{ IntlDateFormatter_object_clone */ zend_object *IntlDateFormatter_object_clone(zend_object *object) { - IntlDateFormatter_object *dfo, *new_dfo; - zend_object *new_obj; + IntlDateFormatter_object *dfo = php_intl_dateformatter_fetch_object(object); + zend_object *new_obj = IntlDateFormatter_ce_ptr->create_object(object->ce); + IntlDateFormatter_object *new_dfo = php_intl_dateformatter_fetch_object(new_obj); - dfo = php_intl_dateformatter_fetch_object(object); - intl_error_reset(INTL_DATA_ERROR_P(dfo)); - - new_obj = IntlDateFormatter_ce_ptr->create_object(object->ce); - new_dfo = php_intl_dateformatter_fetch_object(new_obj); /* clone standard parts */ zend_objects_clone_members(&new_dfo->zo, &dfo->zo); + /* clone formatter object */ - if (dfo->datef_data.udatf != NULL) { - DATE_FORMAT_OBJECT(new_dfo) = udat_clone(DATE_FORMAT_OBJECT(dfo), &INTL_DATA_ERROR_CODE(dfo)); - if (U_FAILURE(INTL_DATA_ERROR_CODE(dfo))) { - /* set up error in case error handler is interested */ - intl_errors_set(INTL_DATA_ERROR_P(dfo), INTL_DATA_ERROR_CODE(dfo), - "Failed to clone IntlDateFormatter object", 0 ); - zend_throw_exception(NULL, "Failed to clone IntlDateFormatter object", 0); + if (DATE_FORMAT_OBJECT(dfo) != NULL) { + UErrorCode error = U_ZERO_ERROR; + DATE_FORMAT_OBJECT(new_dfo) = udat_clone(DATE_FORMAT_OBJECT(dfo), &error); + + if (U_FAILURE(error)) { + zend_throw_error(NULL, "Failed to clone IntlDateFormatter"); } } else { - zend_throw_exception(NULL, "Cannot clone unconstructed IntlDateFormatter", 0); + zend_throw_error(NULL, "Cannot clone uninitialized IntlDateFormatter"); } return new_obj; } diff --git a/ext/intl/dateformat/datepatterngenerator_class.cpp b/ext/intl/dateformat/datepatterngenerator_class.cpp index c77398deb983f..4ce8e005e8587 100644 --- a/ext/intl/dateformat/datepatterngenerator_class.cpp +++ b/ext/intl/dateformat/datepatterngenerator_class.cpp @@ -36,32 +36,21 @@ zend_object_handlers IntlDatePatternGenerator_handlers; static zend_object *IntlDatePatternGenerator_object_clone(zend_object *object) { - intl_error_reset(NULL); - IntlDatePatternGenerator_object *dtpgo_orig = php_intl_datepatterngenerator_fetch_object(object); - intl_error_reset(DTPATTERNGEN_ERROR_P(dtpgo_orig)); - - zend_object *ret_val = IntlDatePatternGenerator_ce_ptr->create_object(object->ce); - IntlDatePatternGenerator_object *dtpgo_new = php_intl_datepatterngenerator_fetch_object(ret_val); + zend_object *ret_val = IntlDatePatternGenerator_ce_ptr->create_object(object->ce); + IntlDatePatternGenerator_object *dtpgo_new = php_intl_datepatterngenerator_fetch_object(ret_val); zend_objects_clone_members(&dtpgo_new->zo, &dtpgo_orig->zo); if (dtpgo_orig->dtpg != NULL) { DateTimePatternGenerator *newDtpg = dtpgo_orig->dtpg->clone(); if (!newDtpg) { - zend_string *err_msg; - intl_errors_set_code(DTPATTERNGEN_ERROR_P(dtpgo_orig), - U_MEMORY_ALLOCATION_ERROR); - intl_errors_set_custom_msg(DTPATTERNGEN_ERROR_P(dtpgo_orig), - "Could not clone IntlDatePatternGenerator", 0); - err_msg = intl_error_get_message(DTPATTERNGEN_ERROR_P(dtpgo_orig)); - zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0); - zend_string_free(err_msg); + zend_throw_error(NULL, "Failed to clone IntlDatePatternGenerator"); } else { dtpgo_new->dtpg = newDtpg; } } else { - zend_throw_exception(NULL, "Cannot clone unconstructed IntlDatePatternGenerator", 0); + zend_throw_error(NULL, "Cannot clone uninitialized IntlDatePatternGenerator"); } return ret_val; diff --git a/ext/intl/formatter/formatter_class.c b/ext/intl/formatter/formatter_class.c index 0c2d8e418153d..89ff5e2b5d4db 100644 --- a/ext/intl/formatter/formatter_class.c +++ b/ext/intl/formatter/formatter_class.c @@ -58,28 +58,22 @@ zend_object *NumberFormatter_object_create(zend_class_entry *ce) /* {{{ NumberFormatter_object_clone */ zend_object *NumberFormatter_object_clone(zend_object *object) { - NumberFormatter_object *nfo, *new_nfo; - zend_object *new_obj; + NumberFormatter_object *nfo = php_intl_number_format_fetch_object(object); + zend_object *new_obj = NumberFormatter_ce_ptr->create_object(object->ce); + NumberFormatter_object *new_nfo = php_intl_number_format_fetch_object(new_obj); - nfo = php_intl_number_format_fetch_object(object); - intl_error_reset(INTL_DATA_ERROR_P(nfo)); - - new_obj = NumberFormatter_ce_ptr->create_object(object->ce); - new_nfo = php_intl_number_format_fetch_object(new_obj); /* clone standard parts */ zend_objects_clone_members(&new_nfo->zo, &nfo->zo); + /* clone formatter object. It may fail, the destruction code must handle this case */ if (FORMATTER_OBJECT(nfo) != NULL) { - FORMATTER_OBJECT(new_nfo) = unum_clone(FORMATTER_OBJECT(nfo), - &INTL_DATA_ERROR_CODE(nfo)); - if (U_FAILURE(INTL_DATA_ERROR_CODE(nfo))) { - /* set up error in case error handler is interested */ - intl_errors_set(INTL_DATA_ERROR_P(nfo), INTL_DATA_ERROR_CODE(nfo), - "Failed to clone NumberFormatter object", 0); - zend_throw_exception(NULL, "Failed to clone NumberFormatter object", 0); + UErrorCode error = U_ZERO_ERROR; + FORMATTER_OBJECT(new_nfo) = unum_clone(FORMATTER_OBJECT(nfo), &error); + if (U_FAILURE(error)) { + zend_throw_error(NULL, "Failed to clone NumberFormatter"); } } else { - zend_throw_exception(NULL, "Cannot clone unconstructed NumberFormatter", 0); + zend_throw_error(NULL, "Cannot clone uninitialized NumberFormatter"); } return new_obj; } diff --git a/ext/intl/msgformat/msgformat_class.c b/ext/intl/msgformat/msgformat_class.c index 57378f19bb010..4e0766a911b9a 100644 --- a/ext/intl/msgformat/msgformat_class.c +++ b/ext/intl/msgformat/msgformat_class.c @@ -56,29 +56,23 @@ zend_object *MessageFormatter_object_create(zend_class_entry *ce) /* {{{ MessageFormatter_object_clone */ zend_object *MessageFormatter_object_clone(zend_object *object) { - MessageFormatter_object *mfo, *new_mfo; - zend_object *new_obj; + MessageFormatter_object *mfo = php_intl_messageformatter_fetch_object(object); + zend_object *new_obj = MessageFormatter_ce_ptr->create_object(object->ce); + MessageFormatter_object *new_mfo = php_intl_messageformatter_fetch_object(new_obj); - mfo = php_intl_messageformatter_fetch_object(object); - intl_error_reset(INTL_DATA_ERROR_P(mfo)); - - new_obj = MessageFormatter_ce_ptr->create_object(object->ce); - new_mfo = php_intl_messageformatter_fetch_object(new_obj); /* clone standard parts */ zend_objects_clone_members(&new_mfo->zo, &mfo->zo); /* clone formatter object */ if (MSG_FORMAT_OBJECT(mfo) != NULL) { - MSG_FORMAT_OBJECT(new_mfo) = umsg_clone(MSG_FORMAT_OBJECT(mfo), - &INTL_DATA_ERROR_CODE(mfo)); + UErrorCode error = U_ZERO_ERROR; + MSG_FORMAT_OBJECT(new_mfo) = umsg_clone(MSG_FORMAT_OBJECT(mfo), &error); - if (U_FAILURE(INTL_DATA_ERROR_CODE(mfo))) { - intl_errors_set(INTL_DATA_ERROR_P(mfo), INTL_DATA_ERROR_CODE(mfo), - "Failed to clone MessageFormatter object", 0); - zend_throw_exception_ex(NULL, 0, "Failed to clone MessageFormatter object"); + if (U_FAILURE(error)) { + zend_throw_error(NULL, "Failed to clone MessageFormatter"); } } else { - zend_throw_exception_ex(NULL, 0, "Cannot clone unconstructed MessageFormatter"); + zend_throw_error(NULL, "Cannot clone uninitialized MessageFormatter"); } return new_obj; } diff --git a/ext/intl/spoofchecker/spoofchecker_class.c b/ext/intl/spoofchecker/spoofchecker_class.c index 557d207351d4d..6fa59edd47b00 100644 --- a/ext/intl/spoofchecker/spoofchecker_class.c +++ b/ext/intl/spoofchecker/spoofchecker_class.c @@ -61,24 +61,25 @@ zend_object *Spoofchecker_object_create(zend_class_entry *ce) static zend_object *spoofchecker_clone_obj(zend_object *object) /* {{{ */ { - zend_object *new_obj_val; - Spoofchecker_object *sfo, *new_sfo; - - sfo = php_intl_spoofchecker_fetch_object(object); - intl_error_reset(SPOOFCHECKER_ERROR_P(sfo)); - - new_obj_val = Spoofchecker_ce_ptr->create_object(object->ce); - new_sfo = php_intl_spoofchecker_fetch_object(new_obj_val); - /* clone standard parts */ - zend_objects_clone_members(&new_sfo->zo, &sfo->zo); - /* clone internal object */ - new_sfo->uspoof = uspoof_clone(sfo->uspoof, SPOOFCHECKER_ERROR_CODE_P(new_sfo)); - if(U_FAILURE(SPOOFCHECKER_ERROR_CODE(new_sfo))) { - /* set up error in case error handler is interested */ - intl_error_set( NULL, SPOOFCHECKER_ERROR_CODE(new_sfo), "Failed to clone SpoofChecker object", 0 ); - Spoofchecker_objects_free(&new_sfo->zo); /* free new object */ - zend_error_noreturn(E_ERROR, "Failed to clone SpoofChecker object"); + Spoofchecker_object *spoofchecker_orig = php_intl_spoofchecker_fetch_object(object); + zend_object *new_obj_val = Spoofchecker_ce_ptr->create_object(object->ce); + Spoofchecker_object *spoofchecker_new = php_intl_spoofchecker_fetch_object(new_obj_val); + + zend_objects_clone_members(&spoofchecker_new->zo, &spoofchecker_orig->zo); + + if (spoofchecker_orig->uspoof != NULL) { + /* guaranteed to return NULL if it fails */ + UErrorCode error = U_ZERO_ERROR; + spoofchecker_new->uspoof = uspoof_clone(spoofchecker_orig->uspoof, &error); + if (U_FAILURE(error)) { + /* free new object */ + Spoofchecker_objects_free(&spoofchecker_new->zo); + zend_throw_error(NULL, "Failed to clone SpoofChecker"); + } + } else { + zend_throw_error(NULL, "Cannot clone uninitialized SpoofChecker"); } + return new_obj_val; } /* }}} */ diff --git a/ext/intl/tests/bug62915.phpt b/ext/intl/tests/bug62915.phpt index fd0c4aeef4b7e..5c5253536958f 100644 --- a/ext/intl/tests/bug62915.phpt +++ b/ext/intl/tests/bug62915.phpt @@ -7,17 +7,17 @@ intl class foo extends IntlTimeZone { public $foo = 'test'; - - public function __construct() { } + public function __construct() { } } $x = new foo; try { - $z = clone $x; -} catch (Exception $e) { - var_dump($e->getMessage()); + $z = clone $x; + var_dump($z); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } ?> --EXPECT-- -string(39) "Cannot clone unconstructed IntlTimeZone" +Error: Cannot clone uninitialized IntlTimeZone diff --git a/ext/intl/tests/dateformat_clone_bad_obj.phpt b/ext/intl/tests/dateformat_clone_bad_obj.phpt index 7e6224e3f4f3a..54117a75b8cdd 100644 --- a/ext/intl/tests/dateformat_clone_bad_obj.phpt +++ b/ext/intl/tests/dateformat_clone_bad_obj.phpt @@ -1,5 +1,5 @@ --TEST-- -Cloning unconstructed IntlDateFormatter +Cloning uninitialized IntlDateFormatter --EXTENSIONS-- intl --FILE-- @@ -12,9 +12,10 @@ class A extends IntlDateFormatter { $a = new A; try { $b = clone $a; -} catch (Exception $e) { - var_dump($e->getMessage()); + var_dump($b); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } ?> ---EXPECTF-- -string(%s) "Cannot clone unconstructed IntlDateFormatter" +--EXPECT-- +Error: Cannot clone uninitialized IntlDateFormatter diff --git a/ext/intl/tests/formatter_clone_bad_obj.phpt b/ext/intl/tests/formatter_clone_bad_obj.phpt index 6598f5d9db696..3291e6d4cb329 100644 --- a/ext/intl/tests/formatter_clone_bad_obj.phpt +++ b/ext/intl/tests/formatter_clone_bad_obj.phpt @@ -1,5 +1,5 @@ --TEST-- -Cloning unconstructed numfmt +Cloning uninitialized NumberFormatter --EXTENSIONS-- intl --FILE-- @@ -12,9 +12,10 @@ class A extends NumberFormatter { $a = new A; try { $b = clone $a; -} catch (Exception $e) { - var_dump($e->getMessage()); + var_dump($b); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } ?> --EXPECT-- -string(42) "Cannot clone unconstructed NumberFormatter" +Error: Cannot clone uninitialized NumberFormatter diff --git a/ext/intl/tests/msgfmt_clone_bad_obj.phpt b/ext/intl/tests/msgfmt_clone_bad_obj.phpt index 543250525b43a..0545117b97ac8 100644 --- a/ext/intl/tests/msgfmt_clone_bad_obj.phpt +++ b/ext/intl/tests/msgfmt_clone_bad_obj.phpt @@ -1,5 +1,5 @@ --TEST-- -Cloning unconstructed MessageFormatter +Cloning uninitialized MessageFormatter --EXTENSIONS-- intl --FILE-- @@ -12,9 +12,10 @@ class A extends MessageFormatter { $a = new A; try { $b = clone $a; -} catch (Exception $e) { - var_dump($e->getMessage()); + var_dump($b); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } ?> ---EXPECTF-- -string(%d) "Cannot clone unconstructed MessageFormatter" +--EXPECT-- +Error: Cannot clone uninitialized MessageFormatter diff --git a/ext/intl/tests/timezone_clone_error.phpt b/ext/intl/tests/timezone_clone_error.phpt index ae3a672dfbf5f..615f306ef77d0 100644 --- a/ext/intl/tests/timezone_clone_error.phpt +++ b/ext/intl/tests/timezone_clone_error.phpt @@ -4,25 +4,19 @@ IntlTimeZone clone handler: error test intl --FILE-- getMessage()); + $b = clone $tz; + var_dump($b); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } ?> --EXPECT-- -object(A)#1 (1) { - ["valid"]=> - bool(false) -} -string(9) "Exception" -string(39) "Cannot clone unconstructed IntlTimeZone" \ No newline at end of file +Error: Cannot clone uninitialized IntlTimeZone diff --git a/ext/intl/timezone/timezone_class.cpp b/ext/intl/timezone/timezone_class.cpp index 8afc7e2bc4a3a..f7b3d4eeb08f2 100644 --- a/ext/intl/timezone/timezone_class.cpp +++ b/ext/intl/timezone/timezone_class.cpp @@ -146,6 +146,8 @@ U_CFUNC TimeZone *timezone_process_timezone_argument(zval *zv_timezone, if (Z_TYPE_P(zv_timezone) == IS_OBJECT && instanceof_function(Z_OBJCE_P(zv_timezone), TimeZone_ce_ptr)) { TimeZone_object *to = Z_INTL_TIMEZONE_P(zv_timezone); + + /* TODO Throw proper Error exceptions for uninitialized classes and failure to clone */ if (to->utimezone == NULL) { spprintf(&message, 0, "%s: passed IntlTimeZone is not " "properly constructed", func); @@ -224,38 +226,22 @@ U_CFUNC TimeZone *timezone_process_timezone_argument(zval *zv_timezone, /* {{{ clone handler for TimeZone */ static zend_object *TimeZone_clone_obj(zend_object *object) { - TimeZone_object *to_orig, - *to_new; - zend_object *ret_val; - intl_error_reset(NULL); - - to_orig = php_intl_timezone_fetch_object(object); - intl_error_reset(TIMEZONE_ERROR_P(to_orig)); - - ret_val = TimeZone_ce_ptr->create_object(object->ce); - to_new = php_intl_timezone_fetch_object(ret_val); + TimeZone_object *to_orig = php_intl_timezone_fetch_object(object); + zend_object *ret_val = TimeZone_ce_ptr->create_object(object->ce); + TimeZone_object *to_new = php_intl_timezone_fetch_object(ret_val); zend_objects_clone_members(&to_new->zo, &to_orig->zo); if (to_orig->utimezone != NULL) { - TimeZone *newTimeZone; - - newTimeZone = to_orig->utimezone->clone(); - to_new->should_delete = 1; + TimeZone *newTimeZone = to_orig->utimezone->clone(); + to_new->should_delete = true; if (!newTimeZone) { - zend_string *err_msg; - intl_errors_set_code(TIMEZONE_ERROR_P(to_orig), - U_MEMORY_ALLOCATION_ERROR); - intl_errors_set_custom_msg(TIMEZONE_ERROR_P(to_orig), - "Could not clone IntlTimeZone", 0); - err_msg = intl_error_get_message(TIMEZONE_ERROR_P(to_orig)); - zend_throw_exception(NULL, ZSTR_VAL(err_msg), 0); - zend_string_free(err_msg); + zend_throw_error(NULL, "Failed to clone IntlTimeZone"); } else { to_new->utimezone = newTimeZone; } } else { - zend_throw_exception(NULL, "Cannot clone unconstructed IntlTimeZone", 0); + zend_throw_error(NULL, "Cannot clone uninitialized IntlTimeZone"); } return ret_val; diff --git a/ext/intl/transliterator/transliterator_class.c b/ext/intl/transliterator/transliterator_class.c index 78c6d562d2f93..438b71c2c417a 100644 --- a/ext/intl/transliterator/transliterator_class.c +++ b/ext/intl/transliterator/transliterator_class.c @@ -128,46 +128,27 @@ static zend_object *Transliterator_object_create( zend_class_entry *ce ) /* {{{ clone handler for Transliterator */ static zend_object *Transliterator_clone_obj( zend_object *object ) { - Transliterator_object *to_orig, - *to_new; - zend_object *ret_val; - intl_error_reset( NULL ); - - to_orig = php_intl_transliterator_fetch_object( object ); - intl_error_reset( INTL_DATA_ERROR_P( to_orig ) ); - ret_val = Transliterator_ce_ptr->create_object( object->ce ); - to_new = php_intl_transliterator_fetch_object( ret_val ); + Transliterator_object *to_orig = php_intl_transliterator_fetch_object(object); + zend_object *ret_val = Transliterator_ce_ptr->create_object(object->ce); + Transliterator_object *to_new = php_intl_transliterator_fetch_object(ret_val); zend_objects_clone_members( &to_new->zo, &to_orig->zo ); - - if( to_orig->utrans != NULL ) - { + if (to_orig->utrans != NULL) { /* guaranteed to return NULL if it fails */ - UTransliterator *utrans = utrans_clone( to_orig->utrans, TRANSLITERATOR_ERROR_CODE_P( to_orig ) ); - - if( U_FAILURE( TRANSLITERATOR_ERROR_CODE( to_orig ) ) ) { - zend_string *err_msg; - - if( utrans != NULL ) - transliterator_object_destroy( to_new ); - - /* set the error anyway, in case in the future we decide not to - * throw an error. It also helps build the error message */ - intl_error_set_code( NULL, INTL_DATA_ERROR_CODE( to_orig ) ); - intl_errors_set_custom_msg( TRANSLITERATOR_ERROR_P( to_orig ), - "Could not clone transliterator", 0 ); - - err_msg = intl_error_get_message( TRANSLITERATOR_ERROR_P( to_orig ) ); - zend_throw_error( NULL, "%s", ZSTR_VAL(err_msg) ); - zend_string_free( err_msg ); /* if it's changed into a warning */ + UErrorCode error = U_ZERO_ERROR; + UTransliterator *utrans = utrans_clone( to_orig->utrans, &error); + + if (U_FAILURE(error)) { + if (utrans != NULL) { + transliterator_object_destroy(to_new); + } + zend_throw_error(NULL, "Failed to clone Transliterator"); } else { to_new->utrans = utrans; } - } - else - { + } else { /* We shouldn't have unconstructed objects in the first place */ - zend_throw_error(NULL, "Unconstructed Transliterator object cannot be cloned"); + zend_throw_error(NULL, "Cannot clone uninitialized Transliterator"); } return ret_val; From 5abf8539777c393ef6b450f1bd6bf0537096babb Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 29 May 2024 15:03:42 +0100 Subject: [PATCH 3/4] ext/intl: idn.c use ValueErrors where appropriate Drive-by refactoring --- ext/intl/idn/idn.c | 26 ++++++++++---------------- ext/intl/tests/idn_uts46_errors.phpt | 26 +++++++++++++++----------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/ext/intl/idn/idn.c b/ext/intl/idn/idn.c index 257e0a00ca109..f3769affcc850 100644 --- a/ext/intl/idn/idn.c +++ b/ext/intl/idn/idn.c @@ -37,7 +37,7 @@ enum { }; /* like INTL_CHECK_STATUS, but as a function and varying the name of the func */ -static int php_intl_idn_check_status(UErrorCode err, const char *msg) +static zend_result php_intl_idn_check_status(UErrorCode err, const char *msg) { intl_error_set_code(NULL, err); if (U_FAILURE(err)) { @@ -53,11 +53,6 @@ static int php_intl_idn_check_status(UErrorCode err, const char *msg) return SUCCESS; } -static inline void php_intl_bad_args(const char *msg) -{ - php_intl_idn_check_status(U_ILLEGAL_ARGUMENT_ERROR, msg); -} - static void php_intl_idn_to_46(INTERNAL_FUNCTION_PARAMETERS, const zend_string *domain, uint32_t option, int mode, zval *idna_info) { @@ -128,18 +123,17 @@ static void php_intl_idn_handoff(INTERNAL_FUNCTION_PARAMETERS, int mode) RETURN_THROWS(); } - if (variant != INTL_IDN_VARIANT_UTS46) { - php_intl_bad_args("invalid variant, must be INTL_IDNA_VARIANT_UTS46"); - RETURN_FALSE; - } - - if (ZSTR_LEN(domain) < 1) { - php_intl_bad_args("empty domain name"); - RETURN_FALSE; + if (ZSTR_LEN(domain) == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } if (ZSTR_LEN(domain) > INT32_MAX - 1) { - php_intl_bad_args("domain name too large"); - RETURN_FALSE; + zend_argument_value_error(1, "must be less than " PRId32 " bytes", INT32_MAX); + RETURN_THROWS(); + } + if (variant != INTL_IDN_VARIANT_UTS46) { + zend_argument_value_error(2, "must be INTL_IDNA_VARIANT_UTS46"); + RETURN_THROWS(); } /* don't check options; it wasn't checked before */ diff --git a/ext/intl/tests/idn_uts46_errors.phpt b/ext/intl/tests/idn_uts46_errors.phpt index ebf7573461de3..435d464f7ac3c 100644 --- a/ext/intl/tests/idn_uts46_errors.phpt +++ b/ext/intl/tests/idn_uts46_errors.phpt @@ -13,10 +13,18 @@ ini_set("intl.error_level", E_WARNING); echo "=> PHP level errors", "\n"; echo "bad variant:", "\n"; -var_dump(idn_to_ascii("", 0, INTL_IDNA_VARIANT_UTS46 + 10)); +try { + var_dump(idn_to_ascii("domain", 0, INTL_IDNA_VARIANT_UTS46 + 10)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} echo "empty domain:", "\n"; -var_dump(idn_to_ascii("", 0, INTL_IDNA_VARIANT_UTS46)); +try { + var_dump(idn_to_ascii("", 0, INTL_IDNA_VARIANT_UTS46)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} echo "with error, but no details arg:", "\n"; var_dump(idn_to_ascii("www.fußball.com-", 0, INTL_IDNA_VARIANT_UTS46)); @@ -26,7 +34,7 @@ var_dump(idn_to_ascii("www.fußball.com-", IDNA_NONTRANSITIONAL_TO_ASCII, INTL_IDNA_VARIANT_UTS46, $foo)); var_dump($foo); -echo "with error, with details arg, contextj:", "\n"; +echo "with error, with details arg, context:", "\n"; var_dump(idn_to_ascii( html_entity_decode("www.a‍b.com", 0, "UTF-8"), IDNA_NONTRANSITIONAL_TO_ASCII | IDNA_CHECK_CONTEXTJ, @@ -35,16 +43,12 @@ var_dump($foo); var_dump($foo["errors"]==IDNA_ERROR_CONTEXTJ); ?> ---EXPECTF-- +--EXPECT-- => PHP level errors bad variant: - -Warning: idn_to_ascii(): idn_to_ascii: invalid variant, must be INTL_IDNA_VARIANT_UTS46 in %s on line %d -bool(false) +ValueError: idn_to_ascii(): Argument #2 ($flags) must be INTL_IDNA_VARIANT_UTS46 empty domain: - -Warning: idn_to_ascii(): idn_to_ascii: empty domain name in %s on line %d -bool(false) +ValueError: idn_to_ascii(): Argument #1 ($domain) cannot be empty with error, but no details arg: bool(false) with error, with details arg: @@ -57,7 +61,7 @@ array(3) { ["errors"]=> int(16) } -with error, with details arg, contextj: +with error, with details arg, context: bool(false) array(3) { ["result"]=> From 665c7fc518bf6b2710f3208dd985ce92be12da2f Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 29 May 2024 15:28:05 +0100 Subject: [PATCH 4/4] ext/intl: Remove some unused headers Probably more cleanup can be done --- ext/intl/grapheme/grapheme_util.c | 2 -- ext/intl/transliterator/transliterator.h | 2 -- 2 files changed, 4 deletions(-) diff --git a/ext/intl/grapheme/grapheme_util.c b/ext/intl/grapheme/grapheme_util.c index 89aaf0e97a008..13ddcb3992162 100644 --- a/ext/intl/grapheme/grapheme_util.c +++ b/ext/intl/grapheme/grapheme_util.c @@ -28,8 +28,6 @@ #include #include -#include "ext/standard/php_string.h" - ZEND_EXTERN_MODULE_GLOBALS( intl ) /* }}} */ diff --git a/ext/intl/transliterator/transliterator.h b/ext/intl/transliterator/transliterator.h index 6824321ac4738..f98ca0682965e 100644 --- a/ext/intl/transliterator/transliterator.h +++ b/ext/intl/transliterator/transliterator.h @@ -18,8 +18,6 @@ #include #include -#include "zend_smart_str.h" - smart_str transliterator_parse_error_to_string( UParseError* pe ); #endif /* #ifndef TRANSLITERATOR_TRANSLITERATOR_H */