From d35e19fe22caed297039d0dd8a3353f8dc33432b Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 16 Nov 2010 21:15:15 +0100 Subject: [PATCH] Added "pre_escape" option for filters This allows filters to get their input pre-escaped for a given context --- lib/Twig/Filter.php | 9 +++ lib/Twig/FilterInterface.php | 3 + lib/Twig/NodeVisitor/Escaper.php | 61 +++++++++++++----- .../Fixtures/tags/autoescape/with_filters.test | 38 ++++++------ .../tags/autoescape/with_pre_escape_filters.test | 68 ++++++++++++++++++++ test/Twig/Tests/integrationTest.php | 28 +++++++- 6 files changed, 169 insertions(+), 38 deletions(-) create mode 100644 test/Twig/Tests/Fixtures/tags/autoescape/with_pre_escape_filters.test diff --git a/lib/Twig/Filter.php b/lib/Twig/Filter.php index 64da1fa..1904d74 100644 --- a/lib/Twig/Filter.php +++ b/lib/Twig/Filter.php @@ -48,4 +48,13 @@ abstract class Twig_Filter implements Twig_FilterInterface return array(); } + + public function getPreEscape() + { + if (isset($this->options['pre_escape'])) { + return $this->options['pre_escape']; + } + + return null; + } } diff --git a/lib/Twig/FilterInterface.php b/lib/Twig/FilterInterface.php index 11346a1..6abeab1 100644 --- a/lib/Twig/FilterInterface.php +++ b/lib/Twig/FilterInterface.php @@ -18,4 +18,7 @@ interface Twig_FilterInterface { public function compile(); + public function needsEnvironment(); + public function getSafe(Twig_Node $filterArgs); + public function getPreEscape(); } diff --git a/lib/Twig/NodeVisitor/Escaper.php b/lib/Twig/NodeVisitor/Escaper.php index 44e2f6b..1f998ee 100644 --- a/lib/Twig/NodeVisitor/Escaper.php +++ b/lib/Twig/NodeVisitor/Escaper.php @@ -40,8 +40,6 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface { if ($node instanceof Twig_Node_AutoEscape) { $this->statusStack[] = $node->getAttribute('value'); - } elseif ($node instanceof Twig_Node_Print) { - return $this->escapeNode($node, $env, $this->needEscaping($env)); } elseif ($node instanceof Twig_Node_Block) { $this->statusStack[] = isset($this->blocks[$node->getAttribute('name')]) ? $this->blocks[$node->getAttribute('name')] : $this->needEscaping($env); } @@ -59,6 +57,12 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface */ public function leaveNode(Twig_NodeInterface $node, Twig_Environment $env) { + if ($node instanceof Twig_Node_Expression_Filter) { + return $this->preEscapeFilterNode($node, $env); + } elseif ($node instanceof Twig_Node_Print) { + return $this->escapePrintNode($node, $env, $this->needEscaping($env)); + } + if ($node instanceof Twig_Node_AutoEscape || $node instanceof Twig_Node_Block) { array_pop($this->statusStack); } elseif ($node instanceof Twig_Node_BlockReference) { @@ -68,14 +72,50 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface return $node; } - protected function escapeNode(Twig_NodeInterface $node, Twig_Environment $env, $type) + protected function escapePrintNode(Twig_Node_Print $node, Twig_Environment $env, $type) { if (false === $type) { return $node; } - $expression = $node instanceof Twig_Node_Print ? $node->getNode('expr') : $node; + $expression = $node->getNode('expr'); + + if ($this->isSafeFor($type, $expression, $env)) { + return $node; + } + + return new Twig_Node_Print( + $this->getEscaperFilter($type, $expression), + $node->getLine() + ); + } + + protected function preEscapeFilterNode(Twig_Node_Expression_Filter $filter, Twig_Environment $env) + { + $filterMap = $env->getFilters(); + $name = $filter->getNode('filter')->getAttribute('value'); + + if (isset($filterMap[$name])) { + $type = $filterMap[$name]->getPreEscape(); + if (null === $type) { + return $filter; + } + + $node = $filter->getNode('node'); + if ($this->isSafeFor($type, $node, $env)) { + return $filter; + } + + $filter->setNode('node', $this->getEscaperFilter($type, $node)); + + return $filter; + } + + return $filter; + } + protected function isSafeFor($type, Twig_NodeInterface $expression, $env) + { $safe = $this->safeAnalysis->getSafe($expression); if (null === $safe) { @@ -86,18 +126,7 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface $safe = $this->safeAnalysis->getSafe($expression); } - if (false !== in_array($type, $safe) || false !== in_array('all', $safe)) { - return $node; - } - - if ($node instanceof Twig_Node_Print) { - return new Twig_Node_Print( - $this->getEscaperFilter($type, $expression), - $node->getLine() - ); - } - - return $this->getEscaperFilter($type, $node); + return in_array($type, $safe) || in_array('all', $safe); } protected function needEscaping(Twig_Environment $env) diff --git a/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test b/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test index ce75c45..53892b1 100644 --- a/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test +++ b/test/Twig/Tests/Fixtures/tags/autoescape/with_filters.test @@ -3,22 +3,22 @@ --TEMPLATE-- {% autoescape on %} -(nl2br is an escaper filter) +(escape_and_nl2br is an escaper filter) 1. Don't escape escaper filter output -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is not escaped ) -{{ var|nl2br }} +{{ var|escape_and_nl2br }} 2. Don't escape escaper filter output -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is not escaped, |raw is redundant ) -{{ var|nl2br|raw }} +{{ var|escape_and_nl2br|raw }} 3. Explicit escape -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is explicitly escaped by |escape ) -{{ var|nl2br|escape }} +{{ var|escape_and_nl2br|escape }} 4. Escape non-escaper filter output ( var is upper-cased by |upper, @@ -26,16 +26,16 @@ {{ var|upper }} 5. Escape if last filter is not an escaper -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_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 }} +{{ var|escape_and_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 }} + the output is escaped by |escape_and_nl2br, line-breaks are added, + the output is not escaped as |escape_and_nl2br is an escaper ) +{{ var|upper|escape_and_nl2br }} 7. Escape if last filter is not an escaper ( the output of |format is "" ~ var ~ "", @@ -64,22 +64,22 @@ return array('var' => "\nTwig") --EXPECT-- -(nl2br is an escaper filter) +(escape_and_nl2br is an escaper filter) 1. Don't escape escaper filter output -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is not escaped ) <Fabien>
Twig 2. Don't escape escaper filter output -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is not escaped, |raw is redundant ) <Fabien>
Twig 3. Explicit escape -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is explicitly escaped by |escape ) &lt;Fabien&gt;<br /> Twig @@ -91,7 +91,7 @@ Twig TWIG 5. Escape if last filter is not an escaper -( var is escaped by |nl2br, line-breaks are added, +( var is escaped by |escape_and_nl2br, line-breaks are added, the output is upper-cased by |upper, the output is auto-escaped as |upper is not an escaper ) &LT;FABIEN&GT;<BR /> @@ -99,8 +99,8 @@ 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 ) + the output is escaped by |escape_and_nl2br, line-breaks are added, + the output is not escaped as |escape_and_nl2br is an escaper ) <FABIEN>
TWIG diff --git a/test/Twig/Tests/Fixtures/tags/autoescape/with_pre_escape_filters.test b/test/Twig/Tests/Fixtures/tags/autoescape/with_pre_escape_filters.test new file mode 100644 index 0000000..a60f7c9 --- /dev/null +++ b/test/Twig/Tests/Fixtures/tags/autoescape/with_pre_escape_filters.test @@ -0,0 +1,68 @@ +--TEST-- +"autoescape" tag applies escaping after calling filters, and before calling pre_escape filters +--TEMPLATE-- +{% autoescape on %} + +(nl2br is pre_escaped for "html" and declared safe for "html") + +1. Pre-escape and don't post-escape +( var|escape|nl2br ) +{{ var|nl2br }} + +2. Don't double-pre-escape +( var|escape|nl2br ) +{{ var|escape|nl2br }} + +3. Don't escape safe values +( var|raw|nl2br ) +{{ var|raw|nl2br }} + +4. Don't escape safe values +( var|escape|nl2br|nl2br ) +{{ var|nl2br|nl2br }} + +5. Re-escape values that are escaped for an other contexts +( var|escape_something|escape|nl2br ) +{{ var|escape_something|nl2br }} + +6. Still escape when using filters not declared safe +( var|escape|nl2br|upper|escape ) +{{ var|nl2br|upper }} + +{% endautoescape %} +--DATA-- +return array('var' => "\nTwig") +--EXPECT-- + +(nl2br is pre_escaped for "html" and declared safe for "html") + +1. Pre-escape and don't post-escape +( var|escape|nl2br ) +<Fabien>
+Twig + +2. Don't double-pre-escape +( var|escape|nl2br ) +<Fabien>
+Twig + +3. Don't escape safe values +( var|raw|nl2br ) +
+Twig + +4. Don't escape safe values +( var|escape|nl2br|nl2br ) +<Fabien>

+Twig + +5. Re-escape values that are escaped for an other contexts +( var|escape_something|escape|nl2br ) +<FABIEN>
+TWIG + +6. Still escape when using filters not declared safe +( var|escape|nl2br|upper|escape ) +&LT;FABIEN&GT;<BR /> +TWIG + diff --git a/test/Twig/Tests/integrationTest.php b/test/Twig/Tests/integrationTest.php index dcaccc5..711cf9d 100644 --- a/test/Twig/Tests/integrationTest.php +++ b/test/Twig/Tests/integrationTest.php @@ -103,12 +103,34 @@ class TestExtension extends Twig_Extension { public function getFilters() { - return array('nl2br' => new Twig_Filter_Method($this, 'nl2br', array('needs_environment' => true, 'is_safe' => array('html')))); + return array( + 'escape_and_nl2br' => new Twig_Filter_Method($this, 'escape_and_nl2br', array('needs_environment' => true, 'is_safe' => array('html'))), + 'nl2br' => new Twig_Filter_Method($this, 'nl2br', array('pre_escape' => 'html', 'is_safe' => array('html'))), + 'escape_something' => new Twig_Filter_Method($this, 'escape_something', array('is_safe' => array('something'))), + ); } - public function nl2br($env, $value, $sep = '
') + /** + * nl2br which also escapes, for testing escaper filters + */ + public function escape_and_nl2br($env, $value, $sep = '
') + { + return $this->nl2br(twig_escape_filter($env, $value, 'html'), $sep); + } + + /** + * nl2br only, for testing filters with pre_escape + */ + public function nl2br($value, $sep = '
') + { + // not secure if $value contains html tags (not only entities) + // don't use + return str_replace("\n", "$sep\n", $value); + } + + public function escape_something($value) { - return str_replace("\n", $sep."\n", twig_escape_filter($env, $value, 'html')); + return strtoupper($value); } public function getName() -- 1.7.2.5