Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:74964
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 108.166.43.115 as permitted sender)
Message-ID: <53A0C6DD.50705@sugarcrm.com>
Date: Tue, 17 Jun 2014 15:53:17 -0700
Organization: SugarCRM
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: Ferenc Kovacs <tyra3l@gmail.com>, Stanislav Malyshev <stas@php.net>, 
 julien pauli <jpauli@php.net>,
 Anatoliy Belsky <ab@php.net>, Remi Collet <remi@php.net>, 
 PHP Internals <internals@lists.php.net>
References: <CAH-PCH6xEXspisLOHtiTbViA4vDTk7q6LM3t_Bh9Dzr1cj_Nag@mail.gmail.com>
In-Reply-To: <CAH-PCH6xEXspisLOHtiTbViA4vDTk7q6LM3t_Bh9Dzr1cj_Nag@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Problems with the fix for the BC break introduced in
 5.4.29 and 5.5.13
From: smalyshev@sugarcrm.com (Stas Malyshev)

Hi!

> Could you check out my last mail about the unserialize stuff?:
> http://news.php.net/php.internals/74947

Reading this message, I gather the situation is as follows:

1. Original fix banned O: unserialization of classes that have custom
serializers.
2. The following fix allowed this behavior for user classes.
3. The second fix means that if the user class descends from internal
class with serializer, we still have a problem.

My opinion is this:

1. Using unserialize() on anything that is not a result of serialize()
is a hack. As such, there are no support guarantees that it would work
in any particular manner, besides continuity of support for serialized
format itself. In particular, we have no guarantees that string that did
not come from serializer would behave in any particular way.

2. We should make reasonable effort to keep code that worked in version
x.y.z working in x.y.z+1. However, "reasonable" is important here, if
there's a behavior that is not documented and not wanted, we can break
it. We don't have to and will try not to, but we can.

3. In light of this, I think we could do with current patch in 5.4 and
5.5 (one that permits userland classes) but we should plug it completely
for 5.6. If somebody needs code that does whatever it did, it should be
in Reflection or someplace else, serializer should do serializing. But I
think for stable version it is a reasonable, if imperfect, compromise -
it would allow most common use cases (userland classes) to work and most
common crashes (internal classes) to not crash. Extending SplFileInfo
looks a bit more exotic so I think we can live with it not being fixed
till 5.6.

-- 
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227