Skip to content

Commit 4808fb6

Browse files
GirgiasGirgias
committed
Merge branch 'PHP-8.1' into PHP-8.2
* PHP-8.1: Fix GH-10519: Array Data Address Reference Issue
2 parents e223bf2 + 49b2ff5 commit 4808fb6

File tree

5 files changed

+176
-8
lines changed

5 files changed

+176
-8
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ PHP NEWS
2727
. Add missing error checks on EVP_MD_CTX_create() and EVP_VerifyInit().
2828
(nielsdos)
2929

30+
- SPL:
31+
. Fixed bug GH-10519 (Array Data Address Reference Issue). (Nathan Freeman)
32+
3033
16 Mar 2023, PHP 8.2.4
3134

3235
- Core:

‎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;
@@ -150,6 +151,8 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
150151
object_properties_init(&intern->std, class_type);
151152

152153
intern->ar_flags = 0;
154+
intern->is_child = false;
155+
intern->bucket = NULL;
153156
intern->ce_get_iterator = spl_ce_ArrayIterator;
154157
if (orig) {
155158
spl_array_object *other = spl_array_from_obj(orig);
@@ -440,6 +443,22 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ
440443
return spl_array_read_dimension_ex(1, object, offset, type, rv);
441444
} /* }}} */
442445

446+
/*
447+
* The assertion(HT_ASSERT_RC1(ht)) failed because the refcount was increased manually when intern->is_child is true.
448+
* We have to set the refcount to 1 to make assertion success and restore the refcount to the original value after
449+
* modifying the array when intern->is_child is true.
450+
*/
451+
static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t refcount) /* {{{ */
452+
{
453+
uint32_t old_refcount = 0;
454+
if (is_child) {
455+
old_refcount = GC_REFCOUNT(ht);
456+
GC_SET_REFCOUNT(ht, refcount);
457+
}
458+
459+
return old_refcount;
460+
} /* }}} */
461+
443462
static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */
444463
{
445464
spl_array_object *intern = spl_array_from_obj(object);
@@ -463,9 +482,16 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
463482
}
464483

465484
Z_TRY_ADDREF_P(value);
485+
486+
uint32_t refcount = 0;
466487
if (!offset || Z_TYPE_P(offset) == IS_NULL) {
467488
ht = spl_array_get_hash_table(intern);
489+
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
468490
zend_hash_next_index_insert(ht, value);
491+
492+
if (refcount) {
493+
spl_array_set_refcount(intern->is_child, ht, refcount);
494+
}
469495
return;
470496
}
471497

@@ -476,12 +502,17 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
476502
}
477503

478504
ht = spl_array_get_hash_table(intern);
505+
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
479506
if (key.key) {
480507
zend_hash_update_ind(ht, key.key, value);
481508
spl_hash_key_release(&key);
482509
} else {
483510
zend_hash_index_update(ht, key.h, value);
484511
}
512+
513+
if (refcount) {
514+
spl_array_set_refcount(intern->is_child, ht, refcount);
515+
}
485516
} /* }}} */
486517

487518
static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */
@@ -511,6 +542,8 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
511542
}
512543

513544
ht = spl_array_get_hash_table(intern);
545+
uint32_t refcount = spl_array_set_refcount(intern->is_child, ht, 1);
546+
514547
if (key.key) {
515548
zval *data = zend_hash_find(ht, key.key);
516549
if (data) {
@@ -533,6 +566,10 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
533566
} else {
534567
zend_hash_index_del(ht, key.h);
535568
}
569+
570+
if (refcount) {
571+
spl_array_set_refcount(intern->is_child, ht, refcount);
572+
}
536573
} /* }}} */
537574

538575
static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */
@@ -994,6 +1031,16 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
9941031
} else {
9951032
//??? TODO: try to avoid array duplication
9961033
ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array)));
1034+
1035+
if (intern->is_child) {
1036+
Z_TRY_DELREF_P(&intern->bucket->val);
1037+
/*
1038+
* replace bucket->val with copied array, so the changes between
1039+
* parent and child object can affect each other.
1040+
*/
1041+
intern->bucket->val = intern->array;
1042+
Z_TRY_ADDREF_P(&intern->array);
1043+
}
9971044
}
9981045
} else {
9991046
if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) {
@@ -1469,6 +1516,22 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren)
14691516
}
14701517
/* }}} */
14711518

1519+
static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */
1520+
{
1521+
object_init_ex(retval, pce);
1522+
spl_array_object *new_intern = Z_SPLARRAY_P(retval);
1523+
/*
1524+
* set new_intern->is_child is true to indicate that the object was created by
1525+
* RecursiveArrayIterator::getChildren() method.
1526+
*/
1527+
new_intern->is_child = true;
1528+
1529+
/* find the bucket of parent object. */
1530+
new_intern->bucket = (Bucket *)((char *)(arg1) - XtOffsetOf(Bucket, val));;
1531+
zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2);
1532+
}
1533+
/* }}} */
1534+
14721535
/* {{{ Create a sub iterator for the current element (same class as $this) */
14731536
PHP_METHOD(RecursiveArrayIterator, getChildren)
14741537
{
@@ -1499,7 +1562,7 @@ PHP_METHOD(RecursiveArrayIterator, getChildren)
14991562
}
15001563

15011564
ZVAL_LONG(&flags, intern->ar_flags);
1502-
spl_instantiate_arg_ex2(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags);
1565+
spl_instantiate_child_arg(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags);
15031566
}
15041567
/* }}} */
15051568

‎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)