Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:56778 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 30057 invoked from network); 5 Dec 2011 03:52:31 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 5 Dec 2011 03:52:31 -0000 Authentication-Results: pb1.pair.com smtp.mail=alan@akbkhome.com; spf=pass; sender-id=pass Authentication-Results: pb1.pair.com header.from=alan@akbkhome.com; sender-id=pass Received-SPF: pass (pb1.pair.com: domain akbkhome.com designates 202.81.246.113 as permitted sender) X-PHP-List-Original-Sender: alan@akbkhome.com X-Host-Fingerprint: 202.81.246.113 246-113.netfront.net Received: from [202.81.246.113] ([202.81.246.113:55560] helo=246-113.netfront.net) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id CD/11-19375-CFF3CDE4 for ; Sun, 04 Dec 2011 22:52:29 -0500 Received: from 154.126.198.203.static.netvigator.com ([203.198.126.154] helo=[192.168.1.10]) by akbkhome.com with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Mailfort v1.2) (envelope-from ) id 1RXPbM-0002l1-DI for internals@lists.php.net; Mon, 05 Dec 2011 11:52:24 +0800 Message-ID: <4EDC3FEE.90305@akbkhome.com> Date: Mon, 05 Dec 2011 11:52:14 +0800 User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.21) Gecko/20110831 Icedove/3.1.13 MIME-Version: 1.0 To: internals@lists.php.net References: <4EDA4989.2010702@akbkhome.com> <4EDC015C.3080701@akbkhome.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-mailfort-sig: d07d2eef5d15c1684354921cb5847882 Subject: Re: [PHP-DEV] Fixing string offsets of strings. From: alan@akbkhome.com (Alan Knowles) On Monday, December 05, 2011 11:21 AM, Laruence wrote: > Hi: > I think we can only trigger notice, then act the same behavior as > before. include isset. > > this would introduce the fewest bc breaks, I think the isset behavior should be fixed (as the BC will be broken anyway with deferenced strings), for the other fix, If it was my project (haha), I'd release the empty string change, and revert it quickly in the next release if there were any serious bug reports saying working code was affected by it. Not sure if that's a great approach, but I'd be interested in hearing opinions about it. Regards Alan > what do you think? > > thanks > > On Mon, Dec 5, 2011 at 7:25 AM, Alan Knowles 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 wrote: >>>> +1. >>>> >>>> thanks. >>>> >>>> On Sun, Dec 4, 2011 at 10:05 PM, Ferenc Kovacs wrote: >>>>> On Sat, Dec 3, 2011 at 5:08 PM, 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 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 >> > >