Skip to content

Commit b8f7265

Browse files
committed
Make var_export/debug_zval_dump first check for infinite recursion on the object
This test case previously failed with an assertion error in debug builds (for updating an index of the array to set it to the same value it already had) Then repeat the existing check on the return value of `get_properties_for` in case there was a reason for doing it that way. 1. `HT_ASSERT_RC1(ht);` would fail for SplFixedArray and related datastructures. 2. 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. This 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. Note that SplFixedArray continues to need to return `object->properties` until php 9.0, when dynamic properties are forbidden. (GitHub wasn't updating the previous PR despite pushing the branch to the right repo earlier, creating a new branch) Closes GH-8044
1 parent 7b90ebe commit b8f7265

File tree

2 files changed

+44
-0
lines changed

2 files changed

+44
-0
lines changed

ext/spl/tests/gh8044.phpt

Lines changed: 20 additions & 0 deletions
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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,11 +343,22 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */
343343
PUTS("}\n");
344344
break;
345345
case IS_OBJECT:
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;
353+
}
354+
Z_PROTECT_RECURSION_P(struc);
355+
346356
myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_DEBUG);
347357
if (myht) {
348358
if (GC_IS_RECURSIVE(myht)) {
349359
PUTS("*RECURSION*\n");
350360
zend_release_properties(myht);
361+
Z_UNPROTECT_RECURSION_P(struc);
351362
return;
352363
}
353364
GC_PROTECT_RECURSION(myht);
@@ -377,6 +388,7 @@ PHPAPI void php_debug_zval_dump(zval *struc, int level) /* {{{ */
377388
php_printf("%*c", level - 1, ' ');
378389
}
379390
PUTS("}\n");
391+
Z_UNPROTECT_RECURSION_P(struc);
380392
break;
381393
case IS_RESOURCE: {
382394
const char *type_name = zend_rsrc_list_get_rsrc_type(Z_RES_P(struc));
@@ -552,9 +564,20 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf) /* {{{ */
552564
break;
553565

554566
case IS_OBJECT:
567+
/* Check if this is already recursing on the object before calling zend_get_properties_for,
568+
* to allow infinite recursion detection to work even if classes return temporary arrays,
569+
* and to avoid the need to update the properties table in place to reflect the state
570+
* if the result won't be used. (https://github.com/php/php-src/issues/8044) */
571+
if (Z_IS_RECURSIVE_P(struc)) {
572+
smart_str_appendl(buf, "NULL", 4);
573+
zend_error(E_WARNING, "var_export does not handle circular references");
574+
return;
575+
}
576+
Z_PROTECT_RECURSION_P(struc);
555577
myht = zend_get_properties_for(struc, ZEND_PROP_PURPOSE_VAR_EXPORT);
556578
if (myht) {
557579
if (GC_IS_RECURSIVE(myht)) {
580+
Z_UNPROTECT_RECURSION_P(struc);
558581
smart_str_appendl(buf, "NULL", 4);
559582
zend_error(E_WARNING, "var_export does not handle circular references");
560583
zend_release_properties(myht);
@@ -595,6 +618,7 @@ PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf) /* {{{ */
595618
GC_TRY_UNPROTECT_RECURSION(myht);
596619
zend_release_properties(myht);
597620
}
621+
Z_UNPROTECT_RECURSION_P(struc);
598622
if (level > 1 && !is_enum) {
599623
buffer_append_spaces(buf, level - 1);
600624
}

0 commit comments

Comments
 (0)