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 960f67f7c5f3dc7cb38679016ab37837f1d10f76..1d8678dd0968b70457e666fa4c549b21816743b1 100644 (file)
--- 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 5210f7bf64a072f21462533c3c42a8af16dec40b..cc22fdb886cfcd04b1f80c4431a1f1b77f584ce2 100644 (file)
--- 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 d6c237b41068bfa382d2b26023e31e0d3d97f652..0b059de8d97844e99b34dcd2fd055e70318ea59f 100644 (file)
--- 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 (file)
index 5404631..0000000
--- 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--
-<?php require 'skipif.inc'; ?>
---FILE--
-<?php
-
-var_dump(mb_convert_case("foobar", 100));
-
-?>
---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 (file)
index 0000000..cd278ae
--- /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--
+<?php require 'skipif.inc'; ?>
+--FILE--
+<?php
+
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_UPPER));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_LOWER));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_TITLE));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_FOLD));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_UPPER_SIMPLE));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_LOWER_SIMPLE));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_TITLE_SIMPLE));
+var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_FOLD_SIMPLE));
+
+// Invalid mode
+try {
+    var_dump(mb_convert_case('foo BAR Spaß', 100));
+} catch (\ValueError $e) {
+    echo $e->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