Bug #67452 [Com]: clone and serialize issue

From: Date: Fri, 23 Jan 2015 18:05:44 +0000
Subject: Bug #67452 [Com]: clone and serialize issue
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-190163@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=67452&edit=1

 ID:                 67452
 Comment by:         aaron dot hamid at gmail dot com
 Reported by:        remi@php.net
 Summary:            clone and serialize issue
 Status:             Open
 Type:               Bug
 Package:            *General Issues
 Operating System:   GNU/LInux (32 bits)
 PHP Version:        5.4+
 Block user comment: N
 Private report:     N

 New Comment:

For completeness my pull request with some questions regarding impl: https://github.com/php/php-src/pull/550


Previous Comments:
------------------------------------------------------------------------
[2015-01-23 17:30:11] aaron dot hamid at gmail dot com

I haven't looked at this patch but from discussion it looks related to #66085 https://bugs.php.net/bug.php?id=66085 I had an
experimental fix for that IIRC in the 5.6 line:

https://github.com/ahamid/php-src/commit/cfe3a0f543fb104d52cf684ddd65e68c3f521375

>I think the only way to fix this issue is to retain a reference to all objects that are
>serialized and drop it after serialization is finished. This avoids the possibility that object
>handles or memory addresses are reused during serialization.

Yes, I came to the same conclusion.

------------------------------------------------------------------------
[2014-12-12 16:42:07] nikic@php.net

Apart from the free_list issue, the patch has the additional problems that object R refs will not be
created for objects created in serialize(). Consider the following script:

<?php

class Test implements Serializable {
    public function serialize() {
        $obj = new stdClass;
        return serialize([$obj, $obj]);
    }
    public function unserialize($str) {
        var_dump(unserialize($str));
    }
}

unserialize(serialize(new Test));

Here an array with two identical objects is serialized and as such it also should unserialize to the
same object.

Current behavior:

~/dev/php-5.6$ sapi/cli/php t18.php 
array(2) {
  [0]=>
  object(stdClass)#2 (0) {
  }
  [1]=>
  object(stdClass)#2 (0) {
  }
}

After patch:

~/dev/php-5.6$ sapi/cli/php t18.php 
array(2) {
  [0]=>
  object(stdClass)#2 (0) {
  }
  [1]=>
  object(stdClass)#3 (0) {
  }
}

So just excluding objects created during serialize() might solve the test failure, but will
introduce problems in other cases.

I think the only way to fix this issue is to retain a reference to all objects that are serialized
and drop it after serialization is finished. This avoids the possibility that object handles or
memory addresses are reused during serialization. This is how this was fixed in PHP 7:

    https://github.com/php/php-src/commit/8be73f2650582423ec1d3c4b65a77c450f6683a0
    https://github.com/php/php-src/commit/75860fa8e1d8ce0c9fd2b505bf7663a4936a7a39

However the same approach is likely not feasible in 5.x due to ABI restrictions.

------------------------------------------------------------------------
[2014-06-20 12:46:44] mbeccati@php.net

Bug was reproducted on NetBSD i386. The patch fixes the issue.

------------------------------------------------------------------------
[2014-06-17 09:58:32] remi@php.net

This first patch seems to fix this runtime issue, and don't break other serialize tests.

This is not a perfect solution, as handle free list is ignored.

------------------------------------------------------------------------
[2014-06-17 09:56:47] remi@php.net

The following patch has been added/updated:

Patch Name: serialize.patch
Revision:   1402999007
URL:        https://bugs.php.net/patch-display.php?bug=67452&patch=serialize.patch&revision=1402999007

------------------------------------------------------------------------


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=67452


--
Edit this bug report at https://bugs.php.net/bug.php?id=67452&edit=1


Thread (9 messages)

« previous php.bugs (#190163) next »