Skip to content

Commit 8b9679e

Browse files
authored
Make var_export/debug_zval_dump check for infinite recursion on the *object* (#9448)
Switch the recursion check from the result of `get_properties_for` (the returned hash table of properties) to just checking for infinite recursion on the object. - In order for a native datastructure to correctly implement `*get_properties_for` for var_export's cycle detection, it would need to return the exact same array every time prior to this PR. Prior to this commit, the requirements for cycle detection would prevent SplFixedArray or similar classes from returning a temporary array that: 1. Wouldn't be affected by unexpected mutations from error handlers 2. Could be garbage collected instead.
1 parent a05add6 commit 8b9679e

File tree

2 files changed

+42
-20
lines changed

2 files changed

+42
-20
lines changed

ext/spl/tests/gh8044.phpt

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Bug GH-8044 (var_export/debug_zval_dump HT_ASSERT_RC1 debug failure for SplFixedArray)
3+
--FILE--
4+
<?php
5+
call_user_func(function () {
6+
$x = new SplFixedArray(1);
7+
$x[0] = $x;
8+
var_export($x); echo "\n";
9+
debug_zval_dump($x); echo "\n";
10+
});
11+
?>
12+
--EXPECTF--
13+
Warning: var_export does not handle circular references in %s on line 5
14+
\SplFixedArray::__set_state(array(
15+
0 => NULL,
16+
))
17+
object(SplFixedArray)#2 (1) refcount(4){
18+
[0]=>
19+
*RECURSION*
20+
}

ext/standard/var.c

+22-20
Original file line numberDiff line numberDiff line change
@@ -343,15 +343,17 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */
343343
PUTS("}\n");
344344
break;
345345
case IS_OBJECT:
346-
myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_DEBUG);
347-
if (myht) {
348-
if (GC_IS_RECURSIVE(myht)) {
349-
PUTS("*RECURSION*\n");
350-
zend_release_properties(myht);
351-
return;
352-
}
353-
GC_PROTECT_RECURSION(myht);
346+
/* Check if this is already recursing on the object before calling zend_get_properties_for,
347+
* to allow infinite recursion detection to work even if classes return temporary arrays,
348+
* and to avoid the need to update the properties table in place to reflect the state
349+
* if the result won't be used. (https://github.com/php/php-src/issues/8044) */
350+
if (Z_IS_RECURSIVE_P(struc)) {
351+
PUTS("*RECURSION*\n");
352+
return;
354353
}
354+
Z_PROTECT_RECURSION_P(struc);
355+
356+
myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_DEBUG);
355357
class_name = Z_OBJ_HANDLER_P(struc, get_class_name)(Z_OBJ_P(struc));
356358
php_printf("object(%s)#%d (%d) refcount(%u){\n", ZSTR_VAL(class_name), Z_OBJ_HANDLE_P(struc), myht ? zend_array_count(myht) : 0, Z_REFCOUNT_P(struc));
357359
zend_string_release_ex(class_name, 0);
@@ -370,13 +372,13 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */
370372
zval_object_property_dump(prop_info, val, index, key, level);
371373
}
372374
} ZEND_HASH_FOREACH_END();
373-
GC_UNPROTECT_RECURSION(myht);
374375
zend_release_properties(myht);
375376
}
376377
if (level > 1) {
377378
php_printf("%*c", level - 1, ' ');
378379
}
379380
PUTS("}\n");
381+
Z_UNPROTECT_RECURSION_P(struc);
380382
break;
381383
case IS_RESOURCE: {
382384
const char *type_name = zend_rsrc_list_get_rsrc_type(Z_RES_P(struc));
@@ -552,17 +554,17 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf) /* {{{ */
552554
break;
553555

554556
case IS_OBJECT:
555-
myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_VAR_EXPORT);
556-
if (myht) {
557-
if (GC_IS_RECURSIVE(myht)) {
558-
smart_str_appendl(buf, "NULL", 4);
559-
zend_error(E_WARNING, "var_export does not handle circular references");
560-
zend_release_properties(myht);
561-
return;
562-
} else {
563-
GC_TRY_PROTECT_RECURSION(myht);
564-
}
557+
/* Check if this is already recursing on the object before calling zend_get_properties_for,
558+
* to allow infinite recursion detection to work even if classes return temporary arrays,
559+
* and to avoid the need to update the properties table in place to reflect the state
560+
* if the result won't be used. (https://github.com/php/php-src/issues/8044) */
561+
if (Z_IS_RECURSIVE_P(struc)) {
562+
smart_str_appendl(buf, "NULL", 4);
563+
zend_error(E_WARNING, "var_export does not handle circular references");
564+
return;
565565
}
566+
Z_PROTECT_RECURSION_P(struc);
567+
myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_VAR_EXPORT);
566568
if (level > 1) {
567569
smart_str_appendc(buf, '\n');
568570
buffer_append_spaces(buf, level - 1);
@@ -593,9 +595,9 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf) /* {{{ */
593595
php_object_element_export(val, index, key, level, buf);
594596
} ZEND_HASH_FOREACH_END();
595597
}
596-
GC_TRY_UNPROTECT_RECURSION(myht);
597598
zend_release_properties(myht);
598599
}
600+
Z_UNPROTECT_RECURSION_P(struc);
599601
if (level > 1 && !is_enum) {
600602
buffer_append_spaces(buf, level - 1);
601603
}

0 commit comments

Comments
 (0)