From d7d36692fda63bd8d628a94e35283cbd142f923d Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 23 May 2023 17:39:17 +0200 Subject: [PATCH] Fix serialization of RC1 objects appearing in object graph twice Previously, if an object had RC1 it would never be recorded in php_serialize_data.ht because it was assumed that it could not be encountered again. This assumption is incorrect though as the object itself may be saved inside an array with RCn. This results in a new instance of the object, instead of a second reference to the same object. This is solved by tracking these objects in php_serialize_data.ht. To retain performance, track if the current object resides in a potentially nested RCn array. If not, and if the object is RC1 itself it may be omitted from php_serialize_data.ht. Additionally, we may treat the array root itself as RC1 because it may not appear in the object graph again without recursion. Recursive arrays are still somewhat broken even with this change, as the tracking of the array only happens when the reference is encountered, thus resulting in a -> a' -> a' for a self recursive array a -> a. Recursive arrays have limited support in serialize anyway, so we ignore this case for now. Co-authored-by: Dmitry Stogov Co-authored-by: Martin Hoch Closes GH-11349 Closes GH-11305 --- NEWS | 3 ++ .../serialize/serialization_objects_019.phpt | 42 +++++++++++++++++++ ext/standard/var.c | 36 +++++++++------- 3 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 ext/standard/tests/serialize/serialization_objects_019.phpt diff --git a/NEWS b/NEWS index 97ecb30e01d..b8f195bd025 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,9 @@ PHP NEWS - Session: . Removed broken url support for transferring session ID. (ilutov) +- Standard: + . Fix serialization of RC1 objects appearing in object graph twice. (ilutov) + 06 Jul 2023, PHP 8.1.21 - CLI: diff --git a/ext/standard/tests/serialize/serialization_objects_019.phpt b/ext/standard/tests/serialize/serialization_objects_019.phpt new file mode 100644 index 00000000000..711217d7961 --- /dev/null +++ b/ext/standard/tests/serialize/serialization_objects_019.phpt @@ -0,0 +1,42 @@ +--TEST-- +Serialization of RC1 objects appearing in object graph twice +--FILE-- +a = [$end]; + $root->b = $root->a; + unset($end); + echo serialize($root), "\n"; +} + +function rcn_rc1() { + $root = new stdClass; + $end = new stdClass; + $root->a = [[$end]]; + $root->b = $root->a; + unset($end); + echo serialize($root), "\n"; +} + +function rcn_properties_ht() { + $object = new stdClass; + $object->object = new stdClass; + $array = (array) $object; + $root = [$object, $array]; + unset($object); + unset($array); + echo serialize($root), "\n"; +} + +rcn(); +rcn_rc1(); +rcn_properties_ht(); + +?> +--EXPECT-- +O:8:"stdClass":2:{s:1:"a";a:1:{i:0;O:8:"stdClass":0:{}}s:1:"b";a:1:{i:0;r:3;}} +O:8:"stdClass":2:{s:1:"a";a:1:{i:0;a:1:{i:0;O:8:"stdClass":0:{}}}s:1:"b";a:1:{i:0;a:1:{i:0;r:4;}}} +a:2:{i:0;O:8:"stdClass":1:{s:6:"object";O:8:"stdClass":0:{}}i:1;a:1:{s:6:"object";r:3;}} diff --git a/ext/standard/var.c b/ext/standard/var.c index c429763eb9c..aab00e52d46 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -652,9 +652,12 @@ PHP_FUNCTION(var_export) } /* }}} */ -static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash); +static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash, bool in_rcn_array, bool is_root); -static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) /* {{{ */ +/** + * @param bool in_rcn_array Whether the element appears in a potentially nested array with RC > 1. + */ +static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var, bool in_rcn_array) /* {{{ */ { zval *zv; zend_ulong key; @@ -666,7 +669,9 @@ static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) / /* pass */ } else if (Z_TYPE_P(var) != IS_OBJECT) { return 0; - } else if (Z_REFCOUNT_P(var) == 1 && (Z_OBJ_P(var)->properties == NULL || GC_REFCOUNT(Z_OBJ_P(var)->properties) == 1)) { + } else if (!in_rcn_array + && Z_REFCOUNT_P(var) == 1 + && (Z_OBJ_P(var)->properties == NULL || GC_REFCOUNT(Z_OBJ_P(var)->properties) == 1)) { return 0; } @@ -923,7 +928,7 @@ static int php_var_serialize_get_sleep_props( } /* }}} */ -static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable *ht, uint32_t count, bool incomplete_class, php_serialize_data_t var_hash) /* {{{ */ +static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable *ht, uint32_t count, bool incomplete_class, php_serialize_data_t var_hash, bool in_rcn_array) /* {{{ */ { smart_str_append_unsigned(buf, count); smart_str_appendl(buf, ":{", 2); @@ -953,19 +958,19 @@ static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable if (Z_TYPE_P(data) == IS_ARRAY) { if (UNEXPECTED(Z_IS_RECURSIVE_P(data)) || UNEXPECTED(Z_TYPE_P(struc) == IS_ARRAY && Z_ARR_P(data) == Z_ARR_P(struc))) { - php_add_var_hash(var_hash, struc); + php_add_var_hash(var_hash, struc, in_rcn_array); smart_str_appendl(buf, "N;", 2); } else { if (Z_REFCOUNTED_P(data)) { Z_PROTECT_RECURSION_P(data); } - php_var_serialize_intern(buf, data, var_hash); + php_var_serialize_intern(buf, data, var_hash, in_rcn_array, false); if (Z_REFCOUNTED_P(data)) { Z_UNPROTECT_RECURSION_P(data); } } } else { - php_var_serialize_intern(buf, data, var_hash); + php_var_serialize_intern(buf, data, var_hash, in_rcn_array, false); } } ZEND_HASH_FOREACH_END(); } @@ -980,13 +985,13 @@ static void php_var_serialize_class(smart_str *buf, zval *struc, HashTable *ht, if (php_var_serialize_get_sleep_props(&props, struc, ht) == SUCCESS) { php_var_serialize_class_name(buf, struc); php_var_serialize_nested_data( - buf, struc, &props, zend_hash_num_elements(&props), /* incomplete_class */ 0, var_hash); + buf, struc, &props, zend_hash_num_elements(&props), /* incomplete_class */ 0, var_hash, GC_REFCOUNT(&props) > 1); } zend_hash_destroy(&props); } /* }}} */ -static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash) /* {{{ */ +static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash, bool in_rcn_array, bool is_root) /* {{{ */ { zend_long var_already; HashTable *myht; @@ -995,7 +1000,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ return; } - if (var_hash && (var_already = php_add_var_hash(var_hash, struc))) { + if (var_hash && (var_already = php_add_var_hash(var_hash, struc, in_rcn_array))) { if (var_already == -1) { /* Reference to an object that failed to serialize, replace with null. */ smart_str_appendl(buf, "N;", 2); @@ -1104,7 +1109,7 @@ again: if (Z_ISREF_P(data) && Z_REFCOUNT_P(data) == 1) { data = Z_REFVAL_P(data); } - php_var_serialize_intern(buf, data, var_hash); + php_var_serialize_intern(buf, data, var_hash, Z_REFCOUNT(retval) > 1, false); } ZEND_HASH_FOREACH_END(); smart_str_appendc(buf, '}'); @@ -1226,7 +1231,7 @@ again: prop = Z_REFVAL_P(prop); } - php_var_serialize_intern(buf, prop, var_hash); + php_var_serialize_intern(buf, prop, var_hash, false, false); } smart_str_appendc(buf, '}'); } else { @@ -1241,7 +1246,7 @@ again: if (count > 0 && incomplete_class) { --count; } - php_var_serialize_nested_data(buf, struc, myht, count, incomplete_class, var_hash); + php_var_serialize_nested_data(buf, struc, myht, count, incomplete_class, var_hash, GC_REFCOUNT(myht) > 1); zend_release_properties(myht); return; } @@ -1249,7 +1254,8 @@ again: smart_str_appendl(buf, "a:", 2); myht = Z_ARRVAL_P(struc); php_var_serialize_nested_data( - buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash); + buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash, + !is_root && (in_rcn_array || GC_REFCOUNT(myht) > 1)); return; case IS_REFERENCE: struc = Z_REFVAL_P(struc); @@ -1263,7 +1269,7 @@ again: PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *data) /* {{{ */ { - php_var_serialize_intern(buf, struc, *data); + php_var_serialize_intern(buf, struc, *data, false, true); smart_str_0(buf); } /* }}} */