Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55576
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: <4E7A763F.1080302@akbkhome.com>
Date: Thu, 22 Sep 2011 07:41:51 +0800
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.21) Gecko/20110831 Thunderbird/3.1.13
MIME-Version: 1.0
To: internals@lists.php.net
References: <4E790B82.6090805@akbkhome.com> <8C.A0.17510.E4DE97E4@pb1.pair.com> <1316615094.2810.5.camel@guybrush> <CAEZPtU51Hk+kmLhNqnCjDbRuW2t7QUumQX2qotPKLbeJkD1rAA@mail.gmail.com> <1316629502-sup-831@fewbar.com>
In-Reply-To: <1316629502-sup-831@fewbar.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Re: is_a() - again - a better fix
From: alan@akbkhome.com (Alan Knowles)


To clarify

  * Code changed to work around this change will not break if it is 
reverted.
    Basically it is to add is_object() before any call to is_a()

  * If left as is, there is reasonable potential for remote exploits in 
many codebases.

  * This change is not really in the wild yet, as people do not upgrade 
that fast, and as mentioned distro's are avoiding upgrading due to the 
rather serious consequences.

  * This code affects anyone using the autoloader (PSR or other) with 
any existing codebase (for example Joomla will probably break if used 
with an autoloader)

  * The 'revert' BC break will only affect someone who
    a) uses undocumented behaviors which are only known about due to 
reading this list.
    b) has written code specifically targeting this controversial 
behavior in the last few weeks.

  * Nobody who has argued for keeping this change has made any commits 
or bug reports to any package (as far as I know) to help fix code 
affected by the change.

  * is_a() in it's previous usage was 'useful', and resulted in clear 
consise code. now practically every use of the code has to be prefixed 
with is_object(). This is wastefull and pointless, and more prone to 
errors.


Saying that  'breaking BC again' should not be done is an utterly bogus 
argument. Please understand the issue properly before coming up with 
such a ridiculous comment. The point of 'not breaking BC' is to stop 
breaking existing code which has been running for years, not breaking 
code that has been created 10 minutes ago. Please be realistic here.

The fix is trivial and it is either to
a) add an extra argument to is_a() - 3rd argument true to autoload & use 
a string
   This I considered to be an unclear way of providing this feature..

b) create a new method
    is_class_of() - as the related method is is_subclass_of()
    instance_of() - might be a little better?
    class_compare() = 0(no) 1(same) 2(instanceof)


Please note that the new behaviour is roughly equivalent to this.
$cna = is_object($a) ? get_class($a) : $a;
$cnb = is_object($b) ? get_class($b) : $b;
if ($cna == $cnb) || is_subclass_of($a,$b) ) { }

It's main usage is really to use with strings so this would work for 
anyone who needs the new feature.
if ($a == $b) || is_subclass_of($a,$b) ) { }


Please think of good names, and let's close this bug sensibly.

Regards
Alan


On Thursday, September 22, 2011 02:31 AM, Clint Byrum wrote:
> Excerpts from Pierre Joye's message of Wed Sep 21 08:01:48 -0700 2011:
>> 2011/9/21 Johannes Schlüter<johannes@schlueters.de>:
>>
>>> Exactly. (while I, at this time, won't argue which behavior is more
>>> "correct") changing this in the first place was wrong. Changing it back
>>> is wrong again. We have two versions out with this change. These
>>> releases reach distributions, reach hosting companies, reach developer
>>> machines, ... changing the behavior again causes more trouble. With a
>>> proper heads up before 5.3.8 we might probably have reverted it there.
>> I agree and I seriously hope that we will stop to do such things from
>> now on and apply the RFC to 5.3 as well.
> Just to give some perspective on this, we specifically did not ship php
> 5.3.8 in Ubuntu 11.10 (in beta right now) because of this confusion.
> This is in spite of the fact that it would have closed many bugs, and
> reduced some of the burden on our security team since they are charged
> with back-porting all security fixes to whatever we ship. I was kind of
> hoping it would be rolled back rapidly in another release.
>
>  From a distribution standpoint, the change in 5.3.8 is a nightmare
> and will require us to either ship with lots of bugs in the dependent
> packages, or lots of patches and delta from upstream. Reverting back to
> the pre 5.3.8 behavior would allow us to ship the new version and move
> forward with future versions without worry.
>
> One question.. if things have been adapted for 5.3.8 .. will they be
> negatively affected by a rollback to pre 5.3.8 behavior? If not, I'd
> wonder why there would be any resistance at all to doing this, as it
> seems more important that 5.3 is consistent from early versions to the
> latest than it is that one version had a serious problem.
>