Merge branch '3.x' into 4.x

* 3.x: [SECURITY] Fix a security issue where escaping was missing when using ?? Fix typo for html_cva code example
twigphp · Jan 29, 2025 · dd69b12 · dd69b12
2 parents d564048 + 38576b1
commit dd69b12
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 45 deletions.
diff --git a/doc/functions/html_cva.rst b/doc/functions/html_cva.rst
@@ -40,7 +40,7 @@ Then use the ``color`` and ``size`` variants to select the needed classes:
 
     {# index.html.twig #}
     {{ include('alert.html.twig', {'color': 'blue', 'size': 'md'}) }}
-    // class="alert bg-red text-md"
+    // class="alert bg-blue text-md"
 
     {{ include('alert.html.twig', {'color': 'green', 'size': 'sm'}) }}
     // class="alert bg-green text-sm"

diff --git a/src/Node/Expression/Binary/NullCoalesceBinary.php b/src/Node/Expression/Binary/NullCoalesceBinary.php
@@ -19,7 +19,6 @@
 use Twig\Node\Expression\Test\DefinedTest;
 use Twig\Node\Expression\Test\NullTest;
 use Twig\Node\Expression\Unary\NotUnary;
-use Twig\Node\Expression\Variable\ContextVariable;
 use Twig\TwigTest;
 
 final class NullCoalesceBinary extends AbstractBinary implements OperatorEscapeInterface
@@ -28,47 +27,31 @@ public function __construct(AbstractExpression $left, AbstractExpression $right,
     {
         parent::__construct($left, $right, $lineno);
 
-        if (!$left instanceof ContextVariable) {
-            $test = new DefinedTest(clone $left, new TwigTest('defined'), new EmptyNode(), $left->getTemplateLine());
-            // for "block()", we don't need the null test as the return value is always a string
-            if (!$left instanceof BlockReferenceExpression) {
-                $test = new AndBinary(
-                    $test,
-                    new NotUnary(new NullTest($left, new TwigTest('null'), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()),
-                    $left->getTemplateLine(),
-                );
-            }
-
-            $this->setNode('test', $test);
-        } else {
-            $left->setAttribute('always_defined', true);
+        $test = new DefinedTest(clone $left, new TwigTest('defined'), new EmptyNode(), $left->getTemplateLine());
+        // for "block()", we don't need the null test as the return value is always a string
+        if (!$left instanceof BlockReferenceExpression) {
+            $test = new AndBinary(
+                $test,
+                new NotUnary(new NullTest($left, new TwigTest('null'), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()),
+                $left->getTemplateLine(),
+            );
         }
+
+        $left->setAttribute('always_defined', true);
+        $this->setNode('test', $test);
     }
 
     public function compile(Compiler $compiler): void
     {
-        /*
-         * This optimizes only one case. PHP 7 also supports more complex expressions
-         * that can return null. So, for instance, if log is defined, log("foo") ?? "..." works,
-         * but log($a["foo"]) ?? "..." does not if $a["foo"] is not defined. More advanced
-         * cases might be implemented as an optimizer node visitor, but has not been done
-         * as benefits are probably not worth the added complexity.
-         */
-        if ($this->hasNode('test')) {
-            $compiler
-                ->raw('((')
-                ->subcompile($this->getNode('test'))
-                ->raw(') ? (')
-                ->subcompile($this->getNode('left'))
-                ->raw(') : (')
-                ->subcompile($this->getNode('right'))
-                ->raw('))')
-            ;
-
-            return;
-        }
-
-        parent::compile($compiler);
+        $compiler
+            ->raw('((')
+            ->subcompile($this->getNode('test'))
+            ->raw(') ? (')
+            ->subcompile($this->getNode('left'))
+            ->raw(') : (')
+            ->subcompile($this->getNode('right'))
+            ->raw('))')
+        ;
     }
 
     public function operator(Compiler $compiler): Compiler
@@ -78,6 +61,6 @@ public function operator(Compiler $compiler): Compiler
 
     public function getOperandNamesToEscape(): array
     {
-        return $this->hasNode('test') ? ['left', 'right'] : ['right'];
+        return ['left', 'right'];
     }
 }
diff --git a/tests/Fixtures/expressions/ternary_operator.test b/tests/Fixtures/expressions/ternary_operator.test
@@ -5,14 +5,16 @@ Twig supports the ternary operator
 {{ 0 ? 'YES' : 'NO' }}
 {{ 0 ? 'YES' : (1 ? 'YES1' : 'NO1') }}
 {{ 0 ? 'YES' : (0 ? 'YES1' : 'NO1') }}
-{{ 1 == 1 ? 'foo<br />':'' }}
+{{ 1 == 1 ? 'foo<br />' : '' }}
 {{ foo ~ (bar ? ('-' ~ bar) : '') }}
+{{ true ? tag : 'KO' }}
 --DATA--
-return ['foo' => 'foo', 'bar' => 'bar']
+return ['foo' => 'foo', 'bar' => 'bar', 'tag' => '<br>']
 --EXPECT--
 YES
 NO
 YES1
 NO1
 foo<br />
 foo-bar
+&lt;br&gt;
diff --git a/tests/Fixtures/expressions/ternary_operator_noelse.test b/tests/Fixtures/expressions/ternary_operator_noelse.test
@@ -3,8 +3,10 @@ Twig supports the ternary operator
 --TEMPLATE--
 {{ 1 ? 'YES' }}
 {{ 0 ? 'YES' }}
+{{ tag ? tag }}
 --DATA--
-return []
+return ['tag' => '<br>']
 --EXPECT--
 YES
 
+&lt;br&gt;
diff --git a/tests/Fixtures/expressions/ternary_operator_nothen.test b/tests/Fixtures/expressions/ternary_operator_nothen.test
@@ -7,12 +7,14 @@ Twig supports the ternary operator
 {{ 0 ? : 'NO' }}
 {{ 'YES' ?  : 'NO' }}
 {{ 0 ?     : 'NO' }}
+{{ tag ?: 'KO' }}
 --DATA--
-return []
+return ['tag' => '<br>']
 --EXPECT--
 YES
 NO
 YES
 NO
 YES
 NO
+&lt;br&gt;
diff --git a/tests/Fixtures/tests/null_coalesce.test b/tests/Fixtures/tests/null_coalesce.test
@@ -16,8 +16,9 @@ Twig supports the ?? operator
 {{ nope ?? nada ?? 2 }}
 {{ obj.null() ?? 'OK' }}
 {{ obj.empty() ?? 'KO' }}
+{{ tag ?? 'KO' }}
 --DATA--
-return ['bar' => 'OK', 'foo' => ['bar' => 'OK'], 'obj' => new Twig\Tests\TwigTestFoo()]
+return ['bar' => 'OK', 'foo' => ['bar' => 'OK'], 'obj' => new Twig\Tests\TwigTestFoo(), 'tag' => '<br>']
 --EXPECT--
 OK
 OK
@@ -33,3 +34,5 @@ OK
 6
 2
 OK
+
+&lt;br&gt;
diff --git a/tests/Node/Expression/Binary/NullCoalesceTest.php b/tests/Node/Expression/Binary/NullCoalesceTest.php
@@ -24,6 +24,6 @@ public static function provideTests(): iterable
         $right = new ConstantExpression(2, 1);
         $node = new NullCoalesceBinary($left, $right, 1);
 
-        return [[$node, "(// line 1\n\$context[\"foo\"] ?? 2)"]];
+        return [[$node, "(((// line 1\narray_key_exists(\"foo\", \$context) &&  !(null === \$context[\"foo\"]))) ? (\$context[\"foo\"]) : (2))"]];
     }
 }