One thing to note. The good test for what's being talked about would be simple:
isset($foo['bar'][1]['baz']) && is_array($foo['bar'][1])
You don't need to check each level. Only the one above the key you're
looking at.
I think it would be a good idea to raise a notice on string index
conversion ($string['foo']). I understand it is legacy behavior, but
given this bug fix, it would be a good way to identify these types of
"issues" people keep talking about.
Anthony
On Wed, Nov 23, 2011 at 8:44 PM, Ferenc Kovacs <tyr...@gmail.com> wrote:
> On Thu, Nov 24, 2011 at 1:38 AM, David Muir <davidkm...@gmail.com> wrote:
>
>> On 24/11/11 06:25, Stas Malyshev wrote:
>> > Hi!
>> >
>> >> The only case where the 5.4 branch works differently as before if you
>> >> reference a string type(int, float, etc. won't trigger this) variable
>> >> using an associative index and you expect it that to be undefined
>> >> variable even though that the documentation explicitly states that the
>> >
>> > Actually, the only change 5.4 did was to make $a['foo']['bar'] work
>> > like ($a['foo'])['bar'] - i.e. chained offsets work the same way as if
>> > they were applied separately. That's it. All the rest has been there
>> > since forever. I can't see how one could argue it should stay this way.
>> >
>> > Now I'm sorry somebody used the fact that chained offsets didn't work
>> > to do a check "do we have any strings there" but that's not how PHP is
>> > supposed to work and it's clearly a side-effect of a bug.
>>
>> One thing that I wasn't aware of was the string to int conversion for
>> string offsets. IMO, that should trigger a notice or something. Good to
>> be reminded of though.
>>
>> Just to clarify, the changes introduced in 5.4 will result in the
>> following:
>>
>> <?php
>>
>> $string = 'foo';
>> $array = array(
>> 'foo' => array(
>> 'bar' => 'baz',
>> //expected structure
>> //'bar' => array('baz' => array('values'))
>> ));
>>
>> var_dump(
>> isset($string['foo']), //true
>> isset($string[0][0]), //false, true in 5.4
>> isset($array['foo']['bar'][0]), //true
>> isset($array['foo']['bar']['baz']), //true
>> isset($array['foo']['bar']['baz']['0']) //false, true as of 5.4
>> isset($string['foo']['bar']['baz']['0']) //false, true as of 5.4
>> );
>>
>
> you are missing a comma from the end of the
> isset($array['foo']['bar']['baz']['0']) //false, true as of 5.4
> line
>
> isset($string['foo']['bar']['baz']['0']) //false, true as of 5.4
> gives me a fatal error on 5.3 ("PHP Fatal error: Cannot use string offset
> as an array" as you can't "chain" string offsets before 5.4)
>
>
>
>
>> If so, then it's a major BC break.
>> Maybe it's a bugfix, but it's a bugfix that allows for behaviour that
>> 1. doesn't really make sense
>>
>
> I think Stas and Etienne summed up the benefits and the reason of this
> change pretty much(mostly internals related).
>
>
>> 2. has little to no practical benefit (when are you ever going to be
>> chaining offsets on strings?)
>>
>
> I agree that there are not (m)any usecase for the ability for method
> chaining in it's current form.
>
>
>> 3. introduces new, hard-to-detect bugs in live code.
>>
> the bugs are there already(you expect an array where you get a string), it
> will just fail differently.
>
>
>
>>
>> What used to be a one-liner, now effectively needs a crapload of
>> boilerplate code.
>>
>
> yeah, if you can't trust the data structures in your code, then you have to
> validate it.
> don't forget that this bug could also bite you with 5.3:
>
> // you expect $foo to be an array
> $foo = 'string';
>
> //this will echo 's'
> echo $foo['bar'];
>
> Personally I think that the main issue is that we silently convert
> $foo['bar'] to $foo[0] for strings, which imo rarely the intended behavior
> and doesn't really makes much sense (except that the offset expects the
> index to be int, and this is how we type juggle between string and int), so
> I think it would be a good idea to raise a notice here, so the developer
> can fix his code, or add some input validation.
>
> --
> Ferenc Kovács
> @Tyr43l - http://tyrael.hu
>
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
