# [RFC] Warn about invalid strings in arithmetic (moving back to discussion)

- Thread: https://externals.io/message/90943
- Original archive: https://news-web.php.net/php.internals/90943
- Started by Andrea Faulds on 2016-01-26T21:19:22+00:00
- Messages: 10
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #90943 — Andrea Faulds — 2016-01-26T21:19:22+00:00

URL: https://externals.io/message/90943#90943

Good evening,

I had opened the vote for this RFC, but then I was reminded by a 
discussion in the pull request about a particular unresolved problem I 
had forgotten about.

In order to deal with that issue, I have cancelled the vote and expanded 
the RFC (both in length and scope) to deal with it.

The specific issue is detailed in a new section, "Fractional and 
scientific notation strings with integer operators", but it affects 
other sections as well.

So, the RFC is back under discussion.

Please read it and tell me your thoughts.

Thanks!
-- 
Andrea Faulds
https://ajf.me/

---

## #90944 — Andrea Faulds — 2016-01-26T21:36:06+00:00

URL: https://externals.io/message/90943#90944 · In reply to: #90943

...I proof-read my email, but I forgot the most important thing. The link!

https://wiki.php.net/rfc/invalid_strings_in_arithmetic

Sorry.

Andrea Faulds wrote:
> Good evening,
>
> I had opened the vote for this RFC, but then I was reminded by a
> discussion in the pull request about a particular unresolved problem I
> had forgotten about.
>
> In order to deal with that issue, I have cancelled the vote and expanded
> the RFC (both in length and scope) to deal with it.
>
> The specific issue is detailed in a new section, "Fractional and
> scientific notation strings with integer operators", but it affects
> other sections as well.
>
> So, the RFC is back under discussion.
>
> Please read it and tell me your thoughts.
>
> Thanks!


-- 
Andrea Faulds
https://ajf.me/

---

## #90952 — Levi Morrison — 2016-01-27T00:54:54+00:00

URL: https://externals.io/message/90943#90952 · In reply to: #90944

I personally consider the new changes with fractional and scientific
notation strings and integer operators to be the more important
change. Thanks to everyone who identified and fixed this
inconsistency.

Does anyone have any good ideas on how to prevent strtol and strtod
from being re-introduced to the repository? I could potentially see
this happening.


> ...I proof-read my email, but I forgot the most important thing. The link!
>
> https://wiki.php.net/rfc/invalid_strings_in_arithmetic
>
> Sorry.
>
>
> Andrea Faulds wrote:
> > […]
>
>
>
> --
> Andrea Faulds
> https://ajf.me/

---

## #90958 — Bishop Bettini — 2016-01-27T01:52:34+00:00

URL: https://externals.io/message/90943#90958 · In reply to: #90952

> I personally consider the new changes with fractional and scientific
> notation strings and integer operators to be the more important
> change. Thanks to everyone who identified and fixed this
> inconsistency.
>

Seconded, thanks all!


> Does anyone have any good ideas on how to prevent strtol and strtod
> from being re-introduced to the repository? I could potentially see
> this happening.


Obviously not portable, but somewhere high up we could:

#pragma GCC poison strtol strtod

---

## #90985 — Andrea Faulds — 2016-01-27T20:08:15+00:00

URL: https://externals.io/message/90943#90985 · In reply to: #90952

Hi Levi,

Levi Morrison wrote:
> I personally consider the new changes with fractional and scientific
> notation strings and integer operators to be the more important
> change. Thanks to everyone who identified and fixed this
> inconsistency.
>
> Does anyone have any good ideas on how to prevent strtol and strtod
> from being re-introduced to the repository? I could potentially see
> this happening.

They're the correct tools to use in some cases. We have a special class 
of numeric strings for array indexes, for example (which can't be fixed).

For the cases this RFC would fix, the tests would spot if the behaviour 
changed.

Thanks.

-- 
Andrea Faulds
https://ajf.me/

---

## #90988 — Michael Wallner — 2016-01-28T07:03:24+00:00

URL: https://externals.io/message/90943#90988 · In reply to: #90952

> I personally consider the new changes with fractional and scientific
> notation strings and integer operators to be the more important
> change. Thanks to everyone who identified and fixed this
> inconsistency.
> 
> Does anyone have any good ideas on how to prevent strtol and strtod
> from being re-introduced to the repository? I could potentially see
> this happening.
>

Pardon? There's no need to "ban" any standard functions, just make sure
you always use is_numeric_string when dealing with userland. It's
perfectly fine to use strtol, when you know what you're doing.
Unfortunately, it seems, we don't have a suitable place to put that notice.

Kudos to everyone discovering that.

-- 
Regards,
Mike

---

## #90989 — Zeev Suraski — 2016-01-28T09:25:37+00:00

URL: https://externals.io/message/90943#90989 · In reply to: #90944

> -----Original Message-----
> From: Andrea Faulds [mailto:ajf@ajf.me]
> Sent: Tuesday, January 26, 2016 11:36 PM
> To: internals@lists.php.net
> Subject: [PHP-DEV] Re: [RFC] Warn about invalid strings in arithmetic (moving
> back todiscussion)
> 
> ...I proof-read my email, but I forgot the most important thing. The link!
> 
> https://wiki.php.net/rfc/invalid_strings_in_arithmetic
> 
> Sorry.
> 
> Andrea Faulds wrote:
> > […]

Andrea,

I think this is a good proposal.  Slightly worried about the `E_NOTICE` for things like "5px" + "10px", which I think are probably not uncommon, and would have been better served by a separate E_STRICT, but that's no longer (practically) available.

Either way seems like a good idea.

Thanks,

Zeev

---

## #91054 — Andrea Faulds — 2016-02-02T01:09:06+00:00

URL: https://externals.io/message/90943#91054 · In reply to: #90943

Hi everyone,

There is one unresolved issue with the current patch that the RFC 
doesn't address, so I'll ask about it here.

As part of supporting exponent notation in all remaining integer 
operations (casts, operators), I would like to have `intval()` support it, 
to match the `(int)` cast.

For strings, `intval()` doesn't use the normal zval-to-long conversion 
functions (zval_get_long/convert_to_long), but instead uses strtol 
directly. This is so it can support multiple bases, e.g. intval("10", 
12) results in 12.

In order for `intval()` to support exponent notation, I'd have to change 
it to now call zval_get_long, but that function doesn't support 
non-decimal bases, nor does is_numeric_string_ex, which underlies it.

So, we'd either have to make `intval()` only support exponent notation for 
base 10, an unfortunate inconsistency, or we could not touch `intval()`, 
but then intval($a, 10) would no longer act the same as (int)$a.

I'm leaning towards the first choice, but I'd like to hear what the 
mailing list thinks. Either way we have a new inconsistency. But then, 
 `intval()` has unfortunate behaviour with its base parameter anyway, in 
that ignores the base for non-string input. That means that intval(123, 
8) and intval("123", 8) aren't equivalent, a violation of weak typing.

Anyway, please tell me your thoughts.

Thanks.

-- 
Andrea Faulds
https://ajf.me/

---

## #91082 — Andrea Faulds — 2016-02-05T10:49:56+00:00

URL: https://externals.io/message/90943#91082 · In reply to: #91054

Hi again,

Andrea Faulds wrote:
> There is one unresolved issue with the current patch that the RFC
> doesn't address, so I'll ask about it here.
>
> As part of supporting exponent notation in all remaining integer
> operations (casts, operators), I would like to have `intval()` support it,
> to match the `(int)` cast.
>
> For strings, `intval()` doesn't use the normal zval-to-long conversion
> functions (zval_get_long/convert_to_long), but instead uses strtol
> directly. This is so it can support multiple bases, e.g. intval("10",
> 12) results in 12.
>
> In order for `intval()` to support exponent notation, I'd have to change
> it to now call zval_get_long, but that function doesn't support
> non-decimal bases, nor does is_numeric_string_ex, which underlies it.
>
> So, we'd either have to make `intval()` only support exponent notation for
> base 10, an unfortunate inconsistency, or we could not touch `intval()`,
> but then intval($a, 10) would no longer act the same as (int)$a.
>
> I'm leaning towards the first choice, but I'd like to hear what the
> mailing list thinks. Either way we have a new inconsistency. But then,
> `intval()` has unfortunate behaviour with its base parameter anyway, in
> that ignores the base for non-string input. That means that intval(123,
> 8) and intval("123", 8) aren't equivalent, a violation of weak typing.
>
> Anyway, please tell me your thoughts.

There hasn't been any response to this so far on the mailing list, 
unfortunately. In discussions I've had elsewhere, making it apply to 
 `intval()` but only for base 10 has seemed to make sense. Both ways result 
in inconsistency.

So, I've updated the RFC to clarify that it does apply to `intval()`, but 
only where $base is 10, and also to clarify that `settype()` should be 
affected.

Thanks.

-- 
Andrea Faulds
https://ajf.me/

---

## #91083 — Michael Wallner — 2016-02-05T12:08:41+00:00

URL: https://externals.io/message/90943#91083 · In reply to: #91082

> 
> 
> Hi again,
> 
> Andrea Faulds wrote:
> > […]
> 
> There hasn't been any response to this so far on the mailing list, unfortunately. In discussions I've had elsewhere, making it apply to `intval()` but only for base 10 has seemed to make sense. Both ways result in inconsistency.
> 
> So, I've updated the RFC to clarify that it does apply to `intval()`, but only where $base is 10, and also to clarify that `settype()` should be affected.


To be honest, I had not even memorised `intval()` supports a base, but that we have `base_convert()`.

I’d opt for dropping base convert support from `intval()`, but that’s obviously not possible, because PHP-7.1 is the target for this RFC.

Cheers,
Mike