From 656046873c9316aae2970fb2997edaafa60478e7 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Thu, 2 Apr 2020 22:38:28 +0200 Subject: [PATCH] Refactor mb_substr_count() Promote empty needle warning to ValueError Convert if branch into an assertion as if mbfl_substr_count fails this now implies a bug Thus mb_substr_count() can only return int now, fix stubs accordingly --- ext/mbstring/mbstring.c | 21 +++++++++++---------- ext/mbstring/mbstring.stub.php | 2 +- ext/mbstring/mbstring_arginfo.h | 2 +- ext/mbstring/tests/mb_substr_count.phpt | 23 ++++++++++++++++------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index e8fb63e675..16452b0a5b 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -2269,23 +2269,24 @@ PHP_FUNCTION(mb_substr_count) RETURN_THROWS(); } + if (needle.len == 0) { + zend_argument_value_error(2, "must not be empty"); + RETURN_THROWS(); + } + haystack.no_language = needle.no_language = MBSTRG(language); haystack.encoding = needle.encoding = php_mb_get_encoding(enc_name, 3); if (!haystack.encoding) { RETURN_THROWS(); } - if (needle.len == 0) { - php_error_docref(NULL, E_WARNING, "Empty substring"); - RETURN_FALSE; - } - n = mbfl_substr_count(&haystack, &needle); - if (!mbfl_is_error(n)) { - RETVAL_LONG(n); - } else { - RETVAL_FALSE; - } + /* An error can only occur if needle is empty, + * an encoding error happens (which should not happen at this stage and is a bug) + * or the haystack is more than sizeof(size_t) bytes + * If one of these things occur this is a bug and should be flagged as such */ + ZEND_ASSERT(!mbfl_is_error(n)); + RETVAL_LONG(n); } /* }}} */ diff --git a/ext/mbstring/mbstring.stub.php b/ext/mbstring/mbstring.stub.php index aa2c7dc0e4..dfc920eb96 100644 --- a/ext/mbstring/mbstring.stub.php +++ b/ext/mbstring/mbstring.stub.php @@ -39,7 +39,7 @@ function mb_stristr(string $haystack, string $needle, bool $part = false, string function mb_strrichr(string $haystack, string $needle, bool $part = false, string $encoding = UNKNOWN): string|false {} -function mb_substr_count(string $haystack, string $needle, string $encoding = UNKNOWN): int|false {} +function mb_substr_count(string $haystack, string $needle, string $encoding = UNKNOWN): int {} function mb_substr(string $str, int $start, ?int $length = null, string $encoding = UNKNOWN): string|false {} diff --git a/ext/mbstring/mbstring_arginfo.h b/ext/mbstring/mbstring_arginfo.h index 09e277d37b..ea6af386e3 100644 --- a/ext/mbstring/mbstring_arginfo.h +++ b/ext/mbstring/mbstring_arginfo.h @@ -78,7 +78,7 @@ ZEND_END_ARG_INFO() #define arginfo_mb_strrichr arginfo_mb_strstr -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_substr_count, 0, 2, MAY_BE_LONG|MAY_BE_FALSE) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_substr_count, 0, 2, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, haystack, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, needle, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 0) diff --git a/ext/mbstring/tests/mb_substr_count.phpt b/ext/mbstring/tests/mb_substr_count.phpt index 84571515de..0d2962866f 100644 --- a/ext/mbstring/tests/mb_substr_count.phpt +++ b/ext/mbstring/tests/mb_substr_count.phpt @@ -7,11 +7,20 @@ output_handler= --FILE-- getMessage() . \PHP_EOL; + } + try { + var_dump(mb_substr_count("��", "")); + } catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; + } + + var_dump(mb_substr_count("", "¤¢")); + var_dump(mb_substr_count("", "¤¢")); + var_dump(mb_substr_count("", chr(0))); $a = str_repeat("abcacba", 100); var_dump(@mb_substr_count($a, "bca")); @@ -32,8 +41,8 @@ output_handler= var_dump(@mb_substr_count($a, "bca")); ?> --EXPECT-- -bool(false) -bool(false) +mb_substr_count(): Argument #2 ($needle) must not be empty +mb_substr_count(): Argument #2 ($needle) must not be empty int(0) int(0) int(0) -- 2.50.1