Bug #80758 [PATCH]: version_compare does not behave as documented (or even consistently)

Edit report at https://bugs.php.net/bug.php?id=80758&edit=1 ID: 80758 Patch added by: brad.jorsch@automattic.com Reported by: brad dot jorsch at automattic dot com Summary: version_compare does not behave as documented (or even consistently) Status: Open Type: Bug Package: *General Issues Operating System: Linux PHP Version: 8.0Git-2021-02-16 (Git) Block user comment: N Private report: N New Comment: 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 Previous Comments: ------------------------------------------------------------------------ [2021-02-16 19:13:14] 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) ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=80758&edit=1