On Thu, Jan 5, 2012 at 12:50 PM, Rasmus Lerdorf <ras...@lerdorf.com> wrote:
> On 01/04/2012 08:13 PM, Laruence wrote:
>> On Thu, Jan 5, 2012 at 5:56 AM, Rasmus Lerdorf <ras...@lerdorf.com> wrote:
>>> On 01/04/2012 01:48 PM, Rasmus Lerdorf wrote:
>>>> On 01/04/2012 01:27 PM, Stas Malyshev wrote:
>>>>> Hi!
>>>>>
>>>>>> Right, like I said in my previous message, if this is caught by
>>>>>> display_start_errors, I am ok with it. We need the default/no php.ini
>>>>>> file case to not leak information like this.
>>>>>
>>>>> Just checked - it does not display error if display_startup_errors if
>>>>> off, does display if it's on.
>>>>
>>>> Right, that seems ok. The other thing is that we need to clarify that it
>>>> actually only limits the number of variables per nesting level. The
>>>> current name and the description doesn't make that clear. You can still
>>>> send 1M post vars in a single POST if you just nest them in a 1000x1000
>>>> 2d array. Of course, this is likely going to hit the post_max_size
>>>> limit, although many sites that do file uploads will have cranked that
>>>> way up.
>>>
>>> Oh, and a final issue to address.
>>>
>>> This code:
>>>
>>> for($data=[],$i=0; $i<=999; $i++) $data[$i] = range(0,1001);
>>> echo curl_post("http://localhost/index.php";,['a'=>$data]);
>>>
>>> will spew the warning 2000 times.
>>>
>>> & php post.php | grep Warning | wc -l
>>> 2000
>>>
>> could you try with this new patch:
>> https://bugs.php.net/patch-display.php?bug_id=60655&patch=max_input_vars.patch&revision=latest
>> ?
>>
>> this patch also restrict the json / serializer ,  all of them must
>> less than PG(max_input_vars).
>>
>> and different with the fix which was commited now,  this patch count
>> the num vars in a global scope, that means if there are 2 elements
>> which both have 500 elements in post,  the restriction will also
>> affect,
>>
>> and this patch will not affect the existsing called to json or
>> serializer,   only affect the zif_json_decode and zif_serialize,
>> after patch, the serialize will have a sencode parameter :"$max_vars".
>>
>> and the restriction can also be closed by set max_input_vars to 0.
>
> Right, I don't think this is going to work. A simple 'make install'
> after applying your patch fails with:
>
> Warning: unserialize(): Unserialized variables exceeded 1000 in
> phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
> on line 1145
>
> Warning: unserialize(): Unserialized variables exceeded 1000 in
> phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
> on line 1145
>
> Warning: unserialize(): Unserialized variables exceeded 1000 in
> phar:///home/rasmus/php-src/branches/PHP_5_4/pear/install-pear-nozlib.phar/PEAR/Registry.php
> on line 1145
> [PEAR] PEAR: pear.php.net/PEAR not installed
>
> I really don't think this manual per-feature limiting is going to cut it
> here. The real problem is the predictability of the hashing which we can
> address by seeding it with a random value. That part is easy enough, the
Hi:
  that will be a better fix, we have disscussed this before in irc,  I
also can have a try in that way.

  and for this fix(in such way), to be honest, yes, this is a little
ugly fix, but quick.

  the default 1000 is a little insufficient when counting the elements
in a global scope.

  IMO it should set to be 4096,  then I think 99% of developers will
not see this warning at all.

thanks

> hard part is figuring out where people may be storing these hashes
> externally and providing some way of associating the seed with these
> external caches so they will still work.
>
> -Rasmus



-- 
Laruence  Xinchen Hui
http://www.laruence.com/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to