From 9d4ceb456383f0f3fd8e7fa6778e6b3604246e21 Mon Sep 17 00:00:00 2001 From: fabien Date: Mon, 14 Dec 2009 16:57:54 +0000 Subject: [PATCH] changed the automatic-escaping rules to be more sensible (the documentation lists all the rules) git-svn-id: http://svn.twig-project.org/trunk@182 93ef8e89-cb99-4229-a87c-7fa0fa45744b --- CHANGELOG | 1 + doc/02-Twig-for-Template-Designers.markdown | 9 +++- doc/03-Twig-for-Developers.markdown | 30 +++++++++++ lib/Twig/Node/Expression/Filter.php | 10 ++++ lib/Twig/NodeTransformer/Escaper.php | 55 +++++++++++++++----- test/fixtures/tags/autoescape/literal.test | 10 ++++ test/fixtures/tags/autoescape/with_filters.test | 14 +++++ .../tags/autoescape/with_filters_arguments.test | 20 +++++++ test/unit/integrationTest.php | 21 +++++++- 9 files changed, 154 insertions(+), 16 deletions(-) create mode 100644 test/fixtures/tags/autoescape/literal.test create mode 100644 test/fixtures/tags/autoescape/with_filters.test create mode 100644 test/fixtures/tags/autoescape/with_filters_arguments.test diff --git a/CHANGELOG b/CHANGELOG index 5334216..18ce387 100644 --- 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 --- 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 --- 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
" }} {# won't be escaped #} + + {% set text as "Twig
" %} + {{ 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 --- 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 --- 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 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 %} +{{ "
" }} +{% endautoescape %} +--DATA-- +return array() +--EXPECT-- +
diff --git a/test/fixtures/tags/autoescape/with_filters.test b/test/fixtures/tags/autoescape/with_filters.test new file mode 100644 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' => "\nTwig") +--EXPECT-- +<Fabien>
+Twig +<Fabien><br /> +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 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("
") }} +{{ var|nl2br("
"|escape) }} +{{ var|nl2br(sep) }} +{{ var|nl2br(sep|safe) }} +{% endautoescape %} +--DATA-- +return array('var' => "\nTwig", 'sep' => '
') +--EXPECT-- +<Fabien>
+Twig +<Fabien><br /> +Twig +<Fabien><br /> +Twig +<Fabien>
+Twig diff --git a/test/unit/integrationTest.php b/test/unit/integrationTest.php index 4a76a21..4310843 100644 --- 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 = '
') + { + 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'); -- 1.7.2.5