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 logic > 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 behavior.
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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
