php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #55801 Behavior of unserialize has changed
Submitted: 2011-09-27 20:06 UTC Modified: 2011-10-19 10:35 UTC
Votes:2
Avg. Score:3.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: mapi at pdepend dot org Assigned: mike
Status: Closed Package: Variables related
PHP Version: 5.4.0beta1 OS: Linux (Fedora 15)
Private report: No CVE-ID:
 [2011-09-27 20:06 UTC] mapi at pdepend dot org
Description:
------------
There is a regression or change in how object structures are serialized or 
unserialized in PHP's 5.4.x branch. I am sorry that I cannot give a more detailed 
explanation and say what has changed and causes this issue, but an object cache 
for one of my applications fails with all PHP 5.4.x version, while it works with 
all PHP 5.2.x and PHP 5.3.x versions.

I would expect that this behavior change would also affect other applications or 
libraries that utilize PHP's (un)serialize functions.

Test script:
---------------
#!/bin/sh

PHP_BIN=/usr/local/bin/php540

cd /tmp

git clone https://github.com/pdepend/pdepend.git
cd pdepend
"$PHP_BIN" ./src/bin/pdepend.php --summary-xml=sum.xml src/main/
"$PHP_BIN" ./src/bin/pdepend.php --summary-xml=sum.xml src/main/

Expected result:
----------------
PHP_Depend @package_version@ by Manuel Pichler

Parsing source files:
............................................................    60
............................................................   120
............................................................   180
................                                               196

Executing CyclomaticComplexity-Analyzer:
............................................................  1200
..............................                                1818

Executing ClassLevel-Analyzer:
............................................................  1200
......................                                        1647

Executing CodeRank-Analyzer:
..........                                                     217

Executing Coupling-Analyzer:
............................................................  1200
..........................................                    2048

Executing Hierarchy-Analyzer:
............................................................  1200
................................                              1852

Executing Inheritance-Analyzer:
...........................                                    549

Executing NPathComplexity-Analyzer:
............................................................  1200
...............................                               1830

Executing NodeCount-Analyzer:
............................................................  1200
..................                                            1564

Executing NodeLoc-Analyzer:
............................................................  1200
............................                                  1766

Generating pdepend log files, this may take a moment.

00:16; Memory: 124.00Mb


Actual result:
--------------
PHP_Depend @package_version@ by Manuel Pichler

Parsing source files:
............................................................    60
............................................................   120
.................PHP Warning:  Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569

Warning: Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
.........................PHP Warning:  Creating default object from empty value 
in /tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569

Warning: Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
.........PHP Warning:  Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569

Warning: Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
PHP Warning:  Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569

Warning: Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
PHP Warning:  Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569

Warning: Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
.........   180
...........PHP Warning:  Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569

Warning: Creating default object from empty value in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
.....                                               196

Executing CyclomaticComplexity-Analyzer:
....................PHP Fatal error:  Call to a member function 
findChildrenOfType() on a non-object in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php on line 
268

Fatal error: Call to a member function findChildrenOfType() on a non-object in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php on line 
268

Patches

fix-serialize-in-sleep-and-wakeup (last revision 2011-10-13 16:21 UTC) by mike@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-09-27 20:16 UTC] pajoye@php.net
-Status: Open +Status: Feedback
 [2011-09-27 20:16 UTC] pajoye@php.net
Is it about objects being serialized and stored using 5.3 and unserialized using 
5.4? And it fails to unserialize them?

Or are you doing this proecess manually?
 [2011-09-28 08:36 UTC] mapi at pdepend dot org
No it's about large object trees serialized in PHP 5.4 and unserialized with the 
same version. This has worked with all PHP versions < 5.4. 

It seems that these object trees are now unserialized or serialized or destructed 
in a different order, so that some of the previous objects are now NULL.
 [2011-09-28 08:50 UTC] mapi at pdepend dot org
What PHP_Depend does is serializing double linked object trees like. It uses 
__sleep() to reset the child-parent link before a tree gets serialized:

class Node {
    protected $parent;
    protected $nodes = array();

    public function __sleep() {
        return array('nodes');
    }
}

And it uses __wakeup() to restore this link during the unserialization:

class Node {
    protected $parent;
    protected $nodes = array();

    public function __sleep() {
        return array('nodes');
    }

    public function __wakeup() {
        foreach ($this->nodes as $node) {
            $node->parent = $this;
        }
    }
}

Now it seems that under certain circumstances the $this->nodes property is an 
array when __wakeup() is called, but instead of objects the array values are 
NULL.

This happens when the following PHP Warning occurs:

PHP Warning:  Creating default object from empty value 
in /tmp/pdepend/src/main/php/PHP/Depend/Code/ASTNode.php on line 569
 [2011-09-28 08:51 UTC] pajoye@php.net
Hm, hard to find what failed without an example then. Can you try to create a 
script to generate an object tree and un/serialize it? That will be very helpful 
to fix this problem.
 [2011-09-28 08:56 UTC] mapi at pdepend dot org
I will try to create a reproducable.
 [2011-09-28 13:30 UTC] mapi at pdepend dot org
Okay, I tried for several hours to create a reproducable outside of PHP_Depend's 
context, but I cannot reproduce this behavior :(

But at least I came up with a small code fragement that illustrates this 
behavior:

  <?php
  class Test {
      function method() {   
          $this->prop[$y]++;
      }
  }

which was translated into the following object graph:

 Statement
   Expression
     MemberPrimaryPrefix  <----------
       Variable                     |
       PropertyPostfix              |
         ArrayIndexExpression  [Same Instance]
           Identifier               |
           Variable                 |
     PostfixExpression              |
       MemberPrimaryPrefix ----------
         Variable
         PropertyPostfix
           ArrayIndexExpression
             Identifier
             Variable

And this second reference to MemberPrimaryPrefix is NULL after unserialization 
with PHP 5.4 and it is just a second clone with PHP < 5.4
 [2011-09-28 13:41 UTC] pajoye@php.net
Any chance to get something that shows us the serialized data with 5.4 and 5.4 so 
we can see how they differ?

Maybe upload the small possible code somewhere?
 [2011-09-28 15:20 UTC] mapi at pdepend dot org
Yes, here you can get the two serialized object structures:

http://manuel-pichler.de/stuff/5.3.ser
http://manuel-pichler.de/stuff/5.4.ser

And there is the difference:

5.3 -> *nodes";a:1:{i:0;r:18;}}}}}}}}}
5.4 -> *nodes";a:1:{i:0;r:36;}}}}}}}}}

It references something different.
 [2011-09-28 16:29 UTC] mapi at pdepend dot org
It seems that this has something todo with the visibility of properties that 
should be serialized. If I do the following:

~ $ rm -rf ~/.pdepend
~ $ git clone https://github.com/pdepend/pdepend.git
~ $ cd pdepend
~ $ vim src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php +86
    -> Change from PROTECTED to PUBLIC
       protected $parentClassReference = null; -> public $parentClassReference = 
null;
~ $ vim src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php +132
    -> Change from PROTECTED to PUBLIC
       protected $nodes = array(); -> public $nodes = array();
~ $ php540 src/bin/pdepend.php --summary-xml=sum.xml 
src/main/php/PHP/Depend/Code/ASTArrayType.php
~ $ php540 src/bin/pdepend.php --summary-xml=sum.xml 
src/main/php/PHP/Depend/Code/ASTArrayType.php
...
Executing CyclomaticComplexity-Analyzer:

Fatal error: Call to a member function findChildrenOfType() on a non-object in 
/tmp/pdepend/src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php on line 
268

If I now revert the visibility of both properties from PUBLIC to PROTECTED, it 
works:

~ $ vim src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php +86
    -> Change from PUBLIC to PROTECTED
       public $parentClassReference = null; -> protected $parentClassReference = 
null;
~ $ vim src/main/php/PHP/Depend/Code/AbstractClassOrInterface.php +132
    -> Change from PUBLIC to PROTECTED
       public $nodes = array(); -> protected $nodes = array();
~ $ php540 src/bin/pdepend.php --summary-xml=sum.xml 
src/main/php/PHP/Depend/Code/ASTArrayType.php

This means that a PUBLIC property that was serialized can be restored in a 
PROTECTED property while unserializing the object structure.
 [2011-09-30 16:09 UTC] johannes@php.net
Can you try reverting SVN revision r299770 and then run your test? - That's mostly a guess, though.

bug #36424
http://svn.php.net/viewvc/?view=revision&revision=299770
 [2011-10-01 12:25 UTC] mapi at pdepend dot org
Bingo! 

With revision 299770 this bug occurs the first time. If I checkout and compile 
revision 299769 of trunk, everything works as expected. With revision 299770 I 
get those annoying unserialized arrays with NULL values.
 [2011-10-01 13:58 UTC] johannes@php.net
-Status: Feedback +Status: Assigned -Assigned To: +Assigned To: mike
 [2011-10-01 13:58 UTC] johannes@php.net
mike, seems like you broke this. Please take a look.
 [2011-10-03 09:45 UTC] mike@php.net
Obviously I did, but it's unclear how...
The reproduce case doesn't help much either.
 [2011-10-03 11:15 UTC] mike@php.net
Ok, I think I found the problematic POC: 
in line 486 of PHP_Depend_Code_AbstractCallable you call serialize() while 
another (the prime) serialize calls __sleep() on an instance of this class.

What's the intention of this __temp__ thing?
 [2011-10-03 11:41 UTC] mike@php.net
OTOH, the following working script suggests that this is not the source of 
failure:


<?php

class node {
    public $parent;
    public $nodes = array();
    public $path;
    public $temp;

    function __toString() {
        return $this->parent ? $this->parent . "/" . $this->path : $this->path;
    }
    function __construct(node $parent = null, $path = ".") {
        $this->parent = $parent;
        $this->path = $path;

        if (is_dir($this)) foreach (scandir($this) as $p) {
            if ($p[0] != ".") {
                $this->nodes[] = new node($this, $p);
            }
        }
    }

    function __sleep() {
        $this->temp = serialize($this->nodes);
        return array("path", "temp");
    }

    function __wakeup() {
        $this->nodes = unserialize($this->temp);
        $this->temp = null;
        foreach ($this->nodes as $n) {
            $n->parent = $this;
        }
    }
}

$tree = new node(null, @$_SERVER["argv"][1] ?: ".");
$s = serialize($tree);
var_dump($s);
$temp = unserialize($s);
print_r($temp);
 [2011-10-04 08:25 UTC] mike@php.net
Ok, now got a reproduce case:

<?php

class node {
    protected $parent;
    protected $nodes = array();
    protected $path;
    protected $temp;

    function __toString() {
        return $this->parent ? $this->parent . "/" . $this->path : $this->path;
    }

    function __construct(node $parent = null, $path = ".") {
        $this->parent = $parent;
        $this->path = $path;

        if (is_dir($this)) foreach (scandir($this) as $p) {
            if ($p[0] != ".") {
                $this->nodes[] = new node($this, $p);
            }   
        }   
    }

    function __sleep() {
        $this->temp = serialize($this->nodes);
        return array("path", "temp");
    }

    function __wakeup() {
        $this->nodes = unserialize($this->temp);
        $this->temp = null;
        foreach ($this->nodes as $n) {
            $n->parent = $this;
        }   
    }

    function createWeirdConnections() {
        foreach ($this->nodes as $n) {
            $a = $this->nodes;
            shuffle($a);
            $n->nodes[] = current($a);
        }   
    }
}

$tree = new node(null, @$_SERVER["argv"][1] ?: ".");
$tree->createWeirdConnections();

$s = serialize($tree);
$temp = unserialize($s);
 [2011-10-04 14:20 UTC] mike@php.net
-Status: Assigned +Status: Analyzed
 [2011-10-04 14:20 UTC] mike@php.net
So, after digging a lot, I can only see two solutions:
 - either disallow serialize/unserialize in __sleep/__wakeup 
 - or revert r299770 which introduced a "persistent" state for serialize() which 
allowed objects which implement the Serializable interface to keep reference 
info through recursive calls to serialize(), see FR #36424

The issue can probably be seen as follows:

serialize(obj)		
 -> obj->__sleep does serialize() (in your code)
 -> then internally serialize(obj->prop) happens
 
unserialize(obj)
 -> internally unserialize(obj->prop) is done
 -> obj->__wakeup is called which does unserialize() (your code)

As you can see the IDs of the referenced objects when unserializing cannot match 
the IDs at serialization time, because of the mixed up call order.
 [2011-10-04 16:20 UTC] mapi at pdepend dot org
Cool that you came up with a reproducable, I have tried it for a couple of hours 
and didn't get a working reproducable outside of PHP_Depend.

But is it such an uncommon use case to serialize/unserialize additional data in 
an object's __sleep() or __wakeup() method?
 [2011-10-04 16:34 UTC] mapi at pdepend dot org
To answer your question, I did't know my initial intention about the __temp__ 
property, but I remember that there was a reason. But it seems that this property 
is obsolete now.
 [2011-10-13 14:39 UTC] mike@php.net
The following patch has been added/updated:

Patch Name: fix-serialize-in-sleep-and-wakeup
Revision:   1318516750
URL:        https://bugs.php.net/patch-display.php?bug=55801&patch=fix-serialize-in-sleep-and-wakeup&revision=1318516750
 [2011-10-13 14:39 UTC] mike@php.net
-Status: Analyzed +Status: Feedback
 [2011-10-13 14:39 UTC] mike@php.net
Could you try attached patch, please?
 [2011-10-13 16:21 UTC] mike@php.net
The following patch has been added/updated:

Patch Name: fix-serialize-in-sleep-and-wakeup
Revision:   1318522878
URL:        https://bugs.php.net/patch-display.php?bug=55801&patch=fix-serialize-in-sleep-and-wakeup&revision=1318522878
 [2011-10-19 08:15 UTC] mike@php.net
So? Running your original test script seems to work fine with this patch...
 [2011-10-19 08:55 UTC] mapi at pdepend dot org
Hello Mike,

I just tested a patched version of "branches/PHP_5_4/" and "trunk/" with the 
original implementation that uses serialize() and unserialize() within 
__sleep()/__wakeup(). Everything works as expected and there are no NULL filled 
arrays anymore.

Thank you very much for help
 [2011-10-19 10:09 UTC] mike@php.net
Automatic comment from SVN on behalf of mike
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=318210
Log: Fix Bug #55801 Behavior of unserialize has changed:
 (un)serialize in __wakeup/__sleep now use clean var_hashes
 [2011-10-19 10:35 UTC] mike@php.net
-Status: Feedback +Status: Closed
 [2011-10-19 10:35 UTC] mike@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2012-04-18 09:48 UTC] laruence@php.net
Automatic comment on behalf of mike
Revision: http://git.php.net/?p=php-src.git;a=commit;h=849e7ae7aa44a682d174ebb894fd03990a51d9e9
Log: Fix Bug #55801 Behavior of unserialize has changed:  (un)serialize in __wakeup/__sleep now use clean var_hashes
 [2012-07-24 23:39 UTC] rasmus@php.net
Automatic comment on behalf of mike
Revision: http://git.php.net/?p=php-src.git;a=commit;h=849e7ae7aa44a682d174ebb894fd03990a51d9e9
Log: Fix Bug #55801 Behavior of unserialize has changed:  (un)serialize in __wakeup/__sleep now use clean var_hashes
 [2013-11-17 09:35 UTC] laruence@php.net
Automatic comment on behalf of mike
Revision: http://git.php.net/?p=php-src.git;a=commit;h=849e7ae7aa44a682d174ebb894fd03990a51d9e9
Log: Fix Bug #55801 Behavior of unserialize has changed:  (un)serialize in __wakeup/__sleep now use clean var_hashes
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Wed Apr 16 22:02:05 2014 UTC