changed the automatic-escaping rules to be more sensible (the documentation lists...

git-svn-id: http://svn.twig-project.org/trunk@182 93ef8e89-cb99-4229-a87c-7fa0fa45744b

diff --git a/CHANGELOG b/CHANGELOG
index 5334216..18ce387 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -12,6 +12,7 @@ environment constant by the "needs_environment" option:
   'even'   => new Twig_Filter_Function('twig_is_even_filter'),
   'escape' => new Twig_Filter_Function('twig_escape_filter', array('needs_environment' => true)),
 
+ * changed the automatic-escaping rules to be more sensible (the documentation lists all the rules)
  * improved the filter system to allow object methods to be used as filters
  * changed the Array and String loaders to actually make use of the cache mechanism
  * included the default filter function definitions in the extension class files directly (Core, Escaper)

diff --git a/doc/02-Twig-for-Template-Designers.markdown b/doc/02-Twig-for-Template-Designers.markdown
index d169fd6..16047ed 100644 (file)
--- a/doc/02-Twig-for-Template-Designers.markdown
+++ b/doc/02-Twig-for-Template-Designers.markdown
@@ -365,12 +365,19 @@ When automatic escaping is enabled everything is escaped by default except for
 values explicitly marked as safe. Those can be marked in the template by using
 the `|safe` filter.
 
-Functions returning template data (like macros and `parent`) return safe markup always.
+Functions returning template data (like macros and `parent`) always return
+safe markup.
 
 >**NOTE**
 >Twig is smart enough to not escape an already escaped value by the `escape`
 >filter.
 
+-
+
+>**NOTE**
+>The chapter for the developers give more information about when and how
+>automatic escaping is applied.
+
 List of Control Structures
 --------------------------
 

diff --git a/doc/03-Twig-for-Developers.markdown b/doc/03-Twig-for-Developers.markdown
index 24fed8a..b17cd10 100644 (file)
--- a/doc/03-Twig-for-Developers.markdown
+++ b/doc/03-Twig-for-Developers.markdown
@@ -311,6 +311,36 @@ You can also change the escaping mode locally by using the `autoescape` tag:
       {% var|escape %}   {# var won't be doubled-escaped #}
     {% endautoescape %}
 
+The escaping rules are implemented as follows:
+
+ * Literals (integers, booleans, arrays, ...) used in the template directly as
+   variables or filter arguments are never automatically escaped:
+
+       [twig]
+       {{ "Twig<br />" }} {# won't be escaped #}
+
+       {% set text as "Twig<br />" %}
+       {{ text }} {# will be escaped #}
+
+ * Escaping is applied before any other filter is applied (the reasoning
+   behind this is that filter transformations should be safe, as the filtered
+   value and all its arguments are escaped):
+
+       [twig]
+       {{ var|nl2br }} {# is equivalent to {{ var|escape|nl2br }} #}
+
+ * The `safe` filter can be used anywhere in the filter chain:
+
+       [twig]
+       {{ var|upper|nl2br|safe }} {# is equivalent to {{ var|safe|upper|nl2br }} #}
+
+ * Automatic escaping is applied to filter arguments, except for literals:
+
+       [twig]
+       {{ var|foo("bar") }} {# "bar" won't be escaped #}
+       {{ var|foo(bar) }} {# bar will be escaped #}
+       {{ var|foo(bar|safe) }} {# bar won't be escaped #}
+
 ### Sandbox Extension
 
 The `sandbox` extension can be used to evaluate untrusted code. Access to

diff --git a/lib/Twig/Node/Expression/Filter.php b/lib/Twig/Node/Expression/Filter.php
index f796ac9..5ad6e41 100644 (file)
--- a/lib/Twig/Node/Expression/Filter.php
+++ b/lib/Twig/Node/Expression/Filter.php
@@ -94,6 +94,16 @@ class Twig_Node_Expression_Filter extends Twig_Node_Expression implements Twig_N
     return $this->filters;
   }
 
+  public function setFilters(array $filters)
+  {
+    $this->filters = $filters;
+  }
+
+  public function prependFilter($filter)
+  {
+    $this->filters = array_merge(array($filter), $this->filters);
+  }
+
   public function appendFilter($filter)
   {
     $this->filters[] = $filter;

diff --git a/lib/Twig/NodeTransformer/Escaper.php b/lib/Twig/NodeTransformer/Escaper.php
index 914d241..1b1c9a3 100644 (file)
--- a/lib/Twig/NodeTransformer/Escaper.php
+++ b/lib/Twig/NodeTransformer/Escaper.php
@@ -37,37 +37,64 @@ class Twig_NodeTransformer_Escaper extends Twig_NodeTransformer
       return $node;
     }
 
-    $expression = $node->getExpression();
+    return $this->escapeNode($node);
+  }
+
+  protected function escapeNode($node)
+  {
+    $expression = $node instanceof Twig_Node_Print ? $node->getExpression() : $node;
+
+    if ($expression instanceof Twig_Node_Expression_Filter)
+    {
+      // don't escape if escape has already been called
+      // or if we want the safe string
+      if ($expression->hasFilter('escape') || $expression->hasFilter('safe'))
+      {
+        return $node;
+      }
 
-    // don't escape if escape has already been called
-    // or if we want the safe string
-    if (
-      $expression instanceof Twig_Node_Expression_Filter
-      &&
-      (
-        $expression->hasFilter('escape')
-        ||
-        $expression->hasFilter('safe')
-      )
-    )
+      // don't escape if the primary node of the filter is not a variable
+      $nodes = $expression->getNodes();
+      if (!$nodes[0] instanceof Twig_Node_Expression_Name)
+      {
+        return $node;
+      }
+    }
+    elseif (!$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)
     {
-      $expression->appendFilter(array('escape', array()));
+      // escape all variables in filters arguments
+      $filters = $expression->getFilters();
+      foreach ($filters as $i => $filter)
+      {
+        foreach ($filter[1] as $j => $argument)
+        {
+          $filters[$i][1][$j] = $this->escapeNode($argument);
+        }
+      }
+
+      $expression->setFilters($filters);
+      $expression->prependFilter(array('escape', array()));
 
       return $node;
     }
-    else
+    elseif ($node instanceof Twig_Node_Print)
     {
       return new Twig_Node_Print(
         new Twig_Node_Expression_Filter($expression, array(array('escape', array())), $node->getLine())
         , $node->getLine()
       );
     }
+    else
+    {
+      return new Twig_Node_Expression_Filter($node, array(array('escape', array())), $node->getLine());
+    }
   }
 
   protected function needEscaping()

diff --git a/test/fixtures/tags/autoescape/literal.test b/test/fixtures/tags/autoescape/literal.test
new file mode 100644 (file)
index 0000000..4c25706
--- /dev/null
+++ b/test/fixtures/tags/autoescape/literal.test
@@ -0,0 +1,10 @@
+--TEST--
+"autoescape" tag does not apply escaping on literals
+--TEMPLATE--
+{% autoescape on %}
+{{ "<br />" }}
+{% endautoescape %}
+--DATA--
+return array()
+--EXPECT--
+<br />

diff --git a/test/fixtures/tags/autoescape/with_filters.test b/test/fixtures/tags/autoescape/with_filters.test
new file mode 100644 (file)
index 0000000..3bb0dd4
--- /dev/null
+++ b/test/fixtures/tags/autoescape/with_filters.test
@@ -0,0 +1,14 @@
+--TEST--
+"autoescape" tag applies escaping before calling filters
+--TEMPLATE--
+{% autoescape on %}
+{{ var|nl2br }}
+{{ var|nl2br|escape }}
+{% endautoescape %}
+--DATA--
+return array('var' => "<Fabien>\nTwig")
+--EXPECT--
+&lt;Fabien&gt;<br />
+Twig
+&lt;Fabien&gt;&lt;br /&gt;
+Twig

diff --git a/test/fixtures/tags/autoescape/with_filters_arguments.test b/test/fixtures/tags/autoescape/with_filters_arguments.test
new file mode 100644 (file)
index 0000000..54cbe99
--- /dev/null
+++ b/test/fixtures/tags/autoescape/with_filters_arguments.test
@@ -0,0 +1,20 @@
+--TEST--
+"autoescape" tag applies escaping on filter arguments, but not on literals
+--TEMPLATE--
+{% autoescape on %}
+{{ var|nl2br("<br />") }}
+{{ var|nl2br("<br />"|escape) }}
+{{ var|nl2br(sep) }}
+{{ var|nl2br(sep|safe) }}
+{% endautoescape %}
+--DATA--
+return array('var' => "<Fabien>\nTwig", 'sep' => '<br />')
+--EXPECT--
+&lt;Fabien&gt;<br />
+Twig
+&lt;Fabien&gt;&lt;br /&gt;
+Twig
+&lt;Fabien&gt;&lt;br /&gt;
+Twig
+&lt;Fabien&gt;<br />
+Twig

diff --git a/test/unit/integrationTest.php b/test/unit/integrationTest.php
index 4a76a21..4310843 100644 (file)
--- a/test/unit/integrationTest.php
+++ b/test/unit/integrationTest.php
@@ -33,7 +33,25 @@ class Foo
   }
 }
 
-$t = new LimeTest(55);
+class TestExtension extends Twig_Extension
+{
+  public function getFilters()
+  {
+    return array('nl2br' => new Twig_Filter_Method($this, 'nl2br'));
+  }
+
+  public function nl2br($value, $sep = '<br />')
+  {
+    return str_replace("\n", $sep."\n", $value);
+  }
+
+  public function getName()
+  {
+    return 'test';
+  }
+}
+
+$t = new LimeTest(58);
 $fixturesDir = realpath(dirname(__FILE__).'/../fixtures/');
 
 foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($fixturesDir), RecursiveIteratorIterator::LEAVES_ONLY) as $file)
@@ -61,6 +79,7 @@ foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($fixturesD
   $loader = new Twig_Loader_Array($templates);
   $twig = new Twig_Environment($loader, array('trim_blocks' => true, 'cache' => false));
   $twig->addExtension(new Twig_Extension_Escaper());
+  $twig->addExtension(new TestExtension());
 
   $template = $twig->loadTemplate('index.twig');