zend_std_has_dimension() doesn't know what (check_empty == 2) means.
check_empty == 0 - ISSET => we don't need to call offsetGet() and relay on
offsetExists() return value.
check_empty == 1 - ISEMPTY => we should call offsetGet() after
offsetExists().
NULL values should be handled by offsetExists().
Thanks. Dmitry.
On Tue, May 6, 2014 at 4:36 PM, Tjerk Anne Meesters <datibbaw@php.net>wrote:
>
>
> On Tue, May 6, 2014 at 7:50 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>
>> Ah, I didn't get what did you mean by (check_empty == 2), but they
>> probably should be changed into (check_empty != 1)
>>
>
> When I said mostly, perhaps I should have mentioned that the isset()
> behaviour was deliberately updated as well; in other words, the following
> works as expected:
>
> $x['foo'] = null;
> isset($x['foo']); // false
>
> After taking a closer look at zend_std_has_dimension() it seems that both
> isset() and empty() behave as if empty() was always used; of course, if
> this behaviour was translated into spl_array.c, the infinite recursion
> would still be an issue. So, at this point I'm unsure what the proper
> action point should be.
>
>
>>
>>
>> It fixes the problem.
>> Could you please verify and commit into all relevant branches.
>>
>> Thanks. Dmitry.
>>
>> --- a/ext/spl/spl_array.c
>> +++ b/ext/spl/spl_array.c
>> @@ -603,7 +603,7 @@ static int spl_array_has_dimension_ex(int
>> check_inherited, zval *object, zval *o
>>
>> if (rv && zend_is_true(rv TSRMLS_CC)) {
>> zval_ptr_dtor(&rv);
>> - if (check_empty == 2) {
>> + if (check_empty != 1) {
>> return 1;
>> } else if (intern->fptr_offset_get) {
>> value = spl_array_read_dimension_ex(1,
>> object, offset, BP_VAR_R TSRMLS_CC);
>> @@ -622,7 +622,7 @@ static int spl_array_has_dimension_ex(int
>> check_inherited, zval *object, zval *o
>> switch(Z_TYPE_P(offset)) {
>> case IS_STRING:
>> if (zend_symtable_find(ht,
>> Z_STRVAL_P(offset), Z_STRLEN_P(offset)+1, (void **) &tmp) != FAILURE) {
>> - if (check_empty == 2) {
>> + if (check_empty != 1) {
>> return 1;
>> }
>> } else {
>> @@ -639,7 +639,7 @@ static int spl_array_has_dimension_ex(int
>> check_inherited, zval *object, zval *o
>> index = Z_LVAL_P(offset);
>> }
>> if (zend_hash_index_find(ht, index, (void
>> **)&tmp) != FAILURE) {
>> - if (check_empty == 2) {
>> + if (check_empty != 1) {
>> return 1;
>> }
>> } else {
>>
>>
>>
>> On Tue, May 6, 2014 at 3:29 PM, Tjerk Anne Meesters <datibbaw@php.net>wrote:
>>
>>> Hi,
>>>
>>>
>>> On Tue, May 6, 2014 at 7:25 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>>>
>>>> I didn't review you patch careful, but why do you need to call
>>>> offsetGet() when offsetExists() must be enough?
>>>> is it for empty()?
>>>>
>>>
>>> Mostly for empty(), yes. It fixes the expected empty() behaviour for
>>> $a['foo'] == 0 when using ArrayObject. This is further explained
>>> in this
>>> bug report: https://bugs.php.net/bug.php?id=66834
>>>
>>>
>>>> Then you probably should implement it a way similar to
>>>> zend_std_has_dimension() in Zend/zend_object_handlers.c.
>>>> call offsetExists() and then offsetGet() if necessary.
>>>>
>>>
>>> That's already the case :)
>>>
>>> http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_object_handlers.c#719
>>>
>>>
>>>>
>>>> Thanks. Dmitry.
>>>>
>>>>
>>>> On Tue, May 6, 2014 at 2:31 PM, Tjerk Anne Meesters <datibbaw@php.net>wrote:
>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>> On Tue, May 6, 2014 at 4:11 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>>>>>
>>>>>> Hi Tjerk,
>>>>>>
>>>>>> your commit broke the code that worked fine before (still works in
>>>>>> 5.5 but broken in 5.6 and above).
>>>>>> It leads into infinity recursion until stack overflow.
>>>>>>
>>>>>> It must be fixed or reverted.
>>>>>>
>>>>>
>>>>> This has been mentioned by Jakub before and a fix to ZF2 has already
>>>>> been merged:
>>>>>
>>>>> https://github.com/zendframework/zf2/pull/6096
>>>>>
>>>>> The previous code and my patch basically cannot coexist; it used to
>>>>> work in 5.6 before, but only by the "virtue" of an unfortunate
>>>>> implementation.
>>>>>
>>>>> I believe this is not the only 5.6 issue that ZF2 is dealing with, but
>>>>> if you feel that this breaks too many things for a 5.x release I suppose we
>>>>> can revert it in PHP-5.6 and keep it for PHP-6?
>>>>>
>>>>> Let me know.
>>>>>
>>>>>
>>>>>> Thanks. Dmitry.
>>>>>>
>>>>>> <?php
>>>>>> class Parameters extends ArrayObject {
>>>>>> public function __construct(array $values = null) {
>>>>>> if (null === $values) {
>>>>>> $values = array();
>>>>>> }
>>>>>> parent::__construct($values, ArrayObject::ARRAY_AS_PROPS);
>>>>>> }
>>>>>> public function offsetGet($name) {
>>>>>> if (isset($this[$name])) {
>>>>>> return parent::offsetGet($name);
>>>>>> }
>>>>>> return null;
>>>>>> }
>>>>>> }
>>>>>> $x = new Parameters();
>>>>>> var_dump($x['foo']);
>>>>>> $x['foo'] = 'bar';
>>>>>> var_dump($x['foo']);
>>>>>> ?>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> --
>>>>> Tjerk
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> --
>>> Tjerk
>>>
>>
>>
>
>
> --
> --
> Tjerk
>