[PATCH] Prevent double decoration in output escaping

in r1321.

I still have double escaping issues with partial inclusions.

The patch works only for objects. Scalars and arrays stille get double escaped.

The only way to prevent double escaping for objects in partials is to use the modified accessors:

$sf_data->getRaw('question')->getTitle();

is double escaped, but

$question->getTitle(ESC_RAW);

is escaped only once.

As explained above, the problem remains for strings.

Multiple usage of partials/components leads to multiple decoration, not only double decoration. Therfore, all existing decorators have to be stripped. The following Patch solves the issue for me:

--- sfOutputEscaper.class.php.orig  2007-09-05 20:06:26.000000000 +0200
+++ sfOutputEscaper.class.php 2007-09-05 20:06:56.000000000 +0200
@@ -98,16 +98,14 @@

     if (is_object($value))
     {
-      if ($value instanceof sfOutputEscaper)
+      while ($value instanceof sfOutputEscaper)
       {
-        // avoid double decoration when passing values from action template to component/partial
-        $copy = clone $value;
-
-        $copy->escapingMethod = $escapingMethod;
-
-        return $copy;
+        // avoid multiple decoration when passing values from
+        // action template to component/partial
+        $value = $value->getRawValue();
       }
-      elseif ($value instanceof Traversable)
+
+      if ($value instanceof Traversable)
       {
         return new sfOutputEscaperIteratorDecorator($escapingMethod, $value);
       }

Be aware that this modification might lead to insecure applications since users might have used getRaw() to circumvent the problem. This should be pointed out in the release notes.

Best regards

Sven Lauritzen

I've recognized that the previous patch lead to arrays decorated with the sfObjectDecorater in partials. They where no more traversable nor accessible. I've reworked the patch. The resulting code is easier to understand and easier to refactore. If include_partial stops decorating the passed variables, the while-loop may simply be removed. Anyhow, it may persist.

--- sfOutputEscaper.class.php.orig  2007-09-11 19:50:04.000000000 +0200
+++ sfOutputEscaper.class.php 2007-09-11 19:49:38.000000000 +0200
@@ -80,6 +80,13 @@
    */
   public static function escape($escapingMethod, $value)
   {
+    // avoid multiple decoration when passing values from
+    // action template to component/partial
+    while ($value instanceof sfOutputEscaper)
+    {
+      $value = $value->getRawValue();
+    }
+
     if (is_null($value) || ($value === false) || ($escapingMethod === 'esc_raw'))   
     {
       return $value;
@@ -98,16 +105,7 @@
       
     if (is_object($value))
     {
-      if ($value instanceof sfOutputEscaper)
-      { 
-        // avoid double decoration when passing values from action template to component/partial
-        $copy = clone $value;
-
-        $copy->escapingMethod = $escapingMethod;
-      
-        return $copy;
-      }
-      elseif ($value instanceof Traversable)
+      if ($value instanceof Traversable)
       {
         return new sfOutputEscaperIteratorDecorator($escapingMethod, $value);
       }

Sven Lauritzen

cristianbaciu: Please leave it to the core team to decide what goes into the next release or not. The current status of this ticket is that the patch is not good enough to be tested by the core team.

That's something that cannot be fixed cleanly in symfony 1. This has been taken into account for Symfony 2.