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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
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

Pull Requests

Pull requests:

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 Nov 21 21:01:28 2024 UTC