php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #66085 Serializable::serialize() breaks if zval addresses are reused.
Submitted: 2013-11-12 20:21 UTC Modified: 2017-06-25 19:33 UTC
Votes:4
Avg. Score:4.5 ± 0.9
Reproduced:4 of 4 (100.0%)
Same Version:3 (75.0%)
Same OS:0 (0.0%)
From: machine dot check dot exception at gmail dot com Assigned: nikic (profile)
Status: Closed Package: Arrays related
PHP Version: 5.5.5 OS: Windows 7
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: machine dot check dot exception at gmail dot com
New email:
PHP Version: OS:

 

 [2013-11-12 20:21 UTC] machine dot check dot exception at gmail dot com
Description:
------------
Doing tests I found a different behaviour between versions of php.

The code in the test script returns in php 5.2.7, 5.3.0, 5.3.9, this:
O:8:"stdClass":1:{s:1:"a";s:4:"tmp1";}
O:8:"stdClass":1:{s:1:"a";s:4:"tmp2";}

In php 5.4.0RC8, 5.4.3 and 5.5.5 this:
O:8:"stdClass":1:{s:1:"a";s:4:"tmp1";}
r:3;


Test script:
---------------
class test implements Serializable {
	public $a;
	public function __construct( $_val ){
		$this->a = $_val;
	}

	public function serialize() {
		$tmp = (object) array();
		$tmp->a = $this->a;
		echo serialize( $tmp ) . "\n";
		return serialize( $tmp );
	}
}

$n = array();
$n[0] = new test('tmp1');
$n[1] = new test('tmp2');
print_r( unserialize( serialize( $n ) ) );

Expected result:
----------------
O:8:"stdClass":1:{s:1:"a";s:4:"tmp1";}
O:8:"stdClass":1:{s:1:"a";s:4:"tmp2";}

Actual result:
--------------
O:8:"stdClass":1:{s:1:"a";s:4:"tmp1";}
r:3;

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-11-12 20:37 UTC] bwoebi@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: bwoebi
 [2013-11-12 20:37 UTC] bwoebi@php.net
Thank you for your bug report. This issue has already been fixed
in the latest released version of PHP, which you can download at 
http://www.php.net/downloads.php

Have a look at: http://3v4l.org/GeU1o#v510 … Was just a bug until 5.5.4.
 [2013-11-13 11:21 UTC] machine dot check dot exception at gmail dot com
I have this version installed: VC11 x86 Thread Safe (2013-Oct-17 00:30:06)
From phpinfo():
Build Date	Oct 15 2013 10:25:39
Compiler	MSVC11 (Visual C++ 2012)
Architecture	x86

And the bug is still there.
 [2013-11-13 12:28 UTC] bwoebi@php.net
Are you sure that that is PHP 5.5.5?

At least I can't reproduce it here with 5.5.5 here. (and http://3v4l.org/GeU1o#v510 tells me the same).
 [2013-11-13 12:53 UTC] machine dot check dot exception at gmail dot com
Here is there info about the system i'm using from phpinfo():

PHP Version 5.5.5

System	Windows NT XYZ-PC 6.1 build 7601 (Windows 7 Ultimate Edition Service Pack 1) i586
Build Date	Oct 15 2013 10:25:39
Compiler	MSVC11 (Visual C++ 2012)
Architecture	x86
Configure Command	cscript /nologo configure.js "--enable-snapshot-build" "--disable-isapi" "--enable-debug-pack" "--without-mssql" "--without-pdo-mssql" "--without-pi3web" "--with-pdo-oci=C:\php-sdk\oracle\instantclient10\sdk,shared" "--with-oci8=C:\php-sdk\oracle\instantclient10\sdk,shared" "--with-oci8-11g=C:\php-sdk\oracle\instantclient11\sdk,shared" "--enable-object-out-dir=../obj/" "--enable-com-dotnet=shared" "--with-mcrypt=static" "--disable-static-analyze" "--with-pgo"
Server API	Apache 2.0 Handler
Virtual Directory Support	enabled
Configuration File (php.ini) Path	C:\Windows
Loaded Configuration File	F:\wamp\bin\apache\apache2.4.6\bin\php.ini


Apache Version	Apache/2.4.6 (Win32) PHP/5.5.5
Apache API Version	20120211


http://3v4l.org/GeU1o#v510 is running under linux. Can it be somethin just related with the windows version?
 [2013-11-13 16:16 UTC] bwoebi@php.net
I really can't believe that.

Code for serialization is platform independent (http://lxr.php.net/xref/PHP_5_5/ext/standard/var.c#719)
 [2013-11-13 20:43 UTC] machine dot check dot exception at gmail dot com
-Status: Closed +Status: Assigned
 [2013-11-13 20:43 UTC] machine dot check dot exception at gmail dot com
The bug is there. Check this: http://3v4l.org/DJg0j

It looks like it is a problem with memory allocation. Windows and linux allocate in different ways, that's why probably windows "crashes" earlier than linux (in maybe any overflow?).
 [2013-11-13 21:01 UTC] bwoebi@php.net
I see; thank you for the report, I'll investigate further tomorrow.

Seems the recursion prevention is buggy. Maybe it needs an extra stack for every serialize() call.
 [2013-11-14 11:27 UTC] bwoebi@php.net
-Status: Assigned +Status: Verified -Assigned To: bwoebi +Assigned To:
 [2013-11-14 11:27 UTC] bwoebi@php.net
That actually is more complicated than I thought. If I fix that, the recursion prevention doesn't work anymore in other edge cases.

Another developer might feel free to fix this.
 [2013-12-16 17:32 UTC] will at johnstonclan dot net
Another test case available here:

https://bugs.php.net/bug.php?id=66292
 [2013-12-23 07:00 UTC] aaron dot hamid at gmail dot com
I debugged this some and discovered that this line is producing object(stdClass)'s with the same zval pointer and zend_objects_get_address address values (actually they alternate two previous values) after the second entry:

$tmp = (object) array();

This causes the object to be detected as already encountered and the reference encoded.  I don't understand how or why this should be the case, are value structures getting reused, is garbage collection occurring?

If I comment out the encountered lookup in php_var_serialize_intern then the desired output is produced, although I assume that defeats the recursion prevention you are talking about bwoebi - is there any other insight you can shed on this?  Is my understanding correct?
 [2013-12-23 07:43 UTC] aaron dot hamid at gmail dot com
Yup, zval ids/addresses are getting reused.  It looks like $tmp is getting collected after the test serialize() method.  If I *prevent* the collection by keeping a reference to the inner $tmp in a global array, then I get the expected output:

$keep_ref = array();

class test implements Serializable {
	public $a;
	public function __construct( $_val ){
		$this->a = $_val;
	}

	public function serialize() {
		$tmp = (object) array();
		$tmp->a = $this->a;
                array_push($keep_ref, $tmp); // keep hold of a reference
		echo serialize( $tmp ) . "\n";
		return serialize( $tmp );
	}
}

I tested with the 9 value array example here: http://3v4l.org/DJg0j

Can anybody confirm?

If zval id/address is used as the visited object key, and these are getting reused/replaced by garbage collection while walking the object graph, then I'm not sure how we can implement references across the entire object graph.  Is there a way to temporarily disable collecting reference-counted objects?  I see there is gc_disable/gc_enable that appears to only affect "circular" reference collector.
 [2013-12-23 07:48 UTC] aaron dot hamid at gmail dot com
sorry i mis-pasted in my last comment, of course there should be 'global $keep_ref;' at beginning of serialize().
 [2017-01-01 11:51 UTC] nikic@php.net
This bug has been fixed in PHP 7 by retaining all relevant zvals during serialization: https://github.com/php/php-src/blob/master/ext/standard/var.c#L629
 [2017-01-01 12:29 UTC] nikic@php.net
-Summary: in function serialize() there is irregular behaviour +Summary: Serializable::serialize() breaks if zval addresses are reused.
 [2017-06-25 19:33 UTC] nikic@php.net
-Status: Verified +Status: Closed -Assigned To: +Assigned To: nikic
 [2017-06-25 19:33 UTC] nikic@php.net
Closing this, as the issue is fixed in PHP 7 and PHP 5.6 is not actively supported anymore.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 11:01:28 2024 UTC