Skip to content

Commit c9a14a3

Browse files
GirgiasGirgias
committed
Promote some warnings in MBString Regex
1 parent 319c21b commit c9a14a3

File tree

6 files changed

+123
-142
lines changed

6 files changed

+123
-142
lines changed

‎ext/mbstring/php_mbregex.c

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ static php_mb_regex_t *php_mbregex_compile_pattern(const char *pattern, size_t p
465465
OnigEncoding enc = MBREX(current_mbctype);
466466

467467
if (!php_mb_check_encoding(pattern, patlen, php_mb_regex_get_mbctype_encoding())) {
468+
// TODO convert to ValueError after adding pattern_arg_num pass through argument
468469
php_error_docref(NULL, E_WARNING,
469470
"Pattern is not valid under %s encoding", _php_mb_regex_mbctype2name(enc));
470471
return NULL;
@@ -474,6 +475,7 @@ static php_mb_regex_t *php_mbregex_compile_pattern(const char *pattern, size_t p
474475
if (!rc || onig_get_options(rc) != options || onig_get_encoding(rc) != enc || onig_get_syntax(rc) != syntax) {
475476
if ((err_code = onig_new(&retval, (OnigUChar *)pattern, (OnigUChar *)(pattern + patlen), options, enc, syntax, &err_info)) != ONIG_NORMAL) {
476477
onig_error_code_to_str(err_str, err_code, &err_info);
478+
// Convert to error?
477479
php_error_docref(NULL, E_WARNING, "mbregex compile err: %s", err_str);
478480
return NULL;
479481
}
@@ -897,15 +899,21 @@ static int _php_mb_onig_search(regex_t* reg, const OnigUChar* str, const OnigUCh
897899
static void _php_mb_regex_ereg_exec(INTERNAL_FUNCTION_PARAMETERS, int icase)
898900
{
899901
zval *array = NULL;
900-
char *arg_pattern, *string;
901-
size_t arg_pattern_len, string_len;
902+
char *string;
903+
size_t string_len;
904+
zend_string *pattern;
902905
php_mb_regex_t *re;
903906
OnigRegion *regs = NULL;
904907
int i, match_len, beg, end;
905908
OnigOptionType options;
906909
char *str;
907910

908-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "ss|z", &arg_pattern, &arg_pattern_len, &string, &string_len, &array) == FAILURE) {
911+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ss|z", &pattern, &string, &string_len, &array) == FAILURE) {
912+
RETURN_THROWS();
913+
}
914+
915+
if (ZSTR_LEN(pattern) == 0) {
916+
zend_argument_value_error(1, "must not be empty");
909917
RETURN_THROWS();
910918
}
911919

@@ -929,13 +937,7 @@ static void _php_mb_regex_ereg_exec(INTERNAL_FUNCTION_PARAMETERS, int icase)
929937
options |= ONIG_OPTION_IGNORECASE;
930938
}
931939

932-
if (arg_pattern_len == 0) {
933-
php_error_docref(NULL, E_WARNING, "Empty pattern");
934-
RETVAL_FALSE;
935-
goto out;
936-
}
937-
938-
re = php_mbregex_compile_pattern(arg_pattern, arg_pattern_len, options, MBREX(regex_default_syntax));
940+
re = php_mbregex_compile_pattern(ZSTR_VAL(pattern), ZSTR_LEN(pattern), options, MBREX(regex_default_syntax));
939941
if (re == NULL) {
940942
RETVAL_FALSE;
941943
goto out;
@@ -1025,15 +1027,9 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
10251027

10261028
const mbfl_encoding *enc;
10271029

1028-
{
1029-
const char *current_enc_name;
1030-
current_enc_name = php_mb_regex_get_mbctype();
1031-
if (current_enc_name == NULL ||
1032-
(enc = mbfl_name2encoding(current_enc_name)) == NULL) {
1033-
php_error_docref(NULL, E_WARNING, "Unknown error");
1034-
RETURN_FALSE;
1035-
}
1036-
}
1030+
enc = mbfl_name2encoding(php_mb_regex_get_mbctype());
1031+
ZEND_ASSERT(enc != NULL);
1032+
10371033
eval = 0;
10381034
{
10391035
char *option_str = NULL;
@@ -1073,8 +1069,12 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
10731069
}
10741070
}
10751071
if (eval && !is_callable) {
1076-
php_error_docref(NULL, E_WARNING, "The 'e' option is no longer supported, use mb_ereg_replace_callback instead");
1077-
RETURN_FALSE;
1072+
zend_argument_value_error(4, "option 'e' is not supported, use mb_ereg_replace_callback() instead");
1073+
RETURN_THROWS();
1074+
}
1075+
if (eval && is_callable) {
1076+
zend_argument_value_error(4, "option 'e' cannot be used with replacement callback");
1077+
RETURN_THROWS();
10781078
}
10791079

10801080
/* create regex pattern buffer */
@@ -1091,13 +1091,6 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
10911091
description = NULL;
10921092
}
10931093

1094-
if (is_callable) {
1095-
if (eval) {
1096-
php_error_docref(NULL, E_WARNING, "Option 'e' cannot be used with replacement callback");
1097-
RETURN_FALSE;
1098-
}
1099-
}
1100-
11011094
/* do the actual work */
11021095
err = 0;
11031096
pos = (OnigUChar *)string;
@@ -1176,6 +1169,7 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
11761169
zval_ptr_dtor(&retval);
11771170
} else {
11781171
if (!EG(exception)) {
1172+
// TODO Is this a return throw?
11791173
php_error_docref(NULL, E_WARNING, "Unable to call custom replacement function");
11801174
}
11811175
}
@@ -1435,11 +1429,13 @@ _php_mb_regex_ereg_search_exec(INTERNAL_FUNCTION_PARAMETERS, int mode)
14351429
}
14361430

14371431
if (MBREX(search_re) == NULL) {
1432+
// TODO which Error/Exception to use, as this seems to be a logical error in function call ordering
14381433
php_error_docref(NULL, E_WARNING, "No regex given");
14391434
RETURN_FALSE;
14401435
}
14411436

14421437
if (str == NULL) {
1438+
// TODO which Error/Exception to use, as this seems to be a logical error in function call ordering
14431439
php_error_docref(NULL, E_WARNING, "No string given");
14441440
RETURN_FALSE;
14451441
}
@@ -1534,19 +1530,19 @@ PHP_FUNCTION(mb_ereg_search_regs)
15341530
PHP_FUNCTION(mb_ereg_search_init)
15351531
{
15361532
int argc = ZEND_NUM_ARGS();
1537-
zend_string *arg_str;
1538-
char *arg_pattern = NULL, *arg_options = NULL;
1539-
size_t arg_pattern_len = 0, arg_options_len = 0;
1533+
zend_string *arg_str, *pattern = NULL;
1534+
char *arg_options = NULL;
1535+
size_t arg_options_len = 0;
15401536
OnigSyntaxType *syntax = NULL;
15411537
OnigOptionType option;
15421538

1543-
if (zend_parse_parameters(argc, "S|ss", &arg_str, &arg_pattern, &arg_pattern_len, &arg_options, &arg_options_len) == FAILURE) {
1539+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|Ss", &arg_str, &pattern, &arg_options, &arg_options_len) == FAILURE) {
15441540
RETURN_THROWS();
15451541
}
15461542

1547-
if (argc > 1 && arg_pattern_len == 0) {
1548-
php_error_docref(NULL, E_WARNING, "Empty pattern");
1549-
RETURN_FALSE;
1543+
if (argc > 1 && ZSTR_LEN(pattern) == 0) {
1544+
zend_argument_value_error(2, "must not be empty");
1545+
RETURN_THROWS();
15501546
}
15511547

15521548
option = MBREX(regex_default_options);
@@ -1559,7 +1555,7 @@ PHP_FUNCTION(mb_ereg_search_init)
15591555

15601556
if (argc > 1) {
15611557
/* create regex pattern buffer */
1562-
if ((MBREX(search_re) = php_mbregex_compile_pattern(arg_pattern, arg_pattern_len, option, syntax)) == NULL) {
1558+
if ((MBREX(search_re) = php_mbregex_compile_pattern(ZSTR_VAL(pattern), ZSTR_LEN(pattern), option, syntax)) == NULL) {
15631559
RETURN_FALSE;
15641560
}
15651561
}
@@ -1660,12 +1656,12 @@ PHP_FUNCTION(mb_ereg_search_setpos)
16601656
}
16611657

16621658
if (position < 0 || (!Z_ISUNDEF(MBREX(search_str)) && Z_TYPE(MBREX(search_str)) == IS_STRING && (size_t)position > Z_STRLEN(MBREX(search_str)))) {
1663-
php_error_docref(NULL, E_WARNING, "Position is out of range");
1664-
MBREX(search_pos) = 0;
1665-
RETURN_FALSE;
1659+
zend_argument_value_error(1, "is out of range");
1660+
RETURN_THROWS();
16661661
}
16671662

16681663
MBREX(search_pos) = position;
1664+
// TODO Return void
16691665
RETURN_TRUE;
16701666
}
16711667
/* }}} */

‎ext/mbstring/tests/bug43994.phpt

Lines changed: 37 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -30,106 +30,76 @@ foreach($inputs as $input) {
3030
}
3131
echo "\n-- Iteration $iterator --\n";
3232
echo "Without \$regs arg:\n";
33-
var_dump( mb_ereg($input, 'hello, world') );
33+
try {
34+
var_dump( mb_ereg($input, 'hello, world') );
35+
} catch (\ValueError $e) {
36+
echo $e->getMessage() . \PHP_EOL;
37+
}
38+
3439
echo "With \$regs arg:\n";
35-
var_dump(mb_ereg($input, 'hello, world', $mb_regs));
40+
try {
41+
var_dump(mb_ereg($input, 'hello, world', $mb_regs));
42+
} catch (\ValueError $e) {
43+
echo $e->getMessage() . \PHP_EOL;
44+
}
45+
3646
var_dump($mb_regs);
3747
$iterator++;
3848
};
3949
?>
40-
--EXPECTF--
50+
--EXPECT--
4151
-- Iteration 1 --
4252
Without $regs arg:
43-
44-
Warning: mb_ereg(): Empty pattern in %s on line %d
45-
bool(false)
53+
mb_ereg(): Argument #1 ($pattern) must not be empty
4654
With $regs arg:
47-
48-
Warning: mb_ereg(): Empty pattern in %s on line %d
49-
bool(false)
50-
array(0) {
51-
}
55+
mb_ereg(): Argument #1 ($pattern) must not be empty
56+
NULL
5257

5358
-- Iteration 2 --
5459
Without $regs arg:
55-
56-
Warning: mb_ereg(): Empty pattern in %s on line %d
57-
bool(false)
60+
mb_ereg(): Argument #1 ($pattern) must not be empty
5861
With $regs arg:
59-
60-
Warning: mb_ereg(): Empty pattern in %s on line %d
61-
bool(false)
62-
array(0) {
63-
}
62+
mb_ereg(): Argument #1 ($pattern) must not be empty
63+
NULL
6464

6565
-- Iteration 3 --
6666
Without $regs arg:
67-
68-
Warning: mb_ereg(): Empty pattern in %s on line %d
69-
bool(false)
67+
mb_ereg(): Argument #1 ($pattern) must not be empty
7068
With $regs arg:
71-
72-
Warning: mb_ereg(): Empty pattern in %s on line %d
73-
bool(false)
74-
array(0) {
75-
}
69+
mb_ereg(): Argument #1 ($pattern) must not be empty
70+
NULL
7671

7772
-- Iteration 4 --
7873
Without $regs arg:
79-
80-
Warning: mb_ereg(): Empty pattern in %s on line %d
81-
bool(false)
74+
mb_ereg(): Argument #1 ($pattern) must not be empty
8275
With $regs arg:
83-
84-
Warning: mb_ereg(): Empty pattern in %s on line %d
85-
bool(false)
86-
array(0) {
87-
}
76+
mb_ereg(): Argument #1 ($pattern) must not be empty
77+
NULL
8878

8979
-- Iteration 5 --
9080
Without $regs arg:
91-
92-
Warning: mb_ereg(): Empty pattern in %s on line %d
93-
bool(false)
81+
mb_ereg(): Argument #1 ($pattern) must not be empty
9482
With $regs arg:
95-
96-
Warning: mb_ereg(): Empty pattern in %s on line %d
97-
bool(false)
98-
array(0) {
99-
}
83+
mb_ereg(): Argument #1 ($pattern) must not be empty
84+
NULL
10085

10186
-- Iteration 6 --
10287
Without $regs arg:
103-
104-
Warning: mb_ereg(): Empty pattern in %s on line %d
105-
bool(false)
88+
mb_ereg(): Argument #1 ($pattern) must not be empty
10689
With $regs arg:
107-
108-
Warning: mb_ereg(): Empty pattern in %s on line %d
109-
bool(false)
110-
array(0) {
111-
}
90+
mb_ereg(): Argument #1 ($pattern) must not be empty
91+
NULL
11292

11393
-- Iteration 7 --
11494
Without $regs arg:
115-
116-
Warning: mb_ereg(): Empty pattern in %s on line %d
117-
bool(false)
95+
mb_ereg(): Argument #1 ($pattern) must not be empty
11896
With $regs arg:
119-
120-
Warning: mb_ereg(): Empty pattern in %s on line %d
121-
bool(false)
122-
array(0) {
123-
}
97+
mb_ereg(): Argument #1 ($pattern) must not be empty
98+
NULL
12499

125100
-- Iteration 8 --
126101
Without $regs arg:
127-
128-
Warning: mb_ereg(): Empty pattern in %s on line %d
129-
bool(false)
102+
mb_ereg(): Argument #1 ($pattern) must not be empty
130103
With $regs arg:
131-
132-
Warning: mb_ereg(): Empty pattern in %s on line %d
133-
bool(false)
134-
array(0) {
135-
}
104+
mb_ereg(): Argument #1 ($pattern) must not be empty
105+
NULL

‎ext/mbstring/tests/bug72164.phpt

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@ if (!function_exists('mb_ereg')) die('skip mbregex support not available');
1010
$var0 = "e";
1111
$var2 = "";
1212
$var3 = NULL;
13-
$var8 = mb_ereg_replace($var2,$var3,$var3,$var0);
14-
var_dump($var8);
13+
try {
14+
$var8 = mb_ereg_replace($var2,$var3,$var3,$var0);
15+
var_dump($var8);
16+
} catch (\ValueError $e) {
17+
echo $e->getMessage() . \PHP_EOL;
18+
}
19+
1520
?>
16-
--EXPECTF--
17-
Warning: mb_ereg_replace(): The 'e' option is no longer supported, use mb_ereg_replace_callback instead in %s on line %d
18-
bool(false)
21+
--EXPECT--
22+
mb_ereg_replace(): Argument #4 ($option) option 'e' is not supported, use mb_ereg_replace_callback() instead

‎ext/mbstring/tests/empty_pattern.phpt

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,22 @@ if (!function_exists('mb_ereg')) die('skip mbregex support not available');
88
--FILE--
99
<?php
1010

11-
mb_ereg_search_init("","","");
11+
try {
12+
mb_ereg_search_init("","","");
13+
} catch (\ValueError $e) {
14+
echo $e->getMessage() . \PHP_EOL;
15+
}
16+
1217
mb_split("","");
13-
mb_ereg_search_regs();
18+
19+
try {
20+
mb_ereg_search_regs();
21+
} catch (\ValueError $e) {
22+
echo $e->getMessage() . \PHP_EOL;
23+
}
1424

1525
?>
1626
--EXPECTF--
17-
Warning: mb_ereg_search_init(): Empty pattern in %s on line %d
27+
mb_ereg_search_init(): Argument #2 ($pattern) must not be empty
1828

1929
Warning: mb_ereg_search_regs(): No regex given in %s on line %d

0 commit comments

Comments
 (0)