diff --git a/NEWS b/NEWS index 7997ecda26ead..1ccdfdc2b3686 100644 --- a/NEWS +++ b/NEWS @@ -1145,6 +1145,7 @@ PHP NEWS non-existing key). (Nikita) . Fixed bug #80724 (FilesystemIterator::FOLLOW_SYMLINKS remove KEY_AS_FILE from bitmask). (Cameron Porter) + . Fixed bug GH-10519 (Array Data Address Reference Issue). (Nathan Freeman) - Standard: . Fixed bug #81441 (gethostbyaddr('::1') returns ip instead of name after diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index e3d75712cd491..1d4683e0c9391 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -29,7 +29,6 @@ #include "php_spl.h" #include "spl_array_arginfo.h" #include "spl_functions.h" -#include "spl_engine.h" #include "spl_iterators.h" #include "spl_array.h" #include "spl_exceptions.h" @@ -63,6 +62,8 @@ typedef struct _spl_array_object { uint32_t ht_iter; int ar_flags; unsigned char nApplyCount; + bool is_child; + Bucket *bucket; zend_function *fptr_offset_get; zend_function *fptr_offset_set; zend_function *fptr_offset_has; @@ -167,6 +168,8 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o object_properties_init(&intern->std, class_type); intern->ar_flags = 0; + intern->is_child = false; + intern->bucket = NULL; intern->ce_get_iterator = spl_ce_ArrayIterator; if (orig) { spl_array_object *other = spl_array_from_obj(orig); @@ -477,6 +480,22 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ return spl_array_read_dimension_ex(1, object, offset, type, rv); } /* }}} */ +/* + * The assertion(HT_ASSERT_RC1(ht)) failed because the refcount was increased manually when intern->is_child is true. + * We have to set the refcount to 1 to make assertion success and restore the refcount to the original value after + * modifying the array when intern->is_child is true. + */ +static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t refcount) /* {{{ */ +{ + uint32_t old_refcount = 0; + if (is_child) { + old_refcount = GC_REFCOUNT(ht); + GC_SET_REFCOUNT(ht, refcount); + } + + return old_refcount; +} /* }}} */ + static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ { spl_array_object *intern = spl_array_from_obj(object); @@ -500,9 +519,16 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec } Z_TRY_ADDREF_P(value); + + uint32_t refcount = 0; if (!offset || Z_TYPE_P(offset) == IS_NULL) { ht = spl_array_get_hash_table(intern); + refcount = spl_array_set_refcount(intern->is_child, ht, 1); zend_hash_next_index_insert(ht, value); + + if (refcount) { + spl_array_set_refcount(intern->is_child, ht, refcount); + } return; } @@ -513,12 +539,17 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec } ht = spl_array_get_hash_table(intern); + refcount = spl_array_set_refcount(intern->is_child, ht, 1); if (key.key) { zend_hash_update_ind(ht, key.key, value); spl_hash_key_release(&key); } else { zend_hash_index_update(ht, key.h, value); } + + if (refcount) { + spl_array_set_refcount(intern->is_child, ht, refcount); + } } /* }}} */ static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */ @@ -548,6 +579,8 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec } ht = spl_array_get_hash_table(intern); + uint32_t refcount = spl_array_set_refcount(intern->is_child, ht, 1); + if (key.key) { zval *data = zend_hash_find(ht, key.key); if (data) { @@ -570,6 +603,10 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec } else { zend_hash_index_del(ht, key.h); } + + if (refcount) { + spl_array_set_refcount(intern->is_child, ht, refcount); + } } /* }}} */ static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */ @@ -1058,6 +1095,16 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar } else { //??? TODO: try to avoid array duplication ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array))); + + if (intern->is_child) { + Z_TRY_DELREF_P(&intern->bucket->val); + /* + * replace bucket->val with copied array, so the changes between + * parent and child object can affect each other. + */ + intern->bucket->val = intern->array; + Z_TRY_ADDREF_P(&intern->array); + } } } else { if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) { @@ -1544,6 +1591,22 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren) } /* }}} */ +static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */ +{ + object_init_ex(retval, pce); + spl_array_object *new_intern = Z_SPLARRAY_P(retval); + /* + * set new_intern->is_child is true to indicate that the object was created by + * RecursiveArrayIterator::getChildren() method. + */ + new_intern->is_child = true; + + /* find the bucket of parent object. */ + new_intern->bucket = (Bucket *)((char *)(arg1) - XtOffsetOf(Bucket, val));; + zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2); +} +/* }}} */ + /* {{{ Create a sub iterator for the current element (same class as $this) */ PHP_METHOD(RecursiveArrayIterator, getChildren) { @@ -1574,7 +1637,7 @@ PHP_METHOD(RecursiveArrayIterator, getChildren) } ZVAL_LONG(&flags, intern->ar_flags); - spl_instantiate_arg_ex2(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags); + spl_instantiate_child_arg(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags); } /* }}} */ diff --git a/ext/spl/tests/bug42654.phpt b/ext/spl/tests/bug42654.phpt index 20aad74b73423..eb72863e08759 100644 --- a/ext/spl/tests/bug42654.phpt +++ b/ext/spl/tests/bug42654.phpt @@ -110,11 +110,11 @@ object(RecursiveArrayIterator)#%d (1) { [2]=> array(2) { [2]=> - string(4) "val2" + string(5) "alter" [3]=> array(1) { [3]=> - string(4) "val3" + string(5) "alter" } } [4]=> @@ -129,11 +129,11 @@ object(RecursiveArrayIterator)#%d (1) { [2]=> array(2) { [2]=> - string(4) "val2" + string(5) "alter" [3]=> array(1) { [3]=> - string(4) "val3" + string(5) "alter" } } [4]=> @@ -146,11 +146,11 @@ array(3) { [2]=> array(2) { [2]=> - string(4) "val2" + string(5) "alter" [3]=> array(1) { [3]=> - string(4) "val3" + string(5) "alter" } } [4]=> diff --git a/ext/spl/tests/bug42654_2.phpt b/ext/spl/tests/bug42654_2.phpt new file mode 100644 index 0000000000000..bd290247dbdc4 --- /dev/null +++ b/ext/spl/tests/bug42654_2.phpt @@ -0,0 +1,33 @@ +--TEST-- +Bug #42654 (RecursiveIteratorIterator modifies only part of leaves) +--FILE-- + 'val1', array('key2' => 'val2'), 'key3' => 'val3'); + +$iterator = new RecursiveIteratorIterator(new RecursiveArrayIterator($data)); +foreach($iterator as $foo) { + $key = $iterator->key(); + switch($key) { + case 'key1': // first level + case 'key2': // recursive level + echo "update $key".PHP_EOL; + $iterator->offsetSet($key, 'alter'); + } +} +$copy = $iterator->getArrayCopy(); +var_dump($copy); +?> +--EXPECT-- +update key1 +update key2 +array(3) { + ["key1"]=> + string(5) "alter" + [0]=> + array(1) { + ["key2"]=> + string(5) "alter" + } + ["key3"]=> + string(4) "val3" +} diff --git a/ext/spl/tests/gh10519.phpt b/ext/spl/tests/gh10519.phpt new file mode 100644 index 0000000000000..1f7572d6e8ca6 --- /dev/null +++ b/ext/spl/tests/gh10519.phpt @@ -0,0 +1,69 @@ +--TEST-- +Bug GH-10519 (Array Data Address Reference Issue) +--FILE-- +getArrayCopy(), $colname); + } + + public function bugySetMethod($key, $value) + { + $data = &$this; + + while ($data->hasChildren()) { + $data = $data->getChildren(); + } + $data->offsetSet($key, $value); + } + + public function jsonSerialize(): mixed + { + return $this; + } +} + +$a = [ + 'test' => [ + 'a' => (object) [2 => '',3 => '',4 => ''], + ] +]; + +$example = A::init($a); +$example->bugySetMethod(5, 'in here'); +var_dump(json_encode($example)); +var_dump(json_encode($a)); + +$b = [ + 'test' => [ + 'b' => [2 => '',3 => '',4 => ''], + ] +]; +$example = A::init($b); + +$example->bugySetMethod(5, 'must be here'); +var_dump(json_encode($example)); +var_dump(json_encode($b)); +?> +--EXPECT-- +string(51) "{"test":{"a":{"2":"","3":"","4":"","5":"in here"}}}" +string(51) "{"test":{"a":{"2":"","3":"","4":"","5":"in here"}}}" +string(56) "{"test":{"b":{"2":"","3":"","4":"","5":"must be here"}}}" +string(37) "{"test":{"b":{"2":"","3":"","4":""}}}"