Skip to content

Commit

Permalink
Make Twig 3.10.2 the minimum requirement (see contao#7214)
Browse files Browse the repository at this point in the history
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
-------

e3f5602 Make Twig 3.10.2 the minimum requirement
4e8915e Remove a Twig 3.8 BC layer
04205f9 Clean up after the upstream merge
10b05f0 Use EscaperExtension::escapeFilterIsSafe() instead of twig_escape_fil…
025eadb Do not use the internal EscaperExtension::escapeFilterIsSafe() method…
6f7ccc2 Apply the suggestions from the pull request
6dea4c8 Drop the "testContaoUsesCorrectTwigFunctionSignatures" test
  • Loading branch information
leofeyer authored May 15, 2024
1 parent f50847e commit 95ac936
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 114 deletions.
4 changes: 1 addition & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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": {
Expand Down
6 changes: 2 additions & 4 deletions core-bundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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": {
Expand Down
50 changes: 26 additions & 24 deletions core-bundle/src/Twig/Extension/ContaoExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -69,24 +69,19 @@ 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.
$this->addContaoEscaperRule('%^@Contao(_[a-zA-Z0-9_-]*)?/%');
$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',
Expand Down Expand Up @@ -242,39 +237,46 @@ 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 = [];

foreach ($string as [$type, $chunk]) {
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 [
Expand Down
5 changes: 2 additions & 3 deletions core-bundle/src/Twig/Interop/ContaoEscaper.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
namespace Contao\CoreBundle\Twig\Interop;

use Contao\StringUtil;
use Twig\Environment;
use Twig\Error\RuntimeError;

/**
Expand All @@ -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));
Expand All @@ -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));
Expand Down
66 changes: 10 additions & 56 deletions core-bundle/tests/Twig/Extension/ContaoExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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),
Expand Down Expand Up @@ -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
*/
Expand All @@ -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([
Expand All @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions core-bundle/tests/Twig/Interop/ContaoEscaperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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());
}
}
8 changes: 1 addition & 7 deletions core-bundle/tests/Twig/Interop/PhpTemplateProxyNodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand All @@ -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());
}
}
8 changes: 1 addition & 7 deletions core-bundle/tests/Twig/ResponseContext/AddNodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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());
}
}

0 comments on commit 95ac936

Please sign in to comment.