Bug #66052 [Com]: Serialized value ids are shared between nested serialization operations
Edit report at https://bugs.php.net/bug.php?id=66052&edit=1
ID: 66052
Comment by: php at laszlokorte dot de
Reported by: crog at gustavus dot edu
Summary: Serialized value ids are shared between nested
serialization operations
Status: Open
Type: Bug
Package: *General Issues
PHP Version: 5.4.21
Block user comment: N
Private report: N
New Comment:
I guess I came across the same issue.
The following gist reproduces it even in php7.0.14:
https://gist.github.com/laszlokorte/3948f40873346cc1fd9b8c11ab06ae04
Previous Comments:
------------------------------------------------------------------------
[2017-01-07 22:15:21] nikic@php.net
Related To: Bug #67363
------------------------------------------------------------------------
[2017-01-01 12:32:51] nikic@php.net
Related To: Bug #70803
------------------------------------------------------------------------
[2017-01-01 12:30:20] nikic@php.net
Note that the issue in the last comment is due to bug #66085, which is resolved in PHP 7. Of course
this still leaves the overall problem.
------------------------------------------------------------------------
[2017-01-01 12:27:58] nikic@php.net
Related To: Bug #73253
------------------------------------------------------------------------
[2014-02-28 17:59:33] crog at gustavus dot edu
Here's another example showing just how screwy serialization is once you throw hooks into the
mix:
http://3v4l.org/XaTAq#v5422
==================================================
Code:
<?php
class ObjectWithReferences {
protected $var1;
protected $var2;
public function __construct() {
$this->var1 = new StdClass();
$this->var2 = $this->var1;
}
}
class WrapperObject implements Serializable
{
private $obj;
public function __construct($obj) {
$this->obj = $obj;
}
public function getObject() {
return $this->obj;
}
public function serialize() {
for ($i = 0; $i < 15; ++$i) {
var_dump(serialize(new \stdClass()));
}
return serialize($this->obj);
}
public function unserialize($serialized) {
$this->obj = unserialize($serialized);
}
}
$wrapper = new WrapperObject(new ObjectWithReferences());
var_dump($wrapper->getObject());
$serialized = serialize($wrapper);
var_dump($serialized);
$wrapper = unserialize($serialized);
=============================================
Output:
string(19) "O:8:"stdClass":0:{}"
string(19) "O:8:"stdClass":0:{}"
string(19) "O:8:"stdClass":0:{}"
string(4) "r:2;"
string(4) "r:3;"
string(4) "r:4;"
string(4) "r:2;"
string(4) "r:3;"
string(4) "r:4;"
string(4) "r:2;"
string(4) "r:3;"
string(4) "r:4;"
string(4) "r:2;"
string(4) "r:3;"
string(4) "r:4;"
string(110)
"C:13:"WrapperObject":84:{O:20:"ObjectWithReferences":2:{s:7:"*var1";O:8:"stdClass":0:{}s:7:"*var2";r:18;}}"
Notice: unserialize(): Error at offset 83 of 84 bytes in /in/XaTAq on line 34
==================================================
The first three iterations make new stdClass instances, and so far we're fine. After that, it
looks like the garbage collection gets involved, starts wiping the unused instances and freeing the
memory for the new instances. As serialization continues, the new instances get put in blocks once
allocated to the now-garbage-collected instances which causes the serialization function to think
it's getting a reference to something it's already serialized (!). This repeats in blocks
of three as the garbage collection is invoked and starts blasting more dead objects.
Then we get to the actual object we're serializing. At this point, PHP thinks it has serialized
15 more objects (!) (should only be three new references, but all as part of a separate operation),
so the reference counts are completely hosed. As soon as this is unserialized, things blow up (as
evident by the error at the end).
The solution here requires improving this lazy reference counting/checking code. The design appears
to have three major issues which are going to continue causing problems for anyone using any of the
serialization hooks (Serializable interface or the __sleep/__wakeup magic methods):
(1) Blindly using memory locations to determine whether or not an object has already been serialized
is going to lead to false positives as long as garbage collection is enabled during the operation.
(2) Not storing an object ID in the serialized object format forces relative referencing. Having
absolute referencing may not fix all of the problems here, but it'd certainly be able fail more
gracefully (malformed input error vs silently rebuilding the object with a corrupted state).
(3) In some cases, a nested serialize calls are are not given a unique state, which makes them
almost-recursive calls except their output is not guaranteed to be part of the parent call's
output.
So long as these issues are left unresolved, any classes using the serialization hooks are going to
be incredibly brittle. I shudder when I think of what problems subclassing adds to this mess
(nevermind delegation, event handling and other callback-oriented patterns).
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=66052
--
Edit this bug report at https://bugs.php.net/bug.php?id=66052&edit=1
Thread (9 messages)