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 <a...@akbkhome.com> wrote: > A few answers... > > $s = "string"; isset($s['offset']) returns false > This is pretty critical, as it's the only way to detect this situation, and > ensure that array tests do not return positive results for strings. It also > catches an obvious, but previously hidden and probably serious bugs in the > PHP code.. > > $s = "string"; $s[' 2 ']; and $s['2foo'] both emit errors, 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 empty > 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: >> I have submit a new patch based on the origin patch, which only >> trigger notice when string offset cast occurred. >> >> thanks >> >> On Sun, Dec 4, 2011 at 10:25 PM, Laruence<larue...@php.net> wrote: >>> >>> +1. >>> >>> thanks. >>> >>> On Sun, Dec 4, 2011 at 10:05 PM, Ferenc Kovacs<tyr...@gmail.com> wrote: >>>> >>>> On Sat, Dec 3, 2011 at 5:08 PM, Alan Knowles<a...@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<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 >>>>> >>>> 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 > -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
