Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56756
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain prohost.org designates 209.85.215.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <4EDB6DB0.2050909@akbkhome.com>
References: <4EDA4989.2010702@akbkhome.com>
	<4EDB6DB0.2050909@akbkhome.com>
Date: Sun, 4 Dec 2011 08:28:29 -0500
Message-ID: <CAJy+15zQgc_gW62XZLoR54nnw9T5ShNNq_=v3Cz5Uoc9ZmynUA@mail.gmail.com>
To: Alan Knowles <alan@akbkhome.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Fixing string offsets of strings.
From: ilia@prohost.org (Ilia Alshanetsky)

Seems like a good change +1.

On Sun, Dec 4, 2011 at 7:55 AM, Alan Knowles <alan@akbkhome.com> wrote:
> This is ready for review now.
>
> https://bugs.php.net/patch-display.php?bug=3D60362&patch=3Dfix_disabling_=
bad_string_offsets&revision=3D1323002696
>
> This resolves the worst behavior changes introduced by the dereferencing =
of
> strings fix.
> https://bugs.php.net/bug.php?id=3D60362
>
> All tests (in Zend/tests) pass (after they have been modified to suit the
> change)
>
> Please review / comment...
>
> Regards
> Alan
>
>
>
> On Sunday, December 04, 2011 12:08 AM, Alan Knowles wrote:
>>
>> I've had a look at making string offsets of strings a bit saner.
>>
>> At present with the fix for array dereferencing : =A0?search=3Dhello and=
 a
>> test like isset($_GET['search']['name']) =A0results in true, which is ha=
s
>> potential security problems and is very confusing for any programmer fin=
ding
>> and working out why something like that may be failing.
>>
>> To solve this quite a few people agreed that not allowing non-numeric
>> string offsets on strings would be the smart way to go, the change is go=
ing
>> to break BC, so the idea is to at least not break it too badly...
>>
>> This patch is a start.
>>
>> https://bugs.php.net/patch-display.php?bug_id=3D60362&patch=3Dfirst_effo=
rt_to_fix_this&revision=3Dlatest
>>
>> It's been quite a while since I hacked on the engine, so the patch only
>> works reasonably well.. (see the FIXME on the tests at the bottom of the
>> patch.)
>>
>> The patch changes the following:
>> =A0* $s =3D "string"; =A0$s['offset'] -- produces a warning (and returns=
 an
>> empty string)
>> =A0* $s =3D "string"; =A0$s['1'] -- works as before..
>> =A0* $s =3D "string"; =A0$s[true] $s[false] $s[0.1] -- give a notice (ca=
st it to
>> an int if you want to get rid of the notice) - however work as before.
>> =A0* changes the warning on invalid indexes to say "Uninitialized or
>> invalid" rather than just "Uninitialized"
>> =A0* fixes most of the related tests
>>
>> I would appreciate if someone with better engine knowledge would have =
=A0a
>> look and work out what the "BAD" usage should return.
>>
>> In theory.. the fetch_dim behavior should be return a empty string if an
>> invalid offset is used, or uninitialized zval if ISSET is calling it
>>
>> Regards
>> Alan
>>
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>