On Mon, Dec 5, 2011 at 12:39 AM, Etienne Kneuss <[email protected]> wrote:
> Hi,
>
> On Sun, Dec 4, 2011 at 15:25, Laruence <[email protected]> wrote:
>>
>> +1.
>>
>> thanks.
>>
>> On Sun, Dec 4, 2011 at 10:05 PM, Ferenc Kovacs <[email protected]> wrote:
>> > On Sat, Dec 3, 2011 at 5:08 PM, Alan Knowles <[email protected]> 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
>
>
> 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.
I agree after a deep think, a notice is enough. thanks
>
> 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
--
Laruence Xinchen Hui
http://www.laruence.com/