From 683e78786070ab77d33f7787598ac1b90d68390a Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 4 Dec 2023 22:00:17 +0000 Subject: [PATCH] Fix GH-12727: NumberFormatter constructor throws an exception on invalid locale. Also re-establishing exception throwing on IntlDateFormatter constructor overwritten by accident most likely so postponing it for next major release. Close GH-12868 --- NEWS | 2 ++ UPGRADING | 4 +++ ext/intl/dateformat/dateformat_create.cpp | 3 ++- ext/intl/formatter/formatter_main.c | 6 +++++ ext/intl/tests/formatter_fail.phpt | 12 ++++++--- ext/intl/tests/gh12282.phpt | 33 ++++++++++++++--------- ext/intl/tests/gh12727.phpt | 15 +++++++++++ 7 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 ext/intl/tests/gh12727.phpt diff --git a/NEWS b/NEWS index 68128f21283..89b82cf594a 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,8 @@ FTP: Intl: . Added IntlDateFormatter::PATTERN constant. (David Carlier) + . Fixed Numberformatter::__construct when the locale is invalid, now + throws an exception. (David Carlier) MBString: . Added mb_trim, mb_ltrim and mb_rtrim. (Yuya Hamada) diff --git a/UPGRADING b/UPGRADING index 2334b1abbf4..9282941272f 100644 --- a/UPGRADING +++ b/UPGRADING @@ -190,6 +190,10 @@ PHP 8.4 UPGRADE NOTES 5. Changed Functions ======================================== +- Intl: + . IntlDateFormatter::__construct() throws a ValueError if the locale is invalid. + . NumberFormatter::__construct() throws a ValueError if the locale is invalid. + - MBString: . The behavior of mb_strcut is more consistent now on invalid UTF-8 and UTF-16 strings. (For valid UTF-8 and UTF-16 strings, there is no change.) diff --git a/ext/intl/dateformat/dateformat_create.cpp b/ext/intl/dateformat/dateformat_create.cpp index 5c96f41fadf..dbc2273f329 100644 --- a/ext/intl/dateformat/dateformat_create.cpp +++ b/ext/intl/dateformat/dateformat_create.cpp @@ -113,7 +113,8 @@ static zend_result datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_error_handlin locale = Locale::createFromName(locale_str); /* get*Name accessors being set does not preclude being bogus */ if (locale.isBogus() || strlen(locale.getISO3Language()) == 0) { - goto error; + zend_argument_value_error(1, "\"%s\" is invalid", locale_str); + return FAILURE; } /* process calendar */ diff --git a/ext/intl/formatter/formatter_main.c b/ext/intl/formatter/formatter_main.c index ed1806d6bbc..53c5a3e9be2 100644 --- a/ext/intl/formatter/formatter_main.c +++ b/ext/intl/formatter/formatter_main.c @@ -17,6 +17,7 @@ #endif #include +#include #include "php_intl.h" #include "formatter_class.h" @@ -63,6 +64,11 @@ static int numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_error_handling *error_ locale = intl_locale_get_default(); } + if (strlen(uloc_getISO3Language(locale)) == 0) { + zend_argument_value_error(1, "\"%s\" is invalid", locale); + return FAILURE; + } + /* Create an ICU number formatter. */ FORMATTER_OBJECT(nfo) = unum_open(style, spattern, spattern_len, locale, NULL, &INTL_DATA_ERROR_CODE(nfo)); diff --git a/ext/intl/tests/formatter_fail.phpt b/ext/intl/tests/formatter_fail.phpt index 2a2908e908c..c9c621c6b76 100644 --- a/ext/intl/tests/formatter_fail.phpt +++ b/ext/intl/tests/formatter_fail.phpt @@ -119,10 +119,14 @@ Deprecated: numfmt_create(): Passing null to parameter #1 ($locale) of type stri Deprecated: numfmt_create(): Passing null to parameter #2 ($style) of type int is deprecated in %s on line %d -IntlException: Constructor failed in %s on line %d -'numfmt_create: number formatter creation failed: U_UNSUPPORTED_ERROR' -'numfmt_create: number formatter creation failed: U_UNSUPPORTED_ERROR' -'numfmt_create: number formatter creation failed: U_UNSUPPORTED_ERROR' +ValueError: NumberFormatter::__construct(): Argument #1 ($locale) "%s" is invalid in %s on line %d +'U_ZERO_ERROR' + +ValueError: NumberFormatter::create(): Argument #1 ($locale) "%s" is invalid in %s on line %d +'U_ZERO_ERROR' + +ValueError: numfmt_create(): Argument #1 ($locale) "%s" is invalid in %s on line %d +'U_ZERO_ERROR' TypeError: NumberFormatter::__construct(): Argument #1 ($locale) must be of type string, array given in %s on line %d 'U_ZERO_ERROR' diff --git a/ext/intl/tests/gh12282.phpt b/ext/intl/tests/gh12282.phpt index a30899a08c7..b12dc655f70 100644 --- a/ext/intl/tests/gh12282.phpt +++ b/ext/intl/tests/gh12282.phpt @@ -5,18 +5,25 @@ intl --FILE-- getMessage() . PHP_EOL; +} + Locale::setDefault('xx'); -var_dump(new IntlDateFormatter(Locale::getDefault())); +try { + new IntlDateFormatter(Locale::getDefault()); +} catch (\ValueError $e) { + echo $e->getMessage(); +} --EXPECT-- -object(IntlDateFormatter)#1 (0) { -} -object(IntlDateFormatter)#1 (0) { -} +IntlDateFormatter::__construct(): Argument #1 ($locale) "xx" is invalid +IntlDateFormatter::__construct(): Argument #1 ($locale) "xx" is invalid diff --git a/ext/intl/tests/gh12727.phpt b/ext/intl/tests/gh12727.phpt new file mode 100644 index 00000000000..e04b5eb26a1 --- /dev/null +++ b/ext/intl/tests/gh12727.phpt @@ -0,0 +1,15 @@ +--TEST-- +numfmt creation failures +--EXTENSIONS-- +intl +--FILE-- +getMessage(); +} +?> +--EXPECTF-- +NumberFormatter::__construct(): Argument #1 ($locale) "%s" is invalid