feature #2934 Make escaping on ternary expressions more fine-grained (fabpot)

This PR was merged into the 1.x branch.

Discussion
----------

Make escaping on ternary expressions more fine-grained

closes #2146
closes #449
closes #539
closes #1174
closes #1320
closes #1739
closes #1333

Commits
-------

dfa94113 made escaping on ternary expressions more fine-grained

CHANGELOG

@@ -1,5 +1,6 @@
 * 1.39.0 (2019-XX-XX)
 
+ * made escaping on ternary expressions (?: and ??) more fine-grained
  * added the possibility to give a nice name to string templates (template_from_string function)
  * fixed the "with" behavior to always include the globals (for consistency with the "include" and "embed" tags)
  * fixed "include" with "ignore missing" when an error loading occurs in the included template

doc/filters/raw.rst

@@ -13,6 +13,8 @@ if ``raw`` is the last filter applied to it:
 
 .. note::
 
+    **This note only applies to Twig before versions 1.39 and 2.8**.
+
     Be careful when using the ``raw`` filter inside expressions:
 
     .. code-block:: jinja

src/Node/Expression/InlinePrint.php

@@ -0,0 +1,35 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Twig\Node\Expression;
+
+use Twig\Compiler;
+use Twig\Node\Node;
+
+/**
+ * @internal
+ */
+final class InlinePrint extends AbstractExpression
+{
+    public function __construct(Node $node, $lineno)
+    {
+        parent::__construct(['node' => $node], [], $lineno);
+    }
+
+    public function compile(Compiler $compiler)
+    {
+        $compiler
+            ->raw('print (')
+            ->subcompile($this->getNode('node'))
+            ->raw(')')
+        ;
+    }
+}

src/NodeVisitor/EscaperNodeVisitor.php

@@ -15,8 +15,11 @@ use Twig\Environment;
 use Twig\Node\AutoEscapeNode;
 use Twig\Node\BlockNode;
 use Twig\Node\BlockReferenceNode;
+use Twig\Node\DoNode;
+use Twig\Node\Expression\ConditionalExpression;
 use Twig\Node\Expression\ConstantExpression;
 use Twig\Node\Expression\FilterExpression;
+use Twig\Node\Expression\InlinePrint;
 use Twig\Node\ImportNode;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
@@ -69,8 +72,13 @@ class EscaperNodeVisitor extends AbstractNodeVisitor
             $this->blocks = [];
         } elseif ($node instanceof FilterExpression) {
             return $this->preEscapeFilterNode($node, $env);
-        } elseif ($node instanceof PrintNode) {
-            return $this->escapePrintNode($node, $env, $this->needEscaping($env));
+        } elseif ($node instanceof PrintNode && false !== $type = $this->needEscaping($env)) {
+            $expression = $node->getNode('expr');
+            if ($expression instanceof ConditionalExpression && $this->shouldUnwrapConditional($expression, $env, $type)) {
+                return new DoNode($this->unwrapConditional($expression, $env, $type), $expression->getTemplateLine());
+            }
+
+            return $this->escapePrintNode($node, $env, $type);
         }
 
         if ($node instanceof AutoEscapeNode || $node instanceof BlockNode) {
@@ -82,6 +90,44 @@ class EscaperNodeVisitor extends AbstractNodeVisitor
         return $node;
     }
 
+    private function shouldUnwrapConditional(ConditionalExpression $expression, Environment $env, $type)
+    {
+        $expr2Safe = $this->isSafeFor($type, $expression->getNode('expr2'), $env);
+        $expr3Safe = $this->isSafeFor($type, $expression->getNode('expr3'), $env);
+
+        return $expr2Safe !== $expr3Safe;
+    }
+
+    private function unwrapConditional(ConditionalExpression $expression, Environment $env, $type)
+    {
+        // convert "echo a ? b : c" to "a ? echo b : echo c" recursively
+        $expr2 = $expression->getNode('expr2');
+        if ($expr2 instanceof ConditionalExpression && $this->shouldUnwrapConditional($expr2, $env, $type)) {
+            $expr2 = $this->unwrapConditional($expr2, $env, $type);
+        } else {
+            $expr2 = $this->escapeInlinePrintNode(new InlinePrint($expr2, $expr2->getTemplateLine()), $env, $type);
+        }
+        $expr3 = $expression->getNode('expr3');
+        if ($expr3 instanceof ConditionalExpression && $this->shouldUnwrapConditional($expr2, $env, $type)) {
+            $expr3 = $this->unwrapConditional($expr3, $env, $type);
+        } else {
+            $expr3 = $this->escapeInlinePrintNode(new InlinePrint($expr3, $expr3->getTemplateLine()), $env, $type);
+        }
+
+        return new ConditionalExpression($expression->getNode('expr1'), $expr2, $expr3, $expression->getTemplateLine());
+    }
+
+    private function escapeInlinePrintNode(InlinePrint $node, Environment $env, $type)
+    {
+        $expression = $node->getNode('node');
+
+        if ($this->isSafeFor($type, $expression, $env)) {
+            return $node;
+        }
+
+        return new InlinePrint($this->getEscaperFilter($type, $expression), $node->getTemplateLine());
+    }
+
     protected function escapePrintNode(PrintNode $node, Environment $env, $type)
     {
         if (false === $type) {
@@ -96,10 +142,7 @@ class EscaperNodeVisitor extends AbstractNodeVisitor
 
         $class = \get_class($node);
 
-        return new $class(
-            $this->getEscaperFilter($type, $expression),
-            $node->getTemplateLine()
-        );
+        return new $class($this->getEscaperFilter($type, $expression), $node->getTemplateLine());
     }
 
     protected function preEscapeFilterNode(FilterExpression $filter, Environment $env)

test/Twig/Tests/Fixtures/tags/autoescape/literal.test

@@ -11,19 +11,36 @@
 
 3. Conditional expression with a variable
 {{ true ? "<br />" : someVar }}
+{{ false ? "<br />" : someVar }}
+{{ true ? someVar : "<br />" }}
+{{ false ? someVar : "<br />" }}
 
 4. Nested conditionals with only literals
 {{ true ? (true ? "<br />" : "<br>") : "\n" }}
 
 5. Nested conditionals with a variable
 {{ true ? (true ? "<br />" : someVar) : "\n" }}
+{{ true ? (false ? "<br />" : someVar) : "\n" }}
+{{ true ? (true ? someVar : "<br />") : "\n" }}
+{{ true ? (false ? someVar : "<br />") : "\n" }}
 
 6. Nested conditionals with a variable marked safe
 {{ true ? (true ? "<br />" : someVar|raw) : "\n" }}
+{{ true ? (false ? "<br />" : someVar|raw) : "\n" }}
+{{ true ? (true ? someVar|raw : "<br />") : "\n" }}
+{{ true ? (false ? someVar|raw : "<br />") : "\n" }}
+
+7. Without then clause
+{{ "<br />" ?: someVar }}
+{{ someFalseVar ?: "<br />" }}
+
+8. NullCoalesce
+{{ aaaa ?? "<br />" }}
+{{ "<br />" ?? someVar }}
 
 {% endautoescape %}
 --DATA--
-return []
+return ['someVar' => '<br />', 'someFalseVar' => false]
 --EXPECT--
 
 1. Simple literal
@@ -33,13 +50,30 @@ return []
 <br />
 
 3. Conditional expression with a variable
+<br />
 &lt;br /&gt;
+&lt;br /&gt;
+<br />
 
 4. Nested conditionals with only literals
 <br />
 
 5. Nested conditionals with a variable
+<br />
 &lt;br /&gt;
+&lt;br /&gt;
+<br />
 
 6. Nested conditionals with a variable marked safe
 <br />
+<br />
+<br />
+<br />
+
+7. Without then clause
+<br />
+<br />
+
+8. NullCoalesce
+<br />
+<br />