Re: Bug 67072 resolution for 5.4/5.5

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. -- Ferenc Kovács @Tyr43l - http://tyrael.hu