Changed escaping rules (switched to post-escaping)

Expression in Print nodes are always escaped before being printed,
except in two conditions :
  * if the expression is an Expression_Constant node
  * if the last filter in the chain is an escaper

diff --git a/lib/Twig/NodeVisitor/Escaper.php b/lib/Twig/NodeVisitor/Escaper.php
index 8cfc08d..7190955 100644 (file)
--- a/lib/Twig/NodeVisitor/Escaper.php
+++ b/lib/Twig/NodeVisitor/Escaper.php
@@ -69,38 +69,30 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface
 
         $expression = $node instanceof Twig_Node_Print ? $node->getNode('expr') : $node;
 
+        if ($expression instanceof Twig_Node_Expression_Constant) {
+
+            return $node;
+        }
+
         if ($expression instanceof Twig_Node_Expression_Filter) {
-            // don't escape if the primary node of the filter is not a variable
-            if (!$expression->getNode('node') instanceof Twig_Node_Expression_GetAttr && !$expression->getNode('node') instanceof Twig_Node_Expression_Name) {
-                return $node;
-            }
 
-            // don't escape if there is already an "escaper" in the filter chain
+            // don't escape if the last filter in the chain is an "escaper"
             $filterMap = $env->getFilters();
-            for ($i = 0; $i < count($expression->getNode('filters')); $i += 2) {
-                $name = $expression->getNode('filters')->getNode($i)->getAttribute('value');
-                if (isset($filterMap[$name]) && $filterMap[$name]->isEscaper()) {
-                    return $node;
-                }
+            $i = count($expression->getNode('filters')) - 2;
+            $name = $expression->getNode('filters')->getNode($i)->getAttribute('value');
+            if (isset($filterMap[$name]) && $filterMap[$name]->isEscaper()) {
+                return $node;
             }
-        } elseif (!$expression instanceof Twig_Node_Expression_GetAttr && !$expression instanceof Twig_Node_Expression_Name) {
-            // don't escape if the node is not a variable
-            return $node;
         }
 
         // escape
         if ($expression instanceof Twig_Node_Expression_Filter) {
-            // escape all variables in filters arguments
-            for ($i = 0; $i < count($expression->getNode('filters')); $i += 2) {
-                foreach ($expression->getNode('filters')->getNode($i + 1) as $j => $n) {
-                    $expression->getNode('filters')->getNode($i + 1)->setNode($j, $this->escapeNode($n, $env, $type));
-                }
-            }
 
             $filter = $this->getEscaperFilter($type, $expression->getLine());
-            $expression->prependFilter($filter[0], $filter[1]);
+            $expression->appendFilter($filter[0], $filter[1]);
 
             return $node;
+
         } elseif ($node instanceof Twig_Node_Print) {
             return new Twig_Node_Print(
                 new Twig_Node_Expression_Filter($expression, new Twig_Node($this->getEscaperFilter($type, $node->getLine())), $node->getLine())

diff --git a/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test b/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test
index 3bb0dd4..ce75c45 100644 (file)
--- a/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test
+++ b/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test
@@ -1,14 +1,131 @@
 --TEST--
-"autoescape" tag applies escaping before calling filters
+"autoescape" tag applies escaping after calling filters
 --TEMPLATE--
 {% autoescape on %}
+
+(nl2br is an escaper filter)
+
+1. Don't escape escaper filter output
+( var is escaped by |nl2br, line-breaks are added, 
+  the output is not escaped )
 {{ var|nl2br }}
+
+2. Don't escape escaper filter output
+( var is escaped by |nl2br, line-breaks are added, 
+  the output is not escaped, |raw is redundant )
+{{ var|nl2br|raw }}
+
+3. Explicit escape
+( var is escaped by |nl2br, line-breaks are added,
+  the output is explicitly escaped by |escape )
 {{ var|nl2br|escape }}
+
+4. Escape non-escaper filter output
+( var is upper-cased by |upper,
+  the output is auto-escaped )
+{{ var|upper }}
+
+5. Escape if last filter is not an escaper
+( var is escaped by |nl2br, line-breaks are added,
+  the output is upper-cased by |upper,
+  the output is auto-escaped as |upper is not an escaper )
+{{ var|nl2br|upper }}
+
+6. Don't escape escaper filter output
+( var is upper cased by upper,
+  the output is escaped by |nl2br, line-breaks are added,
+  the output is not escaped as |nl2br is an escaper )
+{{ var|upper|nl2br }}
+
+7. Escape if last filter is not an escaper
+( the output of |format is "<b>" ~ var ~ "</b>",
+  the output is auto-escaped )
+{{ "<b>%s</b>"|format(var) }}
+
+8. Escape if last filter is not an escaper
+( the output of |format is "<b>" ~ var ~ "</b>",
+  |raw is redundant,
+  the output is auto-escaped )
+{{ "<b>%s</b>"|raw|format(var) }}
+
+9. Don't escape escaper filter output
+( the output of |format is "<b>" ~ var ~ "</b>",
+  the output is not escaped due to |raw filter at the end )
+{{ "<b>%s</b>"|format(var)|raw }}
+
+10. Don't escape escaper filter output
+( the output of |format is "<b>" ~ var ~ "</b>",
+  the output is not escaped due to |raw filter at the end,
+  the |raw filter on var is redundant )
+{{ "<b>%s</b>"|format(var|raw)|raw }}
+
 {% endautoescape %}
 --DATA--
 return array('var' => "<Fabien>\nTwig")
 --EXPECT--
+
+(nl2br is an escaper filter)
+
+1. Don't escape escaper filter output
+( var is escaped by |nl2br, line-breaks are added, 
+  the output is not escaped )
 &lt;Fabien&gt;<br />
 Twig
-&lt;Fabien&gt;&lt;br /&gt;
+
+2. Don't escape escaper filter output
+( var is escaped by |nl2br, line-breaks are added, 
+  the output is not escaped, |raw is redundant )
+&lt;Fabien&gt;<br />
+Twig
+
+3. Explicit escape
+( var is escaped by |nl2br, line-breaks are added,
+  the output is explicitly escaped by |escape )
+&amp;lt;Fabien&amp;gt;&lt;br /&gt;
 Twig
+
+4. Escape non-escaper filter output
+( var is upper-cased by |upper,
+  the output is auto-escaped )
+&lt;FABIEN&gt;
+TWIG
+
+5. Escape if last filter is not an escaper
+( var is escaped by |nl2br, line-breaks are added,
+  the output is upper-cased by |upper,
+  the output is auto-escaped as |upper is not an escaper )
+&amp;LT;FABIEN&amp;GT;&lt;BR /&gt;
+TWIG
+
+6. Don't escape escaper filter output
+( var is upper cased by upper,
+  the output is escaped by |nl2br, line-breaks are added,
+  the output is not escaped as |nl2br is an escaper )
+&lt;FABIEN&gt;<br />
+TWIG
+
+7. Escape if last filter is not an escaper
+( the output of |format is "<b>" ~ var ~ "</b>",
+  the output is auto-escaped )
+&lt;b&gt;&lt;Fabien&gt;
+Twig&lt;/b&gt;
+
+8. Escape if last filter is not an escaper
+( the output of |format is "<b>" ~ var ~ "</b>",
+  |raw is redundant,
+  the output is auto-escaped )
+&lt;b&gt;&lt;Fabien&gt;
+Twig&lt;/b&gt;
+
+9. Don't escape escaper filter output
+( the output of |format is "<b>" ~ var ~ "</b>",
+  the output is not escaped due to |raw filter at the end )
+<b><Fabien>
+Twig</b>
+
+10. Don't escape escaper filter output
+( the output of |format is "<b>" ~ var ~ "</b>",
+  the output is not escaped due to |raw filter at the end,
+  the |raw filter on var is redundant )
+<b><Fabien>
+Twig</b>

diff --git a/test/Twig/Tests/Fixtures/tags/autoescape/with_filters_arguments.test b/test/Twig/Tests/Fixtures/tags/autoescape/with_filters_arguments.test
index c7a5aba..9cf12a1 100644 (file)
--- a/test/Twig/Tests/Fixtures/tags/autoescape/with_filters_arguments.test
+++ b/test/Twig/Tests/Fixtures/tags/autoescape/with_filters_arguments.test
@@ -1,11 +1,12 @@
 --TEST--
-"autoescape" tag applies escaping on filter arguments, but not on literals
+"autoescape" tag do not applies escaping on filter arguments
 --TEMPLATE--
 {% autoescape on %}
 {{ var|nl2br("<br />") }}
 {{ var|nl2br("<br />"|escape) }}
 {{ var|nl2br(sep) }}
 {{ var|nl2br(sep|raw) }}
+{{ var|nl2br(sep|escape) }}
 {% endautoescape %}
 --DATA--
 return array('var' => "<Fabien>\nTwig", 'sep' => '<br />')
@@ -14,7 +15,9 @@ return array('var' => "<Fabien>\nTwig", 'sep' => '<br />')
 Twig
 &lt;Fabien&gt;&lt;br /&gt;
 Twig
-&lt;Fabien&gt;&lt;br /&gt;
+&lt;Fabien&gt;<br />
 Twig
 &lt;Fabien&gt;<br />
 Twig
+&lt;Fabien&gt;&lt;br /&gt;
+Twig

diff --git a/test/Twig/Tests/integrationTest.php b/test/Twig/Tests/integrationTest.php
index 14ac53a..ca6ae7d 100644 (file)
--- a/test/Twig/Tests/integrationTest.php
+++ b/test/Twig/Tests/integrationTest.php
@@ -103,11 +103,12 @@ class TestExtension extends Twig_Extension
 {
     public function getFilters()
     {
-        return array('nl2br' => new Twig_Filter_Method($this, 'nl2br'));
+        return array('nl2br' => new Twig_Filter_Method($this, 'nl2br', array('needs_environment' => true, 'is_escaper' => true)));
     }
 
-    public function nl2br($value, $sep = '<br />')
+    public function nl2br($env, $value, $sep = '<br />')
     {
+        $value = htmlspecialchars($value, ENT_QUOTES, $env->getCharset());
         return str_replace("\n", $sep."\n", $value);
     }