From 4087d087a1107a9471ad317c71935d0639af2c95 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Sat, 6 Nov 2010 03:49:37 +0100 Subject: [PATCH] deeply mark expressions as safe (e.g. a conditonal expression is safe if both operands are literals) --- lib/Twig/Extension/Core.php | 9 +-- lib/Twig/Extension/Escaper.php | 8 +-- lib/Twig/Filter.php | 8 +- lib/Twig/NodeVisitor/Escaper.php | 29 +++++--- lib/Twig/NodeVisitor/SafeAnalysis.php | 76 ++++++++++++++++++++ .../Tests/Fixtures/tags/autoescape/literal.test | 35 +++++++++ 6 files changed, 137 insertions(+), 28 deletions(-) create mode 100644 lib/Twig/NodeVisitor/SafeAnalysis.php diff --git a/lib/Twig/Extension/Core.php b/lib/Twig/Extension/Core.php index 8acf214..7805dec 100644 --- a/lib/Twig/Extension/Core.php +++ b/lib/Twig/Extension/Core.php @@ -251,18 +251,17 @@ function twig_escape_filter(Twig_Environment $env, $string, $type = 'html') } } -function twig_escape_filter_is_safe($for, Twig_Node $filterArgs) +function twig_escape_filter_is_safe(Twig_Node $filterArgs) { - $type = 'html'; foreach($filterArgs as $arg) { if ($arg instanceof Twig_Node_Expression_Constant) { - $type = $arg->getAttribute('value'); + return array($arg->getAttribute('value')); } else { - $type = null; + return array(); } break; } - return $for == $type; + return array('html'); } if (function_exists('iconv')) { diff --git a/lib/Twig/Extension/Escaper.php b/lib/Twig/Extension/Escaper.php index 7fcb17a..62c5a1a 100644 --- a/lib/Twig/Extension/Escaper.php +++ b/lib/Twig/Extension/Escaper.php @@ -45,7 +45,7 @@ class Twig_Extension_Escaper extends Twig_Extension public function getFilters() { return array( - 'raw' => new Twig_Filter_Function('twig_raw_filter', array('is_safe_callback' => 'twig_raw_filter_is_safe')), + 'raw' => new Twig_Filter_Function('twig_raw_filter', array('is_safe' => array('all'))), ); } @@ -71,9 +71,3 @@ function twig_raw_filter($string) return $string; } -// |raw is safe for everything -function twig_raw_filter_is_safe($for, Twig_Node $filterArgs) -{ - return true; -} - diff --git a/lib/Twig/Filter.php b/lib/Twig/Filter.php index 529e500..d9991d5 100644 --- a/lib/Twig/Filter.php +++ b/lib/Twig/Filter.php @@ -37,14 +37,14 @@ abstract class Twig_Filter implements Twig_FilterInterface return $this->options['needs_environment']; } - public function isSafe($for, Twig_Node $filterArgs) + public function getSafe(Twig_Node $filterArgs) { if (isset($this->options['is_safe'])) { - return in_array($for, $this->options['is_safe']); + return $this->options['is_safe']; } if (isset($this->options['is_safe_callback'])) { - return call_user_func($this->options['is_safe_callback'], $for, $filterArgs); + return call_user_func($this->options['is_safe_callback'], $filterArgs); } - return false; + return array(); } } diff --git a/lib/Twig/NodeVisitor/Escaper.php b/lib/Twig/NodeVisitor/Escaper.php index e06aaba..6f0feb7 100644 --- a/lib/Twig/NodeVisitor/Escaper.php +++ b/lib/Twig/NodeVisitor/Escaper.php @@ -21,6 +21,14 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface protected $statusStack = array(); protected $blocks = array(); + protected $safeAnalysis; + protected $traverser; + + function __construct() + { + $this->safeAnalysis = new Twig_NodeVisitor_SafeAnalysis; + } + /** * Called before child nodes are visited. * @@ -69,21 +77,18 @@ class Twig_NodeVisitor_Escaper implements Twig_NodeVisitorInterface $expression = $node instanceof Twig_Node_Print ? $node->getNode('expr') : $node; - if ($expression instanceof Twig_Node_Expression_Constant) { + $safe = $this->safeAnalysis->getSafe($expression); - return $node; + if ($safe === null) { + if ($this->traverser === null) { + $this->traverser = new Twig_NodeTraverser($env, array($this->safeAnalysis)); + } + $this->traverser->traverse($expression); + $safe = $this->safeAnalysis->getSafe($expression); } - if ($expression instanceof Twig_Node_Expression_Filter) { - - // don't escape if the last filter in the chain is safe for $type - $filterMap = $env->getFilters(); - $i = count($expression->getNode('filters')) - 2; - $name = $expression->getNode('filters')->getNode($i)->getAttribute('value'); - $args = $expression->getNode('filters')->getNode($i+1); - if (isset($filterMap[$name]) && $filterMap[$name]->isSafe($type, $args)) { - return $node; - } + if (in_array($type, $safe) !== false || in_array('all', $safe) !== false) { + return $node; } // escape diff --git a/lib/Twig/NodeVisitor/SafeAnalysis.php b/lib/Twig/NodeVisitor/SafeAnalysis.php new file mode 100644 index 0000000..d587d1f --- /dev/null +++ b/lib/Twig/NodeVisitor/SafeAnalysis.php @@ -0,0 +1,76 @@ +data[$hash]) ? $this->data[$hash] : null; + } + + protected function setSafe(Twig_NodeInterface $node, array $safe) + { + $hash = spl_object_hash($node); + $this->data[$hash] = $safe; + } + + protected function intersectSafe(array $a = null, array $b = null) + { + if ($a === null || $b === null) { + return array(); + } + if (in_array('all', $a)) { + return $b; + } + if (in_array('all', $b)) { + return $a; + } + return array_intersect($a, $b); + } + + public function enterNode(Twig_NodeInterface $node, Twig_Environment $env) + { + return $node; + } + + public function leaveNode(Twig_NodeInterface $node, Twig_Environment $env) + { + + // constants are marked safe for all + + if ($node instanceof Twig_Node_Expression_Constant) { + + $this->setSafe($node, array('all')); + + // instersect safeness of both operands + + } else if ($node instanceof Twig_Node_Expression_Conditional) { + + $safe = $this->intersectSafe($this->getSafe($node->getNode('expr2')), $this->getSafe($node->getNode('expr3'))); + $this->setSafe($node, $safe); + + // filter expression is safe when the last filter is safe + + } else if ($node instanceof Twig_Node_Expression_Filter) { + + $filterMap = $env->getFilters(); + $filters = $node->getNode('filters'); + $i = count($filters) - 2; + $name = $filters->getNode($i)->getAttribute('value'); + $args = $filters->getNode($i+1); + if (isset($filterMap[$name])) { + $this->setSafe($node, $filterMap[$name]->getSafe($args)); + } else { + $this->setSafe($node, array()); + } + + } else { + + $this->setSafe($node, array()); + } + + return $node; + } +} diff --git a/test/Twig/Tests/Fixtures/tags/autoescape/literal.test b/test/Twig/Tests/Fixtures/tags/autoescape/literal.test index 4c25706..eb0a987 100644 --- a/test/Twig/Tests/Fixtures/tags/autoescape/literal.test +++ b/test/Twig/Tests/Fixtures/tags/autoescape/literal.test @@ -2,9 +2,44 @@ "autoescape" tag does not apply escaping on literals --TEMPLATE-- {% autoescape on %} + +1. Simple literal {{ "
" }} + +2. Conditional expression with only literals +{{ true ? "
" : "
" }} + +3. Conditonal expression with a variable +{{ true ? "
" : someVar }} + +4. Nested conditionals with only literals +{{ true ? (true ? "
" : "
") : "\n" }} + +5. Nested conditionals with a variable +{{ true ? (true ? "
" : someVar) : "\n" }} + +6. Nested conditionals with a variable marked safe +{{ true ? (true ? "
" : someVar|raw) : "\n" }} + {% endautoescape %} --DATA-- return array() --EXPECT-- + +1. Simple literal +
+ +2. Conditional expression with only literals +
+ +3. Conditonal expression with a variable +<br /> + +4. Nested conditionals with only literals +
+ +5. Nested conditionals with a variable +<br /> + +6. Nested conditionals with a variable marked safe
-- 1.7.2.5