Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:43392
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Message-ID: <49C110B5.9030701@zend.com>
Date: Wed, 18 Mar 2009 18:18:13 +0300
User-Agent: Thunderbird 2.0.0.19 (X11/20090105)
MIME-Version: 1.0
To: Matt Wilmas <php_lists@realplain.com>
CC: internals@lists.php.net, Lukas Kahwe Smith <mls@pooteeweet.org>, 
 =?ISO-8859-1?Q?Johannes_Schl=FCter?= <johannes@php.net>
References: <1113CE12226949C2939A31971420991F@pc1> <49C0A7C7.8000804@zend.com> <12E613FAA1C9422B948F00B61AD32366@pc1>
In-Reply-To: <12E613FAA1C9422B948F00B61AD32366@pc1>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
From: dmitry@zend.com (Dmitry Stogov)

Hi Matt,

Matt Wilmas wrote:
> Hi Dmitry,
> 
> ----- Original Message -----
> From: "Dmitry Stogov"
> Sent: Wednesday, March 18, 2009
> 
>> BTW may be we should eliminate strtol() at all.
>> There's no need to scan the string twice.
> 
> Your change to remove strtol() [1] is not checking for overflow 
> correctly (for example, zend_u_strtol()'s checks are more complicated).  
> This breaks handling of keys above ULONG_MAX (it's correct again after 
> ULONG_MAX+LONG_MAX, until ULONG_MAX * 2, etc.).  See:
> 
> var_dump(array('5000000000' => 1));
> 
> array(1) {
>  [705032704]=>
>  int(1)
> }

Thank you for catching this.
I'll think a bit and then revert patch if don't find better solution.

> And of course the new code is a bit slower for keys that aren't fully 
> numeric, e.g. "123a"

Agree, but it's not a usual case, "1234" would occur much faster.

Thanks. Dmitry.

> [1] http://news.php.net/php.zend-engine.cvs/7465
> 
>> Dmitry.
> 
> - Matt
> 
>> Matt Wilmas wrote:
>>> Hi all,
>>>
>>> Assuming there are no objections, I'll commit this fix in a few hours...
>>>
>>> Besides the bug report(s), I had also found awhile ago that currently 
>>> an array key can be LONG_MAX or LONG_MIN as a string and/or integer 
>>> because of a check in ZEND_HANDLE_NUMERIC() (I assume to avoid a slow 
>>> errno check for ERANGE originally).  I changed it to use the *same 
>>> method* that's used in the scanner, is_numeric_string(), etc., and 
>>> those 2 values are now treated as an integer.
>>>
>>> It's just a few lines changed in zend_hash.h, although I had to move 
>>> some of the definitions from zend_operators.h to zend.h (is that OK?).
>>>
>>> Patches (didn't check HEAD's yet):
>>> http://realplain.com/php/array_key_limit.diff
>>> http://realplain.com/php/array_key_limit_5_3.diff
>>>
>>>
>>> - Matt 
>