bug #4821 Preserve IteratorAggregate identity in sandbox __toString walker (fabpot)

This PR was merged into the 3.x branch.

Discussion
----------

Preserve IteratorAggregate identity in sandbox __toString walker

Fixes #4820

It's not a full fix, but a quick one just that for the specific use case described in the issue. This is the simple case as IteratorAggregate instances can be iterated more than once (I suppose most such iterators don't have side effects).

Commits
-------

d25f98f45b Preserve IteratorAggregate identity in sandbox __toString walker

CHANGELOG

@@ -1,6 +1,6 @@
 # 3.27.1 (2026-XX-XX)
 
- * n/a
+ * Fix sandbox replacing `IteratorAggregate` arguments (e.g. Symfony's `FormView`) by a plain array
 
 # 3.27.0 (2026-05-27)
 

src/Extension/SandboxExtension.php

@@ -172,29 +172,29 @@ final class SandboxExtension extends AbstractExtension
             }
         }
 
-        // A Traversable would later be materialised (e.g. by filters such as `join`
-        // or `replace`) and its elements coerced to string by PHP itself, bypassing
-        // the policy. Materialise it now and recursively check the contents. This
-        // also applies to objects that implement both `Stringable` and `Traversable`:
-        // the `__toString` check above only validates the container's own coercion,
-        // not the elements yielded by `getIterator()`.
+        // Elements yielded by a Traversable may be string-coerced downstream
+        // (e.g. by `join`/`replace`), bypassing the policy. Check them now.
         if ($obj instanceof \Traversable) {
-            // Guard against self-referencing iterables (e.g. an IteratorAggregate
-            // whose getIterator() yields $this): without this check, materialising
-            // and recursing into the elements would overflow the stack. Mirrors
-            // the array-cycle guard in ensureToStringAllowedForArray().
             if (isset($seen[$obj])) {
                 return $obj;
             }
-
             $seen[$obj] = true;
+
+            // IteratorAggregate::getIterator() is idempotent, so we can walk
+            // the elements and return the original object: host code typed
+            // against a specific class (e.g. FormView) keeps working.
+            if ($obj instanceof \IteratorAggregate) {
+                foreach ($obj as $v) {
+                    $this->doEnsureToStringAllowed($v, $lineno, $source, $seen);
+                }
+
+                return $obj;
+            }
+
+            // Single-pass Iterator/Generator: materialise to validate.
             $array = iterator_to_array($obj);
             $this->ensureToStringAllowedForArray($array, $lineno, $source, $seen);
 
-            // Return the materialised array only when the object is not also
-            // Stringable, so that callers that rely on `__toString` (e.g. `{{ obj }}`)
-            // keep working. Plain consumers of iterables (join, replace, ...) call
-            // `iterator_to_array()` again, so the extra materialisation is benign.
             if (!$obj instanceof \Stringable) {
                 return $array;
             }

tests/Extension/SandboxTest.php

@@ -1355,6 +1355,60 @@ EOF
         }
     }
 
+    public function testSandboxPreservesTraversableArgumentIdentity()
+    {
+        // Regression for https://github.com/twigphp/Twig/issues/4820:
+        // a typed Traversable argument (e.g. Symfony's FormView) must reach
+        // host code as-is, not as a plain array.
+        $twig = $this->getEnvironment(
+            true,
+            [],
+            ['index' => '{{ render_traversable(obj) }}'],
+            [],
+            [],
+            ['Twig\Tests\Extension\StringableTraversableObject' => ['__tostring']],
+        );
+        $twig->addFunction(new TwigFunction('render_traversable', static function ($obj) {
+            if (!$obj instanceof StringableTraversableObject) {
+                throw new \RuntimeException(\sprintf('Expected a StringableTraversableObject, got "%s".', get_debug_type($obj)));
+            }
+
+            return (string) $obj;
+        }));
+
+        $policy = $twig->getExtension(SandboxExtension::class)->getSecurityPolicy();
+        $policy->setAllowedFunctions(['render_traversable']);
+
+        $params = ['obj' => new StringableTraversableObject(['a', 'b'])];
+        $this->assertSame('stringable-traversable', $twig->load('index')->render($params));
+    }
+
+    public function testSandboxStillBlocksDisallowedToStringInTraversableArgument()
+    {
+        // The container is returned as-is, but yielded elements must still
+        // be policy-checked since host code can string-coerce them.
+        $twig = $this->getEnvironment(
+            true,
+            [],
+            ['index' => '{{ render_traversable(stringable_iterator) }}'],
+            [],
+            [],
+            ['Twig\Tests\Extension\StringableTraversableObject' => ['__tostring']],
+        );
+        $twig->addFunction(new TwigFunction('render_traversable', static fn ($obj) => (string) $obj));
+
+        $policy = $twig->getExtension(SandboxExtension::class)->getSecurityPolicy();
+        $policy->setAllowedFunctions(['render_traversable']);
+
+        try {
+            $twig->load('index')->render(self::$params);
+            $this->fail('Sandbox should block __toString on objects yielded by a Traversable argument to a user function.');
+        } catch (SecurityNotAllowedMethodError $e) {
+            $this->assertSame('Twig\Tests\Extension\FooObject', $e->getClassName());
+            $this->assertSame('__tostring', $e->getMethodName());
+        }
+    }
+
     public function testColumnFilterUnaffectedOutsideSandbox()
     {
         $params = ['obj' => new ColumnObject()];