Re: Re: com php-src: Improved the fix for bug #67072, thanks Nikita: ext/standard/tests/serialize/005.phpt ext/standard/tests/serialize/bug67072.phpt
ext/standard/var_unserializer.c ext/standard/var_unserializer.re

From: Date: Tue, 06 May 2014 08:26:31 +0000
Subject: Re: Re: com php-src: Improved the fix for bug #67072, thanks Nikita: ext/standard/tests/serialize/005.phpt ext/standard/tests/serialize/bug67072.phpt
ext/standard/var_unserializer.c ext/standard/var_unserializer.re
References: 1 2 3 4  Groups: php.internals 
Request: Send a blank email to internals+get-73953@lists.php.net to get a copy of this message
Hi!

> The initial bug is that an unserialization string misuses "C:" vs "O:"
> format which would call an incorrect method "unserialize" vs. "__wakeup".
> And that leads to a crash with that particular ticket, due to an
> incompletely initialized object. Formally this fix just brings the
> functionality inline with the documentation,
> http://de1.php.net/serializable . It says
> "Classes that implement this
> interface no longer support __sleep() and __wakeup().". So that dirty
> tricks should actually have been impossible, according to the doc. So per
> fact, it breaks something which was already broken but users was relying
> on it.

I'm not sure I understand this point. The manual says __sleep/__wakeup
won't be called, but that doesn't seem to me a problem here, the problem
here seems to be that unserialize throws a warning, not that __wakeup is
not called.

> The patches fixes it globally preventing incomplete objects due to the
> incorrect callback invocation. A fix for that particular ticket could be

This also breaks code that worked before, and does it in stable
versions. I do not think it is an acceptable solution for stable
versions. Can't we make the fix in a way that does not have BC breakage
for stable versions?

> shortly defer the issue because 5.6 is coming out in a few weeks, so the
> bad user code will have to be fixed in the userspace soon. As conclusion -

Only if that code is going to be run on 5.6.0 - which it won't be for
some time because nobody runs .0 versions in production next day they
are released. But even with 5.6, we are trying to minimize amount of
code we're breaking in minor releases, and here we're breaking phpunit,
which is one of the most used PHP tools. So I think we need to look for
a better solution. CCing also Sebastian, maybe he could explain more
about this hack and how it can be made to work more smoothly maybe.
-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227


Thread (4 messages)

« previous php.internals (#73953) next »