From 18863f0371625625e63bfcebc0134f18bba18196 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 7 Sep 2026 09:11:31 +0200 Subject: [PATCH 1/5] Check that the use tag is allowed before resolving trait templates --- CHANGELOG | 1 + src/Node/CheckSecurityCallNode.php | 5 +- src/Node/CheckSecurityNode.php | 26 +++++++++ src/Node/ModuleNode.php | 3 +- src/NodeVisitor/SandboxNodeVisitor.php | 2 +- src/Template.php | 25 +++++++++ tests/Extension/SandboxTest.php | 3 + tests/Sandbox/SandboxTest.php | 78 ++++++++++++++++++++++++++ 8 files changed, 140 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 85043d29f..149e24c6b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,6 @@ # 3.29.0 (2026-XX-XX) + * Fix the sandbox resolving `use` trait templates before checking that the `use` tag is allowed * Fix `html_attr` dropping `style` declarations whose value is `0`, `0.0` or `'0'` * Fix the `default` filter fallback emitting an undefined variable warning when it uses the null-safe operator * Fix the `matches` operator silently treating PCRE execution errors as non-matches diff --git a/src/Node/CheckSecurityCallNode.php b/src/Node/CheckSecurityCallNode.php index a248040b1..b5337aee8 100644 --- a/src/Node/CheckSecurityCallNode.php +++ b/src/Node/CheckSecurityCallNode.php @@ -15,6 +15,9 @@ use Twig\Attribute\YieldReady; use Twig\Compiler; /** + * Wires the security checker at the very top of the template constructor, as + * the constructor resolves `use` traits before the sandbox could check them. + * * @author Fabien Potencier */ #[YieldReady] @@ -26,7 +29,7 @@ class CheckSecurityCallNode extends Node public function compile(Compiler $compiler) { $compiler - ->write("\$this->sandbox = \$this->extensions[SandboxExtension::class]->getChecker();\n") + ->write("\$this->sandbox = \$env->getExtension(SandboxExtension::class)->getChecker();\n") ; } } diff --git a/src/Node/CheckSecurityNode.php b/src/Node/CheckSecurityNode.php index f3944cf50..0a7ed8ff3 100644 --- a/src/Node/CheckSecurityNode.php +++ b/src/Node/CheckSecurityNode.php @@ -47,6 +47,32 @@ class CheckSecurityNode extends Node public function compile(Compiler $compiler): void { + if (isset($this->usedTags['use'])) { + $compiler + ->write("\n") + ->write("protected function checkTraitsAllowed(): void\n") + ->write("{\n") + ->indent() + ->write("if (!\$this->sandbox->isSandboxed(\$this->source)) {\n") + ->indent() + ->write("return;\n") + ->outdent() + ->write("}\n\n") + ->write("try {\n") + ->indent() + ->write("\$this->sandbox->checkSecurity(['use'], [], [], [], \$this->source);\n") + ->outdent() + ->write("} catch (SecurityNotAllowedTagError \$e) {\n") + ->indent() + ->write('$e->setTemplateLine(')->repr($this->usedTags['use'])->raw(");\n\n") + ->write("throw \$e;\n") + ->outdent() + ->write("}\n") + ->outdent() + ->write("}\n") + ; + } + $compiler ->write("\n") ->write("public function ensureSecurityChecked(): void\n") diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index ad67886a2..ca2b0b9c2 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -218,7 +218,8 @@ final class ModuleNode extends Node implements CoercesChildrenToStringInterface $countTraits = \count($this->getNode('traits')); if ($countTraits) { - // traits + $compiler->write("\$this->ensureTraitsAllowed();\n\n"); + foreach ($this->getNode('traits') as $i => $trait) { $node = $trait->getNode('template'); diff --git a/src/NodeVisitor/SandboxNodeVisitor.php b/src/NodeVisitor/SandboxNodeVisitor.php index dba49c8b0..15001f1b2 100644 --- a/src/NodeVisitor/SandboxNodeVisitor.php +++ b/src/NodeVisitor/SandboxNodeVisitor.php @@ -125,7 +125,7 @@ final class SandboxNodeVisitor implements NodeVisitorInterface if ($node instanceof ModuleNode) { $this->inAModule = false; - $node->setNode('constructor_end', new Nodes([new CheckSecurityCallNode(), $node->getNode('constructor_end')])); + $node->setNode('constructor_start', new Nodes([new CheckSecurityCallNode(), $node->getNode('constructor_start')])); $node->setNode('class_end', new Nodes([new CheckSecurityNode($this->filters, $this->tags, $this->functions, $this->tests), $node->getNode('class_end')])); } diff --git a/src/Template.php b/src/Template.php index aeca14677..6078c6c03 100644 --- a/src/Template.php +++ b/src/Template.php @@ -502,6 +502,31 @@ abstract class Template { } + /** + * Checks the "use" tag against the sandbox policy. + * + * The constructor resolves "use" traits eagerly, which reaches the loader, + * so that tag alone is checked here; the rest of the policy still runs at + * render time. + * + * @internal + */ + public function ensureTraitsAllowed(): void + { + try { + $this->checkTraitsAllowed(); + } catch (\Throwable $e) { + $this->handleException($e); + } + } + + /** + * @internal + */ + protected function checkTraitsAllowed(): void + { + } + /** * Auto-generated method to display the template with the given context. * diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index 12f6f6349..3a975f116 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -293,6 +293,9 @@ class SandboxTest extends TestCase { yield ['extends', '{% extends "1_empty" %}']; yield ['use', '{% use "1_empty" %}']; + yield 'use of a missing template is rejected before the loader is reached' => ['use', '{% use "does_not_exist" %}']; + yield 'use of a missing block is rejected before the trait is resolved' => ['use', '{% use "1_layout" with does_not_exist as alias %}']; + yield 'use of a non-traitable template is rejected before the trait is resolved' => ['use', '{% use "1_child" %}']; } /** diff --git a/tests/Sandbox/SandboxTest.php b/tests/Sandbox/SandboxTest.php index 220b072a8..63923d0de 100644 --- a/tests/Sandbox/SandboxTest.php +++ b/tests/Sandbox/SandboxTest.php @@ -13,6 +13,7 @@ namespace Twig\Tests\Sandbox; use PHPUnit\Framework\TestCase; use Twig\Environment; +use Twig\Error\RuntimeError; use Twig\Extension\SandboxExtension; use Twig\Loader\ArrayLoader; use Twig\Markup; @@ -244,6 +245,65 @@ class SandboxTest extends TestCase $denying->render('index'); } + public function testTheUseTagMustBeAllowed(): void + { + $templates = [ + 'index' => '{% use "blocks" with content as base_content %}{{ block("base_content") }}', + 'blocks' => '{% block content %}trait content{% endblock %}', + ]; + + $allowing = new Sandbox(self::env($templates), self::strictPolicy(tags: ['use', 'block'], functions: ['block'])); + $this->assertSame('trait content', $allowing->render('index')); + + $denying = new Sandbox(self::env(['index' => '{% use "missing" with content as base_content %}']), self::strictPolicy()); + $this->expectException(SecurityNotAllowedTagError::class); + $this->expectExceptionMessage('Tag "use" is not allowed'); + $denying->render('index'); + } + + public function testTheUseTagIsCheckedWithoutCheckingTheRestOfThePolicy(): void + { + // "middle" is only reachable as a trait, and the block carrying the + // forbidden filter is overridden by "index", so it never renders. + $sandbox = new Sandbox(self::env([ + 'index' => '{% use "middle" %}{% block content %}SAFE{% endblock %}', + 'middle' => '{% use "leaf" %}{% block content %}{{ "bad"|upper }}{% endblock %}', + 'leaf' => '', + ]), self::strictPolicy(tags: ['use', 'block'])); + + $this->assertSame('SAFE', $sandbox->render('index')); + } + + public function testTheUseTagIsCheckedBeforeTheTraitTemplateIsLoaded(): void + { + $sandbox = new Sandbox(self::env([ + 'index' => '{{ block("b", "receiver") is defined ? "YES" : "NO" }}', + 'receiver' => '{% use "missing" %}', + ]), self::strictPolicy(tags: ['block'], functions: ['block'])); + + $this->expectException(SecurityNotAllowedTagError::class); + $this->expectExceptionMessage('Tag "use" is not allowed'); + $sandbox->render('index'); + } + + public function testAPolicyFailureWhileResolvingTraitsKeepsItsTwigContext(): void + { + $sandbox = new Sandbox(self::env([ + 'index' => "{% use \"empty\" %}\n{{ 'a'|upper }}", + 'empty' => '', + ]), new ThrowingOnUseSecurityPolicy()); + + try { + $sandbox->render('index'); + $this->fail('The policy failure should have been reported.'); + } catch (RuntimeError $e) { + $this->assertStringContainsString('Policy backend unreachable', $e->getMessage()); + $this->assertSame('index', $e->getSourceContext()?->getName()); + $this->assertSame(2, $e->getTemplateLine()); + $this->assertInstanceOf(\RuntimeException::class, $e->getPrevious()); + } + } + public function testARenderOnAnotherEnvironmentDuringASandboxedRenderIsNotSandboxed(): void { $app = new Environment(new ArrayLoader(['trusted' => '{{ value|upper }}']), ['autoescape' => false]); @@ -377,6 +437,24 @@ class SandboxTest extends TestCase } } +final class ThrowingOnUseSecurityPolicy implements SecurityPolicyInterface +{ + public function checkSecurity($tags, $filters, $functions, $tests = []): void + { + if (\in_array('use', $tags, true)) { + throw new \RuntimeException('Policy backend unreachable.'); + } + } + + public function checkMethodAllowed($obj, $method): void + { + } + + public function checkPropertyAllowed($obj, $property): void + { + } +} + final class SandboxTestObject implements \Stringable { public $name = 'fabien'; From bd939c8c3af9cd5801d3ae0e241b358cf47ad6d2 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 11 Sep 2026 10:59:50 +0200 Subject: [PATCH 2/5] Fix wrapping the Twig cache pool in a second tag aware adapter The ".twig.cache.inner" pool is a child of "cache.app". When the application configures a natively tag aware adapter for it (redis, valkey, pdo or mongodb), the child pool is already tag aware and decorating it with a TagAwareAdapter turns every read into a miss. Alias "twig.cache" to the pool in that case, the way Symfony aliases "cache.app.taggable" to "cache.app" instead of decorating it. The pool keeps its own namespace, so it does not start sharing the application pool's one, and the decorator stays in place for the plain adapters that need it. --- CHANGELOG | 1 + .../Compiler/TwigCachePoolPass.php | 57 ++++++++ .../Compiler/TwigCachePoolPassTest.php | 60 ++++++++ .../Tests/Fixture/CacheKernel.php | 129 ++++++++++++++++++ extra/twig-extra-bundle/TwigExtraBundle.php | 6 + 5 files changed, 253 insertions(+) create mode 100644 extra/twig-extra-bundle/DependencyInjection/Compiler/TwigCachePoolPass.php create mode 100644 extra/twig-extra-bundle/Tests/DependencyInjection/Compiler/TwigCachePoolPassTest.php create mode 100644 extra/twig-extra-bundle/Tests/Fixture/CacheKernel.php diff --git a/CHANGELOG b/CHANGELOG index 2dd9742af..ebb942cc4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,6 @@ # 3.29.0 (2026-XX-XX) + * Fix `{% cache %}` always missing in the Symfony bundle when `framework.cache.app` uses a natively tag aware adapter * Fix the sandbox resolving `use` trait templates before checking that the `use` tag is allowed * Fix `html_attr` dropping `style` declarations whose value is `0`, `0.0` or `'0'` * Fix the `default` filter fallback emitting an undefined variable warning when it uses the null-safe operator diff --git a/extra/twig-extra-bundle/DependencyInjection/Compiler/TwigCachePoolPass.php b/extra/twig-extra-bundle/DependencyInjection/Compiler/TwigCachePoolPass.php new file mode 100644 index 000000000..a5bb216fd --- /dev/null +++ b/extra/twig-extra-bundle/DependencyInjection/Compiler/TwigCachePoolPass.php @@ -0,0 +1,57 @@ +hasDefinition('twig.cache') || !$container->hasDefinition('.twig.cache.inner')) { + return; + } + + $definition = $container->getDefinition('.twig.cache.inner'); + $class = $definition->getClass(); + + while (null === $class && $definition instanceof ChildDefinition) { + $parent = $definition->getParent(); + + if (!$container->hasDefinition($parent) && !$container->hasAlias($parent)) { + return; + } + + $definition = $container->findDefinition($parent); + $class = $definition->getClass(); + } + + if (!is_a($class ?? '', TagAwareAdapterInterface::class, true)) { + return; + } + + $container->removeDefinition('twig.cache'); + $container->setAlias('twig.cache', '.twig.cache.inner'); + } +} diff --git a/extra/twig-extra-bundle/Tests/DependencyInjection/Compiler/TwigCachePoolPassTest.php b/extra/twig-extra-bundle/Tests/DependencyInjection/Compiler/TwigCachePoolPassTest.php new file mode 100644 index 000000000..559215ab1 --- /dev/null +++ b/extra/twig-extra-bundle/Tests/DependencyInjection/Compiler/TwigCachePoolPassTest.php @@ -0,0 +1,60 @@ +kernels as $kernel) { + (new Filesystem())->remove($kernel->getTempDir()); + } + + $this->kernels = []; + } + + public function testThePoolIsDecoratedWhenTheAppAdapterIsNotTagAware(): void + { + $pools = $this->compileFor(null); + + $this->assertSame(TagAwareAdapter::class, $pools['twig.cache']); + $this->assertSame(FilesystemAdapter::class, $pools['.twig.cache.inner']); + $this->assertNotSame($pools['cache.app.namespace'], $pools['.twig.cache.inner.namespace']); + } + + public function testThePoolIsNotDecoratedWhenTheAppAdapterIsTagAware(): void + { + $pools = $this->compileFor('cache.adapter.redis_tag_aware'); + + $this->assertSame('@.twig.cache.inner', $pools['twig.cache']); + $this->assertSame(RedisTagAwareAdapter::class, $pools['.twig.cache.inner']); + $this->assertNotSame($pools['cache.app.namespace'], $pools['.twig.cache.inner.namespace']); + } + + private function compileFor(?string $cacheAdapter): array + { + $this->kernels[] = $kernel = new CacheKernel($cacheAdapter); + $kernel->boot(); + + return $kernel->getContainer()->getParameter('twig_extra.test.cache_pools'); + } +} diff --git a/extra/twig-extra-bundle/Tests/Fixture/CacheKernel.php b/extra/twig-extra-bundle/Tests/Fixture/CacheKernel.php new file mode 100644 index 000000000..18dc54c86 --- /dev/null +++ b/extra/twig-extra-bundle/Tests/Fixture/CacheKernel.php @@ -0,0 +1,129 @@ +dir = sys_get_temp_dir().'/twig-extra-bundle/'.bin2hex(random_bytes(6)); + + parent::__construct('test', false); + } + + public function getCacheDir(): string + { + return $this->dir.'/cache'; + } + + public function getLogDir(): string + { + return $this->dir.'/log'; + } + + public function getProjectDir(): string + { + return __DIR__; + } + + public function getTempDir(): string + { + return $this->dir; + } + + public function registerBundles(): iterable + { + yield new FrameworkBundle(); + yield new TwigBundle(); + yield new TwigExtraBundle(); + } + + protected function build(ContainerBuilder $container): void + { + $container->addCompilerPass(new class implements CompilerPassInterface { + public function process(ContainerBuilder $container): void + { + $pools = []; + foreach (['twig.cache', '.twig.cache.inner', 'cache.app'] as $id) { + if ($container->hasAlias($id)) { + $pools[$id] = '@'.$container->getAlias($id); + + continue; + } + + $definition = $container->getDefinition($id); + $pools[$id] = $definition->getClass(); + + // cache adapters take their namespace as their first string argument + foreach ($definition->getArguments() as $argument) { + if (\is_string($argument)) { + $pools[$id.'.namespace'] = $argument; + + break; + } + } + } + + $container->setParameter('twig_extra.test.cache_pools', $pools); + } + }, PassConfig::TYPE_BEFORE_REMOVING, -1000); + } + + protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader): void + { + $config = [ + 'secret' => 'S3CRET', + 'router' => ['utf8' => true], + 'http_method_override' => false, + 'php_errors' => [ + 'log' => true, + ], + ]; + + // the "handle_all_throwables" option was introduced in FrameworkBundle 6.2 (and so was the NotificationAssertionsTrait) + if (trait_exists(NotificationAssertionsTrait::class)) { + $config['handle_all_throwables'] = true; + } + + if (null !== $this->cacheAdapter) { + $config['cache'] = ['app' => $this->cacheAdapter]; + } + + $c->loadFromExtension('framework', $config); + $c->loadFromExtension('twig', [ + 'default_path' => __DIR__.'/views', + ]); + } + + protected function configureRoutes($routes): void + { + } +} diff --git a/extra/twig-extra-bundle/TwigExtraBundle.php b/extra/twig-extra-bundle/TwigExtraBundle.php index 495785623..3475eefc2 100644 --- a/extra/twig-extra-bundle/TwigExtraBundle.php +++ b/extra/twig-extra-bundle/TwigExtraBundle.php @@ -11,10 +11,12 @@ namespace Twig\Extra\TwigExtraBundle; +use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\HttpKernel\KernelInterface; use Twig\Extra\TwigExtraBundle\DependencyInjection\Compiler\MissingExtensionSuggestorPass; +use Twig\Extra\TwigExtraBundle\DependencyInjection\Compiler\TwigCachePoolPass; if (method_exists(KernelInterface::class, 'getShareDir')) { class TwigExtraBundle extends Bundle @@ -24,6 +26,8 @@ if (method_exists(KernelInterface::class, 'getShareDir')) { parent::build($container); $container->addCompilerPass(new MissingExtensionSuggestorPass()); + // priority 64 so that it runs before Symfony's CachePoolPass (priority 32) + $container->addCompilerPass(new TwigCachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 64); } } } else { @@ -35,6 +39,8 @@ if (method_exists(KernelInterface::class, 'getShareDir')) { parent::build($container); $container->addCompilerPass(new MissingExtensionSuggestorPass()); + // priority 64 so that it runs before Symfony's CachePoolPass (priority 32) + $container->addCompilerPass(new TwigCachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 64); } } } From 49f814ea26b64c966c41823d4bbc13dc56d8ea41 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 3 Sep 2026 21:18:47 +0200 Subject: [PATCH 3/5] Template runtime and block composition --- CHANGELOG | 2 + doc/api.rst | 44 +++ src/BlockChain.php | 107 ++++++ src/BlockResolutionContext.php | 91 +++++ src/Node/BlockNode.php | 2 +- src/Node/MacroNode.php | 2 +- src/Template.php | 85 ++++- tests/BlockChainTest.php | 646 +++++++++++++++++++++++++++++++++ tests/Node/BlockTest.php | 2 +- tests/Node/MacroTest.php | 4 +- tests/Node/MacrosTest.php | 2 +- tests/TemplateTest.php | 33 ++ 12 files changed, 1013 insertions(+), 7 deletions(-) create mode 100644 src/BlockChain.php create mode 100644 src/BlockResolutionContext.php create mode 100644 tests/BlockChainTest.php diff --git a/CHANGELOG b/CHANGELOG index 2dd9742af..39040b1a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,8 @@ * Fix the `matches` operator silently treating PCRE execution errors as non-matches * Fix `TemplateWrapper::streamBlock()`, `TemplateWrapper::hasBlock()`, and `TemplateWrapper::getBlockNames()` omitting environment globals * Fix exceptions from dynamic parent expressions escaping without template context + * Add the `BlockChain` class to compose blocks from multiple templates without using template internals + * Fix an output buffer leak when a parent block rendered in an expression throws in non-yield mode * Add the `HtmlExtension::htmlAttrValue()` method to resolve a single HTML attribute value the way the `html_attr` function renders it * Fix `html_attr` JSON encoding a `Stringable` value in a `data-*` attribute instead of using its string representation * Add documentation comments to attach metadata to nodes (experimental) diff --git a/doc/api.rst b/doc/api.rst index d6852bc6d..cbf2047dd 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -68,6 +68,50 @@ If a template defines blocks, they can be rendered individually via the echo $template->renderBlock('block_name', ['the' => 'variables', 'go' => 'here']); +.. caution:: + + Rendering a block on its own does not run the template body, so macros + imported at the top level of the template are missing and the block fails + when it calls one. Move the ``import`` or ``from`` tag inside the block. + Rendering the whole template first also makes them available, but they then + keep the value they resolved to during that render. + +Composing Blocks +---------------- + +.. versionadded:: 3.29 + + The ``BlockChain`` class was introduced in Twig 3.29. + +Use ``BlockChain`` when a form, CMS field, data-grid or similar renderer needs +to select blocks from several templates at runtime. Pass templates from the +highest to the lowest precedence:: + + use Twig\BlockChain; + + $blocks = new BlockChain($twig, [ + 'admin_theme.html.twig', + $twig->load('application_theme.html.twig'), + 'base_theme.html.twig', + ]); + + echo $blocks->renderBlock('field_row', ['field' => $field]); + +Twig considers each template's blocks, blocks imported with ``use`` and parents +before moving to the next template. The first definition of each block wins. +``parent()`` follows the local inheritance or ``use`` hierarchy of the block, +while nested ``block()`` calls use the composed block set unless they name +another template. + +``BlockChain`` composes blocks, not template bodies or macros. Because chained +templates share the composed block set, only combine templates that are trusted +to call one another's blocks. Chained blocks have the same top-level macro +import limitation as blocks rendered directly. + +The optional third constructor argument provides variables used to resolve +dynamic parent expressions. Parent hierarchies are fixed when the chain is +created; create another chain when these variables change. + Streaming Templates ------------------- diff --git a/src/BlockChain.php b/src/BlockChain.php new file mode 100644 index 000000000..0449796db --- /dev/null +++ b/src/BlockChain.php @@ -0,0 +1,107 @@ + */ + private array $blocks; + private Template $template; + + /** + * @param iterable $templates Templates ordered from highest to lowest precedence + */ + public function __construct( + private Environment $env, + iterable $templates, + array $context = [], + ) { + $resolution = new BlockResolutionContext($env, $context + $env->getGlobals()); + $blocks = []; + + foreach ($templates as $template) { + if (\is_string($template)) { + $template = $env->load($template); + } + if (!$template instanceof TemplateWrapper) { + throw new \TypeError(\sprintf('Block chain templates must be strings or "%s" instances, "%s" given.', TemplateWrapper::class, get_debug_type($template))); + } + + $current = $template->unwrap()->freezeLineage($resolution); + $this->template ??= $current; + do { + foreach ($current->getBlocks() as $name => $block) { + if (isset($blocks[$name])) { + continue; + } + if (!\is_array($block) || !isset($block[0], $block[1]) || !$block[0] instanceof Template || !\is_string($block[1])) { + throw new \LogicException('A block must be a method on a \Twig\Template instance.'); + } + + $resolution->assertOwns($block[0]); + $blocks[$name] = $block; + } + } while (false !== $current = $resolution->getParent($current)); + } + + if (!isset($this->template)) { + throw new \InvalidArgumentException('A block chain requires at least one template.'); + } + + $this->blocks = $blocks; + } + + public function hasBlock(string $name): bool + { + return isset($this->blocks[$name]); + } + + /** + * @return string[] + */ + public function getBlockNames(): array + { + return array_keys($this->blocks); + } + + /** + * @return iterable + */ + public function streamBlock(string $name, array $context = []): iterable + { + yield from $this->getBlock($name)->yieldBlock($name, $context + $this->env->getGlobals(), $this->blocks); + } + + public function renderBlock(string $name, array $context = []): string + { + return $this->getBlock($name)->renderBlock($name, $context + $this->env->getGlobals(), $this->blocks); + } + + public function displayBlock(string $name, array $context = []): void + { + $this->getBlock($name)->displayBlock($name, $context + $this->env->getGlobals(), $this->blocks); + } + + private function getBlock(string $name): Template + { + if (isset($this->blocks[$name])) { + return $this->blocks[$name][0]; + } + + throw new RuntimeError(\sprintf('Block "%s" on template "%s" does not exist.', $name, $this->template->getTemplateName()), -1, $this->template->getSourceContext()); + } +} diff --git a/src/BlockResolutionContext.php b/src/BlockResolutionContext.php new file mode 100644 index 000000000..51efb05d1 --- /dev/null +++ b/src/BlockResolutionContext.php @@ -0,0 +1,91 @@ + */ + private \SplObjectStorage $parents; + + /** @var \SplObjectStorage */ + private \SplObjectStorage $frozen; + + /** @var \SplObjectStorage */ + private \SplObjectStorage $freezing; + + public function __construct( + private Environment $env, + private array $context, + ) { + $this->parents = new \SplObjectStorage(); + $this->frozen = new \SplObjectStorage(); + $this->freezing = new \SplObjectStorage(); + } + + public function getParent(Template $template): Template|false + { + if ($this->parents->offsetExists($template)) { + return $this->parents[$template]; + } + + $parent = $template->getParent($this->context); + if ($parent instanceof TemplateWrapper) { + $parent = $parent->unwrap(); + } + + return $this->parents[$template] = $parent; + } + + public function setParent(Template $template, Template|false $parent): void + { + $this->parents[$template] = $parent; + } + + public function assertOwns(Template $template): void + { + if (!$template->isOwnedBy($this->env)) { + throw new \LogicException('A block chain cannot contain templates from different Twig environments.'); + } + } + + public function isFrozen(Template $template): bool + { + return $this->frozen->offsetExists($template); + } + + public function getFrozen(Template $template): Template + { + return $this->frozen[$template]; + } + + public function setFrozen(Template $template, Template $frozen): void + { + $this->frozen[$template] = $frozen; + } + + public function beginFreeze(Template $template): void + { + if ($this->freezing->offsetExists($template)) { + throw new \LogicException(\sprintf('Circular template inheritance detected while building a block chain from "%s".', $template->getTemplateName())); + } + + $this->freezing[$template] = true; + } + + public function endFreeze(Template $template): void + { + $this->freezing->offsetUnset($template); + } +} diff --git a/src/Node/BlockNode.php b/src/Node/BlockNode.php index b4f939cf6..a94f65239 100644 --- a/src/Node/BlockNode.php +++ b/src/Node/BlockNode.php @@ -37,7 +37,7 @@ class BlockNode extends Node ->write(" */\n") ->write(\sprintf("public function block_%s(array \$context, array \$blocks = []): iterable\n", $this->getAttribute('name')), "{\n") ->indent() - ->write("\$macros = \$this->macros;\n") + ->write("\$macros = null === \$this->macroImportSource ? \$this->macros : \$this->rebindMacroImports(\$this->macroImportSource->macros);\n") ; $compiler diff --git a/src/Node/MacroNode.php b/src/Node/MacroNode.php index 255ee8510..a7c0f82c4 100644 --- a/src/Node/MacroNode.php +++ b/src/Node/MacroNode.php @@ -115,7 +115,7 @@ class MacroNode extends Node ->raw("): string|Markup {\n") ->indent() ->addDebugInfo($this) - ->write("\$macros = \$this->macros;\n") + ->write("\$macros = null === \$this->macroImportSource ? \$this->macros : \$this->rebindMacroImports(\$this->macroImportSource->macros);\n") ->write("\$context = [\n") ->indent() ; diff --git a/src/Template.php b/src/Template.php index 212c0f25e..de154ab23 100644 --- a/src/Template.php +++ b/src/Template.php @@ -39,6 +39,7 @@ abstract class Template protected $traitAliases = []; protected $extensions = []; protected $sandbox; + protected ?self $macroImportSource = null; private $useYield; private ?MacroNamespace $macroNamespace = null; @@ -78,6 +79,11 @@ abstract class Template public function getParent(array $context): self|TemplateWrapper|false { if (null !== $this->parent) { + // only block chain clones set macroImportSource; they never run yield(), where the sandbox check normally happens + if (null !== $this->macroImportSource) { + $this->ensureSecurityChecked(); + } + return $this->parent; } @@ -170,12 +176,21 @@ abstract class Template public function renderParentBlock($name, array $context, array $blocks = []): string { if (!$this->useYield) { + $level = ob_get_level(); if ($this->env->isDebug()) { ob_start(); } else { ob_start(static function () { return ''; }); } - $this->displayParentBlock($name, $context, $blocks); + try { + $this->displayParentBlock($name, $context, $blocks); + } catch (\Throwable $e) { + while (ob_get_level() > $level) { + ob_end_clean(); + } + + throw $e; + } return ob_get_clean(); } @@ -355,6 +370,53 @@ abstract class Template return $this; } + /** + * @internal + */ + public function isOwnedBy(Environment $env): bool + { + return $this->env === $env; + } + + /** + * @internal + */ + public function freezeLineage(BlockResolutionContext $resolution): self + { + $resolution->assertOwns($this); + if ($resolution->isFrozen($this)) { + return $resolution->getFrozen($this); + } + + $resolution->beginFreeze($this); + try { + if (false === $parent = $resolution->getParent($this)) { + $resolution->setFrozen($this, $this); + + return $this; + } + + $template = clone $this; + foreach ($template->blocks as &$block) { + if ($block[0] === $this) { + $block[0] = $template; + } + } + unset($block); + + $template->macroNamespace = null; + // Keep module-level imports live while rebinding self imports to the clone. + $template->macroImportSource = $this; + $template->parent = $frozenParent = $parent->freezeLineage($resolution); + $resolution->setParent($template, $frozenParent); + $resolution->setFrozen($this, $template); + + return $template; + } finally { + $resolution->endFreeze($this); + } + } + /** * Returns all blocks. * @@ -499,6 +561,27 @@ abstract class Template return []; } + /** + * @param array $macros + * + * @return array + */ + protected function rebindMacroImports(array $macros): array + { + if (null === $this->macroImportSource) { + return $macros; + } + + $imported = $this->macroImportSource->getMacroNamespace(); + foreach ($macros as $name => $namespace) { + if ($namespace === $imported) { + $macros[$name] = $this->getMacroNamespace(); + } + } + + return $macros; + } + /** * Runs the sandbox security check against the current sandbox state. * diff --git a/tests/BlockChainTest.php b/tests/BlockChainTest.php new file mode 100644 index 000000000..8f10d5e61 --- /dev/null +++ b/tests/BlockChainTest.php @@ -0,0 +1,646 @@ + '{% extends "parent1" %}{% block first %}theme1{% endblock %}', + 'parent1' => '{% block shared %}parent1{% endblock %}{% block parent1 %}parent1{% endblock %}', + 'theme2' => '{% extends "parent2" %}{% block shared %}theme2{% endblock %}{% block second %}theme2{% endblock %}', + 'parent2' => '{% block parent2 %}parent2{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + + $chain = new BlockChain($twig, ['theme1', $twig->load('theme2')]); + + $this->assertSame(['first', 'shared', 'parent1', 'second', 'parent2'], $chain->getBlockNames()); + $this->assertTrue($chain->hasBlock('shared')); + $this->assertFalse($chain->hasBlock('missing')); + $this->assertSame('parent1', $chain->renderBlock('shared')); + $this->assertSame('theme2', $chain->renderBlock('second')); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testNestedBlocksUseTheEffectiveNamespaceAndParentUsesTheFrozenLineage(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% block field %}theme/{{ parent() }}/{{ block("suffix") }}{% endblock %}', + 'parent1' => '{% block field %}parent1{% endblock %}', + 'parent2' => '{% block field %}parent2{% endblock %}', + 'suffix' => '{% block suffix %}suffix{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + + $chain = new BlockChain($twig, ['theme', 'suffix'], ['parent' => 'parent1']); + + $this->assertSame('theme/parent1/suffix', $chain->renderBlock('field', ['parent' => 'parent2'])); + } + + public function testExplicitTemplateBlockCallsResolveOutsideTheChainNamespace(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% block field %}{{ block("suffix", "explicit") }}{% endblock %}', + 'chain' => '{% block suffix %}chain{% endblock %}', + 'explicit' => '{% block suffix %}explicit{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => true]); + + $chain = new BlockChain($twig, ['theme', 'chain']); + + $this->assertSame('explicit', $chain->renderBlock('field')); + } + + public function testFreezingAChainDoesNotChangeTheLoadedTemplate(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% block field %}{{ parent() }}{% endblock %}', + 'parent1' => '{% block field %}one{% endblock %}', + 'parent2' => '{% block field %}two{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => true]); + $template = $twig->load('theme'); + $chain = new BlockChain($twig, [$template], ['parent' => 'parent1']); + + $this->assertSame('one', $chain->renderBlock('field', ['parent' => 'parent2'])); + $this->assertSame('two', $template->renderBlock('field', ['parent' => 'parent2'])); + } + + public function testStructuralContextIncludesEnvironmentGlobals(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends layout %}{% block field %}{{ parent() }}{% endblock %}', + 'parent' => '{% block field %}parent{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => true]); + $twig->addGlobal('layout', 'parent'); + + $chain = new BlockChain($twig, ['theme']); + + $this->assertSame('parent', $chain->renderBlock('field')); + } + + public function testTraitAliasesKeepTheirLocalParentLineage(): void + { + $twig = new Environment(new ArrayLoader([ + 'base_trait' => '{% block field %}base{% endblock %}', + 'trait' => '{% use "base_trait" %}{% block field %}trait/{{ parent() }}{% endblock %}', + 'theme' => '{% use "trait" with field as aliased %}', + ]), ['autoescape' => false, 'use_yield' => true]); + + $chain = new BlockChain($twig, ['theme']); + + $this->assertSame(['aliased'], $chain->getBlockNames()); + $this->assertSame('trait/base', $chain->renderBlock('aliased')); + } + + public function testRenderCapturesLegacyEchoingBlocks(): void + { + $twig = new Environment(new ArrayLoader(), ['use_yield' => false]); + $chain = new BlockChain($twig, [new TemplateWrapper($twig, new EchoingBlockChainTemplate($twig))]); + + $this->assertSame('echo/yield', $chain->renderBlock('field')); + } + + public function testRenderRestoresOutputBuffersOnError(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% block field %}{% set captured %}{{ missing.value }}{% endset %}{% endblock %}', + ]), ['strict_variables' => true, 'use_yield' => false]); + $chain = new BlockChain($twig, ['theme']); + $level = ob_get_level(); + + try { + $chain->renderBlock('field'); + $this->fail('Rendering the block must fail.'); + } catch (RuntimeError) { + $actualLevel = ob_get_level(); + } finally { + while (ob_get_level() > $level) { + ob_end_clean(); + } + } + + $this->assertSame($level, $actualLevel); + } + + public function testRenderingDisplayingAndStreamingAddGlobals(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% block field %}{{ local }}:{{ global|default("none") }}{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => true]); + $twig->addGlobal('global', 'GLOBAL'); + $chain = new BlockChain($twig, ['theme']); + + $this->assertSame('LOCAL:GLOBAL', $chain->renderBlock('field', ['local' => 'LOCAL'])); + + ob_start(); + $chain->displayBlock('field', ['local' => 'LOCAL']); + $this->assertSame('LOCAL:GLOBAL', ob_get_clean()); + + $streamed = ''; + foreach ($chain->streamBlock('field', ['local' => 'LOCAL']) as $data) { + $streamed .= $data; + } + $this->assertSame('LOCAL:GLOBAL', $streamed); + } + + public function testRejectsInvalidBlockDefinitions(): void + { + $twig = new Environment(new ArrayLoader()); + $template = new class($twig) extends EchoingBlockChainTemplate { + public function __construct(Environment $env) + { + parent::__construct($env); + $this->blocks = ['field' => [new \stdClass(), 'block_field']]; + } + }; + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('A block must be a method on a \Twig\Template instance.'); + + new BlockChain($twig, [new TemplateWrapper($twig, $template)]); + } + + public function testRejectsTemplatesThatAreNotStringsOrWrappers(): void + { + $twig = new Environment(new ArrayLoader(['theme' => ''])); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Block chain templates must be strings or "Twig\TemplateWrapper" instances, "stdClass" given.'); + + new BlockChain($twig, [new \stdClass()]); + } + + public function testRejectsWrappersFromAnotherEnvironment(): void + { + $twig = new Environment(new ArrayLoader(['theme' => ''])); + $other = new Environment(new ArrayLoader(['theme' => ''])); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('A block chain cannot contain templates from different Twig environments.'); + + new BlockChain($twig, [$other->load('theme')]); + } + + public function testRejectsWrappersThatHideATemplateFromAnotherEnvironment(): void + { + $twig = new Environment(new ArrayLoader(['theme' => ''])); + $other = new Environment(new ArrayLoader(['theme' => ''])); + $wrapper = new TemplateWrapper($twig, $other->load('theme')->unwrap()); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('A block chain cannot contain templates from different Twig environments.'); + + new BlockChain($twig, [$wrapper]); + } + + public function testRejectsDynamicParentsFromAnotherEnvironment(): void + { + $twig = new Environment(new ArrayLoader(['theme' => '{% extends parent %}'])); + $other = new Environment(new ArrayLoader(['parent' => ''])); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('A block chain cannot contain templates from different Twig environments.'); + + new BlockChain($twig, ['theme'], ['parent' => $other->load('parent')]); + } + + public function testDynamicParentSecurityIsCheckedDuringConstruction(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent|upper %}', + 'PARENT' => '', + ])); + $twig->addExtension(new SandboxExtension(new SecurityPolicy(['extends']), true)); + + $this->expectException(SecurityNotAllowedFilterError::class); + + new BlockChain($twig, ['theme'], ['parent' => 'parent']); + } + + public function testDefiningTemplateSecurityIsCheckedDuringRendering(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% block field %}{{ value|upper }}{% endblock %}', + ])); + $twig->addExtension(new SandboxExtension(new SecurityPolicy(['block']), true)); + $chain = new BlockChain($twig, ['theme']); + + $this->expectException(SecurityNotAllowedFilterError::class); + + $chain->renderBlock('field', ['value' => 'value']); + } + + public function testSandboxPolicyChangesAreObservedAfterConstruction(): void + { + $twig = new Environment(new ArrayLoader([ + 'policy_theme' => '{% extends "parent" %}{% block field %}{{ value|upper }}{% endblock %}', + 'parent' => '', + ]), ['autoescape' => false, 'use_yield' => true]); + $sandbox = new SandboxExtension(new SecurityPolicy(['extends', 'block'], ['upper']), true); + $twig->addExtension($sandbox); + $chain = new BlockChain($twig, ['policy_theme']); + + $this->assertSame('VALUE', $chain->renderBlock('field', ['value' => 'value'])); + + $sandbox->setSecurityPolicy(new SecurityPolicy(['extends', 'block'])); + $this->expectException(SecurityNotAllowedFilterError::class); + + $chain->renderBlock('field', ['value' => 'value']); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testSandboxPolicyChangesAreCheckedOnFrozenIntermediateParents(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends "middle" %}{% block field %}{{ parent() }}{% endblock %}', + 'middle' => '{% extends parent|upper %}', + 'GRANDPARENT' => '{% block field %}safe{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $sandbox = new SandboxExtension(new SecurityPolicy(['extends', 'block'], ['upper'], allowedFunctions: ['parent']), true); + $twig->addExtension($sandbox); + $chain = new BlockChain($twig, ['theme'], ['parent' => 'grandparent']); + + $this->assertSame('safe', $chain->renderBlock('field')); + + $sandbox->setSecurityPolicy(new SecurityPolicy(['extends', 'block'], allowedFunctions: ['parent'])); + $this->expectException(SecurityNotAllowedFilterError::class); + + $chain->renderBlock('field'); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testImportedMacroNamespacesObserveSandboxPolicyChanges(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends "layout" %}{% import "macros" as macros %}{% block field %}{{ macros.label(value) }}{% endblock %}', + 'layout' => '{{ block("field") }}', + 'macros' => '{% macro label(value) %}{{ value|upper }}{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $sandbox = new SandboxExtension(new SecurityPolicy(['extends', 'import', 'block', 'macro'], ['upper'], allowedFunctions: ['block']), true); + $twig->addExtension($sandbox); + $chain = new BlockChain($twig, ['theme']); + + $this->assertSame('VALUE', $twig->render('theme', ['value' => 'value'])); + $this->assertSame('VALUE', $chain->renderBlock('field', ['value' => 'value'])); + + $sandbox->setSecurityPolicy(new SecurityPolicy(['extends', 'import', 'block', 'macro'], allowedFunctions: ['block'])); + $this->expectException(SecurityNotAllowedFilterError::class); + + $chain->renderBlock('field', ['value' => 'value']); + } + + public function testProfilerKeepsTheDefiningTemplateAndBlockAttribution(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends "parent" %}{% block field %}field{% endblock %}', + 'parent' => '', + ]), ['use_yield' => true]); + $profile = new Profile(); + $twig->addExtension(new ProfilerExtension($profile)); + $chain = new BlockChain($twig, ['theme']); + + $chain->renderBlock('field'); + + $profiles = $profile->getProfiles(); + $this->assertCount(1, $profiles); + $this->assertSame('theme', $profiles[0]->getTemplate()); + $this->assertSame(Profile::BLOCK, $profiles[0]->getType()); + $this->assertSame('field', $profiles[0]->getName()); + } + + public function testMacrosStayOwnedByTheirDefiningTemplate(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme1' => '{% macro label() %}one{% endmacro %}{% block field %}{{ _self.label() }}{% endblock %}', + 'theme2' => '{% macro label() %}two{% endmacro %}{% block field %}{{ _self.label() }}{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => true]); + + $chain = new BlockChain($twig, ['theme1', 'theme2']); + + $this->assertSame('one', $chain->renderBlock('field')); + } + + public function testInheritedMacroLookupUsesTheFrozenLineage(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% block field %}{{ _self.label() }}{% endblock %}', + 'parent1' => '{% macro label() %}one{% endmacro %}', + 'parent2' => '{% macro label() %}two{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => true]); + + $chain = new BlockChain($twig, ['theme'], ['parent' => 'parent1']); + + $this->assertSame('one', $chain->renderBlock('field', ['parent' => 'parent2'])); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testSelfMacroImportsInitializedAfterConstructionUseTheFrozenLineage(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% import _self as own %}{% block field %}{{ own.label() }}{% endblock %}', + 'parent1' => '{% macro label() %}one{% endmacro %}{{ block("field") }}', + 'parent2' => '{% macro label() %}two{% endmacro %}{{ block("field") }}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $chain = new BlockChain($twig, ['theme'], ['parent' => 'parent1']); + + $this->assertSame('two', $twig->render('theme', ['parent' => 'parent2'])); + $this->assertSame('one', $chain->renderBlock('field', ['parent' => 'parent2'])); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testPreWarmedSelfMacroImportsUseTheFrozenLineage(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% import _self as own %}{% block field %}{{ own.label() }}{% endblock %}', + 'parent1' => '{% macro label() %}one{% endmacro %}{{ block("field") }}', + 'parent2' => '{% macro label() %}two{% endmacro %}{{ block("field") }}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $this->assertSame('two', $twig->render('theme', ['parent' => 'parent2'])); + + $chain = new BlockChain($twig, ['theme'], ['parent' => 'parent1']); + + $this->assertSame('one', $chain->renderBlock('field', ['parent' => 'parent2'])); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testFromSelfImportsUseTheFrozenLineage(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% from _self import label %}{% macro wrapped() %}{{ label() }}{% endmacro %}{% block field %}{{ label() }}/{{ _self.wrapped() }}{% endblock %}', + 'parent1' => '{% macro label() %}one{% endmacro %}', + 'parent2' => '{% macro label() %}two{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $this->assertSame('', $twig->render('theme', ['parent' => 'parent2'])); + + $chain = new BlockChain($twig, ['theme'], ['parent' => 'parent1']); + + $this->assertSame('one/one', $chain->renderBlock('field', ['parent' => 'parent2'])); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testImportedMacroNamespacesStayOutsideTheFrozenLineage(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% import parent as inherited %}{% block field %}{{ inherited.label() }}{% endblock %}', + 'parent1' => '{% macro label() %}one{% endmacro %}{{ block("field") }}', + 'parent2' => '{% macro label() %}two{% endmacro %}{{ block("field") }}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $this->assertSame('two', $twig->render('theme', ['parent' => 'parent2'])); + + $chain = new BlockChain($twig, ['theme'], ['parent' => 'parent1']); + + $this->assertSame('two', $chain->renderBlock('field')); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testModuleImportsFollowTheDefiningTemplateBodyState(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends "layout" %}{% import helper as macros %}{% block field %}{{ macros.label() }}{% endblock %}', + 'layout' => '{{ block("field") }}', + 'macros1' => '{% macro label() %}one{% endmacro %}', + 'macros2' => '{% macro label() %}two{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $chain = new BlockChain($twig, ['theme']); + + try { + $chain->renderBlock('field', ['helper' => 'macros1']); + $this->fail('Rendering an uninitialized import must fail.'); + } catch (RuntimeError) { + } + + $this->assertSame('one', $twig->render('theme', ['helper' => 'macros1'])); + $this->assertSame('one', $chain->renderBlock('field')); + $this->assertSame('two', $twig->render('theme', ['helper' => 'macros2'])); + $this->assertSame('two', $chain->renderBlock('field')); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testSelfMacroImportsInMacroBodiesUseTheFrozenLineage(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% import _self as own %}{% macro wrapped() %}{{ own.label() }}{% endmacro %}{% block field %}{{ _self.wrapped() }}{% endblock %}', + 'parent1' => '{% macro label() %}one{% endmacro %}', + 'parent2' => '{% macro label() %}two{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $chain = new BlockChain($twig, ['theme'], ['parent' => 'parent1']); + + $this->assertSame('', $twig->render('theme', ['parent' => 'parent2'])); + $this->assertSame('one', $chain->renderBlock('field', ['parent' => 'parent2'])); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testMacroBodiesObserveImportUpdatesAfterConstruction(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends "layout" %}{% import helper as macros %}{% macro wrapped() %}{{ macros.label() }}{% endmacro %}{% block field %}{{ _self.wrapped() }}{% endblock %}', + 'layout' => '{{ block("field") }}', + 'macros1' => '{% macro label() %}one{% endmacro %}', + 'macros2' => '{% macro label() %}two{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $chain = new BlockChain($twig, ['theme']); + + $this->assertSame('one', $twig->render('theme', ['helper' => 'macros1'])); + $this->assertSame('one', $chain->renderBlock('field')); + $this->assertSame('two', $twig->render('theme', ['helper' => 'macros2'])); + $this->assertSame('two', $chain->renderBlock('field')); + } + + /** + * @dataProvider yieldModes + */ + #[DataProvider('yieldModes')] + public function testPreWarmedExternalMacroImportsAreNotReboundByChainOrder(bool $useYield): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends "layout" %}{% import "macros" as macros %}{% block field %}{{ macros.label() }}{% endblock %}', + 'layout' => '{{ block("field") }}', + 'macros' => '{% extends macro_parent %}', + 'macros1' => '{% macro label() %}one{% endmacro %}', + 'macros2' => '{% macro label() %}two{% endmacro %}', + ]), ['autoescape' => false, 'use_yield' => $useYield]); + $this->assertSame('two', $twig->render('theme', ['macro_parent' => 'macros2'])); + + $context = ['macro_parent' => 'macros1']; + $renderContext = ['macro_parent' => 'macros2']; + + $this->assertSame('two', (new BlockChain($twig, ['macros', 'theme'], $context))->renderBlock('field', $renderContext)); + $this->assertSame('two', (new BlockChain($twig, ['theme', 'macros'], $context))->renderBlock('field', $renderContext)); + } + + public function testChainsWithDifferentDynamicParentsCanBeStreamedInterleaved(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => '{% extends parent %}{% block field %}before/{{ parent() }}/after{% endblock %}', + 'parent1' => '{% block field %}one{% endblock %}', + 'parent2' => '{% block field %}two{% endblock %}', + ]), ['autoescape' => false, 'use_yield' => true]); + + $stream1 = (new BlockChain($twig, ['theme'], ['parent' => 'parent1']))->streamBlock('field'); + $stream2 = (new BlockChain($twig, ['theme'], ['parent' => 'parent2']))->streamBlock('field'); + + $output1 = $output2 = ''; + $stream1->rewind(); + $stream2->rewind(); + while ($stream1->valid() || $stream2->valid()) { + if ($stream1->valid()) { + $output1 .= $stream1->current(); + $stream1->next(); + } + if ($stream2->valid()) { + $output2 .= $stream2->current(); + $stream2->next(); + } + } + + $this->assertSame('before/one/after', $output1); + $this->assertSame('before/two/after', $output2); + } + + public function testErrorsKeepTheDefiningSourceAndLine(): void + { + $twig = new Environment(new ArrayLoader([ + 'theme' => "{% block field %}\n{{ missing.value }}\n{% endblock %}", + ]), ['strict_variables' => true, 'use_yield' => true]); + $chain = new BlockChain($twig, ['theme']); + + try { + $chain->renderBlock('field'); + $this->fail('Rendering must fail.'); + } catch (RuntimeError $e) { + $this->assertSame('theme', $e->getSourceContext()->getName()); + $this->assertSame(2, $e->getTemplateLine()); + } + } + + public function testUnknownBlockUsesTheFirstTemplateAsErrorContext(): void + { + $twig = new Environment(new ArrayLoader(['theme' => ''])); + $chain = new BlockChain($twig, ['theme']); + + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('Block "missing" on template "theme" does not exist in "theme".'); + + $chain->renderBlock('missing'); + } + + public function testCircularInheritanceIsRejectedDuringConstruction(): void + { + $twig = new Environment(new ArrayLoader([ + 'one' => '{% extends "two" %}', + 'two' => '{% extends "one" %}', + ])); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Circular template inheritance detected while building a block chain from "one".'); + + new BlockChain($twig, ['one']); + } + + public function testRequiresAtLeastOneTemplate(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('A block chain requires at least one template.'); + + new BlockChain(new Environment(new ArrayLoader()), []); + } + + public static function yieldModes(): iterable + { + yield 'echo and yield' => [false]; + yield 'yield only' => [true]; + } +} + +class EchoingBlockChainTemplate extends Template +{ + public function __construct(Environment $env) + { + parent::__construct($env); + $this->parent = false; + $this->blocks = ['field' => [$this, 'block_field']]; + } + + public function block_field(array $context, array $blocks = []): iterable + { + echo 'echo/'; + yield 'yield'; + } + + public function getTemplateName(): string + { + return 'echoing'; + } + + public function getDebugInfo(): array + { + return []; + } + + public function getSourceContext(): Source + { + return new Source('', 'echoing'); + } + + protected function doDisplay(array $context, array $blocks = []): iterable + { + yield from []; + } +} diff --git a/tests/Node/BlockTest.php b/tests/Node/BlockTest.php index 8606484ba..4fa08fc2d 100644 --- a/tests/Node/BlockTest.php +++ b/tests/Node/BlockTest.php @@ -47,7 +47,7 @@ class BlockTest extends NodeTestCase */ public function block_foo(array \$context, array \$blocks = []): iterable { - \$macros = \$this->macros; + \$macros = null === \$this->macroImportSource ? \$this->macros : \$this->rebindMacroImports(\$this->macroImportSource->macros); yield "foo"; yield from []; } diff --git a/tests/Node/MacroTest.php b/tests/Node/MacroTest.php index 29a0b5cd8..f5bd73b3c 100644 --- a/tests/Node/MacroTest.php +++ b/tests/Node/MacroTest.php @@ -65,7 +65,7 @@ class MacroTest extends NodeTestCase yield 'with use_yield = true' => [self::createNode(), <<macros; + \$macros = null === \$this->macroImportSource ? \$this->macros : \$this->rebindMacroImports(\$this->macroImportSource->macros); \$context = [ "foo" => \$foo, "bar" => \$bar, @@ -86,7 +86,7 @@ EOF, new Environment(new ArrayLoader(), ['use_yield' => true]), yield 'with use_yield = false' => [self::createNode(), <<macros; + \$macros = null === \$this->macroImportSource ? \$this->macros : \$this->rebindMacroImports(\$this->macroImportSource->macros); \$context = [ "foo" => \$foo, "bar" => \$bar, diff --git a/tests/Node/MacrosTest.php b/tests/Node/MacrosTest.php index 7a5687e69..b2f7b503e 100644 --- a/tests/Node/MacrosTest.php +++ b/tests/Node/MacrosTest.php @@ -64,7 +64,7 @@ protected function loadDeclaredMacros(): array return [ "foo" => new \\Twig\\TwigMacro("foo", function (\$foo = null, ...\$varargs): string|Markup { // line 1 - \$macros = \$this->macros; + \$macros = null === \$this->macroImportSource ? \$this->macros : \$this->rebindMacroImports(\$this->macroImportSource->macros); \$context = [ "foo" => \$foo, "varargs" => \$varargs, diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php index 92a80cec1..5bc9520f0 100644 --- a/tests/TemplateTest.php +++ b/tests/TemplateTest.php @@ -226,6 +226,33 @@ class TemplateTest extends TestCase $template->displayBlock('foo', [], ['foo' => [new TemplateForTest($twig, 'index.twig'), 'block_foo']], false); } + /** + * @dataProvider debugModes + */ + #[DataProvider('debugModes')] + public function testRenderParentBlockRestoresOutputBuffersOnError(bool $debug): void + { + $twig = new Environment(new ArrayLoader([ + 'parent' => '{% block content %}{{ missing.value }}{% endblock %}', + 'child' => '{% extends "parent" %}', + ]), ['debug' => $debug, 'strict_variables' => true, 'use_yield' => false]); + $template = $twig->load('child')->unwrap(); + $level = ob_get_level(); + + try { + $template->renderParentBlock('content', []); + $this->fail('Rendering the parent block must fail.'); + } catch (RuntimeError) { + $actualLevel = ob_get_level(); + } finally { + while (ob_get_level() > $level) { + ob_end_clean(); + } + } + + $this->assertSame($level, $actualLevel); + } + public function testGetAttributeOnArrayWithConfusableKey(): void { $twig = new Environment(new ArrayLoader()); @@ -462,6 +489,12 @@ class TemplateTest extends TestCase $this->assertSame(0, $key->toStringCalls); } + public static function debugModes(): iterable + { + yield 'debug disabled' => [false]; + yield 'debug enabled' => [true]; + } + public static function getStrictVariablesModes(): iterable { yield 'lax' => [false]; From c53b6468c9e6c2fc0d6bdd553c518b8bd3cae39d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 11 Sep 2026 06:10:11 -0700 Subject: [PATCH 4/5] Resolve constant parent templates once instead of on every lookup --- CHANGELOG | 1 + src/Node/ModuleNode.php | 19 ++++++++++--------- tests/Node/ModuleTest.php | 2 +- tests/TemplateWrapperTest.php | 13 +++++++++++++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 7b222909e..b0fb15187 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,7 @@ * Fix `TemplateWrapper::streamBlock()`, `TemplateWrapper::hasBlock()`, and `TemplateWrapper::getBlockNames()` omitting environment globals * Fix exceptions from dynamic parent expressions escaping without template context * Add the `BlockChain` class to compose blocks from multiple templates without using template internals + * Fix `TemplateWrapper::hasBlock()` and `TemplateWrapper::getBlockNames()` losing the `extends` line when the parent template does not exist * Fix an output buffer leak when a parent block rendered in an expression throws in non-yield mode * Add the `HtmlExtension::htmlAttrValue()` method to resolve a single HTML attribute value the way the `html_attr` function renders it * Fix `html_attr` JSON encoding a `Stringable` value in a `data-*` attribute instead of using its string representation diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index ca2b0b9c2..13eef99ad 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -143,17 +143,18 @@ final class ModuleNode extends Node implements CoercesChildrenToStringInterface ; if ($parent instanceof ConstantExpression) { - $compiler->subcompile($parent); - } else { - $compiler - ->raw('$this->load(') - ->subcompile($parent) - ->raw(', ') - ->repr($parent->getTemplateLine()) - ->raw(')') - ; + // a constant parent never depends on the context, so resolve it once + $compiler->raw('$this->parent ??= '); } + $compiler + ->raw('$this->load(') + ->subcompile($parent) + ->raw(', ') + ->repr($parent->getTemplateLine()) + ->raw(')') + ; + $compiler ->raw(";\n") ->outdent() diff --git a/tests/Node/ModuleTest.php b/tests/Node/ModuleTest.php index c3cbad02d..670309367 100644 --- a/tests/Node/ModuleTest.php +++ b/tests/Node/ModuleTest.php @@ -232,7 +232,7 @@ class __TwigTemplate_%x extends Template protected function doGetParent(array \$context): bool|string|Template|TemplateWrapper { // line 1 - return "layout.twig"; + return \$this->parent ??= \$this->load("layout.twig", 1); } protected function doDisplay(array \$context, array \$blocks = []): iterable diff --git a/tests/TemplateWrapperTest.php b/tests/TemplateWrapperTest.php index b5a8b6be3..197e3aa13 100644 --- a/tests/TemplateWrapperTest.php +++ b/tests/TemplateWrapperTest.php @@ -139,6 +139,19 @@ class TemplateWrapperTest extends TestCase yield 'exception thrown by parent expression' => ['{% extends boom() %}', new TwigFunction('boom', static function (): never { throw new \DomainException('kaboom'); }), \DomainException::class]; } + public function testBlockIntrospectionReportsTheExtendsLineForAMissingParent(): void + { + $twig = new Environment(new ArrayLoader(['index' => "\n\n{% extends 'missing' %}"])); + + try { + $twig->load('index')->hasBlock('foo'); + $this->fail('Introspecting a template with a missing parent must fail.'); + } catch (LoaderError $e) { + $this->assertSame('Template "missing" is not defined.', $e->getRawMessage()); + $this->assertSame(3, $e->getTemplateLine()); + } + } + public function testRenderBlock(): void { $twig = new Environment(new ArrayLoader([ From 897717d78f0ce627509eda561e10191e19256365 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 12 Sep 2026 00:03:19 +0200 Subject: [PATCH 5/5] Resolve block chains against the render context instead of freezing lineages --- doc/api.rst | 20 +++- src/BlockChain.php | 180 ++++++++++++++++++++++++--------- src/BlockResolutionContext.php | 91 ----------------- src/Node/BlockNode.php | 2 +- src/Node/MacroNode.php | 2 +- src/Template.php | 65 +----------- tests/BlockChainTest.php | 141 +++++++++++++++++--------- tests/Node/BlockTest.php | 2 +- tests/Node/MacroTest.php | 4 +- tests/Node/MacrosTest.php | 2 +- tests/TemplateTest.php | 21 ++++ 11 files changed, 274 insertions(+), 256 deletions(-) delete mode 100644 src/BlockResolutionContext.php diff --git a/doc/api.rst b/doc/api.rst index cbf2047dd..d910530ae 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -108,9 +108,23 @@ templates share the composed block set, only combine templates that are trusted to call one another's blocks. Chained blocks have the same top-level macro import limitation as blocks rendered directly. -The optional third constructor argument provides variables used to resolve -dynamic parent expressions. Parent hierarchies are fixed when the chain is -created; create another chain when these variables change. +Every method takes the render context, and the whole chain is resolved against +it, so a template using a dynamic ``{% extends %}`` behaves exactly as it would +when rendered directly. The optional third constructor argument provides +default variables for that resolution, which the render context can override:: + + $blocks = new BlockChain($twig, ['application_theme.html.twig'], [ + 'layout' => 'wide_layout.html.twig', + ]); + + // resolved against wide_layout.html.twig + $blocks->getBlockNames(); + + // resolved against narrow_layout.html.twig + $blocks->getBlockNames(['layout' => 'narrow_layout.html.twig']); + +Pass the same context to ``hasBlock()``, ``getBlockNames()`` and +``renderBlock()`` to keep them consistent. Streaming Templates ------------------- diff --git a/src/BlockChain.php b/src/BlockChain.php index 0449796db..e68f145da 100644 --- a/src/BlockChain.php +++ b/src/BlockChain.php @@ -18,21 +18,29 @@ use Twig\Error\RuntimeError; */ final class BlockChain { + /** @var list