php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #80758 version_compare does not behave as documented (or even consistently)
Submitted: 2021-02-16 19:13 UTC Modified: -
Votes:2
Avg. Score:3.0 ± 0.0
Reproduced:2 of 2 (100.0%)
Same Version:0 (0.0%)
Same OS:1 (50.0%)
From: brad dot jorsch at automattic dot com Assigned:
Status: Open Package: *General Issues
PHP Version: 8.0Git-2021-02-16 (Git) OS: Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: brad dot jorsch at automattic dot com
New email:
PHP Version: OS:

 

 [2021-02-16 19:13 UTC] brad dot jorsch at automattic dot com
Description:
------------
While I marked the version as 8.0.2, this seems to exist in many versions, in some forms back to 4.3.0 or earlier.

The documentation for version_compare says that "any string not found in this list" comes before various recognized strings, including numbers. But certain strings do not follow this behavior.

1. Any non-alphanumeric character has that character treated as a `.` rather than it being included in the string to be compared. For example, "1.0-alpha 1" < "1.0-alpha 2" because the space is considered a `.`, even though as documented this should not be the case.
2. Exception: If that non-alphanumeric character comes directly after a number, it is not considered as a `.`. So "1.0 alpha" == "1.0 beta" because neither " alpha" nor " beta" are recognized as special strings. (see #75805)
3. Strings beginning with "a", "b", or other recognized strings are considered as being those recognized strings. (see #75806)
4. An empty string as the last component compares as less than itself. This can combine with item 1 in unexpected ways. (see #69268, and https://stackoverflow.com/a/65118939)

IMO, item 1 makes sense and should be documented as such. Items 2 and 4 make no sense and should be fixed. Item 3 could go either way, but since the documentation calls out "a" being the same as "alpha" and so on that implies that "apple" should not be.

The attached patch fixes 2 and 4, and 3 with the interpretation that "apple" should not be the same as "alpha". I'll leave documentation updates to you, since that's done outside of the main repo.

P.S. I see cmb@php.net has a habit of closing reports like these as "not a bug" with the justification that a "PHP-standardized version string" has some sort of meaning that excludes these inputs. If you really want to continue considering this sort of thing "not a bug", you should actually document a "PHP-standardized version string" as only containing alphanumerics plus `.`, `-`, `_`, and `+` somewhere, as currently that is not defined anywhere. You should also reject invalid strings with an Error instead of silently handling them unexpectedly.

But IMO restricting this to only "PHP-standardized version strings" is going against how most people actually use the method.

Test script:
---------------
var_dump( version_compare( "1.0 alpha", "1.0 beta" ) );
var_dump( version_compare( "1.0.apple", "1.0.coconut" ) );
var_dump( version_compare( "1.0.", "1.0." ) );
var_dump( version_compare( "1.0.*", "1.0.*" ) );


Expected result:
----------------
int(-1)
int(0)
int(0)
int(0)


Actual result:
--------------
int(0)
int(1)
int(-1)
int(-1)

Patches

patch (last revision 2021-02-16 19:14 UTC by brad dot jorsch at automattic dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-02-16 19:14 UTC] brad dot jorsch at automattic dot com
The following patch has been added/updated:

Patch Name: patch
Revision:   1613502857
URL:        https://bugs.php.net/patch-display.php?bug=80758&patch=patch&revision=1613502857
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Mon Apr 19 20:01:24 2021 UTC