Merge branch '3.x' into 4.x

* 3.x:
  Handle single-node child template bodies in cleanup
  Keep captured block definitions supported
  Clarify captured block deprecation wording
  Simplify correctness visitor checks
  Move extends validation into correctness visitor
  Fix correctness visitor regressions
  Fix test assertions that did not verify the intended behavior
  Address review: fix block-nesting checks in CorrectnessNodeVisitor
  Move the extends-in-block and extends-in-macro errors into the CorrectnessNodeVisitor
  Introduce a CorrectnessNodeVisitor to validate that templates are semantically correct
  Mark Markup as final
  Allow calling a macro with a dynamic name via the dot operator
  Add an allow-list for tests to the sandbox security policy
  Fix markdown_to_html mangling content that starts with a blank line

# Conflicts:
#	CHANGELOG
#	doc/deprecated.rst
#	doc/sandbox.rst
#	src/ExpressionParser/Infix/DotExpressionParser.php
#	src/Extension/CoreExtension.php
#	src/Markup.php
#	src/Node/CheckSecurityNode.php
#	src/Node/Expression/Filter/DefaultFilter.php
#	src/Node/Expression/NullCoalesceExpression.php
#	src/Node/IfNode.php
#	src/NodeVisitor/SandboxNodeVisitor.php
#	src/Parser.php
#	src/Sandbox/SecurityPolicy.php
#	tests/Extension/SandboxTest.php
#	tests/ParserTest.php

Author: fabpot

.php-cs-fixer.dist.php

@@ -18,7 +18,7 @@
         'phpdoc_to_comment' => ['ignored_tags' => ['var']],
         'ordered_imports' => true,
         'phpdoc_types_order' => ['null_adjustment' => 'always_last', 'sort_algorithm' => 'none'],
-        'no_superfluous_phpdoc_tags' => ['allow_mixed' => true],
+        'no_superfluous_phpdoc_tags' => ['allow_mixed' => true, 'allow_unused_params' => true],
     ])
     ->setRuleCustomisationPolicy(new class implements PhpCsFixer\Config\RuleCustomisationPolicyInterface {
         public function getPolicyVersionForCache(): string

doc/sandbox.rst

@@ -29,13 +29,24 @@ properties and methods on objects::
         'Article' => ['title', 'body'],
     ];
     $functions = ['range'];
-    $policy = new \Twig\Sandbox\SecurityPolicy($tags, $filters, $methods, $properties, $functions);
+    $tests = ['my_test'];
+    $policy = new \Twig\Sandbox\SecurityPolicy($tags, $filters, $methods, $properties, $functions, $tests);
 
 With the above configuration, the security policy will only allow usage of the
-``if`` tag, and the ``upper`` filter. Moreover, the templates will only be able
-to call the ``getTitle()`` and ``getBody()`` methods on ``Article`` objects,
-and the ``title`` and ``body`` public properties. Everything else won't be
-allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception.
+``if`` tag, the ``upper`` filter, and the ``my_test`` test (on top of the
+built-in tests that are always allowed, see below). Moreover, the templates
+will only be able to call the ``getTitle()`` and ``getBody()`` methods on
+``Article`` objects, and the ``title`` and ``body`` public properties.
+Everything else won't be allowed and will generate a
+``\Twig\Sandbox\SecurityError`` exception.
+
+.. note::
+
+    The ``allowedTests`` argument is available since Twig 3.28 (in earlier
+    versions all tests were always allowed). Most built-in tests (``empty``,
+    ``defined``, ``even``, ``same as``, ``iterable``, etc.) are always allowed
+    and do not need to be listed. Only custom tests and the built-in
+    ``constant`` test must be allow-listed like filters and functions.
 
 .. note::
 
@@ -46,13 +57,13 @@ allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception.
     Note that native array-like classes (like ``ArrayObject``) are always
     allowed, you don't need to configure them.
 
-Marking Filters, Functions, and Tags as Always Allowed
-------------------------------------------------------
+Marking Filters, Functions, Tests, and Tags as Always Allowed
+-------------------------------------------------------------
 
-Some filters, functions, and tags are inherently safe and should always be
-usable in sandboxed templates without forcing every policy to allow-list them.
-Mark such callables by setting the ``always_allowed_in_sandbox`` option to
-``true``::
+Some filters, functions, tests, and tags are inherently safe and should always
+be usable in sandboxed templates without forcing every policy to allow-list
+them. Mark such callables by setting the ``always_allowed_in_sandbox`` option
+to ``true``::
 
     $twig->addFilter(new \Twig\TwigFilter('upper', 'strtoupper', [
         'always_allowed_in_sandbox' => true,
@@ -62,6 +73,10 @@ Mark such callables by setting the ``always_allowed_in_sandbox`` option to
         'always_allowed_in_sandbox' => true,
     ]));
 
+    $twig->addTest(new \Twig\TwigTest('even', null, [
+        'always_allowed_in_sandbox' => true,
+    ]));
+
 For tags, override ``isAlwaysAllowedInSandbox()`` on your token parser to
 return ``true``::
 
@@ -75,8 +90,8 @@ return ``true``::
         // ...
     }
 
-Marked filters, functions, and tags are skipped by the sandbox security check
-entirely, so they incur no runtime overhead, and they do not need to be
+Marked filters, functions, tests, and tags are skipped by the sandbox security
+check entirely, so they incur no runtime overhead, and they do not need to be
 listed in the ``SecurityPolicy`` allow-lists.
 
 The sandbox assumes that attackers control template source, not the Twig
@@ -93,9 +108,9 @@ Only mark a callable or tag as always allowed when **all** the following
 conditions hold:
 
 * **No new capability.** The item must not expose anything beyond what the
-  sandbox already accepts. Pure value predicates (``is even``), pure value
-  transformations (``upper``, ``trim``, ``abs``), and pure control flow
-  (``if``, ``for``, ``set``) qualify.
+  sandbox already accepts. Pure value predicates (``is even``, ``is empty``),
+  pure value transformations (``upper``, ``trim``, ``abs``), and pure control
+  flow (``if``, ``for``, ``set``) qualify.
 * **No PHP runtime access.** The item must not read arbitrary PHP constants,
   call arbitrary classes or functions, instantiate objects from
   user-controlled names, or otherwise reach into the PHP runtime. This rules
@@ -154,6 +169,15 @@ When upgrading to 4.0, you can drop these names from your ``SecurityPolicy``
 allow-lists. Leaving them in is harmless: listing a name that is always
 allowed has no effect.
 
+The corresponding built-in tests (``defined``, ``divisible by``, ``empty``,
+``even``, ``iterable``, ``mapping``, ``none``, ``null``, ``odd``, ``same as``,
+``sequence``, ``true``) are **already** flagged as always allowed since Twig
+3.28, so they never need to be allow-listed. This is safe because tests were
+never enforced by the sandbox before 3.28: flagging them keeps existing
+templates working unchanged. The ``constant`` test is the exception: it reaches
+into the PHP runtime, so it is not always allowed and must be allow-listed (it
+is still implicitly allowed in 3.x with a deprecation, and rejected in 4.0).
+
 Enabling the Sandbox
 --------------------
 

doc/tags/macro.rst

@@ -64,6 +64,19 @@ The macros can then be called at will in the *current* template:
     {# You can also use named arguments #}
     <p>{{ forms.input(name: 'password', type: 'password') }}</p>
 
+The macro name can also be dynamic by wrapping an expression with parenthesis
+after the :ref:`dot operator <dot_operator>`:
+
+.. code-block:: html+twig
+
+    {% set field = 'input' %}
+    <p>{{ forms.(field)('username') }}</p>
+    <p>{{ forms.('text' ~ 'area')('comment') }}</p>
+
+.. versionadded:: 3.28
+
+    Support for calling a macro with a dynamic name was added in Twig 3.28.
+
 Alternatively you can import names from the template into the current namespace
 via the ``from`` tag:
 

extra/markdown-extra/MarkdownRuntime.php

@@ -22,11 +22,55 @@ public function __construct(MarkdownInterface $converter)
 
     public function convert(string $body): string
     {
-        // remove indentation
-        if ($white = substr($body, 0, strspn($body, " \t\r\n\0\x0B"))) {
-            $body = preg_replace("{^$white}m", '', $body);
+        return $this->converter->convert(self::stripCommonIndentation($body));
+    }
+
+    /**
+     * Removes the indentation shared by all non-blank lines.
+     *
+     * This lets authors indent a `{% apply markdown_to_html %}` block to match
+     * the surrounding template without that indentation leaking into Markdown
+     * (where leading whitespace is significant, e.g. code blocks).
+     */
+    private static function stripCommonIndentation(string $body): string
+    {
+        $lines = explode("\n", $body);
+
+        $indent = null;
+        foreach ($lines as $line) {
+            if ('' === trim($line)) {
+                continue;
+            }
+
+            $lineIndent = substr($line, 0, strspn($line, " \t"));
+            if (null === $indent) {
+                $indent = $lineIndent;
+                continue;
+            }
+
+            $max = min(\strlen($indent), \strlen($lineIndent));
+            $common = 0;
+            while ($common < $max && $indent[$common] === $lineIndent[$common]) {
+                ++$common;
+            }
+            $indent = substr($indent, 0, $common);
+
+            if ('' === $indent) {
+                return $body;
+            }
+        }
+
+        if (null === $indent || '' === $indent) {
+            return $body;
+        }
+
+        $length = \strlen($indent);
+        foreach ($lines as $i => $line) {
+            if (str_starts_with($line, $indent)) {
+                $lines[$i] = substr($line, $length);
+            }
         }
 
-        return $this->converter->convert($body);
+        return implode("\n", $lines);
     }
 }

extra/markdown-extra/Tests/FunctionalTest.php

@@ -18,6 +18,7 @@
 use Twig\Extra\Markdown\ErusevMarkdown;
 use Twig\Extra\Markdown\LeagueMarkdown;
 use Twig\Extra\Markdown\MarkdownExtension;
+use Twig\Extra\Markdown\MarkdownInterface;
 use Twig\Extra\Markdown\MarkdownRuntime;
 use Twig\Extra\Markdown\MichelfMarkdown;
 use Twig\Loader\ArrayLoader;
@@ -76,6 +77,42 @@ public static function getMarkdownTests()
 {% endapply %}
 EOF, "<h1>Hello</h1>\n+<p>Great!</p>"],
             ["{{ include('html')|markdown_to_html }}", "<h1>Hello</h1>\n+<p>Great!</p>"],
+            [<<<EOF
+{% apply markdown_to_html %}
+
+Paragraph 1
+
+Paragraph 2
+{% endapply %}
+EOF, "<p>Paragraph 1</p>\n+<p>Paragraph 2</p>"],
+        ];
+    }
+
+    /**
+     * @dataProvider getIndentationTests
+     */
+    public function testStripsCommonIndentation(string $body, string $expected)
+    {
+        $runtime = new MarkdownRuntime(new class implements MarkdownInterface {
+            public function convert(string $body): string
+            {
+                return $body;
+            }
+        });
+
+        $this->assertSame($expected, $runtime->convert($body));
+    }
+
+    public static function getIndentationTests()
+    {
+        return [
+            'leading blank line keeps blank lines' => ["\nParagraph 1\n\nParagraph 2", "\nParagraph 1\n\nParagraph 2"],
+            'common indentation is removed' => ["\n    Hello\n    =====\n\n    Great!\n", "\nHello\n=====\n\nGreat!\n"],
+            'minimal common indentation is removed' => ["    a\n      b\n", "a\n  b\n"],
+            'indented code block before non-indented prose is preserved' => ["    Code\n\nParagraph\n", "    Code\n\nParagraph\n"],
+            'tab indentation is removed' => ["\tHello\n\tGreat!\n", "Hello\nGreat!\n"],
+            'mixed tabs and spaces are left untouched' => ["\ta\n    b\n", "\ta\n    b\n"],
+            'blank lines are ignored when computing indentation' => ["    a\n\n    b\n", "a\n\nb\n"],
         ];
     }
 

phpstan-baseline.neon

@@ -1,5 +1,12 @@
 parameters:
 	ignoreErrors:
+		- # The "$tests" parameter is documented now and will be part of the signature in 4.0
+			message: '#^PHPDoc tag @param references unknown parameter\: \$tests$#'
+			identifier: parameter.notFound
+			count: 1
+			path: src/Sandbox/SecurityPolicyInterface.php
+
+
 		- # 2 parameters will be required
 			message: '#^Method Twig\\Node\\IncludeNode\:\:addGetTemplate\(\) invoked with 2 parameters, 1 required\.$#'
 			identifier: arguments.count

src/ExpressionParser/Infix/DotExpressionParser.php

@@ -65,19 +65,25 @@ public function parse(Parser $parser, AbstractExpression $expr, Token $token): A
             $arguments = $this->parseCallableArguments($parser, $token->getLine());
         }
 
+        $isMacroTarget = $expr instanceof ContextVariable
+            && (
+                null !== $parser->getImportedSymbol('template', $expr->getAttribute('name'))
+                || '_self' === $expr->getAttribute('name')
+            );
+
         if (
-            $expr instanceof ContextVariable
+            $isMacroTarget
             && $attribute instanceof ConstantExpression
             && \is_string($name = $attribute->getAttribute('value'))
             && preg_match('#^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$#D', $name)
-            && (
-                null !== $parser->getImportedSymbol('template', $expr->getAttribute('name'))
-                || '_self' === $expr->getAttribute('name')
-            )
         ) {
             return new MacroReferenceExpression(new TemplateVariable($expr->getAttribute('name'), $expr->getTemplateLine()), 'macro_'.$name, $arguments, $expr->getTemplateLine());
         }
 
+        if ($isMacroTarget && !$attribute instanceof ConstantExpression) {
+            return new MacroReferenceExpression(new TemplateVariable($expr->getAttribute('name'), $expr->getTemplateLine()), $attribute, $arguments, $expr->getTemplateLine());
+        }
+
         return new GetAttrExpression($expr, $attribute, $arguments, $type, $lineno, $nullSafe);
     }
 

src/Extension/CoreExtension.php

@@ -87,6 +87,7 @@
 use Twig\Node\Expression\Unary\SpreadUnary;
 use Twig\Node\Expression\Variable\ContextVariable;
 use Twig\Node\Node;
+use Twig\NodeVisitor\CorrectnessNodeVisitor;
 use Twig\Parser;
 use Twig\Sandbox\SecurityNotAllowedMethodError;
 use Twig\Sandbox\SecurityNotAllowedPropertyError;
@@ -312,25 +313,27 @@ public function getFunctions(): array
     public function getTests(): array
     {
         return [
-            new TwigTest('even', null, ['node_class' => EvenTest::class]),
-            new TwigTest('odd', null, ['node_class' => OddTest::class]),
-            new TwigTest('defined', null, ['node_class' => DefinedTest::class]),
-            new TwigTest('same as', null, ['node_class' => SameasTest::class, 'one_mandatory_argument' => true]),
-            new TwigTest('none', null, ['node_class' => NullTest::class]),
-            new TwigTest('null', null, ['node_class' => NullTest::class]),
-            new TwigTest('divisible by', null, ['node_class' => DivisiblebyTest::class, 'one_mandatory_argument' => true]),
+            new TwigTest('even', null, ['node_class' => EvenTest::class, 'always_allowed_in_sandbox' => true]),
+            new TwigTest('odd', null, ['node_class' => OddTest::class, 'always_allowed_in_sandbox' => true]),
+            new TwigTest('defined', null, ['node_class' => DefinedTest::class, 'always_allowed_in_sandbox' => true]),
+            new TwigTest('same as', null, ['node_class' => SameasTest::class, 'one_mandatory_argument' => true, 'always_allowed_in_sandbox' => true]),
+            new TwigTest('none', null, ['node_class' => NullTest::class, 'always_allowed_in_sandbox' => true]),
+            new TwigTest('null', null, ['node_class' => NullTest::class, 'always_allowed_in_sandbox' => true]),
+            new TwigTest('divisible by', null, ['node_class' => DivisiblebyTest::class, 'one_mandatory_argument' => true, 'always_allowed_in_sandbox' => true]),
             new TwigTest('constant', null, ['node_class' => ConstantTest::class]),
-            new TwigTest('empty', self::testEmpty(...)),
-            new TwigTest('iterable', 'is_iterable'),
-            new TwigTest('sequence', self::testSequence(...)),
-            new TwigTest('mapping', self::testMapping(...)),
-            new TwigTest('true', null, ['node_class' => TrueTest::class]),
+            new TwigTest('empty', self::testEmpty(...), ['always_allowed_in_sandbox' => true]),
+            new TwigTest('iterable', 'is_iterable', ['always_allowed_in_sandbox' => true]),
+            new TwigTest('sequence', self::testSequence(...), ['always_allowed_in_sandbox' => true]),
+            new TwigTest('mapping', self::testMapping(...), ['always_allowed_in_sandbox' => true]),
+            new TwigTest('true', null, ['node_class' => TrueTest::class, 'always_allowed_in_sandbox' => true]),
         ];
     }
 
     public function getNodeVisitors(): array
     {
-        return [];
+        return [
+            new CorrectnessNodeVisitor(),
+        ];
     }
 
     public function getExpressionParsers(): array

src/Extension/SandboxExtension.php

@@ -65,11 +65,29 @@ public function getSecurityPolicy(): SecurityPolicyInterface
         return $this->policy;
     }
 
-    public function checkSecurity($tags, $filters, $functions, ?Source $source = null): void
+    public function checkSecurity($tags, $filters, $functions, $tests = [], $source = null): void
     {
-        if ($this->isSandboxed($source)) {
-            $this->policy->checkSecurity($tags, $filters, $functions);
+        // BC: previous signature was checkSecurity($tags, $filters, $functions, ?Source $source = null);
+        // detect a legacy call where the 4th positional argument was the Source.
+        if ($tests instanceof Source || (null === $tests && \func_num_args() < 5)) {
+            trigger_deprecation('twig/twig', '3.28', 'Passing a "Twig\Source" as the 4th argument of "%s()" is deprecated; pass an array of tests instead.', __METHOD__);
+            $source = $tests;
+            $tests = [];
         }
+
+        if (!$this->isSandboxed($source)) {
+            return;
+        }
+
+        if ((new \ReflectionMethod($this->policy, 'checkSecurity'))->getNumberOfParameters() >= 4) {
+            $this->policy->checkSecurity($tags, $filters, $functions, $tests);
+
+            return;
+        }
+
+        trigger_deprecation('twig/twig', '3.28', 'The "%s::checkSecurity()" method will take a 4th "array $tests" argument in 4.0; not declaring it is deprecated.', $this->policy::class);
+
+        $this->policy->checkSecurity($tags, $filters, $functions);
     }
 
     public function checkMethodAllowed($obj, $method, int $lineno = -1, ?Source $source = null): void