Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94232
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: softfail (pb1.pair.com: domain golemon.com does not designate 209.85.214.46 as permitted sender)
MIME-Version: 1.0
Sender: php@golemon.com
Date: Thu, 23 Jun 2016 19:53:58 +0200
Message-ID: <CAESVnVpQGHe9eDCbdgkmJw3qNFX7BJ3af0xcRr7DtK9NXwykrg@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: [Bug #68319] unserialize() with modified class definition.
From: pollita@php.net (Sara Golemon)

https://bugs.php.net/bug.php?id=68319
https://3v4l.org/irnRC

The crux is this:
* Object instance gets serialized with one definition, maybe stored in
DB/file, whatever, the serialized value lives on.
* Class definition changes slightly. In this case, a property changes
visibility.
* Serialized value is unserialized.  The prop visibilities don't match.
* PHP says, "Eh, whatevs, I'll make a dynamic prop of the same name."

Possible resolutions:
1: Raise a warning and return false (as unserialize already does for
parse errors)
2: Raise a warning and "correct" the visibility to match the current
class definition
3: Raise a warning and continue duplicating the properties

I don't think we need to be as terrible as option 3 since any code
facing this problem right now can't actually access the unserialized
value and is therefore broken in much worse ways.  I think option 2
presents its own unquantified risks and should probably be avoided.

So obviously, I vote option 1, but I'd like to get other's thoughts
and opinions before addressing this bug.

I'm going to go ahead and say ignore what HHVM does here.  In this
specific case they basically take option 2, but in the inverse case
https://3v4l.org/ecM1Q they're precisely as broken as we are.

-Sara