Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56551
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAH-PCH7GCDOdnHve-mNdkPYbBN+WGLJN2BP8pdpXmVz=0FQ0mg@mail.gmail.com>
References: <20111123015008.GA12933@panix.com>
	<CACQesk6UuGUgyJz3hsGVE0crJx3GCkjTTpLb4xNGTm1goLzO8Q@mail.gmail.com>
	<20111123023108.GA172@panix.com>
	<CACQesk5kj=4kzvTDiLL1oOLBXZrn4WdUcLXdbyMLTa=OdhGTcw@mail.gmail.com>
	<4ECCB549.904@lsces.co.uk>
	<4ECCBC56.3050602@sugarcrm.com>
	<20111123141408.GA11940@panix.com>
	<4ECD40E2.1000601@sugarcrm.com>
	<CAH-PCH6f30p6AfNT9z=wVPURo4ko08sMTwHiEQsdpauN-cYtwQ@mail.gmail.com>
	<4ECD48AD.1020207@sugarcrm.com>
	<4ECD91F4.5040303@gmail.com>
	<CAH-PCH7GCDOdnHve-mNdkPYbBN+WGLJN2BP8pdpXmVz=0FQ0mg@mail.gmail.com>
Date: Wed, 23 Nov 2011 21:30:30 -0500
Message-ID: <CAAyV7nEcXNZGUFLJzUMp_miGMba1ZujWi6+zKWCm43JUYhqW1A@mail.gmail.com>
To: Ferenc Kovacs <tyra3l@gmail.com>
Cc: David Muir <davidkmuir@gmail.com>, Stas Malyshev <smalyshev@sugarcrm.com>, 
	Daniel Convissor <danielc@analysisandsolutions.com>, Lester Caine <lester@lsces.co.uk>, 
	PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] 5.4 regression: non-existent sub-sub keys now have values
From: ircmaxell@gmail.com (Anthony Ferrara)

One thing to note.  The good test for what's being talked about would be si=
mple:

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 th=
e
>> >
>> > 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 =3D 'foo';
>> $array =3D array(
>> =A0 =A0'foo' =3D> array(
>> =A0 =A0 =A0 =A0'bar' =3D> 'baz',
>> =A0 =A0 =A0 =A0//expected structure
>> =A0 =A0 =A0 =A0//'bar' =3D> array('baz' =3D> array('values'))
>> ));
>>
>> var_dump(
>> =A0 =A0isset($string['foo']), //true
>> =A0 =A0isset($string[0][0]), //false, true in 5.4
>> =A0 =A0isset($array['foo']['bar'][0]), //true
>> =A0 =A0isset($array['foo']['bar']['baz']), //true
>> =A0 =A0isset($array['foo']['bar']['baz']['0']) //false, true as of 5.4
>> =A0 =A0isset($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: =A0Cannot use string off=
set
> 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), i=
t
> 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 =3D '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 behavio=
r
> 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=E1cs
> @Tyr43l - http://tyrael.hu
>