Re: [Request][Discussion] Double value as array key improvement
| From: | Andrew Nester | Date: | Sun, 13 Aug 2017 18:39:09 +0000 |
| Subject: | Re: [Request][Discussion] Double value as array key improvement | ||
| References: | 1 2 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-100205@lists.php.net to get a copy of this message | ||
> 11 авг. 2017 г., в 15:53, Andrew Nester <newaltgroup@bk.ru> написал(а):
>
>
>> On Aug 11, 2017, at 2:10 PM, Andrew Nester <newaltgroup@bk.ru> wrote:
>>
>> Hello everyone!
>>
>> I was working on following request
>> https://bugs.php.net/bug.php?id=75053 which resulted in following
>> pull request https://github.com/php/php-src/pull/2676
>>
>> The problem here is following: when we’re using large numbers as array index when adding
>> new elements it could overwrite already existing value.
>> Assume we have 2 indexes 5076964154930102272 and 999999999999999999999999999999 with
>> different value set for them.
>>
>> Because 999999999999999999999999999999 is larger than maximum long int number for 64-bit
>> systems, it will be converted to double. (corresponding code here
>> https://github.com/php/php-src/blob/master/Zend/zend_language_scanner.l#L1648)
>> But when double value is used as array indexes, it is converted to long integer. (f.e.,
>> code is here
>> https://github.com/php/php-src/blob/master/Zend/zend_execute.c#L1573)
>> At this case it causes overflow and we’ve got index equal to 5076964154930102272 and as a
>> result - we’re overwriting previously set value.
>>
>> My suggestion is following:
>> 1) when double key is less than maximum possible long integer - convert it to integer
>> 2) if it’s larger - convert it to string.
>>
>> That’s what implemented in proposed PR.
>>
>> Another possible option is just to throw warning in this case (proposed by Nikita Popov)
>>
>> I would happy to hear any feedback and suggestions about this solution.
>> Thanks!
>
> Here is the alternative solution which emits E_WARNING in case of integer array index overflow.
> https://github.com/php/php-src/pull/2677
My preferred solution is 2nd one (emitting warning) as it more obvious for users, doesn't break
previous behaviour.
Cheers,
Andrew