Skip to content

Commit 1b6f61e

Browse files
GirgiasGirgias
committed
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.
1 parent a34e73d commit 1b6f61e

File tree

5 files changed

+68
-54
lines changed

5 files changed

+68
-54
lines changed

‎ext/mbstring/mbstring.c

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2760,8 +2760,8 @@ static char *mbstring_convert_case(
27602760
MBSTRG(current_filter_illegal_mode), MBSTRG(current_filter_illegal_substchar));
27612761
}
27622762

2763-
/* {{{ proto string mb_convert_case(string sourcestring, int mode [, string encoding])
2764-
Returns a case-folded version of sourcestring */
2763+
/* {{{ proto string mb_convert_case(string source_string, int mode [, string encoding])
2764+
Returns a case-folded version of source_string */
27652765
PHP_FUNCTION(mb_convert_case)
27662766
{
27672767
zend_string *from_encoding = NULL;
@@ -2772,9 +2772,7 @@ PHP_FUNCTION(mb_convert_case)
27722772
size_t ret_len;
27732773
const mbfl_encoding *enc;
27742774

2775-
RETVAL_FALSE;
2776-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sl|S!", &str, &str_len,
2777-
&case_mode, &from_encoding) == FAILURE) {
2775+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sl|S!", &str, &str_len, &case_mode, &from_encoding) == FAILURE) {
27782776
RETURN_THROWS();
27792777
}
27802778

@@ -2784,22 +2782,23 @@ PHP_FUNCTION(mb_convert_case)
27842782
}
27852783

27862784
if (case_mode < 0 || case_mode > PHP_UNICODE_CASE_MODE_MAX) {
2787-
php_error_docref(NULL, E_WARNING, "Invalid case mode");
2788-
return;
2785+
zend_argument_value_error(2, "must be one of MB_CASE_UPPER, MB_CASE_LOWER, MB_CASE_TITLE, MB_CASE_FOLD,"
2786+
" MB_CASE_UPPER_SIMPLE, MB_CASE_LOWER_SIMPLE, MB_CASE_TITLE_SIMPLE, or MB_CASE_FOLD_SIMPLE");
2787+
RETURN_THROWS();
27892788
}
27902789

27912790
newstr = mbstring_convert_case(case_mode, str, str_len, &ret_len, enc);
2791+
/* If newstr is NULL something went wrong in mbfl and this is a bug */
2792+
ZEND_ASSERT(newstr != NULL);
27922793

2793-
if (newstr) {
2794-
// TODO: avoid reallocation ???
2795-
RETVAL_STRINGL(newstr, ret_len);
2796-
efree(newstr);
2797-
}
2794+
// TODO: avoid reallocation ???
2795+
RETVAL_STRINGL(newstr, ret_len);
2796+
efree(newstr);
27982797
}
27992798
/* }}} */
28002799

2801-
/* {{{ proto string mb_strtoupper(string sourcestring [, string encoding])
2802-
* Returns a uppercased version of sourcestring
2800+
/* {{{ proto string mb_strtoupper(string source_string [, string encoding])
2801+
* Returns a upper cased version of source_string
28032802
*/
28042803
PHP_FUNCTION(mb_strtoupper)
28052804
{
@@ -2810,8 +2809,7 @@ PHP_FUNCTION(mb_strtoupper)
28102809
size_t ret_len;
28112810
const mbfl_encoding *enc;
28122811

2813-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len,
2814-
&from_encoding) == FAILURE) {
2812+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len, &from_encoding) == FAILURE) {
28152813
RETURN_THROWS();
28162814
}
28172815

@@ -2821,19 +2819,17 @@ PHP_FUNCTION(mb_strtoupper)
28212819
}
28222820

28232821
newstr = mbstring_convert_case(PHP_UNICODE_CASE_UPPER, str, str_len, &ret_len, enc);
2822+
/* If newstr is NULL something went wrong in mbfl and this is a bug */
2823+
ZEND_ASSERT(newstr != NULL);
28242824

2825-
if (newstr) {
2826-
// TODO: avoid reallocation ???
2827-
RETVAL_STRINGL(newstr, ret_len);
2828-
efree(newstr);
2829-
return;
2830-
}
2831-
RETURN_FALSE;
2825+
// TODO: avoid reallocation ???
2826+
RETVAL_STRINGL(newstr, ret_len);
2827+
efree(newstr);
28322828
}
28332829
/* }}} */
28342830

2835-
/* {{{ proto string mb_strtolower(string sourcestring [, string encoding])
2836-
* Returns a lowercased version of sourcestring
2831+
/* {{{ proto string mb_strtolower(string source_string [, string encoding])
2832+
* Returns a lower cased version of source_string
28372833
*/
28382834
PHP_FUNCTION(mb_strtolower)
28392835
{
@@ -2844,8 +2840,7 @@ PHP_FUNCTION(mb_strtolower)
28442840
size_t ret_len;
28452841
const mbfl_encoding *enc;
28462842

2847-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len,
2848-
&from_encoding) == FAILURE) {
2843+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|S!", &str, &str_len, &from_encoding) == FAILURE) {
28492844
RETURN_THROWS();
28502845
}
28512846

@@ -2855,14 +2850,12 @@ PHP_FUNCTION(mb_strtolower)
28552850
}
28562851

28572852
newstr = mbstring_convert_case(PHP_UNICODE_CASE_LOWER, str, str_len, &ret_len, enc);
2853+
/* If newstr is NULL something went wrong in mbfl and this is a bug */
2854+
ZEND_ASSERT(newstr != NULL);
28582855

2859-
if (newstr) {
2860-
// TODO: avoid reallocation ???
2861-
RETVAL_STRINGL(newstr, ret_len);
2862-
efree(newstr);
2863-
return;
2864-
}
2865-
RETURN_FALSE;
2856+
// TODO: avoid reallocation ???
2857+
RETVAL_STRINGL(newstr, ret_len);
2858+
efree(newstr);
28662859
}
28672860
/* }}} */
28682861

‎ext/mbstring/mbstring.stub.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ function mb_strimwidth(string $str, int $start, int $width, string $trimmarker =
5151

5252
function mb_convert_encoding(array|string $str, string $to, array|string $from = UNKNOWN): array|string|false {}
5353

54-
function mb_convert_case(string $sourcestring, int $mode, ?string $encoding = null): string|false {}
54+
function mb_convert_case(string $source_string, int $mode, ?string $encoding = null): string {}
5555

56-
function mb_strtoupper(string $sourcestring, ?string $encoding = null): string|false {}
56+
function mb_strtoupper(string $source_string, ?string $encoding = null): string {}
5757

58-
function mb_strtolower(string $sourcestring, ?string $encoding = null): string|false {}
58+
function mb_strtolower(string $source_string, ?string $encoding = null): string {}
5959

6060
function mb_detect_encoding(string $str, array|string|null $encoding_list = null, bool $strict = false): string|false {}
6161

‎ext/mbstring/mbstring_arginfo.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,14 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_convert_encoding, 0, 2, MAY_B
112112
ZEND_ARG_TYPE_MASK(0, from, MAY_BE_ARRAY|MAY_BE_STRING)
113113
ZEND_END_ARG_INFO()
114114

115-
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_convert_case, 0, 2, MAY_BE_STRING|MAY_BE_FALSE)
116-
ZEND_ARG_TYPE_INFO(0, sourcestring, IS_STRING, 0)
115+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_convert_case, 0, 2, IS_STRING, 0)
116+
ZEND_ARG_TYPE_INFO(0, source_string, IS_STRING, 0)
117117
ZEND_ARG_TYPE_INFO(0, mode, IS_LONG, 0)
118118
ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 1)
119119
ZEND_END_ARG_INFO()
120120

121-
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_strtoupper, 0, 1, MAY_BE_STRING|MAY_BE_FALSE)
122-
ZEND_ARG_TYPE_INFO(0, sourcestring, IS_STRING, 0)
121+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_strtoupper, 0, 1, IS_STRING, 0)
122+
ZEND_ARG_TYPE_INFO(0, source_string, IS_STRING, 0)
123123
ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 1)
124124
ZEND_END_ARG_INFO()
125125

‎ext/mbstring/tests/mb_convert_case_invalid_mode.phpt

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎ext/mbstring/tests/mb_convert_case_various_mode.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
Calling mb_convert_case() with an invalid casing mode
3+
--SKIPIF--
4+
<?php require 'skipif.inc'; ?>
5+
--FILE--
6+
<?php
7+
8+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_UPPER));
9+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_LOWER));
10+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_TITLE));
11+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_FOLD));
12+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_UPPER_SIMPLE));
13+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_LOWER_SIMPLE));
14+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_TITLE_SIMPLE));
15+
var_dump(mb_convert_case('foo BAR Spaß', MB_CASE_FOLD_SIMPLE));
16+
17+
// Invalid mode
18+
try {
19+
var_dump(mb_convert_case('foo BAR Spaß', 100));
20+
} catch (\ValueError $e) {
21+
echo $e->getMessage() . \PHP_EOL;
22+
}
23+
24+
?>
25+
--EXPECT--
26+
string(13) "FOO BAR SPASS"
27+
string(13) "foo bar spaß"
28+
string(13) "Foo Bar Spaß"
29+
string(13) "foo bar spass"
30+
string(13) "FOO BAR SPAß"
31+
string(13) "foo bar spaß"
32+
string(13) "Foo Bar Spaß"
33+
string(13) "foo bar spaß"
34+
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

0 commit comments

Comments
 (0)