Skip to content

Commit 0444158

Browse files
GirgiasGirgias
committed
Promote some warnings in MBString Regexes
Closes GH-5341
1 parent 8f415d4 commit 0444158

10 files changed

+154
-155
lines changed

‎ext/mbstring/php_mbregex.c

+37-37
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,8 @@ static size_t _php_mb_regex_get_option_string(char *str, size_t len, OnigOptionT
591591
/* }}} */
592592

593593
/* {{{ _php_mb_regex_init_options */
594-
static void
595-
_php_mb_regex_init_options(const char *parg, size_t narg, OnigOptionType *option, OnigSyntaxType **syntax, int *eval)
594+
static bool _php_mb_regex_init_options(const char *parg, size_t narg, OnigOptionType *option,
595+
OnigSyntaxType **syntax)
596596
{
597597
size_t n;
598598
char c;
@@ -650,15 +650,14 @@ _php_mb_regex_init_options(const char *parg, size_t narg, OnigOptionType *option
650650
case 'd':
651651
*syntax = ONIG_SYNTAX_POSIX_EXTENDED;
652652
break;
653-
case 'e':
654-
if (eval != NULL) *eval = 1;
655-
break;
656653
default:
657-
break;
654+
zend_value_error("Option \"%c\" is not supported", c);
655+
return false;
658656
}
659657
}
660658
if (option != NULL) *option|=optm;
661659
}
660+
return true;
662661
}
663662
/* }}} */
664663

@@ -900,6 +899,11 @@ static void _php_mb_regex_ereg_exec(INTERNAL_FUNCTION_PARAMETERS, int icase)
900899
RETURN_THROWS();
901900
}
902901

902+
if (arg_pattern_len == 0) {
903+
zend_argument_value_error(1, "must not be empty");
904+
RETURN_THROWS();
905+
}
906+
903907
if (array != NULL) {
904908
array = zend_try_array_init(array);
905909
if (!array) {
@@ -920,12 +924,6 @@ static void _php_mb_regex_ereg_exec(INTERNAL_FUNCTION_PARAMETERS, int icase)
920924
options |= ONIG_OPTION_IGNORECASE;
921925
}
922926

923-
if (arg_pattern_len == 0) {
924-
php_error_docref(NULL, E_WARNING, "Empty pattern");
925-
RETVAL_FALSE;
926-
goto out;
927-
}
928-
929927
re = php_mbregex_compile_pattern(arg_pattern, arg_pattern_len, options, MBREX(regex_default_syntax));
930928
if (re == NULL) {
931929
RETVAL_FALSE;
@@ -1007,15 +1005,14 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
10071005
smart_str out_buf = {0};
10081006
smart_str eval_buf = {0};
10091007
smart_str *pbuf;
1010-
int err, eval, n;
1008+
int err, n;
10111009
OnigUChar *pos;
10121010
OnigUChar *string_lim;
10131011
char *description = NULL;
10141012

10151013
const mbfl_encoding *enc = php_mb_regex_get_mbctype_encoding();
10161014
ZEND_ASSERT(enc != NULL);
10171015

1018-
eval = 0;
10191016
{
10201017
char *option_str = NULL;
10211018
size_t option_str_len = 0;
@@ -1043,20 +1040,15 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
10431040
}
10441041

10451042
if (option_str != NULL) {
1046-
_php_mb_regex_init_options(option_str, option_str_len, &options, &syntax, &eval);
1043+
/* Initialize option and in case of failure it means there is a value error */
1044+
if (!_php_mb_regex_init_options(option_str, option_str_len, &options, &syntax)) {
1045+
RETURN_THROWS();
1046+
}
10471047
} else {
10481048
options |= MBREX(regex_default_options);
10491049
syntax = MBREX(regex_default_syntax);
10501050
}
10511051
}
1052-
if (eval) {
1053-
if (is_callable) {
1054-
php_error_docref(NULL, E_WARNING, "Option 'e' cannot be used with replacement callback");
1055-
} else {
1056-
php_error_docref(NULL, E_WARNING, "The 'e' option is no longer supported, use mb_ereg_replace_callback instead");
1057-
}
1058-
RETURN_FALSE;
1059-
}
10601052

10611053
/* create regex pattern buffer */
10621054
re = php_mbregex_compile_pattern(arg_pattern, arg_pattern_len, options, syntax);
@@ -1122,7 +1114,9 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
11221114
zval_ptr_dtor(&retval);
11231115
} else {
11241116
if (!EG(exception)) {
1125-
php_error_docref(NULL, E_WARNING, "Unable to call custom replacement function");
1117+
zend_throw_error(NULL, "Unable to call custom replacement function");
1118+
zval_ptr_dtor(&subpats);
1119+
RETURN_THROWS();
11261120
}
11271121
}
11281122
zval_ptr_dtor(&subpats);
@@ -1251,6 +1245,7 @@ PHP_FUNCTION(mb_split)
12511245
onig_region_free(regs, 1);
12521246

12531247
/* see if we encountered an error */
1248+
// ToDo investigate if this can actually/should happen ...
12541249
if (err <= -2) {
12551250
OnigUChar err_str[ONIG_MAX_ERROR_MESSAGE_LEN];
12561251
onig_error_code_to_str(err_str, err);
@@ -1295,7 +1290,9 @@ PHP_FUNCTION(mb_ereg_match)
12951290
}
12961291

12971292
if (option_str != NULL) {
1298-
_php_mb_regex_init_options(option_str, option_str_len, &option, &syntax, NULL);
1293+
if(!_php_mb_regex_init_options(option_str, option_str_len, &option, &syntax)) {
1294+
RETURN_THROWS();
1295+
}
12991296
} else {
13001297
option |= MBREX(regex_default_options);
13011298
syntax = MBREX(regex_default_syntax);
@@ -1348,7 +1345,7 @@ static void _php_mb_regex_ereg_search_exec(INTERNAL_FUNCTION_PARAMETERS, int mod
13481345
}
13491346

13501347
if (arg_options) {
1351-
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax, NULL);
1348+
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax);
13521349
} else {
13531350
option |= MBREX(regex_default_options);
13541351
syntax = MBREX(regex_default_syntax);
@@ -1375,13 +1372,13 @@ static void _php_mb_regex_ereg_search_exec(INTERNAL_FUNCTION_PARAMETERS, int mod
13751372
}
13761373

13771374
if (MBREX(search_re) == NULL) {
1378-
php_error_docref(NULL, E_WARNING, "No regex given");
1379-
RETURN_FALSE;
1375+
zend_throw_error(NULL, "No pattern was provided");
1376+
RETURN_THROWS();
13801377
}
13811378

13821379
if (str == NULL) {
1383-
php_error_docref(NULL, E_WARNING, "No string given");
1384-
RETURN_FALSE;
1380+
zend_throw_error(NULL, "No string was provided");
1381+
RETURN_THROWS();
13851382
}
13861383

13871384
MBREX(search_regs) = onig_region_new();
@@ -1480,13 +1477,13 @@ PHP_FUNCTION(mb_ereg_search_init)
14801477
}
14811478

14821479
if (arg_pattern && arg_pattern_len == 0) {
1483-
php_error_docref(NULL, E_WARNING, "Empty pattern");
1484-
RETURN_FALSE;
1480+
zend_argument_value_error(2, "must not be empty");
1481+
RETURN_THROWS();
14851482
}
14861483

14871484
if (arg_options) {
14881485
option = 0;
1489-
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax, NULL);
1486+
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax);
14901487
} else {
14911488
option = MBREX(regex_default_options);
14921489
syntax = MBREX(regex_default_syntax);
@@ -1557,6 +1554,7 @@ PHP_FUNCTION(mb_ereg_search_getregs)
15571554
onig_foreach_name(MBREX(search_re), mb_regex_groups_iter, &args);
15581555
}
15591556
} else {
1557+
// TODO This seems to be some logical error, promote to Error
15601558
RETVAL_FALSE;
15611559
}
15621560
}
@@ -1588,12 +1586,12 @@ PHP_FUNCTION(mb_ereg_search_setpos)
15881586
}
15891587

15901588
if (position < 0 || (!Z_ISUNDEF(MBREX(search_str)) && Z_TYPE(MBREX(search_str)) == IS_STRING && (size_t)position > Z_STRLEN(MBREX(search_str)))) {
1591-
php_error_docref(NULL, E_WARNING, "Position is out of range");
1592-
MBREX(search_pos) = 0;
1593-
RETURN_FALSE;
1589+
zend_argument_value_error(1, "is out of range");
1590+
RETURN_THROWS();
15941591
}
15951592

15961593
MBREX(search_pos) = position;
1594+
// TODO Return void
15971595
RETURN_TRUE;
15981596
}
15991597
/* }}} */
@@ -1628,7 +1626,9 @@ PHP_FUNCTION(mb_regex_set_options)
16281626
if (string != NULL) {
16291627
opt = 0;
16301628
syntax = NULL;
1631-
_php_mb_regex_init_options(string, string_len, &opt, &syntax, NULL);
1629+
if(!_php_mb_regex_init_options(string, string_len, &opt, &syntax)) {
1630+
RETURN_THROWS();
1631+
}
16321632
_php_mb_regex_set_options(opt, syntax, &prev_opt, &prev_syntax);
16331633
opt = prev_opt;
16341634
syntax = prev_syntax;

‎ext/mbstring/tests/bug43994.phpt

+37-67
Original file line numberDiff line numberDiff line change
@@ -25,106 +25,76 @@ foreach($inputs as $input) {
2525
}
2626
echo "\n-- Iteration $iterator --\n";
2727
echo "Without \$regs arg:\n";
28-
var_dump( mb_ereg($input, 'hello, world') );
28+
try {
29+
var_dump( mb_ereg($input, 'hello, world') );
30+
} catch (\ValueError $e) {
31+
echo $e->getMessage() . \PHP_EOL;
32+
}
33+
2934
echo "With \$regs arg:\n";
30-
var_dump(mb_ereg($input, 'hello, world', $mb_regs));
35+
try {
36+
var_dump(mb_ereg($input, 'hello, world', $mb_regs));
37+
} catch (\ValueError $e) {
38+
echo $e->getMessage() . \PHP_EOL;
39+
}
40+
3141
var_dump($mb_regs);
3242
$iterator++;
3343
};
3444
?>
35-
--EXPECTF--
45+
--EXPECT--
3646
-- Iteration 1 --
3747
Without $regs arg:
38-
39-
Warning: mb_ereg(): Empty pattern in %s on line %d
40-
bool(false)
48+
mb_ereg(): Argument #1 ($pattern) must not be empty
4149
With $regs arg:
42-
43-
Warning: mb_ereg(): Empty pattern in %s on line %d
44-
bool(false)
45-
array(0) {
46-
}
50+
mb_ereg(): Argument #1 ($pattern) must not be empty
51+
NULL
4752

4853
-- Iteration 2 --
4954
Without $regs arg:
50-
51-
Warning: mb_ereg(): Empty pattern in %s on line %d
52-
bool(false)
55+
mb_ereg(): Argument #1 ($pattern) must not be empty
5356
With $regs arg:
54-
55-
Warning: mb_ereg(): Empty pattern in %s on line %d
56-
bool(false)
57-
array(0) {
58-
}
57+
mb_ereg(): Argument #1 ($pattern) must not be empty
58+
NULL
5959

6060
-- Iteration 3 --
6161
Without $regs arg:
62-
63-
Warning: mb_ereg(): Empty pattern in %s on line %d
64-
bool(false)
62+
mb_ereg(): Argument #1 ($pattern) must not be empty
6563
With $regs arg:
66-
67-
Warning: mb_ereg(): Empty pattern in %s on line %d
68-
bool(false)
69-
array(0) {
70-
}
64+
mb_ereg(): Argument #1 ($pattern) must not be empty
65+
NULL
7166

7267
-- Iteration 4 --
7368
Without $regs arg:
74-
75-
Warning: mb_ereg(): Empty pattern in %s on line %d
76-
bool(false)
69+
mb_ereg(): Argument #1 ($pattern) must not be empty
7770
With $regs arg:
78-
79-
Warning: mb_ereg(): Empty pattern in %s on line %d
80-
bool(false)
81-
array(0) {
82-
}
71+
mb_ereg(): Argument #1 ($pattern) must not be empty
72+
NULL
8373

8474
-- Iteration 5 --
8575
Without $regs arg:
86-
87-
Warning: mb_ereg(): Empty pattern in %s on line %d
88-
bool(false)
76+
mb_ereg(): Argument #1 ($pattern) must not be empty
8977
With $regs arg:
90-
91-
Warning: mb_ereg(): Empty pattern in %s on line %d
92-
bool(false)
93-
array(0) {
94-
}
78+
mb_ereg(): Argument #1 ($pattern) must not be empty
79+
NULL
9580

9681
-- Iteration 6 --
9782
Without $regs arg:
98-
99-
Warning: mb_ereg(): Empty pattern in %s on line %d
100-
bool(false)
83+
mb_ereg(): Argument #1 ($pattern) must not be empty
10184
With $regs arg:
102-
103-
Warning: mb_ereg(): Empty pattern in %s on line %d
104-
bool(false)
105-
array(0) {
106-
}
85+
mb_ereg(): Argument #1 ($pattern) must not be empty
86+
NULL
10787

10888
-- Iteration 7 --
10989
Without $regs arg:
110-
111-
Warning: mb_ereg(): Empty pattern in %s on line %d
112-
bool(false)
90+
mb_ereg(): Argument #1 ($pattern) must not be empty
11391
With $regs arg:
114-
115-
Warning: mb_ereg(): Empty pattern in %s on line %d
116-
bool(false)
117-
array(0) {
118-
}
92+
mb_ereg(): Argument #1 ($pattern) must not be empty
93+
NULL
11994

12095
-- Iteration 8 --
12196
Without $regs arg:
122-
123-
Warning: mb_ereg(): Empty pattern in %s on line %d
124-
bool(false)
97+
mb_ereg(): Argument #1 ($pattern) must not be empty
12598
With $regs arg:
126-
127-
Warning: mb_ereg(): Empty pattern in %s on line %d
128-
bool(false)
129-
array(0) {
130-
}
99+
mb_ereg(): Argument #1 ($pattern) must not be empty
100+
NULL

‎ext/mbstring/tests/bug69151.phpt

+8-7
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ if (!function_exists('mb_ereg')) die('skip mbregex support not available');
88
--FILE--
99
<?php
1010
$str = "\x80";
11-
var_dump(
12-
false === mb_eregi('.', $str, $matches),
13-
[] === $matches,
14-
NULL === mb_ereg_replace('.', "\\0", $str),
15-
false === mb_ereg_search_init("\x80", '.'),
16-
false === mb_ereg_search()
17-
);
11+
12+
var_dump(false === mb_eregi('.', $str, $matches));
13+
var_dump([] === $matches);
14+
15+
var_dump(NULL === mb_ereg_replace('.', "\\0", $str));
16+
17+
var_dump(false === mb_ereg_search_init("\x80", '.'));
18+
var_dump(false === mb_ereg_search());
1819
?>
1920
--EXPECT--
2021
bool(true)

‎ext/mbstring/tests/bug72164.phpt

+9-5
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+
Option "e" is not supported

0 commit comments

Comments
 (0)