Everything

php

Don't compare zero exponentials in strings as equal

php.internals

Nikita Popov

5 years ago

Hi internals,

PHP's == comparison semantics for strings have a peculiar edge-case, where
comparisons of the form "0e123" == "0e456" return true, because they are
interpreted as floating point zero numbers. This is problematic, because
strings of that form are usually not numbers, but hex-encoded hashes or
similar.

I'm wondering if it may make sense to special-case the comparison semantics
to not consider strings of the form "0e[DIGITS]" equal, unless they are
exactly equal (i.e., fall back to lexicographical if both sides of the
comparison are zero exponentials).

Here's a possible implementation: https://github.com/php/php-src/pull/6749

Of course, the usual rule that you should always use === still holds, but
this at least eliminates the most dangerous edge case.

Regards,
Nikita

Ben Ramsey

5 years ago

> On Mar 3, 2021, at 08:04, Nikita Popov <nikita.ppv@gmail.com> wrote:
>
> Hi internals,
>
> PHP's == comparison semantics for strings have a peculiar edge-case, where
> comparisons of the form "0e123" == "0e456" return true, because they are
> interpreted as floating point zero numbers. This is problematic, because
> strings of that form are usually not numbers, but hex-encoded hashes or
> similar.
>
> I'm wondering if it may make sense to special-case the comparison semantics
> to not consider strings of the form "0e[DIGITS]" equal, unless they are
> exactly equal (i.e., fall back to lexicographical if both sides of the
> comparison are zero exponentials).
>
> Here's a possible implementation: https://github.com/php/php-src/pull/6749
>
> Of course, the usual rule that you should always use === still holds, but
> this at least eliminates the most dangerous edge case.

I encountered a similar situation a few years back.

We were testing whether a value was numeric and, if so, adding 0 to it in order to convert it to an appropriate number type. The code looked something like this:

if (is_numeric($value)) {
$value += 0;
}

We chose not to do an explicit cast because the string could represent a float or an int, so we wanted the type coercion to do its magic.

We did this before calling `json_encode()` on the data structure, so that string numbers coming out of a database would be converted to numbers in JSON. For some reason, `JSON_NUMERIC_CHECK` wasn’t giving us what we wanted, but I can’t recall the issue we were having.

Anyway, we ran into some fun issues with hashes that looked like this:

'131124826899e4096767887418316466'

That value should have remained a string in the JSON output, but `is_numeric()` returns `true` for it, so it became `INF`.

We were able to come up with a work-around, but it’s not foolproof.

Cheers,
Ben

Stas Malyshev

5 years ago

Hi!

> PHP's == comparison semantics for strings have a peculiar edge-case, where
> comparisons of the form "0e123" == "0e456" return true, because they are
> interpreted as floating point zero numbers. This is problematic, because
> strings of that form are usually not numbers, but hex-encoded hashes or
> similar.

This particular argument makes sense, but in more generic sense I feel
it leads us to a dangerous path of implying it's ok to use "==" to
compare strings, because we'll take care of the corner cases. Which I
think is wrong to imply because there are so many corner cases where it
still doesn't work and probably never will. I mean, "000" == "0000000"
is still true. "010" == "0000010" is still true. "1e23" == "001e023" is
still true. Nobody who applies == to strings and expects it to work out
as stri g comparison is doing the right thing. If you apply == to
hex-encoded hashes, that code is fubar, and fixing one particular corner
case won't rescue it. So I wonder if fixing one particular corner case
while leaving many others in would do much.

--
Stas Malyshev
smalyshev@gmail.com

Christian Schneider

5 years ago

Am 03.03.2021 um 19:21 schrieb Stanislav Malyshev <smalyshev@gmail.com>:

> Nobody who applies == to strings and expects it to work out as stri g comparison is doing the right thing. If you apply == to hex-encoded hashes, that code is fubar, and fixing one particular corner case won't rescue it. So I wonder if fixing one particular corner case while leaving many others in would do much.

Just brainstorming, but following that logic, if we would like to remedy this in the long run:
a) Should == with two strings where one (or both?) of them is numeric cause an E_NOTICE or E_WARNING that === should be used?
b) Should == with two strings leave them as string? Possibly after a period of notice/warning as described in a)?

I haven't really thought this through but I still wanted to throw it out there.
I'm sure you'll tell me how foolish I am and why this is a very bad idea ;-)

- Chris

Kamil Tekiela

5 years ago

Oh, I like Chris's idea. Yes, please.
Let's deprecate numerical comparison when both operands are strings and
remove that behaviour in PHP 9.0.
Type juggling can be useful when one of them is an integer or float but
when both are strings then chances are that this is an error.
Sorry Nikita, but adding special handling for edge-cases is only going to
make things messier.

Christian Schneider

5 years ago

Am 03.03.2021 um 21:25 schrieb Kamil Tekiela <tekiela246@gmail.com>:

> Sorry Nikita, but adding special handling for edge-cases is only going to
> make things messier.

I didn't want to say that, as there is plenty of code out there who might fall in this trap this intermediate measure might still make sense.
The good old quick-fix - warning - better fix cycle :-)

- Chris

Ben Ramsey

5 years ago

On Mar 3, 2021, at 14:25, Kamil Tekiela <tekiela246@gmail.com> wrote:

>
> when both are strings then chances are that this is an error.

Except when comparing two values from sources known to provide numbers as strings, such as form input and database results. :-)

Cheers,
Ben

Rowan Collins

5 years ago

On 04/03/2021 00:37, Ben Ramsey wrote:

> On Mar 3, 2021, at 14:25, Kamil Tekiela <tekiela246@gmail.com> wrote:
>> when both are strings then chances are that this is an error.
>
> Except when comparing two values from sources known to provide numbers as strings, such as form input and database results. :-)

The juggling only makes a difference if the two sources provide
different representations of the same number - "12345" is equal to
"12345" whether you cast both sides to int or leave both as strings.

Regards,

--
Rowan Tommins
[IMSoP]

Christian Schneider

5 years ago

Am 04.03.2021 um 01:37 schrieb Ben Ramsey <ben@benramsey.com>:

> On Mar 3, 2021, at 14:25, Kamil Tekiela <tekiela246@gmail.com> wrote:
>>
>> when both are strings then chances are that this is an error.
>
> Except when comparing two values from sources known to provide numbers as strings, such as form input and database results. :-)

This would be a problem for leading zeroes and leading/training spaces, right?

Leading zeroes theoretically could happen in databases, leading/training spaces happen in form input and possibly databases.
Are there other 'common' cases?

- Chris

Nikita Popov

5 years ago

On Thu, Mar 4, 2021 at 10:54 AM Christian Schneider <cschneid@cschneid.com>
wrote:

> Am 04.03.2021 um 01:37 schrieb Ben Ramsey <ben@benramsey.com>:
> > On Mar 3, 2021, at 14:25, Kamil Tekiela <tekiela246@gmail.com> wrote:
> >>
> >> when both are strings then chances are that this is an error.
> >
> > Except when comparing two values from sources known to provide numbers
> as strings, such as form input and database results. :-)
>
>
> This would be a problem for leading zeroes and leading/training spaces,
> right?
>
> Leading zeroes theoretically could happen in databases, leading/training
> spaces happen in form input and possibly databases.
> Are there other 'common' cases?
>

The main one that comes to mind is something like '0' == '0.0'. However,
the real problem is something else: Comparison behavior doesn't affect just
== and !=, but also < and >. And I can see how people would want '2' < '10'
to be true (numeric comparison) rather than false (lexicographical
comparison).

I generally agree that we should remove the special "numeric string"
handling for equality comparisons, and I don't think that removing that
behavior would have a major impact. But we do need to carefully consider
the impact it has on relational operators. There are two ways I can see
this going:

1. Decouple equality comparison from relational comparison. Don't handle
numeric strings for == and !=, but do handle them for <, >, etc. The
disadvantage is that comparison results may not be trichotomous, e.g. for
"0" op "0.0" all of ==, < and > would return false. (To be fair, this can
already happen in other cases, e.g. non-comparable objects.)

2. Don't allow relational comparison on strings. If you want to compare
them lexicographically, use strcmp(), otherwise cast to number first.
("Don't allow" here could be a warning to start with.)

Regards,
Nikita

Nikita Popov

5 years ago

On Thu, Mar 4, 2021 at 11:54 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> On Thu, Mar 4, 2021 at 10:54 AM Christian Schneider <cschneid@cschneid.com>
> wrote:
>
>> Am 04.03.2021 um 01:37 schrieb Ben Ramsey <ben@benramsey.com>:
>> > On Mar 3, 2021, at 14:25, Kamil Tekiela <tekiela246@gmail.com> wrote:
>> >>
>> >> when both are strings then chances are that this is an error.
>> >
>> > Except when comparing two values from sources known to provide numbers
>> as strings, such as form input and database results. :-)
>>
>>
>> This would be a problem for leading zeroes and leading/training spaces,
>> right?
>>
>> Leading zeroes theoretically could happen in databases, leading/training
>> spaces happen in form input and possibly databases.
>> Are there other 'common' cases?
>>
>
> The main one that comes to mind is something like '0' == '0.0'. However,
> the real problem is something else: Comparison behavior doesn't affect just
> == and !=, but also < and >. And I can see how people would want '2' < '10'
> to be true (numeric comparison) rather than false (lexicographical
> comparison).
>
> I generally agree that we should remove the special "numeric string"
> handling for equality comparisons, and I don't think that removing that
> behavior would have a major impact. But we do need to carefully consider
> the impact it has on relational operators. There are two ways I can see
> this going:
>
> 1. Decouple equality comparison from relational comparison. Don't handle
> numeric strings for == and !=, but do handle them for <, >, etc. The
> disadvantage is that comparison results may not be trichotomous, e.g. for
> "0" op "0.0" all of ==, < and > would return false. (To be fair, this can
> already happen in other cases, e.g. non-comparable objects.)
>
> 2. Don't allow relational comparison on strings. If you want to compare
> them lexicographically, use strcmp(), otherwise cast to number first.
> ("Don't allow" here could be a warning to start with.)
>

Regarding the last point, while I think that lexicographical comparison
with explicit < and > operators is pretty uncommon, sorting an array of
strings and expecting lexicographical order probably isn't unusual. While
SORT_STRING can be passed to enforce that, people probably expect that as
the default behavior. So just not allowing relational comparison is not a
great option either.

Nikita

Kamil Tekiela

5 years ago

I actually do a lot of lexicographical comparison with ISO8601 date
strings.
I also have numerical string comparisons, but the truth is I didn't think
they would be cast to integers so that is a potential bug in my code. When
I use < on strings I expect that they will both compare as strings. In
other places, I explicitly cast one or both operands to integers or float.
I do see the problem that many people could use < and > on strings
expecting numerical comparison, but I believe most people expect
lexicographical comparison like me.

Rowan Collins

5 years ago

On 04/03/2021 10:54, Nikita Popov wrote:

> The main one that comes to mind is something like '0' == '0.0'. However,
> the real problem is something else: Comparison behavior doesn't affect just
> == and !=, but also < and >. And I can see how people would want '2' < '10'
> to be true (numeric comparison) rather than false (lexicographical
> comparison).

That's a very good point, and I think the existence of the <=> makes
this even more complicated.


Considering your two options:

> 1. Decouple equality comparison from relational comparison. Don't handle
> numeric strings for == and !=, but do handle them for <, >, etc.

What would then be the result of '0' <=> '0.0'? Would the operator need
to special case the fact that they are numerically equal but
lexicographically unequal?

> 2. Don't allow relational comparison on strings. If you want to compare
> them lexicographically, use strcmp(), otherwise cast to number first.

This is easy to *implement* for the <=> operator, but makes it much less
useful. Part of the appeal of the operator is that you can write code
like $sortCallback = fn($a,$b) => $a[$sortField] <=> $b[$sortField];
without needing different cases for different data types.

Granted, that's not going to use an appropriate sorting collation for
many languages, but nor is strcmp().


I think further narrowing the definition of "numeric string" is a more
useful course. If we were designing from scratch, the straight-forward
definition would be:

- all digits: /^\d+$/
- or, all digits with leading hyphen-minus: /^-\d+$/
- or, at least one digit, a dot, and at least one more digit: /^\d+\.\d+$/
- or, as above, but with leading hyphen-minus: /^-\d+\.\d+$/

I think anything beyond that list needs to be carefully justified.

- Leading and trailing spaces are probably OK. Other whitespace
(newlines, tabs, etc) probably not.
- Alternative notations like hexadecimal and exponentials are easy to
have false positive matches, and how common are they in practice?
- Leading and trailing dots (".5", "1.") might be used sometimes, but
I'd probably lean against

So, ignoring BC concerns, I would be happy with "numeric string" defined
as "maybe space, maybe hyphen, some digits, maybe a dot and more digits,
maybe space", which I think in regex form looks like /^ *-?\d+(\.\d+)? *$/


Regards,

--
Rowan Tommins
[IMSoP]

Nikita Popov

5 years ago

On Thu, Mar 4, 2021 at 1:03 PM Rowan Tommins <rowan.collins@gmail.com>
wrote:

> On 04/03/2021 10:54, Nikita Popov wrote:
> > The main one that comes to mind is something like '0' == '0.0'. However,
> > the real problem is something else: Comparison behavior doesn't affect
> just
> > == and !=, but also < and >. And I can see how people would want '2' <
> '10'
> > to be true (numeric comparison) rather than false (lexicographical
> > comparison).
>
>
> That's a very good point, and I think the existence of the <=> makes
> this even more complicated.
>
>
> Considering your two options:
>
> > 1. Decouple equality comparison from relational comparison. Don't handle
> > numeric strings for == and !=, but do handle them for <, >, etc.
>
>
> What would then be the result of '0' <=> '0.0'? Would the operator need
> to special case the fact that they are numerically equal but
> lexicographically unequal?
>

Both '0' <=> '0.0' and '0.0' <=> '0' would return 1 in that case, which is
PHP's indication that values are non-comparable. It's definitely not a good
option.

> > 2. Don't allow relational comparison on strings. If you want to compare
> > them lexicographically, use strcmp(), otherwise cast to number first.
>
>
> This is easy to *implement* for the <=> operator, but makes it much less
> useful. Part of the appeal of the operator is that you can write code
> like $sortCallback = fn($a,$b) => $a[$sortField] <=> $b[$sortField];
> without needing different cases for different data types.
>
> Granted, that's not going to use an appropriate sorting collation for
> many languages, but nor is strcmp().
>
>
> I think further narrowing the definition of "numeric string" is a more
> useful course. If we were designing from scratch, the straight-forward
> definition would be:
>
> - all digits: /^\d+$/
> - or, all digits with leading hyphen-minus: /^-\d+$/
> - or, at least one digit, a dot, and at least one more digit: /^\d+\.\d+$/
> - or, as above, but with leading hyphen-minus: /^-\d+\.\d+$/
>
> I think anything beyond that list needs to be carefully justified.
>
> - Leading and trailing spaces are probably OK. Other whitespace
> (newlines, tabs, etc) probably not.
> - Alternative notations like hexadecimal and exponentials are easy to
> have false positive matches, and how common are they in practice?
> - Leading and trailing dots (".5", "1.") might be used sometimes, but
> I'd probably lean against
>
> So, ignoring BC concerns, I would be happy with "numeric string" defined
> as "maybe space, maybe hyphen, some digits, maybe a dot and more digits,
> maybe space", which I think in regex form looks like /^ *-?\d+(\.\d+)? *$/
>

A disadvantage of narrowing the definition in such a fashion is that it
introduces a discrepancy with (float) casts. I believe these currently
recognize the same values, with the exception that (float) discards
trailing garbage.

Another disadvantage is that exponential notation is commonly returned for
large numbers by various data source -- e.g. if you stored a large float in
a database, I'd expect you'd get it back in exponential notation (if you
get it back as a string). This means that your code could suddenly break
because the range of a value passes some heuristic threshold for how it
gets printed.

Regards,
Nikita

Rowan Collins

5 years ago

On 04/03/2021 13:17, Nikita Popov wrote:

>
> A disadvantage of narrowing the definition in such a fashion is that
> it introduces a discrepancy with (float) casts. I believe these
> currently recognize the same values, with the exception that (float)
> discards trailing garbage.

I don't think that's a big problem; as you say, explicit casts are
already more lax than implicit ones, and that's always going to be the
case in some sense, because they never "fail". Opinions may vary, though.

>
> Another disadvantage is that exponential notation is commonly returned
> for large numbers by various data source -- e.g. if you stored a large
> float in a database, I'd expect you'd get it back in exponential
> notation (if you get it back as a string). This means that your code
> could suddenly break because the range of a value passes some
> heuristic threshold for how it gets printed.

That may be a more compelling reason, at least given backwards
compatibility requirements. I don't know how common that is, but it
certainly sounds plausible.

Regards,

--
Rowan Tommins
[IMSoP]

Nikita Popov

5 years ago

On Wed, Mar 3, 2021 at 3:04 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

> Hi internals,
>
> PHP's == comparison semantics for strings have a peculiar edge-case, where
> comparisons of the form "0e123" == "0e456" return true, because they are
> interpreted as floating point zero numbers. This is problematic, because
> strings of that form are usually not numbers, but hex-encoded hashes or
> similar.
>
> I'm wondering if it may make sense to special-case the comparison
> semantics to not consider strings of the form "0e[DIGITS]" equal, unless
> they are exactly equal (i.e., fall back to lexicographical if both sides of
> the comparison are zero exponentials).
>
> Here's a possible implementation: https://github.com/php/php-src/pull/6749
>
> Of course, the usual rule that you should always use === still holds, but
> this at least eliminates the most dangerous edge case.
>

So, I gather the consensus here is that we should leave this alone as a
lost cause?

Regards,
Nikita