php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77853 Inconsistent substr_compare behaviour with empty haystack
Submitted: 2019-04-05 11:50 UTC Modified: 2019-04-08 09:44 UTC
Votes:1
Avg. Score:4.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: riikka dot kalliomaki at gmail dot com Assigned: nikic (profile)
Status: Closed Package: Strings related
PHP Version: 7.3.4 OS:
Private report: No CVE-ID: None
 [2019-04-05 11:50 UTC] 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)

Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-04-08 09:42 UTC] 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-08 09:42 UTC] nikic@php.net
-Status: Open +Status: Closed
 [2019-04-08 09:44 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2019-04-08 09:44 UTC] nikic@php.net
I went with option 3 here. As we're allowing more inputs rather than less, I'm not particularly concerned about BC impact.
 [2019-05-12 08:56 UTC]
The following pull request has been associated:

Patch Name: Patch download link directly from cron mails
On GitHub:  https://github.com/php/web-doc-editor/pull/9
Patch:      https://github.com/php/web-doc-editor/pull/9.patch
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 16:01:28 2024 UTC