Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56755
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain akbkhome.com designates 202.81.246.113 as permitted sender)
Message-ID: <4EDB6DB0.2050909@akbkhome.com>
Date: Sun, 04 Dec 2011 20:55:12 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111124 Thunderbird/8.0
MIME-Version: 1.0
To: internals@lists.php.net
References: <4EDA4989.2010702@akbkhome.com>
In-Reply-To: <4EDA4989.2010702@akbkhome.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Fixing string offsets  of strings.
From: alan@akbkhome.com (Alan Knowles)

This is ready for review now.

https://bugs.php.net/patch-display.php?bug=60362&patch=fix_disabling_bad_string_offsets&revision=1323002696

This resolves the worst behavior changes introduced by the dereferencing 
of strings fix.
https://bugs.php.net/bug.php?id=60362

All tests (in Zend/tests) pass (after they have been modified to suit 
the change)

Please review / comment...

Regards
Alan


On Sunday, December 04, 2011 12:08 AM, Alan Knowles 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 
>
>
> 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 would appreciate if someone with better engine knowledge would have  
> a look and work out what the "BAD" usage should return.
>
> In theory.. the fetch_dim behavior should be return a empty string if 
> an invalid offset is used, or uninitialized zval if ISSET is calling it
>
> Regards
> Alan
>