Skip to content

Commit d988897

Browse files
NathanFreemanNathanFreeman
committed
Fix bug php#10519 
1 parent 729f006 commit d988897

File tree

5 files changed

+157
-9
lines changed

5 files changed

+157
-9
lines changed

‎NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,7 @@ PHP NEWS
11451145
non-existing key). (Nikita)
11461146
. Fixed bug #80724 (FilesystemIterator::FOLLOW_SYMLINKS remove KEY_AS_FILE
11471147
from bitmask). (Cameron Porter)
1148+
. Fixed bug GH-10519 (Array Data Address Reference Issue). (Nathan Freeman)
11481149

11491150
- Standard:
11501151
. Fixed bug #81441 (gethostbyaddr('::1') returns ip instead of name after

‎ext/spl/spl_array.c

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "php_spl.h"
3030
#include "spl_array_arginfo.h"
3131
#include "spl_functions.h"
32-
#include "spl_engine.h"
3332
#include "spl_iterators.h"
3433
#include "spl_array.h"
3534
#include "spl_exceptions.h"
@@ -63,6 +62,8 @@ typedef struct _spl_array_object {
6362
uint32_t ht_iter;
6463
int ar_flags;
6564
unsigned char nApplyCount;
65+
bool is_child;
66+
Bucket *bucket;
6667
zend_function *fptr_offset_get;
6768
zend_function *fptr_offset_set;
6869
zend_function *fptr_offset_has;
@@ -167,6 +168,8 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
167168
object_properties_init(&intern->std, class_type);
168169

169170
intern->ar_flags = 0;
171+
intern->is_child = false;
172+
intern->bucket = NULL;
170173
intern->ce_get_iterator = spl_ce_ArrayIterator;
171174
if (orig) {
172175
spl_array_object *other = spl_array_from_obj(orig);
@@ -477,11 +480,23 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ
477480
return spl_array_read_dimension_ex(1, object, offset, type, rv);
478481
} /* }}} */
479482

483+
static uint32_t spl_array_set_refcount(spl_array_object *intern, HashTable *ht, uint32_t refcount) /* {{{ */
484+
{
485+
uint32_t old_refcount = 0;
486+
if (intern->is_child) {
487+
old_refcount = GC_REFCOUNT(ht);
488+
GC_SET_REFCOUNT(ht, refcount);
489+
}
490+
491+
return old_refcount;
492+
} /* }}} */
493+
480494
static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */
481495
{
482496
spl_array_object *intern = spl_array_from_obj(object);
483497
HashTable *ht;
484498
spl_hash_key key;
499+
uint32_t refcount = 0;
485500

486501
if (check_inherited && intern->fptr_offset_set) {
487502
zval tmp;
@@ -502,7 +517,12 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
502517
Z_TRY_ADDREF_P(value);
503518
if (!offset || Z_TYPE_P(offset) == IS_NULL) {
504519
ht = spl_array_get_hash_table(intern);
520+
refcount = spl_array_set_refcount(intern, ht, 1);
505521
zend_hash_next_index_insert(ht, value);
522+
523+
if (refcount) {
524+
spl_array_set_refcount(intern, ht, refcount);
525+
}
506526
return;
507527
}
508528

@@ -513,12 +533,17 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
513533
}
514534

515535
ht = spl_array_get_hash_table(intern);
536+
refcount = spl_array_set_refcount(intern, ht, 1);
516537
if (key.key) {
517538
zend_hash_update_ind(ht, key.key, value);
518539
spl_hash_key_release(&key);
519540
} else {
520541
zend_hash_index_update(ht, key.h, value);
521542
}
543+
544+
if (refcount) {
545+
spl_array_set_refcount(intern, ht, refcount);
546+
}
522547
} /* }}} */
523548

524549
static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */
@@ -531,6 +556,7 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
531556
HashTable *ht;
532557
spl_array_object *intern = spl_array_from_obj(object);
533558
spl_hash_key key;
559+
uint32_t refcount = 0;
534560

535561
if (check_inherited && intern->fptr_offset_del) {
536562
zend_call_method_with_1_params(object, object->ce, &intern->fptr_offset_del, "offsetUnset", NULL, offset);
@@ -548,6 +574,7 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
548574
}
549575

550576
ht = spl_array_get_hash_table(intern);
577+
refcount = spl_array_set_refcount(intern, ht, 1);
551578
if (key.key) {
552579
zval *data = zend_hash_find(ht, key.key);
553580
if (data) {
@@ -570,6 +597,10 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
570597
} else {
571598
zend_hash_index_del(ht, key.h);
572599
}
600+
601+
if (refcount) {
602+
spl_array_set_refcount(intern, ht, refcount);
603+
}
573604
} /* }}} */
574605

575606
static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */
@@ -1056,8 +1087,12 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
10561087
if (Z_REFCOUNT_P(array) == 1) {
10571088
ZVAL_COPY(&intern->array, array);
10581089
} else {
1059-
//??? TODO: try to avoid array duplication
10601090
ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array)));
1091+
if (intern->is_child) {
1092+
Z_TRY_DELREF_P(&intern->bucket->val);
1093+
intern->bucket->val = intern->array;
1094+
Z_TRY_ADDREF_P(&intern->array);
1095+
}
10611096
}
10621097
} else {
10631098
if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) {
@@ -1544,6 +1579,16 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren)
15441579
}
15451580
/* }}} */
15461581

1582+
static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */
1583+
{
1584+
object_init_ex(retval, pce);
1585+
spl_array_object *new_intern = Z_SPLARRAY_P(retval);
1586+
new_intern->is_child = true;
1587+
new_intern->bucket = (Bucket *)((char*)(arg1) - XtOffsetOf(Bucket, val));
1588+
zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2);
1589+
}
1590+
/* }}} */
1591+
15471592
/* {{{ Create a sub iterator for the current element (same class as $this) */
15481593
PHP_METHOD(RecursiveArrayIterator, getChildren)
15491594
{
@@ -1574,7 +1619,7 @@ PHP_METHOD(RecursiveArrayIterator, getChildren)
15741619
}
15751620

15761621
ZVAL_LONG(&flags, intern->ar_flags);
1577-
spl_instantiate_arg_ex2(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags);
1622+
spl_instantiate_child_arg(Z_OBJCE_P(ZEND_THIS), return_value, entry, &flags);
15781623
}
15791624
/* }}} */
15801625

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