Bug #77853 [Csd]: Inconsistent substr_compare behaviour with empty haystack

Edit report at https://bugs.php.net/bug.php?id=77853&edit=1 ID: 77853 Updated by: nikic@php.net Reported by: riikka dot kalliomaki at gmail dot com Summary: Inconsistent substr_compare behaviour with empty haystack Status: Closed Type: Bug Package: Strings related PHP Version: 7.3.4 -Assigned To: +Assigned To: nikic Block user comment: N Private report: N New Comment: I went with option 3 here. As we're allowing more inputs rather than less, I'm not particularly concerned about BC impact. Previous Comments: ------------------------------------------------------------------------ [2019-04-08 09:42:51] nikic@php.net Automatic comment on behalf of nikita.ppv@gmail.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=d7b5954f2818aff6db29a995f407797a7902f38f Log: Fixed bug #77853 ------------------------------------------------------------------------ [2019-04-05 11:50:41] riikka dot kalliomaki at gmail dot com Description: ------------ The behavior of substr_compare is a bit odd and somewhat inconsistent, especially when compared to other php functions. substr_compare provides an error if the offset is *equal* or greater than the length of the first paremeter, i.e. the haystack. Other functions like substr() and substr_count() have no problems with offset that are *equal* to the haystack, e.g. substr('foo', 3) === ''; substr('foo', 4) === false; substr_count('foo', 'o', 3) === 0; substr_count('foo', 'o', 4) === false; // Produces PHP Warning: substr_count(): Offset not contained in string This creates a somewhat strange problem, that you cannot compare an empty string with anything, e.g. substr_compare('a', 'a', 0) === 0; substr_compare('', '', 0) === false; // Produces PHP Warning: substr_compare(): The start position cannot exceed initial string length That is, unless you set the length to 0 as well (because the internal function short circuits 0 length comparison): substr_compare('', '', 0, 0) === 0; To be slightly pedantic, the manual also states the following about the length parameter: > The length of the comparison. The default value is the largest of the length of the str > compared to the length of main_str less the offset. Thus, substr_compare('', '', 0, 0) should be same as substr_compare('', '', 0), but the short circuiting doesn't happen due to usage of default parameter, so it causes an error. In my opinion, this should be fixed in one of the following ways to make a function more usable: 1. Make sure that substr_compare('', '', 0) === substr_compare('', '', 0, 0) 2. Allow special case of offset 0, if the haystack is empty 3. Allow offset that is *equal* to the length of the haystack The case 1 is a minimal fix to address what I consider the main "bug" or inconsistency. The case 2 would allow some leeway that is probably a very minimal BC break. The case 3 would make most of sense in PHP, but is a somewhat of a BC break. As it stands right now, general use of substr_compare is a bit unwieldy, as you have to add additional guard clauses to check if the haystack is empty to use the function in general case. For example, the following function will fail with empty haystack and nonempty needle: function ends_with($haystack, $needle): bool { $length = strlen($needle); return substr_compare($haystack, $needle, -$length, $length) === 0; } Test script: --------------- <?php var_dump(substr_compare('', '', 0, 0)); var_dump(substr_compare('', '', 0)); Expected result: ---------------- int(0) int(0) Actual result: -------------- int(0) Warning: substr_compare(): The start position cannot exceed initial string length in /Users/riikka/Source/opcodes2.php on line 4 bool(false) ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=77853&edit=1