Parcourir la source

Removed logic that tried to avoid double-escaping

Because that's just not possible (have a look at the unit tests to see all possibilities
-- as you will notice, there is no way we can determine the context and whether the
data are already escaped or not).

So, we always escape data, which means that sometimes, we will try to escape already
escaped data. This is not a problem for everything except strings. That's because
strings are not wrapped with an object like everything else (for performance reason).

This means that all escapers must be able to avoid double-escaping (that's the case
for the default escapers as both htmlspecialchars() and htmlentities() have a flag
that does just this).
Fabien Potencier il y a 14 ans
Parent
commit
13f36b1657

+ 2 - 13
src/Symfony/Bundle/FrameworkBundle/Templating/Engine.php

@@ -27,7 +27,6 @@ class Engine extends BaseEngine
 {
     protected $container;
     protected $escaper;
-    protected $level;
 
     /**
      * Constructor.
@@ -39,7 +38,6 @@ class Engine extends BaseEngine
      */
     public function __construct(ContainerInterface $container, LoaderInterface $loader, array $renderers = array(), $escaper = false)
     {
-        $this->level = 0;
         $this->container = $container;
         $this->escaper = $escaper;
 
@@ -61,8 +59,6 @@ class Engine extends BaseEngine
 
     public function render($name, array $parameters = array())
     {
-        ++$this->level;
-
         list(, $options) = $this->splitTemplateName($name);
 
         $renderer = $options['renderer'];
@@ -73,17 +69,10 @@ class Engine extends BaseEngine
         }
 
         if ('php' === $renderer) {
-            // escape only once
-            if (1 === $this->level && !isset($parameters['_data'])) {
-                $parameters = $this->escapeParameters($parameters);
-            }
+            $parameters = $this->escapeParameters($parameters);
         }
 
-        $content = parent::render($name, $parameters);
-
-        --$this->level;
-
-        return $content;
+        return parent::render($name, $parameters);
     }
 
     /**

+ 41 - 0
src/Symfony/Bundle/FrameworkBundle/Tests/Templating/EngineTest.php

@@ -13,9 +13,50 @@ namespace Symfony\Bundle\FrameworkBundle\Tests\Templating;
 
 use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
 use Symfony\Bundle\FrameworkBundle\Templating\Engine;
+use Symfony\Component\Templating\Storage\StringStorage;
+use Symfony\Component\Templating\Storage\Storage;
+use Symfony\Component\Templating\Renderer\PhpRenderer;
+use Symfony\Component\OutputEscaper\Escaper;
+
+// simulate the rendering of another controller
+function foo($engine)
+{
+    return $engine->render('FooBundle:Foo:tpl1.php', array('foo' => 'foo <br />'));
+}
 
 class EngineTest extends TestCase
 {
+    public function testRenderEscaping()
+    {
+        $templates = array(
+            'tpl1'  => '<?php echo $foo ?>',
+            'tpl2'  => '<?php echo $foo.$view->render("FooBundle:Foo:tpl1.php", array("foo" => $foo)) ?>',
+            'tpl3'  => '<?php echo $foo.$view->render("FooBundle:Foo:tpl1.php", array("foo" => "foo <br />")) ?>',
+            'tpl4'  => '<?php echo $foo.Symfony\Bundle\FrameworkBundle\Tests\Templating\foo($view) ?>',
+        );
+
+        $loader = $this->getMock('Symfony\Component\Templating\Loader\LoaderInterface');
+        $loader->expects($this->exactly(4))
+            ->method('load')
+            ->with($this->anything(), $this->anything())
+            ->will($this->onConsecutiveCalls(
+                new StringStorage($templates['tpl1']),
+                new StringStorage($templates['tpl2']),
+                new StringStorage($templates['tpl3']),
+                new StringStorage($templates['tpl4'])
+            ))
+        ;
+
+        $engine = new Engine($this->getContainerMock(), $loader, array('php' => new PhpRenderer()), 'htmlspecialchars');
+
+        $this->assertEquals('foo &lt;br /&gt;', $engine->render('FooBundle:Foo:tpl1.php', array('foo' => 'foo <br />')));
+        $this->assertEquals('foo &lt;br /&gt;', $engine->render('FooBundle:Foo:tpl1.php', array('foo' => 'foo <br />')));
+
+        $this->assertEquals('foo &lt;br /&gt;foo &lt;br /&gt;', $engine->render('FooBundle:Foo:tpl2.php', array('foo' => 'foo <br />')));
+        $this->assertEquals('foo &lt;br /&gt;foo &lt;br /&gt;', $engine->render('FooBundle:Foo:tpl3.php', array('foo' => 'foo <br />')));
+        $this->assertEquals('foo &lt;br /&gt;foo &lt;br /&gt;', $engine->render('FooBundle:Foo:tpl4.php', array('foo' => 'foo <br />')));
+    }
+
     /**
      * @dataProvider getSplitTemplateNameTests
      */

+ 5 - 2
src/Symfony/Component/OutputEscaper/Escaper.php

@@ -206,6 +206,9 @@ class Escaper
     /**
      * Adds a named escaper.
      *
+     * Warning: An escaper must be able to deal with
+     * double-escaping correctly.
+     *
      * @param string $name    The escaper name
      * @param mixed  $escaper A PHP callable
      */
@@ -262,7 +265,7 @@ class Escaper
                 {
                     // Numbers and boolean values get turned into strings which can cause problems
                     // with type comparisons (e.g. === or is_int() etc).
-                    return is_string($value) ? htmlspecialchars($value, ENT_QUOTES, Escaper::getCharset()) : $value;
+                    return is_string($value) ? htmlspecialchars($value, ENT_QUOTES, Escaper::getCharset(), false) : $value;
                 },
 
             'entities' =>
@@ -276,7 +279,7 @@ class Escaper
                 {
                     // Numbers and boolean values get turned into strings which can cause problems
                     // with type comparisons (e.g. === or is_int() etc).
-                    return is_string($value) ? htmlentities($value, ENT_QUOTES, Escaper::getCharset()) : $value;
+                    return is_string($value) ? htmlentities($value, ENT_QUOTES, Escaper::getCharset(), false) : $value;
                 },
 
             'raw' =>