Edit report at https://bugs.php.net/bug.php?id=62097&edit=1

 ID:                 62097
 Updated by:         s...@php.net
 Reported by:        kazuo at o-ishi dot jp
 Summary:            New behavior of string == has a compatibility
                     problem
 Status:             Assigned
 Type:               Bug
 Package:            Scripting Engine problem
 Operating System:   Gentoo Linux
 PHP Version:        5.4.4RC1
 Assigned To:        stas
 Block user comment: N
 Private report:     N

 New Comment:

I am sorry, but I still do not see the explanation why any code would rely on 
comparison such as "09223372036854775808" == "9223372036854775808" to return 
true. If you have such reason - please bring it forward. Repeating "HISTORICAL 
REASON" does not explain anything. If you have valid use case why it needs to 
be 
preserved - please provide it ASAP. Otherwise the only thing to conclude will 
be 
that no application has a valid reason to rely on that and the right thing will 
be to change it, as this code should behave this way from the start.


Previous Comments:
------------------------------------------------------------------------
[2012-05-29 03:25:08] kazuo at o-ishi dot jp

> Old comparison ("09223372036854775808" == "9223372036854775808") is
> a problem since it may lead to md5() hashes accepting wrong
> passwords. This has very high risk of negative consequences for many
> PHP users. Thus we decided to fix it ASAP.

Clearly, it have to be compared using === instead of ==.

We should issue the statement at the PHP site:

  "String to string comparison using == has many problem including
   security problems.  We strongly recommend to use === instead."

And we should add new Security section at
http://www.php.net/manual/en/security.php

In addition, string to string comparison using == should be mentioned as NOT
RECOMMENDED explicitly on
http://www.php.net/manual/en/language.operators.comparison.php

These actions can be done at right now, and it is effective for all
PHP users including users of old versions.


Fortunately, it's rare the return value is constructed only decimal
number since md5() returns 32-character hexdecimal string
[(10/16)^32 -> 0.000029% ?].
And, for the application developers, it is comparatively easy to point
out and correct a part with such a problem.


> This change can not be backward compatible - since the whole point
> was to change how this comparison works.

Security fix which breaks compatibility in wide and unexpectable area
will not work well, because it makes the users difficult to migrate to new 
version.


> I have very hard time understanding why your code would rely on comparison 
> between two completely distinct strings return "true" - what exactly this 
> code 
> does and why it relates on == comparison truncating long numbers?

That's HISTORICAL REASON.
There are many of legacy code with == comparison in the world,
they need to work AS IS.

If there is such a incompatibility, the maintainers of that code have
to check whole code using == operator before upgrading to PHP 5.4.4.

------------------------------------------------------------------------
[2012-05-29 01:14:30] s...@php.net

Old comparison ("09223372036854775808" == "9223372036854775808") is a problem 
since it may lead to md5() hashes accepting wrong passwords. This has very high 
risk of negative consequences for many PHP users. Thus we decided to fix it 
ASAP.
This change can not be backward compatible - since the whole point was to 
change 
how this comparison works. 

I have very hard time understanding why your code would rely on comparison 
between two completely distinct strings return "true" - what exactly this code 
does and why it relates on == comparison truncating long numbers? Maybe if you 
explain the need better there would be a reason to postpone this change but for 
now I do not see a reason that would override very real security concern from 
bug #54547.

------------------------------------------------------------------------
[2012-05-28 11:02:48] cataphr...@php.net

I'm reassigning to stas, as which branches changes go to is primarily his 
decision.

------------------------------------------------------------------------
[2012-05-28 10:29:56] kazuo at o-ishi dot jp

> The new rule is the same as the old rule, with the following
> limitation of numeric comparisons: If both strings look like integers
> (no decimal separator nor exponent) but they were both converted to
> doubles because of being too large in absolute value, if they both
> compare equal in a double comparison, and if they're both larger than
> 2^53-1 in absolute value, then compare them as a string.

Why this change is Right, in spite of breaking backward compatibility
from 5.4.3 to 5.4.4?  What kind of tests are needed to the users to
migrate PHP 5.4.3 to PHP 5.4.4 safely?

I know this change was introduced to fix Bug #54547, but it's side
effects are not small.

I agree your first comment on #54547 :

>> Maybe this should be Won't Fix to keep it consistent with
>> 9223372036854775807 == 9223372036854775808 (with number literals).

------------------

> I agree that this is not a model of simplicity -- I'd have preferred a
> custom string comparison for number-like values -- but it's working as
> expected.

This need "Backward Incompatible Changes" section like
http://www.php.net/manual/en/migration54.incompatible.php ...

Please keep compatibility. 
Even though this change will be done, I would like you to do it for PHP 5.5.

------------------------------------------------------------------------
[2012-05-28 09:07:31] cataphr...@php.net

The old rule is:
If the strings look like numbers (i.e they follow the notation for a decimal or 
hexadecimal integer once any leading whitespace or leading zeros -- immediately 
before the first non-zero digit -- are ignored), then they are compared as 
numbers, except if the conversion result in infinite values with the same side, 
in which case they are compared as strings. The number comparison is a double 
comparison if any of the strings is converted to a double (due to a decimal 
separator, exponent or the number being too large in absolute value) and it's 
an integer comparison otherwise.

The new rule is the same as the old rule, with the following limitation of 
numeric comparisons:
If both strings look like integers (no decimal separator nor exponent) but they 
were both converted to doubles because of being too large in absolute value, if 
they both compare equal in a double comparison, and if they're both larger than 
2^53-1 in absolute value, then compare them as a string.

In light of this:

"1234567890123456789" == "12345678901.23456789E8" (32-bit)
1234567890123456789 > 2^31-1, so it cannot be represented as long => converted 
to double => compared as double => compares equal => 1234567890123456789 > 
2^53-1  => compare as string => FALSE

"9223372036854775808" == "09223372036854775808" (64-bit)
1234567890123456789 > 2^63-1, the same follows

"9223372036854775808" == " 9223372036854775808" (64-bit)
idem

For the rest of the 32-bit examples, 9007199254740992 is also larger than 
2^53-1, so the the string comparison will be triggered.

I agree that this is not a model of simplicity -- I'd have preferred a custom 
string comparison for number-like values -- but it's working as expected.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=62097


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=62097&edit=1

Reply via email to