Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:75833
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.45 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <F5324819-A85F-47BA-8FE5-27FBE87E1B89@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>
	<53A62AFF.4080302@sugarcrm.com>
	<CADyq6s+N_mNc-n1g=S5LwWBZ9fkZ+hqRc3Ycu8M7EBd9Hc1ABw@mail.gmail.com>
	<53B10D59.4060206@sugarcrm.com>
	<F5324819-A85F-47BA-8FE5-27FBE87E1B89@gmail.com>
Date: Tue, 22 Jul 2014 12:27:38 +0200
Message-ID: <CAH-PCH6Tb-QbTaB3_qthpiGo4fcs0afViDhACmcur=vZ+=HnFg@mail.gmail.com>
To: Alexey Zakhlestin <indeyets@gmail.com>
Cc: Stas Malyshev <smalyshev@sugarcrm.com>, Marco Pivetta <ocramius@gmail.com>, 
	Sebastian Bergmann <sebastian@php.net>, Julien Pauli <jpauli@php.net>, Remi Collet <remi@fedoraproject.org>, 
	PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11398720418eb104fec5acb8
Subject: Re: [PHP-DEV] Problems with the fix for the BC break introduced in
 5.4.29 and 5.5.13
From: tyra3l@gmail.com (Ferenc Kovacs)

--001a11398720418eb104fec5acb8
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 30, 2014 at 8:10 PM, Alexey Zakhlestin <indeyets@gmail.com>
wrote:

>
> On 30 Jun 2014, at 11:10, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
>
> > I think we should move away from the practice of using serializer for
> > something it was never made for, namely a weird way of instantiating
> > classes. Serializer should be working only with serialized data.
>
> > Now, the question is can we instantiate the internal class without
> > calling its ctor, and the answer here would probably be "no", at least
> > not safely. While in the case of user class the engine can be reasonabl=
e
> > sure even if you don't call the ctor the basic structures are
> > initialized properly, in the case of the internal class all bets are
> > off. I'm not sure yet which use cases require ctor not to be called, bu=
t
> > I'm not sure how we can deliver on internal classes here.
>
> Sorry, I=E2=80=99m a bit late to discussion and might be missing somethin=
g obvious.
>
> As far as I remember, internal classes have 2 constructors.
>
> 1. implementation of __construct() function
> 2. create_object hook
>
> It should be possible to keep unsafe stuff in create_object which is
> called unconditionally and leave safe initialisation in __construct.
>
> so, in case when __construct is not called object will have properties
> initialised with nulls, empty strings, etc.
>
> I understand that is a lot of work on case-by-case basis but it is doable=
.
>
>
sorry for the late reply.
you are right and after looking into the implementation I think classes
having custom object storage (eg. create_object) shouldn't assume that
their __construct will be called, but either do the initialization in the
create_object hook or validate in the other methods that the object was
properly initialized.
given that this lack of initialization problem is already present(you can
extend such a class and have a __construct() in the subclass which doesn't
call the parent constructor), and we want to keep the unserialize trick
fixed (as that exposes this problems to the remote attacker when some code
accepts arbitrary serialized data from the client) I see no reason to keep
the limitation in the ReflectionClass::newInstanceWithoutConstructor() and
allowing the instantiation of internal classes will provide a clean upgrade
path to doctrine and co.
ofc. we have to fix the internal classes misusing the constructor for
proper initialization one by one, but that can happen after the initial
5.6.0 release (ofc it would be wonderful if people could lend me a hand for
fixing as much as we can before the release), but we have to fix those
anyways.

I consider this problem the biggest impediment for releasing the 5.6.0
final, so I would really like to hear what do you guys think about my
proposed solution?
--=20
Ferenc Kov=C3=A1cs
@Tyr43l - http://tyrael.hu

--001a11398720418eb104fec5acb8--