Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:75046
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.181 as permitted sender)
MIME-Version: 1.0
Sender: julienpauli@gmail.com
In-Reply-To: <CAH-PCH7Bt0vtmOCG99=jqHgi_A-4YFD0Ndba91rMfi9+-w+aRg@mail.gmail.com>
References: <CAH-PCH6xEXspisLOHtiTbViA4vDTk7q6LM3t_Bh9Dzr1cj_Nag@mail.gmail.com>
 <53A1C722.9060501@fedoraproject.org> <53A21137.6010705@sugarcrm.com>
 <CAMUwpuT_S2EE6cdJM95RzY4vmORGx3vJ9+gau8G3EZCMsNSydA@mail.gmail.com>
 <53A2A9BD.1070603@sugarcrm.com> <CAH-PCH5-vtCVvGvceMqt6rDXq4zo0Vb34cyDK3KdBUNfg_oLyg@mail.gmail.com>
 <CADyq6sKQbV+Avzfs3OAdGM9Hc85i18t0LSATD6Rk=HjqKOMgBg@mail.gmail.com>
 <53A3874E.20704@sugarcrm.com> <CAH-PCH6nSk0ir_TSjhCd0BVhiY5UsOsLKzqKhqfaU1G0ghi4Tg@mail.gmail.com>
 <53A65578.6000701@sugarcrm.com> <CAH-PCH7tw0=OacG2HFDvBZxAho0HjdOtpLR=J9aXYGxsv40ehA@mail.gmail.com>
 <53A772D0.3010404@sugarcrm.com> <CAMUwpuR7YPnR6jv+0OfOO5J59vcjBdNJT6e03KHDiJtiGiA6KA@mail.gmail.com>
 <CAH-PCH7Bt0vtmOCG99=jqHgi_A-4YFD0Ndba91rMfi9+-w+aRg@mail.gmail.com>
Date: Mon, 23 Jun 2014 10:55:42 +0200
Message-ID: <CAMUwpuTk6MNCh_pC+UoYQp-vD3AUetCK9qHuHSGymC8tm60HHw@mail.gmail.com>
To: Ferenc Kovacs <tyra3l@gmail.com>
Cc: Stas Malyshev <smalyshev@sugarcrm.com>, Sebastian Bergmann <sebastian@php.net>, 
	Remi Collet <remi@fedoraproject.org>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: Bug 67072 resolution for 5.4/5.5
From: jpauli@php.net (Julien Pauli)

On Mon, Jun 23, 2014 at 10:07 AM, Ferenc Kovacs <tyra3l@gmail.com> wrote:
>
>
>
> On Mon, Jun 23, 2014 at 9:54 AM, Julien Pauli <jpauli@php.net> wrote:
>>
>> On Mon, Jun 23, 2014 at 2:20 AM, Stas Malyshev <smalyshev@sugarcrm.com>
>> wrote:
>> > Hi!
>> >
>> >> for the issue to materialize you need to feed hand-crafted input to
>> >> unserialize,
>> >
>> > True.
>> >
>> >> anybody doing that with user controlled data already asking
>> >> for problems,
>> >
>> > True in theory, in practice this is widely and commonly done.
>> >
>> >> I prefer this over what we have in 5.4/5.5 and given how few classes
>> >> does 1, actually mean, I think it would be an acceptable compromise,
>> >> but
>> >> let's hear what others think.
>> >
>> > Cool, waiting for others to chime in.
>> >
>> >> ps: I've seen that you created a pull request with the patch, if
>> >> somebody don't wanna copypaste the patch from the mail, here it is:
>> >> https://github.com/php/php-src/pull/701
>> >
>> > Yes, thanks for quoting it, it seems to be green on Travis and phpunit
>> > also seems to work fine with it. I also added a unit tests with a couple
>> > of cases to see how it's supposed to work.
>> >
>> > --
>> > Stanislav Malyshev, Software Architect
>> > SugarCRM: http://www.sugarcrm.com/
>> > (408)454-6900 ext. 227
>>
>>
>> Hello,
>>
>> I find the compromise nice.
>> The goal is to have something barely working in most use cases for 5.4
>> and 5.5, and prepare something nicer and stronger for 5.6.
>>
>> So, the proposed patch ( Stas' ) is nice for this, as comon tools still
>> work.
>>
>> I'm also ok for the 5.6 statements :
>> - Disalow O: for classes with custom serializer
>> - Unlock newInstanceArgWithoutConstructor() for internal classes
>>
>> Note that unlocking newInstanceArgWithoutConstructor() for internal
>> classes may require lot of work.
>> Remi already tried to patch some extensions for them to work AFAIR.
>
>
> and maybe not even possible to fix all those cases, yet we already have the
> same problem with:
> MyClass extends InternalClassDependingOnConstructor {
>   public function __construct(){
>     //not calling parent::__construct
>   }
> }
>
> so that shouldn't be a blocker for enabling internal classes for
> newInstanceWithoutConstructor
> but I would discuss this separately/later, as the 5.4/5.5 decision/fix is a
> bit more urgent.

Yes, for 5.4 and 5.5 , Stas' patch looks right to me.

Julien