Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:43391
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 209.151.69.1 cause and error)
Message-ID: <12E613FAA1C9422B948F00B61AD32366@pc1>
To: <internals@lists.php.net>,
	"Dmitry Stogov" <dmitry@zend.com>
Cc: "Lukas Kahwe Smith" <mls@pooteeweet.org>,
	=?iso-8859-1?Q?Johannes_Schl=FCter?= <johannes@php.net>
References: <1113CE12226949C2939A31971420991F@pc1> <49C0A7C7.8000804@zend.com>
Date: Wed, 18 Mar 2009 10:05:18 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=response
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
From: php_lists@realplain.com ("Matt Wilmas")

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)
}

And of course the new code is a bit slower for keys that aren't fully 
numeric, e.g. "123a"

[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