php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #76606 Widespread regression with Serializable interface and legacy __wakeup method
Submitted: 2018-07-10 14:26 UTC Modified: 2018-07-13 08:21 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: westie at typefish dot co dot uk Assigned: cmb (profile)
Status: Closed Package: Class/Object related
PHP Version: Irrelevant OS: All tested
Private report: No CVE-ID: None
 [2018-07-10 14:26 UTC] westie at typefish dot co dot uk
Description:
------------
According to the documentation, developers are allowed to intermingle the legacy method of unserialising an object (using __wakeup) AND using the much more modern and cleaner way, using methods provided in the Serializable interface.

> Note, that when an old instance of a class that implements this interface
> now, which had been serialized before the class implemeted the interface,
> is unserialized, __wakeup() is called instead of the unserialize method,
> which might be useful for migration purposes.
> ~ http://uk1.php.net/manual/en/class.serializable.php

I appear to have found an interesting regression, where this is no longer the case.

Multiple versions affected (possibly including EOL in this list):

- 5.4.29 (EOL?)
- 5.5.13 (EOL?)
- 5.6.0 - 5.6.30
- 5.6.36 specifically
- 7.0.0 - 7.3.0alpha1 (will literally presume ALL of PHP7)

I was planning to implement this functionality in a new project but yeah, I cannot anymore!

Test script:
---------------
<?php

# author note: please review the code on https://3v4l.org/XRr5t to see
# the full extent of this regression


# base class
class Test_TestClassBase
{
	public $x = 4;
	
	public function __wakeup()
	{
		var_dump("__wakeup");
	}
	
	public function __sleep()
	{
		return array_keys(get_object_vars($this));
	}
	
	public function unserialize($input)
	{
		var_dump("unserialize");
	}
	
	public function serialize()
	{
		var_dump("serialize");
		
		return serialize(get_object_vars($this));
	}
}


# derived classes
class Test_TestClassA extends Test_TestClassBase {}
class Test_TestClassB extends Test_TestClassBase implements Serializable {}
class Test_TestClassAA extends Test_TestClassA implements Serializable {}
class Test_TestClassBB extends Test_TestClassB {}


# run our serialisation
foreach(array("Test_TestClassA", "Test_TestClassB", "Test_TestClassAA", "Test_TestClassBB") as $class)
{
	$serialised = 'O:'.strlen($class).':"'.$class.'":1:{s:1:"x";i:4;}';
	
	var_dump("(input)  ".$serialised);
	var_dump("(output) ".(unserialize($serialised) instanceof $class ? "true (passing)" : "false (failing)"));
}

Expected result:
----------------
string(48) "(input)  O:15:"Test_TestClassA":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(48) "(input)  O:15:"Test_TestClassB":1:{s:1:"x";i:4;}"

Warning: Erroneous data format for unserializing 'Test_TestClassB' in /in/XRr5t on line 48

Notice: unserialize(): Error at offset 26 of 39 bytes in /in/XRr5t on line 48
string(24) "(output) false (failing)"
string(49) "(input)  O:16:"Test_TestClassAA":1:{s:1:"x";i:4;}"

Warning: Erroneous data format for unserializing 'Test_TestClassAA' in /in/XRr5t on line 48

Notice: unserialize(): Error at offset 27 of 40 bytes in /in/XRr5t on line 48
string(24) "(output) false (failing)"
string(49) "(input)  O:16:"Test_TestClassBB":1:{s:1:"x";i:4;}"

Warning: Erroneous data format for unserializing 'Test_TestClassBB' in /in/XRr5t on line 48

Notice: unserialize(): Error at offset 27 of 40 bytes in /in/XRr5t on line 48
string(24) "(output) false (failing)"

Actual result:
--------------
string(48) "(input)  O:15:"Test_TestClassA":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(48) "(input)  O:15:"Test_TestClassB":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(49) "(input)  O:16:"Test_TestClassAA":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(49) "(input)  O:16:"Test_TestClassBB":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-07-10 14:28 UTC] westie at typefish dot co dot uk
Please reverse the 'expected' and 'actual' results, I swapped them around.

To confirm what the expect result should be:

string(48) "(input)  O:15:"Test_TestClassA":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(48) "(input)  O:15:"Test_TestClassB":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(49) "(input)  O:16:"Test_TestClassAA":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
string(49) "(input)  O:16:"Test_TestClassBB":1:{s:1:"x";i:4;}"
string(8) "__wakeup"
string(23) "(output) true (passing)"
 [2018-07-10 15:09 UTC] westie at typefish dot co dot uk
https://3v4l.org/rWFHv

To re-iterate, failing on all normal PHP versions yet working as expected on a third party PHP implementation (HHVM)
 [2018-07-10 16:33 UTC] cmb@php.net
-Status: Open +Status: Verified -Type: Bug +Type: Documentation Problem
 [2018-07-10 16:33 UTC] cmb@php.net
The behavioral change has been introduced with commit 5328d42[1]
(which fixed bug #67072).  Restoring the documented behavior (if
that even will be possible) doesn't appear to make sense after
such a long time, so the documentation needs to be fixed.

[1] <https://github.com/php/php-src/commit/5328d4289946e260232f3195ba2e0f0eb173d5ef>
 [2018-07-10 16:44 UTC] westie at typefish dot co dot uk
I would not call this a documentation issue.

With the exception of say, creating an alternative serialisation parser either in PHP or as a module, how would one go about using the features of Serializable whilst keeping backwards compatibility with older serialised data?

I don't seem to recall when implementing JsonSerializable being unable to use serialize()...
 [2018-07-10 16:57 UTC] cmb@php.net
-Assigned To: +Assigned To: ab
 [2018-07-10 16:57 UTC] cmb@php.net
I, personally, would advise against storing long-lasting data in
serialized format, anyway.

Anyhow, Anatol, what do you think?  Would it be possible and
sensible to restore the old behavior?
 [2018-07-10 19:47 UTC] ab@php.net
Thanks for digging that deep, Christoph. The original issue was local but turned out to reveal a common vulnerability with handling of the internal classe unserialisation. There was fixes on top of that as well. In the end, the decision of the 5.4, 5.5 and 5.6 RMs was to keep the essential part of the fix. Please check the links from Ferenc in the ticket you've linked. In the light of the issues revealed, the sentence "Classes that implement this interface no longer support __sleep() and __wakeup()." has still the precedence.  

@westie sure in some grades it might be painful to migrate an app from 5.4 to 7.x. There has been much more change than just this one in the meanwhile. The most viable way would be to recreate a clean serialized data for your app. And, manipulated serialize strings was never promised to work. In the end, it is a documentation issue today.

Thanks.
 [2018-07-10 21:45 UTC] westie at typefish dot co dot uk
Thank you @cmb and @ab for your interest and insights into this issue.

Firstly, despite a warning or two in the comments in the documentation for serialize (https://secure.php.net/serialize) the documentation itself does not imply that output of serialize is mutable; to quote, it says that it "[...] generates a *storable* representation of a value" (emphasis mine)

I hope you don't find the tone of what I am going to suggest too abrasive but since this has now been classed a documentation issue as opposed to an undocumented breaking change, perhaps the documentation for serialize() also needs to be edited, to specifically warn against the use of it for any planned long term storage of object data, as well as a warning against relying on data being able to be unserialised between different versions of PHP?

Anyhow, I'm investigating alternative ways around this.

Thanks for your support!
 [2018-07-11 12:41 UTC] cmb@php.net
-Assigned To: ab +Assigned To: cmb
 [2018-07-11 12:41 UTC] cmb@php.net
Sorry, all my mistake!  I committed the respective note to the
manual[1] *after* the behavioral change had been introduced.

> […] to specifically warn against the use of it for any planned
> long term storage of object data […]

That's just my personal opinion, and it's not so much regarding
potential breakages in the serialization format, but rather to
prevent issues with changes to userland classes.

> […] as well as a warning against relying on data being able to
> be unserialised between different versions of PHP?

Well, exchanging serialized data between different versions of PHP
should be fine, if the class definitions are identical.

[1] <http://svn.php.net/viewvc/?view=revision&amp;revision=339383>
 [2018-07-11 12:53 UTC] cmb@php.net
Automatic comment from SVN on behalf of cmb
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=345303
Log: Fix #76606: Widespread regression with Serializable interface and legacy __wakeup method

We revert revision 339383.  While this info was valid for PHP 5.1.0 up to
PHP 5.4.28 and 5.5.12, respectively, it is no longer, and as such it's
rather confusing.
 [2018-07-11 12:54 UTC] cmb@php.net
-Status: Verified +Status: Closed
 [2018-07-11 12:54 UTC] cmb@php.net
This bug has been fixed in the documentation's XML sources. Since the
online and downloadable versions of the documentation need some time
to get updated, we would like to ask you to be a bit patient.

Thank you for the report, and for helping us make our documentation better.
 [2018-07-13 08:21 UTC] westie at typefish dot co dot uk
For further reference, I have (for migration and curiosity purposes) created a library that "fixes" this behaviour.

Any objects that were serialised before the class definition was changed to implement the `Serializable` interface will become unserialisable once again, using oddly enough, the unserialize method.

(The inverse is also true, __wakeup would be called when unserialising objects that no longer support unserialisation via `Serializable`)

At the time of writing the support is limited to the types I can easily replicate, but I hope to change that.

https://github.com/OUTRAGElib/unserialize
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 08:01:28 2019 UTC