Skip to content

Commit 1d9d1fe

Browse files
GirgiasGirgias
committed
Merge branch 'PHP-8.2'
* PHP-8.2: Fix GH-10519: Array Data Address Reference Issue
2 parents 6f1e5ff + 4808fb6 commit 1d9d1fe

File tree

4 files changed

+173
-8
lines changed

4 files changed

+173
-8
lines changed

‎ext/spl/spl_array.c

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
#include "php_spl.h"
3030
#include "spl_functions.h"
31-
#include "spl_engine.h"
3231
#include "spl_iterators.h"
3332
#include "spl_array.h"
3433
#include "spl_array_arginfo.h"
@@ -46,6 +45,8 @@ typedef struct _spl_array_object {
4645
uint32_t ht_iter;
4746
int ar_flags;
4847
unsigned char nApplyCount;
48+
bool is_child;
49+
Bucket *bucket;
4950
zend_function *fptr_offset_get;
5051
zend_function *fptr_offset_set;
5152
zend_function *fptr_offset_has;
@@ -165,6 +166,8 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
165166
object_properties_init(&intern->std, class_type);
166167

167168
intern->ar_flags = 0;
169+
intern->is_child = false;
170+
intern->bucket = NULL;
168171
intern->ce_get_iterator = spl_ce_ArrayIterator;
169172
if (orig) {
170173
spl_array_object *other = spl_array_from_obj(orig);
@@ -451,6 +454,22 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ
451454
return spl_array_read_dimension_ex(1, object, offset, type, rv);
452455
} /* }}} */
453456

457+
/*
458+
* The assertion(HT_ASSERT_RC1(ht)) failed because the refcount was increased manually when intern->is_child is true.
459+
* We have to set the refcount to 1 to make assertion success and restore the refcount to the original value after
460+
* modifying the array when intern->is_child is true.
461+
*/
462+
static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t refcount) /* {{{ */
463+
{
464+
uint32_t old_refcount = 0;
465+
if (is_child) {
466+
old_refcount = GC_REFCOUNT(ht);
467+
GC_SET_REFCOUNT(ht, refcount);
468+
}
469+
470+
return old_refcount;
471+
} /* }}} */
472+
454473
static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */
455474
{
456475
spl_array_object *intern = spl_array_from_obj(object);
@@ -474,9 +493,16 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
474493
}
475494

476495
Z_TRY_ADDREF_P(value);
496+
497+
uint32_t refcount = 0;
477498
if (!offset || Z_TYPE_P(offset) == IS_NULL) {
478499
ht = spl_array_get_hash_table(intern);
500+
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
479501
zend_hash_next_index_insert(ht, value);
502+
503+
if (refcount) {
504+
spl_array_set_refcount(intern->is_child, ht, refcount);
505+
}
480506
return;
481507
}
482508

@@ -487,12 +513,17 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
487513
}
488514

489515
ht = spl_array_get_hash_table(intern);
516+
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
490517
if (key.key) {
491518
zend_hash_update_ind(ht, key.key, value);
492519
spl_hash_key_release(&key);
493520
} else {
494521
zend_hash_index_update(ht, key.h, value);
495522
}
523+
524+
if (refcount) {
525+
spl_array_set_refcount(intern->is_child, ht, refcount);
526+
}
496527
} /* }}} */
497528

498529
static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */
@@ -522,6 +553,8 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
522553
}
523554

524555
ht = spl_array_get_hash_table(intern);
556+
uint32_t refcount = spl_array_set_refcount(intern->is_child, ht, 1);
557+
525558
if (key.key) {
526559
zval *data = zend_hash_find(ht, key.key);
527560
if (data) {
@@ -544,6 +577,10 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
544577
} else {
545578
zend_hash_index_del(ht, key.h);
546579
}
580+
581+
if (refcount) {
582+
spl_array_set_refcount(intern->is_child, ht, refcount);
583+
}
547584
} /* }}} */
548585

549586
static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */
@@ -1005,6 +1042,16 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
10051042
} else {
10061043
//??? TODO: try to avoid array duplication
10071044
ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array)));
1045+
1046+
if (intern->is_child) {
1047+
Z_TRY_DELREF_P(&intern->bucket->val);
1048+
/*
1049+
* replace bucket->val with copied array, so the changes between
1050+
* parent and child object can affect each other.
1051+
*/
1052+
intern->bucket->val = intern->array;
1053+
Z_TRY_ADDREF_P(&intern->array);
1054+
}
10081055
}
10091056
} else {
10101057
if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) {
@@ -1480,6 +1527,22 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren)
14801527
}
14811528
/* }}} */
14821529

1530+
static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */
1531+
{
1532+
object_init_ex(retval, pce);
1533+
spl_array_object *new_intern = Z_SPLARRAY_P(retval);
1534+
/*
1535+
* set new_intern->is_child is true to indicate that the object was created by
1536+
* RecursiveArrayIterator::getChildren() method.
1537+
*/
1538+
new_intern->is_child = true;
1539+
1540+
/* find the bucket of parent object. */
1541+
new_intern->bucket = (Bucket *)((char *)(arg1) - XtOffsetOf(Bucket, val));;
1542+
zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2);
1543+
}
1544+
/* }}} */
1545+
14831546
/* {{{ Create a sub iterator for the current element (same class as $this) */
14841547
PHP_METHOD(RecursiveArrayIterator, getChildren)
14851548
{
@@ -1510,7 +1573,7 @@ PHP_METHOD(RecursiveArrayIterator, getChildren)
15101573
}
15111574

15121575
ZVAL_LONG(&flags, intern->ar_flags);
1513-
spl_instantiate_arg_ex2(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags);
1576+
spl_instantiate_child_arg(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags);
15141577
}
15151578
/* }}} */
15161579

‎ext/spl/tests/bug42654.phpt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ object(RecursiveArrayIterator)#%d (1) {
110110
[2]=>
111111
array(2) {
112112
[2]=>
113-
string(4) "val2"
113+
string(5) "alter"
114114
[3]=>
115115
array(1) {
116116
[3]=>
117-
string(4) "val3"
117+
string(5) "alter"
118118
}
119119
}
120120
[4]=>
@@ -129,11 +129,11 @@ object(RecursiveArrayIterator)#%d (1) {
129129
[2]=>
130130
array(2) {
131131
[2]=>
132-
string(4) "val2"
132+
string(5) "alter"
133133
[3]=>
134134
array(1) {
135135
[3]=>
136-
string(4) "val3"
136+
string(5) "alter"
137137
}
138138
}
139139
[4]=>
@@ -146,11 +146,11 @@ array(3) {
146146
[2]=>
147147
array(2) {
148148
[2]=>
149-
string(4) "val2"
149+
string(5) "alter"
150150
[3]=>
151151
array(1) {
152152
[3]=>
153-
string(4) "val3"
153+
string(5) "alter"
154154
}
155155
}
156156
[4]=>

‎ext/spl/tests/bug42654_2.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
Bug #42654 (RecursiveIteratorIterator modifies only part of leaves)
3+
--FILE--
4+
<?php
5+
$data = array ('key1' => 'val1', array('key2' => 'val2'), 'key3' => 'val3');
6+
7+
$iterator = new RecursiveIteratorIterator(new RecursiveArrayIterator($data));
8+
foreach($iterator as $foo) {
9+
$key = $iterator->key();
10+
switch($key) {
11+
case 'key1': // first level
12+
case 'key2': // recursive level
13+
echo "update $key".PHP_EOL;
14+
$iterator->offsetSet($key, 'alter');
15+
}
16+
}
17+
$copy = $iterator->getArrayCopy();
18+
var_dump($copy);
19+
?>
20+
--EXPECT--
21+
update key1
22+
update key2
23+
array(3) {
24+
["key1"]=>
25+
string(5) "alter"
26+
[0]=>
27+
array(1) {
28+
["key2"]=>
29+
string(5) "alter"
30+
}
31+
["key3"]=>
32+
string(4) "val3"
33+
}

‎ext/spl/tests/gh10519.phpt

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
--TEST--
2+
Bug GH-10519 (Array Data Address Reference Issue)
3+
--FILE--
4+
<?php
5+
interface DataInterface extends JsonSerializable, RecursiveIterator, Iterator
6+
{
7+
public static function init(Traversable $data): DataInterface;
8+
}
9+
10+
class A extends RecursiveArrayIterator implements DataInterface
11+
{
12+
public function __construct($data)
13+
{
14+
parent::__construct($data);
15+
}
16+
17+
public static function init($data): DataInterface
18+
{
19+
return new static($data);
20+
}
21+
22+
public function getCols(string $colname): array
23+
{
24+
return array_column($this->getArrayCopy(), $colname);
25+
}
26+
27+
public function bugySetMethod($key, $value)
28+
{
29+
$data = &$this;
30+
31+
while ($data->hasChildren()) {
32+
$data = $data->getChildren();
33+
}
34+
$data->offsetSet($key, $value);
35+
}
36+
37+
public function jsonSerialize(): mixed
38+
{
39+
return $this;
40+
}
41+
}
42+
43+
$a = [
44+
'test' => [
45+
'a' => (object) [2 => '',3 => '',4 => ''],
46+
]
47+
];
48+
49+
$example = A::init($a);
50+
$example->bugySetMethod(5, 'in here');
51+
var_dump(json_encode($example));
52+
var_dump(json_encode($a));
53+
54+
$b = [
55+
'test' => [
56+
'b' => [2 => '',3 => '',4 => ''],
57+
]
58+
];
59+
$example = A::init($b);
60+
61+
$example->bugySetMethod(5, 'must be here');
62+
var_dump(json_encode($example));
63+
var_dump(json_encode($b));
64+
?>
65+
--EXPECT--
66+
string(51) "{"test":{"a":{"2":"","3":"","4":"","5":"in here"}}}"
67+
string(51) "{"test":{"a":{"2":"","3":"","4":"","5":"in here"}}}"
68+
string(56) "{"test":{"b":{"2":"","3":"","4":"","5":"must be here"}}}"
69+
string(37) "{"test":{"b":{"2":"","3":"","4":""}}}"

0 commit comments

Comments
 (0)