diff --git a/CHANGELOG b/CHANGELOG index 5eda0f3ec49..10d87e482ae 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,6 @@ # 3.19.0 (2025-XX-XX) + * Fix a security issue where escaping was missing when using `??` * Deprecate `Token::getType()`, use `Token::test()` instead * Add `Token::toEnglish()` * Add `ForElseNode` diff --git a/src/Node/Expression/Binary/NullCoalesceBinary.php b/src/Node/Expression/Binary/NullCoalesceBinary.php index 1472e8e2e7a..b4fb750321c 100644 --- 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 index 37eccc0f545..3617a8eb0db 100644 --- a/tests/Fixtures/expressions/ternary_operator.test +++ b/tests/Fixtures/expressions/ternary_operator.test @@ -5,10 +5,11 @@ Twig supports the ternary operator {{ 0 ? 'YES' : 'NO' }} {{ 0 ? 'YES' : (1 ? 'YES1' : 'NO1') }} {{ 0 ? 'YES' : (0 ? 'YES1' : 'NO1') }} -{{ 1 == 1 ? 'foo
':'' }} +{{ 1 == 1 ? 'foo
' : '' }} {{ foo ~ (bar ? ('-' ~ bar) : '') }} +{{ true ? tag : 'KO' }} --DATA-- -return ['foo' => 'foo', 'bar' => 'bar'] +return ['foo' => 'foo', 'bar' => 'bar', 'tag' => '
'] --EXPECT-- YES NO @@ -16,3 +17,4 @@ YES1 NO1 foo
foo-bar +<br> diff --git a/tests/Fixtures/expressions/ternary_operator_noelse.test b/tests/Fixtures/expressions/ternary_operator_noelse.test index 8b0f7284b9b..e82f465554d 100644 --- 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' => '
'] --EXPECT-- YES +<br> diff --git a/tests/Fixtures/expressions/ternary_operator_nothen.test b/tests/Fixtures/expressions/ternary_operator_nothen.test index 53f8c0b3caf..4aebb778677 100644 --- a/tests/Fixtures/expressions/ternary_operator_nothen.test +++ b/tests/Fixtures/expressions/ternary_operator_nothen.test @@ -7,8 +7,9 @@ Twig supports the ternary operator {{ 0 ? : 'NO' }} {{ 'YES' ? : 'NO' }} {{ 0 ? : 'NO' }} +{{ tag ?: 'KO' }} --DATA-- -return [] +return ['tag' => '
'] --EXPECT-- YES NO @@ -16,3 +17,4 @@ YES NO YES NO +<br> diff --git a/tests/Fixtures/tests/null_coalesce.test b/tests/Fixtures/tests/null_coalesce.test index b73ec4634d8..685415758bb 100644 --- a/tests/Fixtures/tests/null_coalesce.test +++ b/tests/Fixtures/tests/null_coalesce.test @@ -15,8 +15,9 @@ Twig supports the ?? operator {{ 1 + (nope ?? 3) + (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' => '
'] --EXPECT-- OK OK @@ -31,3 +32,5 @@ OK 3 6 OK + +<br> diff --git a/tests/Node/Expression/Binary/NullCoalesceTest.php b/tests/Node/Expression/Binary/NullCoalesceTest.php index b2c79320ad6..294dd14613a 100644 --- 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))"]]; } }