Skip to content

Commit d7d3669

Browse files
committed
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 <dmitry@zend.com> Co-authored-by: Martin Hoch <martin@littlerobot.de> Closes GH-11349 Closes GH-11305
1 parent dc73b73 commit d7d3669

File tree

3 files changed

+66
-15
lines changed

3 files changed

+66
-15
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ PHP NEWS
3232
- Session:
3333
. Removed broken url support for transferring session ID. (ilutov)
3434

35+
- Standard:
36+
. Fix serialization of RC1 objects appearing in object graph twice. (ilutov)
37+
3538
06 Jul 2023, PHP 8.1.21
3639

3740
- CLI:
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
Serialization of RC1 objects appearing in object graph twice
3+
--FILE--
4+
<?php
5+
6+
function rcn() {
7+
$root = new stdClass;
8+
$end = new stdClass;
9+
$root->a = [$end];
10+
$root->b = $root->a;
11+
unset($end);
12+
echo serialize($root), "\n";
13+
}
14+
15+
function rcn_rc1() {
16+
$root = new stdClass;
17+
$end = new stdClass;
18+
$root->a = [[$end]];
19+
$root->b = $root->a;
20+
unset($end);
21+
echo serialize($root), "\n";
22+
}
23+
24+
function rcn_properties_ht() {
25+
$object = new stdClass;
26+
$object->object = new stdClass;
27+
$array = (array) $object;
28+
$root = [$object, $array];
29+
unset($object);
30+
unset($array);
31+
echo serialize($root), "\n";
32+
}
33+
34+
rcn();
35+
rcn_rc1();
36+
rcn_properties_ht();
37+
38+
?>
39+
--EXPECT--
40+
O:8:"stdClass":2:{s:1:"a";a:1:{i:0;O:8:"stdClass":0:{}}s:1:"b";a:1:{i:0;r:3;}}
41+
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;}}}
42+
a:2:{i:0;O:8:"stdClass":1:{s:6:"object";O:8:"stdClass":0:{}}i:1;a:1:{s:6:"object";r:3;}}

ext/standard/var.c

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -652,9 +652,12 @@ PHP_FUNCTION(var_export)
652652
}
653653
/* }}} */
654654

655-
static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash);
655+
static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash, bool in_rcn_array, bool is_root);
656656

657-
static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) /* {{{ */
657+
/**
658+
* @param bool in_rcn_array Whether the element appears in a potentially nested array with RC > 1.
659+
*/
660+
static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var, bool in_rcn_array) /* {{{ */
658661
{
659662
zval *zv;
660663
zend_ulong key;
@@ -666,7 +669,9 @@ static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) /
666669
/* pass */
667670
} else if (Z_TYPE_P(var) != IS_OBJECT) {
668671
return 0;
669-
} else if (Z_REFCOUNT_P(var) == 1 && (Z_OBJ_P(var)->properties == NULL || GC_REFCOUNT(Z_OBJ_P(var)->properties) == 1)) {
672+
} else if (!in_rcn_array
673+
&& Z_REFCOUNT_P(var) == 1
674+
&& (Z_OBJ_P(var)->properties == NULL || GC_REFCOUNT(Z_OBJ_P(var)->properties) == 1)) {
670675
return 0;
671676
}
672677

@@ -923,7 +928,7 @@ static int php_var_serialize_get_sleep_props(
923928
}
924929
/* }}} */
925930

926-
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) /* {{{ */
931+
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) /* {{{ */
927932
{
928933
smart_str_append_unsigned(buf, count);
929934
smart_str_appendl(buf, ":{", 2);
@@ -953,19 +958,19 @@ static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable
953958
if (Z_TYPE_P(data) == IS_ARRAY) {
954959
if (UNEXPECTED(Z_IS_RECURSIVE_P(data))
955960
|| UNEXPECTED(Z_TYPE_P(struc) == IS_ARRAY && Z_ARR_P(data) == Z_ARR_P(struc))) {
956-
php_add_var_hash(var_hash, struc);
961+
php_add_var_hash(var_hash, struc, in_rcn_array);
957962
smart_str_appendl(buf, "N;", 2);
958963
} else {
959964
if (Z_REFCOUNTED_P(data)) {
960965
Z_PROTECT_RECURSION_P(data);
961966
}
962-
php_var_serialize_intern(buf, data, var_hash);
967+
php_var_serialize_intern(buf, data, var_hash, in_rcn_array, false);
963968
if (Z_REFCOUNTED_P(data)) {
964969
Z_UNPROTECT_RECURSION_P(data);
965970
}
966971
}
967972
} else {
968-
php_var_serialize_intern(buf, data, var_hash);
973+
php_var_serialize_intern(buf, data, var_hash, in_rcn_array, false);
969974
}
970975
} ZEND_HASH_FOREACH_END();
971976
}
@@ -980,13 +985,13 @@ static void php_var_serialize_class(smart_str *buf, zval *struc, HashTable *ht,
980985
if (php_var_serialize_get_sleep_props(&props, struc, ht) == SUCCESS) {
981986
php_var_serialize_class_name(buf, struc);
982987
php_var_serialize_nested_data(
983-
buf, struc, &props, zend_hash_num_elements(&props), /* incomplete_class */ 0, var_hash);
988+
buf, struc, &props, zend_hash_num_elements(&props), /* incomplete_class */ 0, var_hash, GC_REFCOUNT(&props) > 1);
984989
}
985990
zend_hash_destroy(&props);
986991
}
987992
/* }}} */
988993

989-
static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash) /* {{{ */
994+
static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash, bool in_rcn_array, bool is_root) /* {{{ */
990995
{
991996
zend_long var_already;
992997
HashTable *myht;
@@ -995,7 +1000,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_
9951000
return;
9961001
}
9971002

998-
if (var_hash && (var_already = php_add_var_hash(var_hash, struc))) {
1003+
if (var_hash && (var_already = php_add_var_hash(var_hash, struc, in_rcn_array))) {
9991004
if (var_already == -1) {
10001005
/* Reference to an object that failed to serialize, replace with null. */
10011006
smart_str_appendl(buf, "N;", 2);
@@ -1104,7 +1109,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_
11041109
if (Z_ISREF_P(data) && Z_REFCOUNT_P(data) == 1) {
11051110
data = Z_REFVAL_P(data);
11061111
}
1107-
php_var_serialize_intern(buf, data, var_hash);
1112+
php_var_serialize_intern(buf, data, var_hash, Z_REFCOUNT(retval) > 1, false);
11081113
} ZEND_HASH_FOREACH_END();
11091114
smart_str_appendc(buf, '}');
11101115

@@ -1226,7 +1231,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_
12261231
prop = Z_REFVAL_P(prop);
12271232
}
12281233

1229-
php_var_serialize_intern(buf, prop, var_hash);
1234+
php_var_serialize_intern(buf, prop, var_hash, false, false);
12301235
}
12311236
smart_str_appendc(buf, '}');
12321237
} else {
@@ -1241,15 +1246,16 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_
12411246
if (count > 0 && incomplete_class) {
12421247
--count;
12431248
}
1244-
php_var_serialize_nested_data(buf, struc, myht, count, incomplete_class, var_hash);
1249+
php_var_serialize_nested_data(buf, struc, myht, count, incomplete_class, var_hash, GC_REFCOUNT(myht) > 1);
12451250
zend_release_properties(myht);
12461251
return;
12471252
}
12481253
case IS_ARRAY:
12491254
smart_str_appendl(buf, "a:", 2);
12501255
myht = Z_ARRVAL_P(struc);
12511256
php_var_serialize_nested_data(
1252-
buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash);
1257+
buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash,
1258+
!is_root && (in_rcn_array || GC_REFCOUNT(myht) > 1));
12531259
return;
12541260
case IS_REFERENCE:
12551261
struc = Z_REFVAL_P(struc);
@@ -1263,7 +1269,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_
12631269

12641270
PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *data) /* {{{ */
12651271
{
1266-
php_var_serialize_intern(buf, struc, *data);
1272+
php_var_serialize_intern(buf, struc, *data, false, true);
12671273
smart_str_0(buf);
12681274
}
12691275
/* }}} */

0 commit comments

Comments
 (0)