Skip to content

Refactoring mb_str(r)(i)pos out of bound checks/errors #5109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Refactoring mb_str(r)(i)pos out of bound checks/errors #5109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ext/mbstring/mbstring.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2084,13 +2084,13 @@ static void handle_strpos_error(size_t error) {
case MBFL_ERROR_NOT_FOUND:
break;
case MBFL_ERROR_ENCODING:
php_error_docref(NULL, E_WARNING, "Unknown encoding or conversion error");
zend_value_error("Unknown encoding or conversion error");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave this one as a warning. This error doesn't actually occur (as far as I know), but if it did, it would be due to invalid encoding in the input string, which shouldn't throw. You can change the error message to just "Conversion error" to make that clear. (Unknown encodings are caught separately.)

break;
case MBFL_ERROR_OFFSET:
php_error_docref(NULL, E_WARNING, "Offset not contained in string");
zend_value_error("Offset not contained in string");
break;
default:
php_error_docref(NULL, E_WARNING, "Unknown error in mb_strpos");
zend_value_error("Unknown error in mb_strpos");
break;
}
}
Expand Down
38 changes: 17 additions & 21 deletions ext/mbstring/tests/bug43840.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,20 @@ $needle = base64_decode('44CC');
foreach($offsets as $i) {
echo "\n-- Offset is $i --\n";
echo "--Multibyte String:--\n";
var_dump( mb_strpos($string_mb, $needle, $i, 'UTF-8') );
try {
var_dump( mb_strpos($string_mb, $needle, $i, 'UTF-8') );
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
echo"--ASCII String:--\n";
var_dump(mb_strpos('This is na English ta', 'a', $i));
try {
var_dump(mb_strpos('This is na English ta', 'a', $i));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
}
?>
--EXPECTF--
--EXPECT--
-- Offset is 20 --
--Multibyte String:--
int(20)
Expand All @@ -46,30 +54,18 @@ bool(false)

-- Offset is 22 --
--Multibyte String:--

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
--ASCII String:--

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

-- Offset is 53 --
--Multibyte String:--

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
--ASCII String:--

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

-- Offset is 54 --
--Multibyte String:--

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
--ASCII String:--

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
41 changes: 20 additions & 21 deletions ext/mbstring/tests/bug43841.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,50 +17,49 @@ function_exists('mb_strrpos') or die("skip mb_strrpos() is not available in this
*/

$offsets = array(-25, -24, -13, -12);
$string_mb =
base64_decode('5pel5pys6Kqe44OG44Kt44K544OI44Gn44GZ44CCMDEyMzTvvJXvvJbvv
JfvvJjvvJnjgII=');
$needle = base64_decode('44CC');
// Japanese string in UTF-8
$string_mb = "日本語テキストです。01234５６７８９。";
$needle = "。";

foreach ($offsets as $i) {
echo "\n-- Offset is $i --\n";
echo "Multibyte String:\n";
var_dump( mb_strrpos($string_mb, $needle, $i, 'UTF-8') );
try {
var_dump( mb_strrpos($string_mb, $needle, $i, 'UTF-8') );
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
echo "ASCII String:\n";
echo "mb_strrpos:\n";
var_dump(mb_strrpos('This is na English ta', 'a', $i));
try {
var_dump(mb_strrpos('This is na English ta', 'a', $i));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
echo "strrpos:\n";
try {
var_dump(strrpos('This is na English ta', 'a', $i));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
}
?>
--EXPECTF--
--EXPECT--
-- Offset is -25 --
Multibyte String:

Warning: mb_strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
ASCII String:
mb_strrpos:

Warning: mb_strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
strrpos:
Offset not contained in string

-- Offset is -24 --
Multibyte String:

Warning: mb_strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
ASCII String:
mb_strrpos:

Warning: mb_strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
strrpos:
Offset not contained in string

Expand Down
46 changes: 15 additions & 31 deletions ext/mbstring/tests/bug45923.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ Bug #45923 (mb_st[r]ripos() offset not handled correctly)
function section($func, $haystack, $needle)
{
echo "\n------- $func -----------\n\n";
foreach(array(0, 3, 6, 9, 11, 12, -1, -3, -6, -20) as $offset) {
foreach([0, 3, 6, 9, 11, 12, -1, -3, -6, -20] as $offset) {
echo "> Offset: $offset\n";
try {
var_dump($func($haystack,$needle,$offset));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
try {
var_dump($func($haystack, $needle, $offset));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
}
}

Expand All @@ -30,7 +30,7 @@ section('mb_strrpos' , "●○◆ ●○◆ ●○◆", "●○◆");
section('strripos' , "abc abc abc" , "abc");
section('mb_strripos', "●○◆ ●○◆ ●○◆", "●○◆");
?>
--EXPECTF--
--EXPECT--
------- strpos -----------

> Offset: 0
Expand Down Expand Up @@ -67,19 +67,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
bool(false)
> Offset: -3
int(8)
> Offset: -6
int(8)
> Offset: -20

Warning: mb_strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- stripos -----------

Expand Down Expand Up @@ -117,19 +113,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: mb_stripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
bool(false)
> Offset: -3
int(8)
> Offset: -6
int(8)
> Offset: -20

Warning: mb_stripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- strrpos -----------

Expand Down Expand Up @@ -167,19 +159,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: mb_strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
int(8)
> Offset: -3
int(8)
> Offset: -6
int(4)
> Offset: -20

Warning: mb_strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- strripos -----------

Expand Down Expand Up @@ -217,16 +205,12 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: mb_strripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
int(8)
> Offset: -3
int(8)
> Offset: -6
int(4)
> Offset: -20

Warning: mb_strripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
55 changes: 0 additions & 55 deletions ext/mbstring/tests/mb_stripos.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,6 @@ print mb_stripos($euc_jp, 0, -15, 'EUC-JP') . "\n";
print mb_stripos($euc_jp, 0, -43, 'EUC-JP') . "\n";


// Invalid offset - should return false with warning
print ("== INVALID OFFSET ==\n");

$r = mb_stripos($euc_jp, '���ܸ�', 44, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, '���ܸ�', 50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, '0', 50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, 3, 50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, 0, 50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, '���ܸ�', -50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, '0', -50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, 3, -50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, 0, -50, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";
$r = mb_stripos($euc_jp, 0, -44, 'EUC-JP');
($r === FALSE) ? print "OK_INVALID_OFFSET\n" : print "NG_INVALID_OFFSET\n";

// Out of range - should return false
print ("== OUT OF RANGE ==\n");

Expand Down Expand Up @@ -143,37 +119,6 @@ String len: 43
33
30
0
== INVALID OFFSET ==

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET

Warning: mb_stripos(): Offset not contained in string in %s on line %d
OK_INVALID_OFFSET
== OUT OF RANGE ==
OK_OUT_RANGE
OK_OUT_RANGE
Expand Down
Loading