Re: [PATCH] array_count_values bug: mishandling of numbers with leading whitespace

From: Date: Sun, 13 Aug 2006 12:08:26 +0000
Subject: Re: [PATCH] array_count_values bug: mishandling of numbers with leading whitespace
References: 1  Groups: php.internals 
Request: Send a blank email to internals+get-25338@lists.php.net to get a copy of this message
Hello, On 8/13/06, Matt W <php_lists@realplain.com> wrote:
Hi there, I didn't know whether to make an official bug report or send to the list -- so I'm trying this first. :-)
You already tried in the previous thread about is_numeric.
var_dump(array_count_values(array(' 001', 1, ' 1 ', '1'))); Expected result (and what PHP 4 gives): array(3) { [" 001"]=> int(1) [1]=> int(2) [" 1 "]=> int(1) } Actual result: array(2) { [1]=> int(3) [" 1 "]=> int(1) } Can this patch please be applied for 5.2's release? To maintain what I'm sure is the correct behavior from PHP 4, the function needs to use the zend_[u_]symtable* functions, which take care of *correctly* handling numeric strings, as zend_hash_[find|update] did in 4.x. If they would've been used in 5+ in the first place, there wouldn't have been bugs like #34723 (still present with leading whitespace, as you can see), #30833, #29808, etc. (I guess that's all, actually). :-) The updated code should be faster, too...
As I said in the previous discussion (why in the world have you splitted it out?), I don't think it is reasonable or safe to apply this patch in 5.2. Secondly it seems to wrong address the problem. You work around the actual is_numeric "problems" by suppressing its usage. Doing so reintroduce the numeric string keys problem (what addresses the removed code).
http://realplain.com/php/array_count_values_bug.diff http://realplain.com/php/array_count_values_bug_5_2.diff
I think it is much more safer to first determine the general policy for is_numeric and then address this problem. It is too late in the game to change such behaviors in 5.x (I care little about 4.x compatibility). Cheers, --Pierre

« previous php.internals (#25338) next »