On Mon, Jun 30, 2014 at 8:10 PM, Alexey Zakhlestin <indey...@gmail.com> wrote:
> > On 30 Jun 2014, at 11:10, Stas Malyshev <smalys...@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 reasonable > > 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, but > > I'm not sure how we can deliver on internal classes here. > > Sorry, I’m a bit late to discussion and might be missing something 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? -- Ferenc Kovács @Tyr43l - http://tyrael.hu
