Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56766
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.42 as permitted sender)
MIME-Version: 1.0
Sender: ekneuss@gmail.com
In-Reply-To: <CABBJUpfNRaa580ZCtsYHCbDat1Sxbo7tNz2MiNQ68foPFTq61g@mail.gmail.com>
References: <4EDA4989.2010702@akbkhome.com>
	<CAH-PCH76g4LwEwzaEvU3Hp7PeeyMUN4sc0uZExkedhHvYYnm9A@mail.gmail.com>
	<CABBJUpfNRaa580ZCtsYHCbDat1Sxbo7tNz2MiNQ68foPFTq61g@mail.gmail.com>
Date: Sun, 4 Dec 2011 17:39:30 +0100
Message-ID: <CACQesk4xNrm3VfreZX5NAQZ+DqDt5w6ecOKw70e=Yjn1EPeDag@mail.gmail.com>
To: Laruence <laruence@php.net>
Cc: Ferenc Kovacs <tyra3l@gmail.com>, Alan Knowles <alan@akbkhome.com>, 
	PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=0015173feb669d933204b346d856
Subject: Re: [PHP-DEV] Fixing string offsets of strings.
From: colder@php.net (Etienne Kneuss)

--0015173feb669d933204b346d856
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi,

On Sun, Dec 4, 2011 at 15:25, Laruence <laruence@php.net> wrote:

> +1.
>
> thanks.
>
> On Sun, Dec 4, 2011 at 10:05 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote:
> > On Sat, Dec 3, 2011 at 5:08 PM, Alan Knowles <alan@akbkhome.com> wrote:
> >
> >> I've had a look at making string offsets of strings a bit saner.
> >>
> >> At present with the fix for array dereferencing :  ?search=3Dhello and=
 a
> >> test like isset($_GET['search']['name'])  results in true, which is ha=
s
> >> 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_effor=
t_to_fix_this&revision=3Dlatest
> >
> >>
> >> It's been quite a while since I hacked on the engine, so the patch onl=
y
> >> works reasonably well.. (see the FIXME on the tests at the bottom of t=
he
> >> patch.)
> >>
> >> The patch changes the following:
> >>  * $s =3D "string";  $s['offset'] -- produces a warning (and returns a=
n
> >> empty string)
> >>  * $s =3D "string";  $s['1'] -- works as before..
> >>  * $s =3D "string";  $s[true] $s[false] $s[0.1] -- give a notice (cast=
 it
> to
> >> an int if you want to get rid of the notice) - however work as before.
> >>  * changes the warning on invalid indexes to say "Uninitialized or
> >> invalid" rather than just "Uninitialized"
> >>  * fixes most of the related tests
>

What about other edge cases like $string['   2   '], $string['2foo']?

I like the idea of the patch, I just find it a bit inconsistent for
$s['offset'] to return an empty string while other cases of implicit
conversions work as before. It doesn't bring much to return an empty string
instead of the first char. I believe every case should work as before,
throwing a notice is enough IMO.

Also, you don't mention whether your patch modifies the behavior of
isset(), is $str =3D "foo"; isset($str['bar'])  true or false ?

Best,


> >>
> >
> > 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  Xinchen Hui
> http://www.laruence.com/
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>


--=20
Etienne Kneuss
http://www.colder.ch

--0015173feb669d933204b346d856--