Re: 5.4 regression: non-existent sub-sub keys now have values

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 <tyra3l@gmail.com> wrote: > On Thu, Nov 24, 2011 at 1:38 AM, David Muir <davidkmuir@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 >