php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #69996 Changing the property of a cloned object affects the original
Submitted: 2015-07-05 11:33 UTC Modified: 2015-07-10 08:00 UTC
Votes:4
Avg. Score:4.5 ± 0.9
Reproduced:3 of 4 (75.0%)
Same Version:3 (100.0%)
Same OS:3 (100.0%)
From: berdir@php.net Assigned:
Status: Closed Package: Scripting Engine problem
PHP Version: 7.0Git-2015-07-05 (Git) OS: Ubuntu
Private report: No CVE-ID:
 [2015-07-05 11:33 UTC] berdir@php.net
Description:
------------
... under not yet identified descriptions.

This is part of our efforts to get Drupal 8 green on PHP 7, see https://www.drupal.org/node/2454439.

At some point a very strange bug was introduced in PHP 7, this is a relatively new bug. Many tests have fails, I haven't checked yet if it's the same bug for all of them.

You can reproduce by running MemoryBackendUnitTest. https://bugs.php.net/bug.php?id=69371 for example has instructions on how to run a test.

The problematic code is in MemoryBackend::prepareItem():

    $prepared = clone $cache;
    $prepared->data = unserialize($prepared->data);

This is called repeatedly and on the second call, $cache->data is already unserialized which results in a notice. So it looks like the clone doesn't fully work. $cache is a stdClass object.

I tried to extract it into a standalone script but that doesn't fail.

When debugging, I noticed that both cloned object have the same hash (spl_object_hash()) and that both $prepared->data and $cache->data are already unserialized.

Expected result:
----------------
No error

Actual result:
--------------
unserialize(): Error at offset 0 of 6 bytes
unserialize('foobar')
Drupal\Core\Cache\MemoryBackend->prepareItem(Object, 1)
Drupal\Core\Cache\MemoryBackend->get('test3', 1)
Drupal\system\Tests\Cache\GenericCacheBackendUnitTestBase->testSetGet()
Drupal\simpletest\TestBase->run()
_simpletest_batch_operation(Array, '7', Array)
_batch_process()
_batch_do()
_batch_page(Object)
Drupal\system\Controller\BatchController->batchPage(Object)
call_user_func_array(Array, Array)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1)
Stack\StackedHttpKernel->handle(Object, 1, 1)
Drupal\Core\DrupalKernel->handle(Object)

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-07-09 17:59 UTC] neclimdul at gmail dot com
Seems it has to be in a function and it only happens on the 3rd unserialize. Weird characteristics but hopefully that'll help someone debug it.

Test case:
<?php

function method($cache) {
  $prepared = clone $cache;
  $prepared->data = unserialize($prepared->data);
  return $prepared;
}

$cache = new stdClass();
$cache->data = serialize(['foo' => 'bar']);

for ($i = 0; $i < 5; ++$i) {
  echo "$i\n";
  method($cache);
}
?>
Output:
0
1
2

Warning: unserialize() expects parameter 1 to be string, array given in /.../test2.php on line 5
3
 [2015-07-10 08:00 UTC] laruence@php.net
it turns out we can not use fast_copy here:
diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c
index 9694cd6..2cc7c6d 100644
--- a/Zend/zend_objects.c
+++ b/Zend/zend_objects.c
@@ -179,7 +179,9 @@ ZEND_API void zend_objects_clone_members(zend_object *new_object, zend_object *o
 			src++;
 			dst++;
 		} while (src != end);
-	} else if (old_object->properties && !old_object->ce->clone) {
+	}
+#if 0
+   	else if (old_object->properties && !old_object->ce->clone) {
 		/* fast copy */
 		if (EXPECTED(old_object->handlers == &std_object_handlers)) {
 			if (EXPECTED(!(GC_FLAGS(old_object->properties) & IS_ARRAY_IMMUTABLE))) {
@@ -189,6 +191,7 @@ ZEND_API void zend_objects_clone_members(zend_object *new_object, zend_object *o
 			return;
 		}
 	}
+#endif

 	if (old_object->properties &&
 	    EXPECTED(zend_hash_num_elements(old_object->properties))) {

thanks
 [2015-07-10 08:37 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=f930d6ea0e8ca8723212ab234973d74dbccfc226
Log: Fixed Bug #69996 (Changing the property of a cloned object affects the original)
 [2015-07-10 08:37 UTC] laruence@php.net
-Status: Open +Status: Closed
 [2015-07-21 14:21 UTC] ab@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=f930d6ea0e8ca8723212ab234973d74dbccfc226
Log: Fixed Bug #69996 (Changing the property of a cloned object affects the original)
 [2016-07-20 11:37 UTC] davey@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=f930d6ea0e8ca8723212ab234973d74dbccfc226
Log: Fixed Bug #69996 (Changing the property of a cloned object affects the original)
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Fri Apr 28 08:01:50 2017 UTC