Skip to content

Commit ffd7018

Browse files
nielsdosnielsdos
committed
Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
When you do an assignment between two zvals (no, not zval*), you copy all fields. This includes the additional u2 data. So that means for example the Z_NEXT index gets copied, which in some cases can therefore cause a cycle in zend_hash lookups. Instead of doing an assignment, we should be doing a ZVAL_COPY (or ZVAL_COPY_VALUE for non-refcounting cases). This avoids copying u2. Closes GH-12086.
1 parent f2c16b7 commit ffd7018

File tree

3 files changed

+202
-3
lines changed

3 files changed

+202
-3
lines changed

‎NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ PHP NEWS
1818
. Fixed bug GH-10270 (Invalid error message when connection via SSL fails:
1919
"trying to connect via (null)"). (Kamil Tekiela)
2020

21+
- SPL:
22+
. Fixed bug GH-11972 (RecursiveCallbackFilterIterator regression in 8.1.18).
23+
(nielsdos)
24+
2125
31 Aug 2023, PHP 8.1.23
2226

2327
- CLI:

‎ext/spl/spl_array.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,13 +1128,12 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
11281128
ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array)));
11291129

11301130
if (intern->is_child) {
1131-
Z_TRY_DELREF_P(&intern->bucket->val);
1131+
Z_TRY_DELREF(intern->bucket->val);
11321132
/*
11331133
* replace bucket->val with copied array, so the changes between
11341134
* parent and child object can affect each other.
11351135
*/
1136-
intern->bucket->val = intern->array;
1137-
Z_TRY_ADDREF_P(&intern->array);
1136+
ZVAL_COPY(&intern->bucket->val, &intern->array);
11381137
}
11391138
}
11401139
} else {

‎ext/spl/tests/gh11972.phpt

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
--TEST--
2+
GH-11972 (RecursiveCallbackFilterIterator regression in 8.1.18)
3+
--EXTENSIONS--
4+
spl
5+
--FILE--
6+
<?php
7+
8+
class RecursiveFilterTest {
9+
public function traverse(array $variables): array {
10+
$array_iterator = new \RecursiveArrayIterator($variables);
11+
$filter_iterator = new \RecursiveCallbackFilterIterator($array_iterator, [
12+
$this, 'isCyclic',
13+
]);
14+
$recursive_iterator = new \RecursiveIteratorIterator($filter_iterator, \RecursiveIteratorIterator::SELF_FIRST);
15+
$recursive_iterator->setMaxDepth(20);
16+
foreach ($recursive_iterator as $value) {
17+
// Avoid recursion by marking where we've been.
18+
$value['#override_mode_breadcrumb'] = true;
19+
}
20+
return \iterator_to_array($recursive_iterator);
21+
}
22+
23+
public function isCyclic($current, string $key, \RecursiveArrayIterator $iterator): bool {
24+
var_dump($current);
25+
if (!is_array($current)) {
26+
return false;
27+
}
28+
// Avoid infinite loops by checking if we've been here before.
29+
// e.g. View > query > view > query ...
30+
if (isset($current['#override_mode_breadcrumb'])) {
31+
return false;
32+
}
33+
return true;
34+
}
35+
}
36+
37+
$test_array['e']['p'][] = ['a', 'a'];
38+
$test_array['e']['p'][] = ['b', 'b'];
39+
$test_array['e']['p'][] = ['c', 'c'];
40+
$serialized = serialize($test_array);
41+
$unserialized = unserialize($serialized);
42+
43+
$test_class = new RecursiveFilterTest();
44+
$test_class->traverse($unserialized);
45+
46+
echo "Done\n";
47+
48+
?>
49+
--EXPECT--
50+
array(1) {
51+
["p"]=>
52+
array(3) {
53+
[0]=>
54+
array(2) {
55+
[0]=>
56+
string(1) "a"
57+
[1]=>
58+
string(1) "a"
59+
}
60+
[1]=>
61+
array(2) {
62+
[0]=>
63+
string(1) "b"
64+
[1]=>
65+
string(1) "b"
66+
}
67+
[2]=>
68+
array(2) {
69+
[0]=>
70+
string(1) "c"
71+
[1]=>
72+
string(1) "c"
73+
}
74+
}
75+
}
76+
array(3) {
77+
[0]=>
78+
array(2) {
79+
[0]=>
80+
string(1) "a"
81+
[1]=>
82+
string(1) "a"
83+
}
84+
[1]=>
85+
array(2) {
86+
[0]=>
87+
string(1) "b"
88+
[1]=>
89+
string(1) "b"
90+
}
91+
[2]=>
92+
array(2) {
93+
[0]=>
94+
string(1) "c"
95+
[1]=>
96+
string(1) "c"
97+
}
98+
}
99+
array(2) {
100+
[0]=>
101+
string(1) "a"
102+
[1]=>
103+
string(1) "a"
104+
}
105+
string(1) "a"
106+
string(1) "a"
107+
array(2) {
108+
[0]=>
109+
string(1) "b"
110+
[1]=>
111+
string(1) "b"
112+
}
113+
string(1) "b"
114+
string(1) "b"
115+
array(2) {
116+
[0]=>
117+
string(1) "c"
118+
[1]=>
119+
string(1) "c"
120+
}
121+
string(1) "c"
122+
string(1) "c"
123+
array(1) {
124+
["p"]=>
125+
array(3) {
126+
[0]=>
127+
array(2) {
128+
[0]=>
129+
string(1) "a"
130+
[1]=>
131+
string(1) "a"
132+
}
133+
[1]=>
134+
array(2) {
135+
[0]=>
136+
string(1) "b"
137+
[1]=>
138+
string(1) "b"
139+
}
140+
[2]=>
141+
array(2) {
142+
[0]=>
143+
string(1) "c"
144+
[1]=>
145+
string(1) "c"
146+
}
147+
}
148+
}
149+
array(3) {
150+
[0]=>
151+
array(2) {
152+
[0]=>
153+
string(1) "a"
154+
[1]=>
155+
string(1) "a"
156+
}
157+
[1]=>
158+
array(2) {
159+
[0]=>
160+
string(1) "b"
161+
[1]=>
162+
string(1) "b"
163+
}
164+
[2]=>
165+
array(2) {
166+
[0]=>
167+
string(1) "c"
168+
[1]=>
169+
string(1) "c"
170+
}
171+
}
172+
array(2) {
173+
[0]=>
174+
string(1) "a"
175+
[1]=>
176+
string(1) "a"
177+
}
178+
string(1) "a"
179+
string(1) "a"
180+
array(2) {
181+
[0]=>
182+
string(1) "b"
183+
[1]=>
184+
string(1) "b"
185+
}
186+
string(1) "b"
187+
string(1) "b"
188+
array(2) {
189+
[0]=>
190+
string(1) "c"
191+
[1]=>
192+
string(1) "c"
193+
}
194+
string(1) "c"
195+
string(1) "c"
196+
Done

0 commit comments

Comments
 (0)