From 95ac936f88765af00e9c4d790708e70fc2f3e47e Mon Sep 17 00:00:00 2001 From: Leo Feyer <1192057+leofeyer@users.noreply.github.com> Date: Wed, 15 May 2024 17:06:03 +0200 Subject: [PATCH] Make Twig 3.10.2 the minimum requirement (see #7214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description ----------- This pull request makes Twig 3.10.2 the minimum version and therefore does not need all the BC layers. @contao/developers Do we want to add this to Contao 5.3 or only in Contao 5.4? Commits ------- e3f56025 Make Twig 3.10.2 the minimum requirement 4e8915e4 Remove a Twig 3.8 BC layer 04205f95 Clean up after the upstream merge 10b05f0e Use EscaperExtension::escapeFilterIsSafe() instead of twig_escape_fil… 025eadbd Do not use the internal EscaperExtension::escapeFilterIsSafe() method… 6f7ccc2f Apply the suggestions from the pull request 6dea4c84 Drop the "testContaoUsesCorrectTwigFunctionSignatures" test --- composer.json | 4 +- core-bundle/composer.json | 6 +- .../src/Twig/Extension/ContaoExtension.php | 50 +++++++------- .../src/Twig/Interop/ContaoEscaper.php | 5 +- .../Twig/Extension/ContaoExtensionTest.php | 66 +++---------------- .../tests/Twig/Interop/ContaoEscaperTest.php | 5 +- .../PhpTemplateParentReferenceNodeTest.php | 8 +-- .../Twig/Interop/PhpTemplateProxyNodeTest.php | 8 +-- .../Twig/ResponseContext/AddNodeTest.php | 8 +-- 9 files changed, 46 insertions(+), 114 deletions(-) diff --git a/composer.json b/composer.json index ecf2148e25e..a55d94f2eb9 100644 --- a/composer.json +++ b/composer.json @@ -165,7 +165,7 @@ "toflar/psr6-symfony-http-cache-store": "^4.0", "twig/extra-bundle": "^3.0", "twig/string-extra": "^3.0", - "twig/twig": "^3.8", + "twig/twig": "^3.10.2", "ua-parser/uap-php": "^3.9", "webignition/robots-txt-file": "^3.0", "wikimedia/less.php": "^1.7" @@ -202,8 +202,6 @@ "nikic/php-parser": "4.7.0", "terminal42/contao-ce-access": "<3.0", "thecodingmachine/safe": "<1.2", - "twig/intl-extra": "3.9.0", - "twig/twig": "3.9.0 || 3.10.0 || 3.10.1", "zendframework/zend-code": "<3.3.1" }, "autoload": { diff --git a/core-bundle/composer.json b/core-bundle/composer.json index f6cc3500d96..33e7d47dcaf 100644 --- a/core-bundle/composer.json +++ b/core-bundle/composer.json @@ -151,7 +151,7 @@ "terminal42/service-annotation-bundle": "^1.1", "toflar/cronjob-supervisor": "^2.0", "twig/string-extra": "^3.0", - "twig/twig": "^3.8", + "twig/twig": "^3.10.2", "ua-parser/uap-php": "^3.9", "webignition/robots-txt-file": "^3.0", "wikimedia/less.php": "^1.7" @@ -179,9 +179,7 @@ "contao/manager-plugin": "<2.0 || >=3.0", "doctrine/cache": "<1.10", "terminal42/contao-ce-access": "<3.0", - "thecodingmachine/safe": "<1.2", - "twig/intl-extra": "3.9.0", - "twig/twig": "3.9.0 || 3.10.0 || 3.10.1" + "thecodingmachine/safe": "<1.2" }, "autoload": { "psr-4": { diff --git a/core-bundle/src/Twig/Extension/ContaoExtension.php b/core-bundle/src/Twig/Extension/ContaoExtension.php index 3d1ce37129a..b07b2be4e31 100644 --- a/core-bundle/src/Twig/Extension/ContaoExtension.php +++ b/core-bundle/src/Twig/Extension/ContaoExtension.php @@ -48,10 +48,10 @@ use Twig\Environment; use Twig\Extension\AbstractExtension; use Twig\Extension\CoreExtension; -use Twig\Extension\EscaperExtension; use Twig\Extension\GlobalsInterface; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Node; +use Twig\Runtime\EscaperRuntime; use Twig\TwigFilter; use Twig\TwigFunction; @@ -69,15 +69,10 @@ public function __construct( private readonly ContaoVariable $contaoVariable, ) { $contaoEscaper = new ContaoEscaper(); - $escaperExtension = $environment->getExtension(EscaperExtension::class); - // Forward compatibility with twig/twig >=3.10.0 - if (method_exists($escaperExtension, 'setEnvironment')) { - $escaperExtension->setEnvironment($environment); - } - - $escaperExtension->setEscaper('contao_html', $contaoEscaper->escapeHtml(...)); - $escaperExtension->setEscaper('contao_html_attr', $contaoEscaper->escapeHtmlAttr(...)); + $escaperRuntime = $this->environment->getRuntime(EscaperRuntime::class); + $escaperRuntime->setEscaper('contao_html', $contaoEscaper->escapeHtml(...)); + $escaperRuntime->setEscaper('contao_html_attr', $contaoEscaper->escapeHtmlAttr(...)); // Use our escaper on all templates in the "@Contao" and "@Contao_*" namespaces, // as well as the existing bundle templates we're already shipping. @@ -85,8 +80,8 @@ public function __construct( $this->addContaoEscaperRule('%^@ContaoCore/%'); // Mark classes as safe for HTML that already escape their output themselves - $escaperExtension->addSafeClass(HtmlAttributes::class, ['html', 'contao_html']); - $escaperExtension->addSafeClass(HighlightResult::class, ['html', 'contao_html']); + $escaperRuntime->addSafeClass(HtmlAttributes::class, ['html', 'contao_html']); + $escaperRuntime->addSafeClass(HighlightResult::class, ['html', 'contao_html']); $this->environment->addGlobal( 'request_token', @@ -242,7 +237,9 @@ function (Environment $env, $context, $template, $variables = [], $withContext = public function getFilters(): array { - $escaperFilter = static function (Environment $env, $string, $strategy = 'html', $charset = null, $autoescape = false) { + $escaperFilter = static function (Environment $env, $string, string $strategy = 'html', string|null $charset = null, bool $autoescape = false) { + $runtime = $env->getRuntime(EscaperRuntime::class); + if ($string instanceof ChunkedText) { $parts = []; @@ -250,31 +247,36 @@ public function getFilters(): array if (ChunkedText::TYPE_RAW === $type) { $parts[] = $chunk; } else { - $parts[] = twig_escape_filter($env, $chunk, $strategy, $charset); + $parts[] = $runtime->escape($chunk, $strategy, $charset); } } return implode('', $parts); } - return twig_escape_filter($env, $string, $strategy, $charset, $autoescape); + return $runtime->escape($string, $strategy, $charset, $autoescape); }; + /** @see \Twig\Extension\EscaperExtension::escapeFilterIsSafe() */ $twigEscaperFilterIsSafe = static function (Node $filterArgs): array { - $expression = iterator_to_array($filterArgs)[0] ?? null; - - if ($expression instanceof ConstantExpression) { - $value = $expression->getAttribute('value'); + foreach ($filterArgs as $arg) { + if ($arg instanceof ConstantExpression) { + $value = $arg->getAttribute('value'); + + // Our escaper strategy variants that tolerate input encoding are also safe in + // the original context (e.g. for the filter argument 'contao_html' we will + // return ['contao_html', 'html']). + if (\in_array($value, ['contao_html', 'contao_html_attr'], true)) { + return [$value, substr($value, 7)]; + } - // Our escaper strategy variants that tolerate input encoding are also safe in - // the original context (e.g. for the filter argument 'contao_html' we will - // return ['contao_html', 'html']). - if (\in_array($value, ['contao_html', 'contao_html_attr'], true)) { - return [$value, substr($value, 7)]; + return [$value]; } + + return []; } - return twig_escape_filter_is_safe($filterArgs); + return ['html']; }; return [ diff --git a/core-bundle/src/Twig/Interop/ContaoEscaper.php b/core-bundle/src/Twig/Interop/ContaoEscaper.php index ac7a67a048d..2cdbcc11fb2 100644 --- a/core-bundle/src/Twig/Interop/ContaoEscaper.php +++ b/core-bundle/src/Twig/Interop/ContaoEscaper.php @@ -13,7 +13,6 @@ namespace Contao\CoreBundle\Twig\Interop; use Contao\StringUtil; -use Twig\Environment; use Twig\Error\RuntimeError; /** @@ -33,7 +32,7 @@ final class ContaoEscaper * * @see twig_escape_filter */ - public function escapeHtml(Environment $environment, mixed $string, string|null $charset): string + public function escapeHtml(mixed $string, string|null $charset): string { if (null !== $charset && 'UTF-8' !== strtoupper($charset)) { throw new RuntimeError(sprintf('The "contao_html" escape filter does not support the %s charset, use UTF-8 instead.', $charset)); @@ -50,7 +49,7 @@ public function escapeHtml(Environment $environment, mixed $string, string|null * * @see twig_escape_filter */ - public function escapeHtmlAttr(Environment $environment, mixed $string, string|null $charset): string + public function escapeHtmlAttr(mixed $string, string|null $charset): string { if (null !== $charset && 'UTF-8' !== strtoupper($charset)) { throw new RuntimeError(sprintf('The "contao_html_attr" escape filter does not support the %s charset, use UTF-8 instead.', $charset)); diff --git a/core-bundle/tests/Twig/Extension/ContaoExtensionTest.php b/core-bundle/tests/Twig/Extension/ContaoExtensionTest.php index 0b7d48a3204..cae6f8b7d2b 100644 --- a/core-bundle/tests/Twig/Extension/ContaoExtensionTest.php +++ b/core-bundle/tests/Twig/Extension/ContaoExtensionTest.php @@ -176,6 +176,11 @@ public function testIncludeFunctionDelegatesToTwigInclude(): void public function testThrowsIfCoreIncludeFunctionIsNotFound(): void { $environment = $this->createMock(Environment::class); + $environment + ->method('getRuntime') + ->willReturn(new EscaperRuntime()) + ; + $environment ->method('getExtension') ->willReturnMap([ @@ -185,14 +190,6 @@ public function testThrowsIfCoreIncludeFunctionIsNotFound(): void ]) ; - // Forward compatibility with twig/twig >=3.10.0 - if (class_exists(EscaperRuntime::class)) { - $environment - ->method('getRuntime') - ->willReturn(new EscaperRuntime()) - ; - } - $extension = new ContaoExtension( $environment, $this->createMock(ContaoFilesystemLoader::class), @@ -379,46 +376,6 @@ public static function provideTemplateNames(): iterable yield 'core-bundle template' => ['@ContaoCore/Image/Studio/figure.html.twig']; } - /** - * We need to adjust some of Twig's core functions (e.g. the escape filter) but - * still delegate to the original implementation for maximum compatibility. This - * test makes sure the function's signatures remains the same and changes to the - * original codebase do not stay unnoticed. - * - * @dataProvider provideTwigFunctionSignatures - */ - public function testContaoUsesCorrectTwigFunctionSignatures(\ReflectionFunction|\ReflectionMethod $reflector, array $expectedParameters): void - { - $parameters = array_map( - static fn (\ReflectionParameter $parameter): array => [ - ($type = $parameter->getType()) instanceof \ReflectionNamedType ? $type->getName() : null, - $parameter->getName(), - ], - $reflector->getParameters(), - ); - - $this->assertSame($parameters, $expectedParameters); - } - - public static function provideTwigFunctionSignatures(): iterable - { - // Backwards compatibility with twig/twig <3.9.0 - new \ReflectionClass(EscaperExtension::class); - - yield [ - new \ReflectionFunction('twig_escape_filter'), - [ - [Environment::class, 'env'], - [null, 'string'], - [null, 'strategy'], - [null, 'charset'], - [null, 'autoescape'], - ], - ]; - - yield [new \ReflectionFunction('twig_escape_filter_is_safe'), [[Node::class, 'filterArgs']]]; - } - /** * @param Environment&MockObject $environment */ @@ -427,6 +384,11 @@ private function getContaoExtension(Environment|null $environment = null, Contao $environment ??= $this->createMock(Environment::class); $filesystemLoader ??= $this->createMock(ContaoFilesystemLoader::class); + $environment + ->method('getRuntime') + ->willReturn(new EscaperRuntime()) + ; + $environment ->method('getExtension') ->willReturnMap([ @@ -435,14 +397,6 @@ private function getContaoExtension(Environment|null $environment = null, Contao ]) ; - // Forward compatibility with twig/twig >=3.10.0 - if (class_exists(EscaperRuntime::class)) { - $environment - ->method('getRuntime') - ->willReturn(new EscaperRuntime()) - ; - } - return new ContaoExtension( $environment, $filesystemLoader, diff --git a/core-bundle/tests/Twig/Interop/ContaoEscaperTest.php b/core-bundle/tests/Twig/Interop/ContaoEscaperTest.php index 270f20ac7b0..7eec2775710 100644 --- a/core-bundle/tests/Twig/Interop/ContaoEscaperTest.php +++ b/core-bundle/tests/Twig/Interop/ContaoEscaperTest.php @@ -21,7 +21,6 @@ use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpKernel\Fragment\FragmentHandler; -use Twig\Environment; use Twig\Error\RuntimeError; class ContaoEscaperTest extends TestCase @@ -140,11 +139,11 @@ public function testEscapeHtmlAttrThrowsErrorIfCharsetIsNotUtf8(): void private function invokeEscapeHtml(int|string $input, string|null $charset): string { - return (new ContaoEscaper())->escapeHtml($this->createMock(Environment::class), $input, $charset); + return (new ContaoEscaper())->escapeHtml($input, $charset); } private function invokeEscapeHtmlAttr(int|string $input, string|null $charset): string { - return (new ContaoEscaper())->escapeHtmlAttr($this->createMock(Environment::class), $input, $charset); + return (new ContaoEscaper())->escapeHtmlAttr($input, $charset); } } diff --git a/core-bundle/tests/Twig/Interop/PhpTemplateParentReferenceNodeTest.php b/core-bundle/tests/Twig/Interop/PhpTemplateParentReferenceNodeTest.php index cf11c609d46..a2ebe1e31a3 100644 --- a/core-bundle/tests/Twig/Interop/PhpTemplateParentReferenceNodeTest.php +++ b/core-bundle/tests/Twig/Interop/PhpTemplateParentReferenceNodeTest.php @@ -14,7 +14,6 @@ use Contao\CoreBundle\Tests\TestCase; use Contao\CoreBundle\Twig\Interop\PhpTemplateParentReferenceNode; -use Twig\Attribute\YieldReady; use Twig\Compiler; use Twig\Environment; @@ -27,15 +26,10 @@ public function testCompilesParentReferenceCode(): void (new PhpTemplateParentReferenceNode())->compile($compiler); $expectedSource = <<<'SOURCE' - echo sprintf('[[TL_PARENT_%s]]', \Contao\CoreBundle\Framework\ContaoFramework::getNonce()); + yield sprintf('[[TL_PARENT_%s]]', \Contao\CoreBundle\Framework\ContaoFramework::getNonce()); SOURCE; - // Forward compatibility with twig/twig >=3.9.0 - if (class_exists(YieldReady::class)) { - $expectedSource = str_replace('echo', 'yield', $expectedSource); - } - $this->assertSame($expectedSource, $compiler->getSource()); } } diff --git a/core-bundle/tests/Twig/Interop/PhpTemplateProxyNodeTest.php b/core-bundle/tests/Twig/Interop/PhpTemplateProxyNodeTest.php index 7edb743ffc3..a2784730cc7 100644 --- a/core-bundle/tests/Twig/Interop/PhpTemplateProxyNodeTest.php +++ b/core-bundle/tests/Twig/Interop/PhpTemplateProxyNodeTest.php @@ -15,7 +15,6 @@ use Contao\CoreBundle\Tests\TestCase; use Contao\CoreBundle\Twig\Extension\ContaoExtension; use Contao\CoreBundle\Twig\Interop\PhpTemplateProxyNode; -use Twig\Attribute\YieldReady; use Twig\Compiler; use Twig\Environment; @@ -28,7 +27,7 @@ public function testCompilesProxyCode(): void (new PhpTemplateProxyNode(ContaoExtension::class))->compile($compiler); $expectedSource = <<<'SOURCE' - echo $this->extensions["Contao\\CoreBundle\\Twig\\Extension\\ContaoExtension"]->renderLegacyTemplate( + yield $this->extensions["Contao\\CoreBundle\\Twig\\Extension\\ContaoExtension"]->renderLegacyTemplate( $this->getTemplateName(), array_map( function(callable $block) use ($context): string { @@ -47,11 +46,6 @@ function(callable $block) use ($context): string { SOURCE; - // Forward compatibility with twig/twig >=3.9.0 - if (class_exists(YieldReady::class)) { - $expectedSource = str_replace('echo', 'yield', $expectedSource); - } - $this->assertSame($expectedSource, $compiler->getSource()); } } diff --git a/core-bundle/tests/Twig/ResponseContext/AddNodeTest.php b/core-bundle/tests/Twig/ResponseContext/AddNodeTest.php index 04a8c6cdcff..8fbdc4cac22 100644 --- a/core-bundle/tests/Twig/ResponseContext/AddNodeTest.php +++ b/core-bundle/tests/Twig/ResponseContext/AddNodeTest.php @@ -16,7 +16,6 @@ use Contao\CoreBundle\Twig\Extension\ContaoExtension; use Contao\CoreBundle\Twig\ResponseContext\AddNode; use Contao\CoreBundle\Twig\ResponseContext\DocumentLocation; -use Twig\Attribute\YieldReady; use Twig\Compiler; use Twig\Environment; use Twig\Node\Expression\ConstantExpression; @@ -43,7 +42,7 @@ public function testCompilesAddNode(): void $__contao_document_content = ''; foreach((function () use (&$context, $macros, $blocks) { // line 42 - echo "foobar"; + yield "foobar"; yield ''; })() as $__contao_document_chunk) { $__contao_document_content .= ob_get_contents() . $__contao_document_chunk; @@ -56,11 +55,6 @@ public function testCompilesAddNode(): void SOURCE; - // Forward compatibility with twig/twig >=3.9.0 - if (class_exists(YieldReady::class)) { - $expectedSource = str_replace('echo', 'yield', $expectedSource); - } - $this->assertSame($expectedSource, $compiler->getSource()); } }