Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56777
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.170 as permitted sender)
MIME-Version: 1.0
Sender: laruence@gmail.com
In-Reply-To: <4EDC015C.3080701@akbkhome.com>
References: <4EDA4989.2010702@akbkhome.com> <CAH-PCH76g4LwEwzaEvU3Hp7PeeyMUN4sc0uZExkedhHvYYnm9A@mail.gmail.com>
 <CABBJUpfNRaa580ZCtsYHCbDat1Sxbo7tNz2MiNQ68foPFTq61g@mail.gmail.com>
 <CABBJUpe2tWo68hpPrfovhDtFYVczU3-XhDD8ynh0i5E=vUnuFg@mail.gmail.com> <4EDC015C.3080701@akbkhome.com>
Date: Mon, 5 Dec 2011 11:21:29 +0800
Message-ID: <CABBJUpd5srxLCJotQzdDK39vga1PhqbFcF1XF=D1eh2Rbjwr1w@mail.gmail.com>
To: Alan Knowles <alan@akbkhome.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Fixing string offsets of strings.
From: laruence@php.net (Laruence)

Hi:
   I think we can only trigger notice, then act the same behavior as
before. include isset.

   this would introduce the fewest bc breaks,

   what do you think?

thanks

On Mon, Dec 5, 2011 at 7:25 AM, Alan Knowles <alan@akbkhome.com> wrote:
> A few answers...
>
> $s =3D "string"; =C2=A0isset($s['offset']) returns false
> This is pretty critical, as it's the only way to detect this situation, a=
nd
> ensure that array tests do not return positive results for strings. It al=
so
> catches an obvious, but previously hidden and probably serious bugs in th=
e
> PHP code..
>
> $s =3D "string"; =C2=A0$s[' =C2=A02 =C2=A0']; and $s['2foo'] both emit er=
rors, and return
> false from isset() - the code pretty much uses is_numeric() internally. -
> I'm pretty sure these would be un-intentional bugs, so behaving as such
> seems consistent.
>
> I do not mind either way if $s['offset'] returns the first char or an emp=
ty
> string, however it seemed a little inconstant to return false on isset(),
> yet actually return a string. (although technically an empty string does
> kind of indicate it is 'isset') - it at least leaves the engine in a
> consistent state. Perhaps that change can wait's until 5.5.. thoughts...
>
> As for dropping the syntax for Strings eventually.. It would be nice, but
> I'm not sure it is feasible anymore unfortunately.
>
> Regards
> Alan
>
>
>
>
>
> On Monday, December 05, 2011 01:28 AM, Laruence wrote:
>>
>> Hi:
>> =C2=A0 =C2=A0 I have submit a new patch based on the origin patch, =C2=
=A0which only
>> trigger notice when string offset cast occurred.
>>
>> thanks
>>
>> On Sun, Dec 4, 2011 at 10:25 PM, Laruence<laruence@php.net> =C2=A0wrote:
>>>
>>> +1.
>>>
>>> thanks.
>>>
>>> On Sun, Dec 4, 2011 at 10:05 PM, Ferenc Kovacs<tyra3l@gmail.com> =C2=A0=
wrote:
>>>>
>>>> On Sat, Dec 3, 2011 at 5:08 PM, Alan Knowles<alan@akbkhome.com> =C2=A0=
wrote:
>>>>
>>>>> I've had a look at making string offsets of strings a bit saner.
>>>>>
>>>>> At present with the fix for array dereferencing : =C2=A0?search=3Dhel=
lo and a
>>>>> test like isset($_GET['search']['name']) =C2=A0results in true, which=
 is has
>>>>> potential security problems and is very confusing for any programmer
>>>>> finding 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
>>>>> going
>>>>> 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_effort_to_fix_**this&revision=3Dlatest<https://bugs.php=
.net/patch-display.php?bug_id=3D60362&patch=3Dfirst_effort_to_fix_this&revi=
sion=3Dlatest>
>>>>>
>>>>> It's been quite a while since I hacked on the engine, so the patch on=
ly
>>>>> works reasonably well.. (see the FIXME on the tests at the bottom of
>>>>> the
>>>>> patch.)
>>>>>
>>>>> The patch changes the following:
>>>>> =C2=A0* $s =3D "string"; =C2=A0$s['offset'] -- produces a warning (an=
d returns an
>>>>> empty string)
>>>>> =C2=A0* $s =3D "string"; =C2=A0$s['1'] -- works as before..
>>>>> =C2=A0* $s =3D "string"; =C2=A0$s[true] $s[false] $s[0.1] -- give a n=
otice (cast it
>>>>> to
>>>>> an int if you want to get rid of the notice) - however work as before=
.
>>>>> =C2=A0* changes the warning on invalid indexes to say "Uninitialized =
or
>>>>> invalid" rather than just "Uninitialized"
>>>>> =C2=A0* fixes most of the related tests
>>>>>
>>>> I think that those changes are pretty much in line with the discussion
>>>> that
>>>> we had.
>>>> Thanks for fixing this!
>>>>
>>>>
>>>> --
>>>> Ferenc Kov=C3=A1cs
>>>> @Tyr43l - http://tyrael.hu
>>>
>>>
>>>
>>> --
>>> Laruence =C2=A0Xinchen Hui
>>> http://www.laruence.com/
>>
>>
>>
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>



--=20
Laruence =C2=A0Xinchen Hui
http://www.laruence.com/