Skip to content

Commit

Permalink
Promote some warnings in MBString Regexes
Browse files Browse the repository at this point in the history 
Closes GH-5341
  • Loading branch information
@Girgias
Girgias committed Sep 9, 2020
1 parent 8f415d4 commit 0444158
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 155 deletions.
74 changes: 37 additions & 37 deletions ext/mbstring/php_mbregex.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,8 @@ static size_t _php_mb_regex_get_option_string(char *str, size_t len, OnigOptionT
/* }}} */

/* {{{ _php_mb_regex_init_options */
static void
_php_mb_regex_init_options(const char *parg, size_t narg, OnigOptionType *option, OnigSyntaxType **syntax, int *eval)
static bool _php_mb_regex_init_options(const char *parg, size_t narg, OnigOptionType *option,
OnigSyntaxType **syntax)
{
size_t n;
char c;
Expand Down Expand Up @@ -650,15 +650,14 @@ _php_mb_regex_init_options(const char *parg, size_t narg, OnigOptionType *option
case 'd':
*syntax = ONIG_SYNTAX_POSIX_EXTENDED;
break;
case 'e':
if (eval != NULL) *eval = 1;
break;
default:
break;
zend_value_error("Option \"%c\" is not supported", c);
return false;
}
}
if (option != NULL) *option|=optm;
}
return true;
}
/* }}} */

Expand Down Expand Up @@ -900,6 +899,11 @@ static void _php_mb_regex_ereg_exec(INTERNAL_FUNCTION_PARAMETERS, int icase)
RETURN_THROWS();
}

if (arg_pattern_len == 0) {
zend_argument_value_error(1, "must not be empty");
RETURN_THROWS();
}

if (array != NULL) {
array = zend_try_array_init(array);
if (!array) {
Expand All @@ -920,12 +924,6 @@ static void _php_mb_regex_ereg_exec(INTERNAL_FUNCTION_PARAMETERS, int icase)
options |= ONIG_OPTION_IGNORECASE;
}

if (arg_pattern_len == 0) {
php_error_docref(NULL, E_WARNING, "Empty pattern");
RETVAL_FALSE;
goto out;
}

re = php_mbregex_compile_pattern(arg_pattern, arg_pattern_len, options, MBREX(regex_default_syntax));
if (re == NULL) {
RETVAL_FALSE;
Expand Down Expand Up @@ -1007,15 +1005,14 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
smart_str out_buf = {0};
smart_str eval_buf = {0};
smart_str *pbuf;
int err, eval, n;
int err, n;
OnigUChar *pos;
OnigUChar *string_lim;
char *description = NULL;

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

eval = 0;
{
char *option_str = NULL;
size_t option_str_len = 0;
Expand Down Expand Up @@ -1043,20 +1040,15 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
}

if (option_str != NULL) {
_php_mb_regex_init_options(option_str, option_str_len, &options, &syntax, &eval);
/* Initialize option and in case of failure it means there is a value error */
if (!_php_mb_regex_init_options(option_str, option_str_len, &options, &syntax)) {
RETURN_THROWS();
}
} else {
options |= MBREX(regex_default_options);
syntax = MBREX(regex_default_syntax);
}
}
if (eval) {
if (is_callable) {
php_error_docref(NULL, E_WARNING, "Option 'e' cannot be used with replacement callback");
} else {
php_error_docref(NULL, E_WARNING, "The 'e' option is no longer supported, use mb_ereg_replace_callback instead");
}
RETURN_FALSE;
}

/* create regex pattern buffer */
re = php_mbregex_compile_pattern(arg_pattern, arg_pattern_len, options, syntax);
Expand Down Expand Up @@ -1122,7 +1114,9 @@ static void _php_mb_regex_ereg_replace_exec(INTERNAL_FUNCTION_PARAMETERS, OnigOp
zval_ptr_dtor(&retval);
} else {
if (!EG(exception)) {
php_error_docref(NULL, E_WARNING, "Unable to call custom replacement function");
zend_throw_error(NULL, "Unable to call custom replacement function");
zval_ptr_dtor(&subpats);
RETURN_THROWS();
}
}
zval_ptr_dtor(&subpats);
Expand Down Expand Up @@ -1251,6 +1245,7 @@ PHP_FUNCTION(mb_split)
onig_region_free(regs, 1);

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

if (option_str != NULL) {
_php_mb_regex_init_options(option_str, option_str_len, &option, &syntax, NULL);
if(!_php_mb_regex_init_options(option_str, option_str_len, &option, &syntax)) {
RETURN_THROWS();
}
} else {
option |= MBREX(regex_default_options);
syntax = MBREX(regex_default_syntax);
Expand Down Expand Up @@ -1348,7 +1345,7 @@ static void _php_mb_regex_ereg_search_exec(INTERNAL_FUNCTION_PARAMETERS, int mod
}

if (arg_options) {
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax, NULL);
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax);
} else {
option |= MBREX(regex_default_options);
syntax = MBREX(regex_default_syntax);
Expand All @@ -1375,13 +1372,13 @@ static void _php_mb_regex_ereg_search_exec(INTERNAL_FUNCTION_PARAMETERS, int mod
}

if (MBREX(search_re) == NULL) {
php_error_docref(NULL, E_WARNING, "No regex given");
RETURN_FALSE;
zend_throw_error(NULL, "No pattern was provided");
RETURN_THROWS();
}

if (str == NULL) {
php_error_docref(NULL, E_WARNING, "No string given");
RETURN_FALSE;
zend_throw_error(NULL, "No string was provided");
RETURN_THROWS();
}

MBREX(search_regs) = onig_region_new();
Expand Down Expand Up @@ -1480,13 +1477,13 @@ PHP_FUNCTION(mb_ereg_search_init)
}

if (arg_pattern && arg_pattern_len == 0) {
php_error_docref(NULL, E_WARNING, "Empty pattern");
RETURN_FALSE;
zend_argument_value_error(2, "must not be empty");
RETURN_THROWS();
}

if (arg_options) {
option = 0;
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax, NULL);
_php_mb_regex_init_options(arg_options, arg_options_len, &option, &syntax);
} else {
option = MBREX(regex_default_options);
syntax = MBREX(regex_default_syntax);
Expand Down Expand Up @@ -1557,6 +1554,7 @@ PHP_FUNCTION(mb_ereg_search_getregs)
onig_foreach_name(MBREX(search_re), mb_regex_groups_iter, &args);
}
} else {
// TODO This seems to be some logical error, promote to Error
RETVAL_FALSE;
}
}
Expand Down Expand Up @@ -1588,12 +1586,12 @@ PHP_FUNCTION(mb_ereg_search_setpos)
}

if (position < 0 || (!Z_ISUNDEF(MBREX(search_str)) && Z_TYPE(MBREX(search_str)) == IS_STRING && (size_t)position > Z_STRLEN(MBREX(search_str)))) {
php_error_docref(NULL, E_WARNING, "Position is out of range");
MBREX(search_pos) = 0;
RETURN_FALSE;
zend_argument_value_error(1, "is out of range");
RETURN_THROWS();
}

MBREX(search_pos) = position;
// TODO Return void
RETURN_TRUE;
}
/* }}} */
Expand Down Expand Up @@ -1628,7 +1626,9 @@ PHP_FUNCTION(mb_regex_set_options)
if (string != NULL) {
opt = 0;
syntax = NULL;
_php_mb_regex_init_options(string, string_len, &opt, &syntax, NULL);
if(!_php_mb_regex_init_options(string, string_len, &opt, &syntax)) {
RETURN_THROWS();
}
_php_mb_regex_set_options(opt, syntax, &prev_opt, &prev_syntax);
opt = prev_opt;
syntax = prev_syntax;
Expand Down
104 changes: 37 additions & 67 deletions ext/mbstring/tests/bug43994.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,106 +25,76 @@ foreach($inputs as $input) {
}
echo "\n-- Iteration $iterator --\n";
echo "Without \$regs arg:\n";
var_dump( mb_ereg($input, 'hello, world') );
try {
var_dump( mb_ereg($input, 'hello, world') );
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}

echo "With \$regs arg:\n";
var_dump(mb_ereg($input, 'hello, world', $mb_regs));
try {
var_dump(mb_ereg($input, 'hello, world', $mb_regs));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}

var_dump($mb_regs);
$iterator++;
};
?>
--EXPECTF--
--EXPECT--
-- Iteration 1 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 2 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 3 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 4 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 5 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 6 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 7 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL

-- Iteration 8 --
Without $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
mb_ereg(): Argument #1 ($pattern) must not be empty
With $regs arg:

Warning: mb_ereg(): Empty pattern in %s on line %d
bool(false)
array(0) {
}
mb_ereg(): Argument #1 ($pattern) must not be empty
NULL
15 changes: 8 additions & 7 deletions ext/mbstring/tests/bug69151.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ if (!function_exists('mb_ereg')) die('skip mbregex support not available');
--FILE--
<?php
$str = "\x80";
var_dump(
false === mb_eregi('.', $str, $matches),
[] === $matches,
NULL === mb_ereg_replace('.', "\\0", $str),
false === mb_ereg_search_init("\x80", '.'),
false === mb_ereg_search()
);

var_dump(false === mb_eregi('.', $str, $matches));
var_dump([] === $matches);

var_dump(NULL === mb_ereg_replace('.', "\\0", $str));

var_dump(false === mb_ereg_search_init("\x80", '.'));
var_dump(false === mb_ereg_search());
?>
--EXPECT--
bool(true)
Expand Down
14 changes: 9 additions & 5 deletions ext/mbstring/tests/bug72164.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@ if (!function_exists('mb_ereg')) die('skip mbregex support not available');
$var0 = "e";
$var2 = "";
$var3 = NULL;
$var8 = mb_ereg_replace($var2,$var3,$var3,$var0);
var_dump($var8);
try {
$var8 = mb_ereg_replace($var2,$var3,$var3,$var0);
var_dump($var8);
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}

?>
--EXPECTF--
Warning: mb_ereg_replace(): The 'e' option is no longer supported, use mb_ereg_replace_callback instead in %s on line %d
bool(false)
--EXPECT--
Option "e" is not supported

0 comments on commit 0444158

Please sign in to comment.