Package: php8.2-cli

Version: 8.2.7-1~deb12u1

Source: php8.2

Severity: important

Tags: patch

When I execute the following PHP code with the above mentioned PHP version
it simply stays in an endless loop:

<?php

$a = [

    'a' => [

        'b' => [],

    ],

];

$recursive = new RecursiveIteratorIterator(

    new RecursiveArrayIterator($a),

    RecursiveIteratorIterator::CHILD_FIRST

);

foreach($recursive as $b) {

    isset($b['r']);

}

?>

It does not throw any error message, exception or anything else. It simply
stocks in an endless loop.

With PHP 8.2.11 and newer, this issue does not exist anymore as it has been
fixed there. When it's fixed, the PHP script immediately exits without any
error message.

I suggest that the changes from
https://github.com/php/php-src/commit/ffd7018fcdd13ca2966149e5141197a02707aff1
get backported to PHP 8.2.7 on Debian 12 (Bookworm). When I apply these
changes on top of the above mentioned PHP version, the issue is resolved.
At the bottom, you’ll find our tested patch.

I am using Debian GNU/Linux 12.4.

—-

I created a patch file (see below) from commit
ffd7018fcdd13ca2966149e5141197a02707aff1
<https://github.com/php/php-src/commit/ffd7018fcdd13ca2966149e5141197a02707aff1>
using git format-patch and removed the NEWS hook as it didn't apply. With
this patch file, I succeeded in building packages that fixed the bug.

Fix-GH-11972-RecursiveCallbackFilterIterator-regress.patch:

>From ffd7018fcdd13ca2966149e5141197a02707aff1 Mon Sep 17 00:00:00 2001

From: Niels Dossche <[email protected]>

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.

Closes GH-12086.

---

 ext/spl/spl_array.c        |   5 +-

 ext/spl/tests/gh11972.phpt | 196 +++++++++++++++++++++++++++++++++++++

 3 files changed, 202 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 676f68fb6b..01756186fa 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 0000000000..d88d7c5eca

--- /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

-- 

2.39.2

Reply via email to