php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #54454 substr_compare incorrectly reports equality in some cases
Submitted: 2011-04-02 23:52 UTC Modified: 2012-06-08 07:25 UTC
From: zweibieren at yahoo dot com Assigned: pierrick (profile)
Status: Closed Package: Strings related
PHP Version: Irrelevant OS: linux
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: zweibieren at yahoo dot com
New email:
PHP Version: OS:

 

 [2011-04-02 23:52 UTC] zweibieren at yahoo dot com 
Description:
------------
---
From manual page: http://www.php.net/function.substr-compare#Description

<blockquote>
int substr_compare ( string $main_str , string $str , int $offset , int $length ... )

substr_compare() compares main_str from position offset with str up to length characters. 

Return Values

Returns < 0 if main_str from position offset is less than str, > 0 if it is greater than str, and 0 if they are equal. ...
</blockquote>
---

HOWEVER, if main_str is shorter than str, substr_compare checks only up to the length of main_str.  This is NOT how I read the description.  I believe the comparison should extend to length characters and render a non-zero value if the two strings differ at any character in the first length characters.

WORKAROUND: Use strncmp. 

{Note that substr_compare advertises itself as "binary safe." Since it is stopping at '\0', it fails to actually BE binary safe.}





Test script:
---------------
<html><head></head><body>
<?php echo 'four comparisons of "/" and "/asd":'
		. '<br/> &nbsp; substr_compare("/", "/asd", 0, 4) => ' 
				. substr_compare("/", "/asd", 0, 4) 
				. ' <b>(SHOULD BE -3)</b>'  
		. '<br/> &nbsp; substr_compare("/asd", "/", 0, 4) => ' 
				. substr_compare("/asd", "/", 0, 4) 
		. '<br/> &nbsp; strncmp("/", "/asd", 4) => ' 
				. strncmp("/", "/asd", 4) 
		. '<br/> &nbsp; strncmp("/asd", "/", 4) => ' 
				. strncmp("/asd", "/", 4);  ?>
</body></html>


Expected result:
----------------
four comparisons of "/" and "/asd":
  substr_compare("/", "/asd", 0, 4) => -3
  substr_compare("/asd", "/", 0, 4) => 3
  strncmp("/", "/asd", 4) => -3
  strncmp("/asd", "/", 4) => 3



Actual result:
--------------
four comparisons of "/" and "/asd":
  substr_compare("/", "/asd", 0, 4) => 0 (SHOULD BE -3)
  substr_compare("/asd", "/", 0, 4) => 3
  strncmp("/", "/asd", 4) => -3
  strncmp("/asd", "/", 4) => 3

As the script says, the first reported value should be -3.

Patches

54454.diff (last revision 2011-04-03 06:38 UTC by pierrick at webstart dot fr)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-04-03 09:18 UTC] pierrick@php.net 
Automatic comment from SVN on behalf of pierrick
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=309910
Log: Fixed bug #54454 (substr_compare incorrectly reports equality in some cases)
 [2011-04-03 09:21 UTC] pierrick@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: pierrick
 [2011-04-03 09:23 UTC] pierrick@php.net 
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.
 [2011-04-06 10:16 UTC] pajoye@php.net
-Status: Closed +Status: Re-Opened
 [2011-04-06 10:16 UTC] pajoye@php.net 
Chris and Scott are seeing issues with this change. Reopening.
 [2011-04-06 20:58 UTC] sixd@php.net 
I was initially concerned that the security implications of the patch were 
reviewed.  Scott did this but has a concern about BC.
 [2011-04-07 01:16 UTC] zweibieren at yahoo dot com 
Parkinson would be glad to claim as one of his laws: 
   The smaller the software change the bigger the discussion. !-)
 [2011-04-21 03:51 UTC] pierrick@php.net 
Automatic comment from SVN on behalf of pierrick
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=310401
Log: Revert bug #54454 fix from PHP_5_3 branch to avoid BC break in minor release
 [2012-06-08 07:25 UTC] pierrick@php.net 
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.
 [2012-06-08 07:25 UTC] pierrick@php.net
-Status: Re-Opened +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Wed Nov 27 10:01:27 2024 UTC