From 716076bd838769135894f779b358b4580b17646f Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Wed, 30 Aug 2023 21:25:03 +0200
Subject: [PATCH] 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.
---
 ext/spl/spl_array.c        |   5 +-
 ext/spl/tests/gh11972.phpt | 196 +++++++++++++++++++++++++++++++++++++
 2 files changed, 198 insertions(+), 3 deletions(-)
 create mode 100644 ext/spl/tests/gh11972.phpt

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index 676f68fb6b276..01756186fa525 100644
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -1128,13 +1128,12 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
 			ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array)));
 
 			if (intern->is_child) {
-				Z_TRY_DELREF_P(&intern->bucket->val);
+				Z_TRY_DELREF(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);
+				ZVAL_COPY(&intern->bucket->val, &intern->array);
 			}
 		}
 	} else {
diff --git a/ext/spl/tests/gh11972.phpt b/ext/spl/tests/gh11972.phpt
new file mode 100644
index 0000000000000..d88d7c5ecae40
--- /dev/null
+++ b/ext/spl/tests/gh11972.phpt
@@ -0,0 +1,196 @@
+--TEST--
+GH-11972 (RecursiveCallbackFilterIterator regression in 8.1.18)
+--EXTENSIONS--
+spl
+--FILE--
+<?php
+
+class RecursiveFilterTest {
+    public function traverse(array $variables): array {
+        $array_iterator = new \RecursiveArrayIterator($variables);
+        $filter_iterator = new \RecursiveCallbackFilterIterator($array_iterator, [
+            $this, 'isCyclic',
+        ]);
+        $recursive_iterator = new \RecursiveIteratorIterator($filter_iterator, \RecursiveIteratorIterator::SELF_FIRST);
+        $recursive_iterator->setMaxDepth(20);
+        foreach ($recursive_iterator as $value) {
+            // Avoid recursion by marking where we've been.
+            $value['#override_mode_breadcrumb'] = true;
+        }
+        return \iterator_to_array($recursive_iterator);
+    }
+
+    public function isCyclic($current, string $key, \RecursiveArrayIterator $iterator): bool {
+        var_dump($current);
+        if (!is_array($current)) {
+            return false;
+        }
+        // Avoid infinite loops by checking if we've been here before.
+        // e.g. View > query > view > query ...
+        if (isset($current['#override_mode_breadcrumb'])) {
+            return false;
+        }
+        return true;
+    }
+}
+
+$test_array['e']['p'][] = ['a', 'a'];
+$test_array['e']['p'][] = ['b', 'b'];
+$test_array['e']['p'][] = ['c', 'c'];
+$serialized = serialize($test_array);
+$unserialized = unserialize($serialized);
+
+$test_class = new RecursiveFilterTest();
+$test_class->traverse($unserialized);
+
+echo "Done\n";
+
+?>
+--EXPECT--
+array(1) {
+  ["p"]=>
+  array(3) {
+    [0]=>
+    array(2) {
+      [0]=>
+      string(1) "a"
+      [1]=>
+      string(1) "a"
+    }
+    [1]=>
+    array(2) {
+      [0]=>
+      string(1) "b"
+      [1]=>
+      string(1) "b"
+    }
+    [2]=>
+    array(2) {
+      [0]=>
+      string(1) "c"
+      [1]=>
+      string(1) "c"
+    }
+  }
+}
+array(3) {
+  [0]=>
+  array(2) {
+    [0]=>
+    string(1) "a"
+    [1]=>
+    string(1) "a"
+  }
+  [1]=>
+  array(2) {
+    [0]=>
+    string(1) "b"
+    [1]=>
+    string(1) "b"
+  }
+  [2]=>
+  array(2) {
+    [0]=>
+    string(1) "c"
+    [1]=>
+    string(1) "c"
+  }
+}
+array(2) {
+  [0]=>
+  string(1) "a"
+  [1]=>
+  string(1) "a"
+}
+string(1) "a"
+string(1) "a"
+array(2) {
+  [0]=>
+  string(1) "b"
+  [1]=>
+  string(1) "b"
+}
+string(1) "b"
+string(1) "b"
+array(2) {
+  [0]=>
+  string(1) "c"
+  [1]=>
+  string(1) "c"
+}
+string(1) "c"
+string(1) "c"
+array(1) {
+  ["p"]=>
+  array(3) {
+    [0]=>
+    array(2) {
+      [0]=>
+      string(1) "a"
+      [1]=>
+      string(1) "a"
+    }
+    [1]=>
+    array(2) {
+      [0]=>
+      string(1) "b"
+      [1]=>
+      string(1) "b"
+    }
+    [2]=>
+    array(2) {
+      [0]=>
+      string(1) "c"
+      [1]=>
+      string(1) "c"
+    }
+  }
+}
+array(3) {
+  [0]=>
+  array(2) {
+    [0]=>
+    string(1) "a"
+    [1]=>
+    string(1) "a"
+  }
+  [1]=>
+  array(2) {
+    [0]=>
+    string(1) "b"
+    [1]=>
+    string(1) "b"
+  }
+  [2]=>
+  array(2) {
+    [0]=>
+    string(1) "c"
+    [1]=>
+    string(1) "c"
+  }
+}
+array(2) {
+  [0]=>
+  string(1) "a"
+  [1]=>
+  string(1) "a"
+}
+string(1) "a"
+string(1) "a"
+array(2) {
+  [0]=>
+  string(1) "b"
+  [1]=>
+  string(1) "b"
+}
+string(1) "b"
+string(1) "b"
+array(2) {
+  [0]=>
+  string(1) "c"
+  [1]=>
+  string(1) "c"
+}
+string(1) "c"
+string(1) "c"
+Done