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))"]];
}
}