Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int

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