From: George Peter Banyard Date: Sun, 5 Apr 2020 01:33:08 +0000 (+0200) Subject: Promote invalid case mode to ValueError in mb_case_converter X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1b6f61e7c4330b0f7abcc29c80f3c94bc6d13ce5;p=php Promote invalid case mode to ValueError in mb_case_converter Add assertions to check the return value is not NULL as this indicates a bug. Add identical assertion to mb_strtoupper and mb_strtolower. This means these functions can't return false anymore, ammend stubs accordingly. --- diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index 960f67f7c5..1d8678dd09 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -2760,8 +2760,8 @@ static char *mbstring_convert_case( MBSTRG(current_filter_illegal_mode), MBSTRG(current_filter_illegal_substchar)); } -/* {{{ proto string mb_convert_case(string sourcestring, int mode [, string encoding]) - Returns a case-folded version of sourcestring */ +/* {{{ proto string mb_convert_case(string source_string, int mode [, string encoding]) + Returns a case-folded version of source_string */ PHP_FUNCTION(mb_convert_case) { zend_string *from_encoding = NULL; @@ -2772,9 +2772,7 @@ PHP_FUNCTION(mb_convert_case) size_t ret_len; const mbfl_encoding *enc; - RETVAL_FALSE; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "sl|S!", &str, &str_len, - &case_mode, &from_encoding) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "sl|S!", &str, &str_len, &case_mode, &from_encoding) == FAILURE) { RETURN_THROWS(); } @@ -2784,22 +2782,23 @@ PHP_FUNCTION(mb_convert_case) } if (case_mode < 0 || case_mode > PHP_UNICODE_CASE_MODE_MAX) { - php_error_docref(NULL, E_WARNING, "Invalid case mode"); - return; + zend_argument_value_error(2, "must be one of MB_CASE_UPPER, MB_CASE_LOWER, MB_CASE_TITLE, MB_CASE_FOLD," + " MB_CASE_UPPER_SIMPLE, MB_CASE_LOWER_SIMPLE, MB_CASE_TITLE_SIMPLE, or MB_CASE_FOLD_SIMPLE"); + RETURN_THROWS(); } newstr = mbstring_convert_case(case_mode, str, str_len, &ret_len, enc); + /* If newstr is NULL something went wrong in mbfl and this is a bug */ + ZEND_ASSERT(newstr != NULL); - if (newstr) { - // TODO: avoid reallocation ??? - RETVAL_STRINGL(newstr, ret_len); - efree(newstr); - } + // TODO: avoid reallocation ??? + RETVAL_STRINGL(newstr, ret_len); + efree(newstr); } /* }}} */ -/* {{{ proto string mb_strtoupper(string sourcestring [, string encoding]) - * Returns a uppercased version of sourcestring +/* {{{ proto string mb_strtoupper(string source_string [, string encoding]) + * Returns a upper cased version of source_string */ PHP_FUNCTION(mb_strtoupper) { @@ -2810,8 +2809,7 @@ PHP_FUNCTION(mb_strtoupper) size_t ret_len; const mbfl_encoding *enc; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len, - &from_encoding) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len, &from_encoding) == FAILURE) { RETURN_THROWS(); } @@ -2821,19 +2819,17 @@ PHP_FUNCTION(mb_strtoupper) } newstr = mbstring_convert_case(PHP_UNICODE_CASE_UPPER, str, str_len, &ret_len, enc); + /* If newstr is NULL something went wrong in mbfl and this is a bug */ + ZEND_ASSERT(newstr != NULL); - if (newstr) { - // TODO: avoid reallocation ??? - RETVAL_STRINGL(newstr, ret_len); - efree(newstr); - return; - } - RETURN_FALSE; + // TODO: avoid reallocation ??? + RETVAL_STRINGL(newstr, ret_len); + efree(newstr); } /* }}} */ -/* {{{ proto string mb_strtolower(string sourcestring [, string encoding]) - * Returns a lowercased version of sourcestring +/* {{{ proto string mb_strtolower(string source_string [, string encoding]) + * Returns a lower cased version of source_string */ PHP_FUNCTION(mb_strtolower) { @@ -2844,8 +2840,7 @@ PHP_FUNCTION(mb_strtolower) size_t ret_len; const mbfl_encoding *enc; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len, - &from_encoding) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len, &from_encoding) == FAILURE) { RETURN_THROWS(); } @@ -2855,14 +2850,12 @@ PHP_FUNCTION(mb_strtolower) } newstr = mbstring_convert_case(PHP_UNICODE_CASE_LOWER, str, str_len, &ret_len, enc); + /* If newstr is NULL something went wrong in mbfl and this is a bug */ + ZEND_ASSERT(newstr != NULL); - if (newstr) { - // TODO: avoid reallocation ??? - RETVAL_STRINGL(newstr, ret_len); - efree(newstr); - return; - } - RETURN_FALSE; + // TODO: avoid reallocation ??? + RETVAL_STRINGL(newstr, ret_len); + efree(newstr); } /* }}} */ diff --git a/ext/mbstring/mbstring.stub.php b/ext/mbstring/mbstring.stub.php index 5210f7bf64..cc22fdb886 100644 --- a/ext/mbstring/mbstring.stub.php +++ b/ext/mbstring/mbstring.stub.php @@ -51,11 +51,11 @@ function mb_strimwidth(string $str, int $start, int $width, string $trimmarker = function mb_convert_encoding(array|string $str, string $to, array|string $from = UNKNOWN): array|string|false {} -function mb_convert_case(string $sourcestring, int $mode, ?string $encoding = null): string|false {} +function mb_convert_case(string $source_string, int $mode, ?string $encoding = null): string {} -function mb_strtoupper(string $sourcestring, ?string $encoding = null): string|false {} +function mb_strtoupper(string $source_string, ?string $encoding = null): string {} -function mb_strtolower(string $sourcestring, ?string $encoding = null): string|false {} +function mb_strtolower(string $source_string, ?string $encoding = null): string {} function mb_detect_encoding(string $str, array|string|null $encoding_list = null, bool $strict = false): string|false {} diff --git a/ext/mbstring/mbstring_arginfo.h b/ext/mbstring/mbstring_arginfo.h index d6c237b410..0b059de8d9 100644 --- a/ext/mbstring/mbstring_arginfo.h +++ b/ext/mbstring/mbstring_arginfo.h @@ -112,14 +112,14 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_convert_encoding, 0, 2, MAY_B ZEND_ARG_TYPE_MASK(0, from, MAY_BE_ARRAY|MAY_BE_STRING) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_convert_case, 0, 2, MAY_BE_STRING|MAY_BE_FALSE) - ZEND_ARG_TYPE_INFO(0, sourcestring, IS_STRING, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_convert_case, 0, 2, IS_STRING, 0) + ZEND_ARG_TYPE_INFO(0, source_string, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, mode, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 1) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_strtoupper, 0, 1, MAY_BE_STRING|MAY_BE_FALSE) - ZEND_ARG_TYPE_INFO(0, sourcestring, IS_STRING, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_strtoupper, 0, 1, IS_STRING, 0) + ZEND_ARG_TYPE_INFO(0, source_string, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 1) ZEND_END_ARG_INFO() diff --git a/ext/mbstring/tests/mb_convert_case_invalid_mode.phpt b/ext/mbstring/tests/mb_convert_case_invalid_mode.phpt deleted file mode 100644 index 540463143a..0000000000 --- a/ext/mbstring/tests/mb_convert_case_invalid_mode.phpt +++ /dev/null @@ -1,13 +0,0 @@ ---TEST-- -Calling mb_convert_case() with an invalid casing mode ---SKIPIF-- - ---FILE-- - ---EXPECTF-- -Warning: mb_convert_case(): Invalid case mode in %s on line %d -bool(false) diff --git a/ext/mbstring/tests/mb_convert_case_various_mode.phpt b/ext/mbstring/tests/mb_convert_case_various_mode.phpt new file mode 100644 index 0000000000..cd278ae52f --- /dev/null +++ b/ext/mbstring/tests/mb_convert_case_various_mode.phpt @@ -0,0 +1,34 @@ +--TEST-- +Calling mb_convert_case() with an invalid casing mode +--SKIPIF-- + +--FILE-- +getMessage() . \PHP_EOL; +} + +?> +--EXPECT-- +string(13) "FOO BAR SPASS" +string(13) "foo bar spaß" +string(13) "Foo Bar Spaß" +string(13) "foo bar spass" +string(13) "FOO BAR SPAß" +string(13) "foo bar spaß" +string(13) "Foo Bar Spaß" +string(13) "foo bar spaß" +mb_convert_case(): Argument #2 ($mode) must be one of MB_CASE_UPPER, MB_CASE_LOWER, MB_CASE_TITLE, MB_CASE_FOLD, MB_CASE_UPPER_SIMPLE, MB_CASE_LOWER_SIMPLE, MB_CASE_TITLE_SIMPLE, or MB_CASE_FOLD_SIMPLE