Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116856
Message-ID: <0f503b90-c341-daf7-67a7-ee73598b1a7d@gmx.de>
Date: Mon, 10 Jan 2022 19:16:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.1
Content-Language: de-DE
To: Sara Golemon <pollita@php.net>, PHP internals <internals@lists.php.net>
References: <CAESVnVpdbUCBhCib873wyZQCb-+Oh13epHygmrqDRN_1nH0RTA@mail.gmail.com>
Cc: lbarnaud@php.net
In-Reply-To: <CAESVnVpdbUCBhCib873wyZQCb-+Oh13epHygmrqDRN_1nH0RTA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: zend_atol() and zend_atoi()
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 08.05.2019 at 20:58, Sara Golemon wrote:

> I fell down a WTF hole today that led me to zend_atol().
> The end result is the PR which I'd like to present for discussion (I'll =
add
> tests before I push anything, though it might necessitate a vote).
> https://github.com/php/php-src/pull/4132
>
> The issue is explained in the commit message, but I'll copy here:
>
> zend_ato[il] don't just do number parsing.
> They also check for a 'K', 'M', or 'G' at the end of the string,
> and multiply the parsed value out accordingly.
>
> Unfortunately, they ignore any other non-numerics between the
> numeric component and the last character in the string.
> This means that numbers such as the following are both valid
> and non-intuitive in their final output.
>
>    - "123KMG" is interpreted as "123G" -> 132070244352
>    - "123G " is interpreted as "123 " -> 123
>    - "123GB" is interpreted as "123B" -> 123
>    - "123 I like tacos." is also interpreted as "123." -> 123
>
> This diff primarily adds warnings for these cases when the output
> would be a potentially unexpected, and unhelpful value.
>
> Additionally, several places in php-src use these functions
> despite not actually wanting their KMG behavior such as
> session.upload_progress.freq which will happily parse "1 banana"
> as a valid value.
>
> For these settings, I've switched them to ZEND_STRTOL which preserves
> their existing /intended/ behavior.
>
>    - It won't respect KMG suffixes, but they never really wanted that lo=
gic
>    anyway.
>    - It will ignore non-numeric suffixes so as to not introduce new
>    failures.
>
> We should probably reexamine that second bullet point separately.
>
> Lastly, with these changes, zend_atoi() is no longer used in php-src,
> but I left it as a valid API for 3rd party extensions.
> Note that I did make it a proxy to zend_atol() since deferring the
> truncation till the end is effectively the same as truncation during
> multiplication, but this avoid code duplication.
>
> I think we should consider removing zend_atoi() entirely (perhaps in 8.0=
?)
> and rename zend_atol() to something reflecting it's KMG specific behavio=
r.

Arnaud Le Blanc grabbed that up, and submitted
<https://github.com/php/php-src/pull/7788> a while ago.  I'm generally
in favor of this PR; are there any objections to merging it?

Christoph