From c453c0ef8239bc79e484dd3feb9e275e942e9d48 Mon Sep 17 00:00:00 2001
From: Andy Staudacher <andy.st@gmail.com>
Date: Fri, 4 Sep 2009 10:11:42 -0700
Subject: Simplifying SafeString a bit: From a XSS HTML security point of view,
 treat clean() and purify() the same. No longer run a safe HTML string through
 the HTML purifier (since it's already marked as safe).

This also addresses the issue of calling purify() when no purifier is installed. In that case, we'd run clean() on a clean string (double HTML encoding).

If this approach doesn't work out, we can still modify the fallback code of purify() to check if the string is already clean before calling clean() instead of purify().
---
 modules/gallery/libraries/SafeString.php  | 11 ++---------
 modules/gallery/tests/SafeString_Test.php |  6 ++++++
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php
index 3328fed5..ba3a8ffd 100644
--- a/modules/gallery/libraries/SafeString.php
+++ b/modules/gallery/libraries/SafeString.php
@@ -24,13 +24,11 @@
 class SafeString_Core {
   private $_raw_string;
   protected $_is_safe_html = false;
-  protected $_is_purified_html = false;
 
   /** Constructor */
   function __construct($string) {
     if ($string instanceof SafeString) {
       $this->_is_safe_html = $string->_is_safe_html;
-      $this->_is_purified_html = $string->_is_purified_html;
       $string = $string->unescaped();
     }
     $this->_raw_string = (string) $string;
@@ -49,14 +47,13 @@ class SafeString_Core {
    */
   static function purify($string) {
     if ($string instanceof SafeString) {
-      if ($string->_is_purified_html) {
+      if ($string->_is_safe_html) {
         return $string;
       } else {
         $string = $string->unescaped();
       }
     }
     $safe_string = self::of_safe_html(self::_purify_for_html($string));
-    $safe_string->_is_purified_html = true;
     return $safe_string;
   }
 
@@ -135,11 +132,7 @@ class SafeString_Core {
    * @return the string escaped for use in HTML.
    */
   function purified_html() {
-    if ($this->_is_purified_html) {
-      return $this;
-    } else {
-      return self::purify($this);
-    }
+    return self::purify($this);
   }
 
   /**
diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php
index fdab5c58..2c07d934 100644
--- a/modules/gallery/tests/SafeString_Test.php
+++ b/modules/gallery/tests/SafeString_Test.php
@@ -106,6 +106,12 @@ class SafeString_Test extends Unit_Test_Case {
     $this->assert_equal($expected, $safe_string_2);
   }
 
+  public function purify_safe_html_test() {
+    $safe_string = SafeString::of_safe_html("hello <p  >world</p>");
+    $actual = SafeString::purify($safe_string);
+    $this->assert_equal("hello <p  >world</p>", $actual);
+  }
+
   public function of_fluid_api_test() {
     $escaped_string = SafeString::of("Foo's bar")->for_js();
     $this->assert_equal('"Foo\'s bar"', $escaped_string);
-- 
cgit v1.2.3