From b76677d1cf8ce13b6803fa99dfb3599e0821bf9e Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 15 Feb 2020 10:26:42 +0100 Subject: [PATCH 1/4] Disallow passing array as number to NumberFormatter::format() Array to number conversion makes no sense here, and is likely to hide a programming error. Therefore we disallow passing an array in the first place. --- ext/intl/formatter/formatter_format.c | 10 +++++----- ext/intl/tests/bug79212.phpt | 7 ++++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ext/intl/formatter/formatter_format.c b/ext/intl/formatter/formatter_format.c index 8c6a567eb2b78..80740f1da7a29 100644 --- a/ext/intl/formatter/formatter_format.c +++ b/ext/intl/formatter/formatter_format.c @@ -44,15 +44,15 @@ PHP_FUNCTION( numfmt_format ) { RETURN_THROWS(); } + if(Z_TYPE_P(number) == IS_ARRAY) { + zend_type_error("given value must not be an array"); + RETURN_THROWS(); + } /* Fetch the object. */ FORMATTER_METHOD_FETCH_OBJECT; - if(Z_TYPE_P(number) != IS_ARRAY) { - convert_scalar_to_number_ex(number); - } else { - convert_to_long(number); - } + convert_scalar_to_number_ex(number); if(type == FORMAT_TYPE_DEFAULT) { switch(Z_TYPE_P(number)) { diff --git a/ext/intl/tests/bug79212.phpt b/ext/intl/tests/bug79212.phpt index 3d2083013b8f8..f27b0686fb492 100644 --- a/ext/intl/tests/bug79212.phpt +++ b/ext/intl/tests/bug79212.phpt @@ -15,4 +15,9 @@ var_dump($fmt->format([1], NumberFormatter::TYPE_INT64)); ?> --EXPECTF-- string(21) "823749273428379%c%c%c%c%c%c" -string(1) "1" + +Fatal error: Uncaught TypeError: given value must not be an array in %s:%d +Stack trace: +#0 %s(%d): NumberFormatter->format(Array, 2) +#1 {main} + thrown in %s on line %d From bf1444734608e8ce99a1239fe0af3bca9726fd6e Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 15 Feb 2020 10:55:09 +0100 Subject: [PATCH 2/4] Capitalize error message --- ext/intl/formatter/formatter_format.c | 2 +- ext/intl/tests/bug79212.phpt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/intl/formatter/formatter_format.c b/ext/intl/formatter/formatter_format.c index 80740f1da7a29..3174b233561a0 100644 --- a/ext/intl/formatter/formatter_format.c +++ b/ext/intl/formatter/formatter_format.c @@ -45,7 +45,7 @@ PHP_FUNCTION( numfmt_format ) RETURN_THROWS(); } if(Z_TYPE_P(number) == IS_ARRAY) { - zend_type_error("given value must not be an array"); + zend_type_error("Given value must not be an array"); RETURN_THROWS(); } diff --git a/ext/intl/tests/bug79212.phpt b/ext/intl/tests/bug79212.phpt index f27b0686fb492..03633e3bc6b5f 100644 --- a/ext/intl/tests/bug79212.phpt +++ b/ext/intl/tests/bug79212.phpt @@ -16,7 +16,7 @@ var_dump($fmt->format([1], NumberFormatter::TYPE_INT64)); --EXPECTF-- string(21) "823749273428379%c%c%c%c%c%c" -Fatal error: Uncaught TypeError: given value must not be an array in %s:%d +Fatal error: Uncaught TypeError: Given value must not be an array in %s:%d Stack trace: #0 %s(%d): NumberFormatter->format(Array, 2) #1 {main} From 7f88d2e300f2a82acab8b874d7c486ef4138588e Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 16 Feb 2020 11:12:06 +0100 Subject: [PATCH 3/4] Introduce n specifier for classic ZPP This port of `Z_PARAM_NUMBER` is particularly useful for `zend_parse_method_parameters()`. --- Zend/zend_API.c | 12 +++++++++++- docs/parameter-parsing-api.md | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index a3e452bf7d9c5..ed8440f1047ea 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -522,6 +522,16 @@ static const char *zend_parse_arg_impl(int arg_num, zval *arg, va_list *va, cons } break; + case 'n': + { + zval **p = va_arg(*va, zval **); + + if (!zend_parse_arg_number(arg, p, check_null)) { + return "number"; + } + } + break; + case 's': { char **p = va_arg(*va, char **); @@ -793,7 +803,7 @@ static int zend_parse_va_args(int num_args, const char *type_spec, va_list *va, case 'f': case 'A': case 'H': case 'p': case 'S': case 'P': - case 'L': + case 'L': case 'n': max_num_args++; break; diff --git a/docs/parameter-parsing-api.md b/docs/parameter-parsing-api.md index e038a20dee3cd..6bf035673b54f 100644 --- a/docs/parameter-parsing-api.md +++ b/docs/parameter-parsing-api.md @@ -75,6 +75,7 @@ f - function or array containing php method call info (returned as h - array (returned as HashTable*) H - array or HASH_OF(object) (returned as HashTable*) l - long (zend_long) +n - long or double (zval*) o - object of any type (zval*) O - object of specific type given by class entry (zval*, zend_class_entry) p - valid path (string without null bytes in the middle) and its length (char*, size_t) From 7dd996375ee1ff933b329c207785654a16044f19 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 16 Feb 2020 11:52:20 +0100 Subject: [PATCH 4/4] Constrain number paramater to int|float We also remove two test cases, since these have been degenerated to ZPP tests. --- ext/intl/formatter/formatter.stub.php | 4 ++-- ext/intl/formatter/formatter_arginfo.h | 4 ++-- ext/intl/formatter/formatter_format.c | 8 +------- ext/intl/tests/bug48227.phpt | 21 --------------------- ext/intl/tests/bug53735.phpt | 2 ++ ext/intl/tests/bug79212.phpt | 23 ----------------------- 6 files changed, 7 insertions(+), 55 deletions(-) delete mode 100644 ext/intl/tests/bug48227.phpt delete mode 100644 ext/intl/tests/bug79212.phpt diff --git a/ext/intl/formatter/formatter.stub.php b/ext/intl/formatter/formatter.stub.php index 8d976f79e2270..2edc28d6fd4c4 100644 --- a/ext/intl/formatter/formatter.stub.php +++ b/ext/intl/formatter/formatter.stub.php @@ -8,7 +8,7 @@ public function __construct(string $locale, int $style, string $pattern = "") {} public static function create(string $locale, int $style, string $pattern = "") {} /** @return string|false */ - public function format($value, int $type = NumberFormatter::TYPE_DEFAULT) {} + public function format(int|float $value, int $type = NumberFormatter::TYPE_DEFAULT) {} /** @return int|float|false */ public function parse(string $value, int $type = NumberFormatter::TYPE_DOUBLE, &$position = null) {} @@ -58,7 +58,7 @@ public function getErrorMessage() {} function numfmt_create(string $locale, int $style, string $pattern = ""): ?NumberFormatter {} -function numfmt_format(NumberFormatter $fmt, $value, int $type = NumberFormatter::TYPE_DEFAULT): string|false {} +function numfmt_format(NumberFormatter $fmt, int|float $value, int $type = NumberFormatter::TYPE_DEFAULT): string|false {} function numfmt_parse(NumberFormatter $fmt, string $value, int $type = NumberFormatter::TYPE_DOUBLE, &$position = null): int|float|false {} diff --git a/ext/intl/formatter/formatter_arginfo.h b/ext/intl/formatter/formatter_arginfo.h index 313d0dd714dce..eced76d82b428 100644 --- a/ext/intl/formatter/formatter_arginfo.h +++ b/ext/intl/formatter/formatter_arginfo.h @@ -9,7 +9,7 @@ ZEND_END_ARG_INFO() #define arginfo_class_NumberFormatter_create arginfo_class_NumberFormatter___construct ZEND_BEGIN_ARG_INFO_EX(arginfo_class_NumberFormatter_format, 0, 0, 1) - ZEND_ARG_INFO(0, value) + ZEND_ARG_TYPE_MASK(0, value, MAY_BE_LONG|MAY_BE_DOUBLE) ZEND_ARG_TYPE_INFO(0, type, IS_LONG, 0) ZEND_END_ARG_INFO() @@ -73,7 +73,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_numfmt_format, 0, 2, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_OBJ_INFO(0, fmt, NumberFormatter, 0) - ZEND_ARG_INFO(0, value) + ZEND_ARG_TYPE_MASK(0, value, MAY_BE_LONG|MAY_BE_DOUBLE) ZEND_ARG_TYPE_INFO(0, type, IS_LONG, 0) ZEND_END_ARG_INFO() diff --git a/ext/intl/formatter/formatter_format.c b/ext/intl/formatter/formatter_format.c index 3174b233561a0..4f87a981f29a8 100644 --- a/ext/intl/formatter/formatter_format.c +++ b/ext/intl/formatter/formatter_format.c @@ -39,21 +39,15 @@ PHP_FUNCTION( numfmt_format ) FORMATTER_METHOD_INIT_VARS; /* Parse parameters. */ - if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "Oz|l", + if( zend_parse_method_parameters( ZEND_NUM_ARGS(), getThis(), "On|l", &object, NumberFormatter_ce_ptr, &number, &type ) == FAILURE ) { RETURN_THROWS(); } - if(Z_TYPE_P(number) == IS_ARRAY) { - zend_type_error("Given value must not be an array"); - RETURN_THROWS(); - } /* Fetch the object. */ FORMATTER_METHOD_FETCH_OBJECT; - convert_scalar_to_number_ex(number); - if(type == FORMAT_TYPE_DEFAULT) { switch(Z_TYPE_P(number)) { case IS_LONG: diff --git a/ext/intl/tests/bug48227.phpt b/ext/intl/tests/bug48227.phpt deleted file mode 100644 index 42a4ffaf867a2..0000000000000 --- a/ext/intl/tests/bug48227.phpt +++ /dev/null @@ -1,21 +0,0 @@ ---TEST-- -Bug #48227 (NumberFormatter::format leaks memory) ---SKIPIF-- - ---FILE-- -format('')); -var_dump($x->format(1)); -var_dump($x->format(NULL)); -var_dump($x->format($x)); - -?> ---EXPECTF-- -string(1) "0" -string(1) "1" -string(1) "0" - -Notice: Object of class NumberFormatter could not be converted to number in %s on line %d -string(1) "1" diff --git a/ext/intl/tests/bug53735.phpt b/ext/intl/tests/bug53735.phpt index 1deebb2276632..66370e61f2e4c 100644 --- a/ext/intl/tests/bug53735.phpt +++ b/ext/intl/tests/bug53735.phpt @@ -25,6 +25,8 @@ var_dump($f->format(0.26)); --EXPECTF-- string(%d) "5,50 kr%A" string(%d) "5,50 kr%A" + +Notice: A non well formed numeric value encountered in %s on line %d string(%d) "5,00 kr%A" string(5) "23,25" string(3) "26%" diff --git a/ext/intl/tests/bug79212.phpt b/ext/intl/tests/bug79212.phpt deleted file mode 100644 index 03633e3bc6b5f..0000000000000 --- a/ext/intl/tests/bug79212.phpt +++ /dev/null @@ -1,23 +0,0 @@ ---TEST-- -Bug #79212 (NumberFormatter::format() may detect wrong type) ---SKIPIF-- - ---FILE-- -format(gmp_init('823749273428379492374'))); - -$fmt = new NumberFormatter('en_US', NumberFormatter::PATTERN_DECIMAL); -var_dump($fmt->format([1], NumberFormatter::TYPE_INT64)); -?> ---EXPECTF-- -string(21) "823749273428379%c%c%c%c%c%c" - -Fatal error: Uncaught TypeError: Given value must not be an array in %s:%d -Stack trace: -#0 %s(%d): NumberFormatter->format(Array, 2) -#1 {main} - thrown in %s on line %d