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.

5 files changed, 68 insertions, 54 deletions

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--
-<?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
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--
+<?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