Re: Fixing string offsets of strings.

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=hello and a > >> test like isset($_GET['search']['name']) results 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=60362&** > >> patch=first_effort_to_fix_**this&revision=latest< > ÍÆž<Ò¸‡ºª > ÍtNéhttps://bugs.php.net/patch-display.php?bug_id=60362&patch=first_effort_to_fix_this&revision=latest > > > >> > >> 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: > >> * $s = "string"; $s['offset'] -- produces a warning (and returns > >> an > >> empty string) > >> * $s = "string"; $s['1'] -- works as before.. > >> * $s = "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 = "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ács > > @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 > > -- Etienne Kneuss http://www.colder.ch