Add more factory methods for convenience:

SafeString::purify() and SafeString::of_safe_html().

Removing SafeString::mark_html_safe() since it's no longer needed.

5 files changed, 43 insertions, 22 deletions

diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php
index b4b7f352..6092a9d8 100644
--- a/modules/gallery/helpers/MY_url.php
+++ b/modules/gallery/helpers/MY_url.php
@@ -31,7 +31,7 @@ class url extends url_Core {
       $uri = model_cache::get("item", $parts[1])->relative_path();
     }
     $url = parent::site($uri . $query, $protocol);
-    return SafeString::of($url)->mark_html_safe();
+    return SafeString::of_safe_html($url);
   }
 
   static function parse_url() {
@@ -103,22 +103,22 @@ class url extends url_Core {
 
   public static function base($index=false, $protocol=false) {
     $url = parent::base($index, $protocol);
-    return SafeString::of($url)->mark_html_safe();
+    return SafeString::of_safe_html($url);
   }
 
   public static function current($qs=false) {
     $url = parent::current($qs);
-    return SafeString::of($url)->mark_html_safe();
+    return SafeString::of_safe_html($url);
   }
 
   public static function file($file, $index=false) {
     $url = parent::file($file, $index);
-    return SafeString::of($url)->mark_html_safe();
+    return SafeString::of_safe_html($url);
   }
 
   public static function merge(array $arguments) {
     $url = parent::merge($arguments);
-    return SafeString::of($url)->mark_html_safe();
+    return SafeString::of_safe_html($url);
   }
 
 }
diff --git a/modules/gallery/libraries/I18n.php b/modules/gallery/libraries/I18n.php
index 8dc42e04..a53d5ae9 100644
--- a/modules/gallery/libraries/I18n.php
+++ b/modules/gallery/libraries/I18n.php
@@ -116,7 +116,7 @@ class I18n_Core {
 
     $entry = $this->interpolate($locale, $entry, $values);
 
-    return SafeString::of($entry)->mark_html_safe();
+    return SafeString::of_safe_html($entry);
   }
 
   private function lookup($locale, $message) {
diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php
index 709ab5f6..9a269ed4 100644
--- a/modules/gallery/libraries/SafeString.php
+++ b/modules/gallery/libraries/SafeString.php
@@ -24,6 +24,7 @@
 class SafeString_Core {
   private $_raw_string;
   protected $_is_safe_html = false;
+  protected $_is_purified_html = false;
 
   private static $_purifier = null;
 
@@ -44,11 +45,25 @@ class SafeString_Core {
   }
 
   /**
-   * Marks this string as safe to be used in HTML without any escaping.
+   * Factory method returning a new SafeString instance after HTML purifying
+   * the given string.
    */
-  function mark_html_safe() {
-    $this->_is_safe_html = true;
-    return $this;
+  static function purify($string) {
+    if ($string instanceof SafeString) {
+      $string = $string->unescaped();
+    }
+    $safe_string = self::of_safe_html(self::_purify_for_html($string));
+    $safe_string->_is_purified_html = true;
+    return $safe_string;
+  }
+
+  /**
+   * Factory method returning a new SafeString instance which won't HTML escape.
+   */
+  static function of_safe_html($string) {
+    $safe_string = new SafeString($string);
+    $safe_string->_is_safe_html = true;
+    return $safe_string;
   }
 
   /**
@@ -117,10 +132,10 @@ class SafeString_Core {
    * @return the string escaped for use in HTML.
    */
   function purified_html() {
-    if ($this->_is_safe_html) {
+    if ($this->_is_purified_html) {
       return $this;
     } else {
-      return SafeString::of(self::_purify_for_html($this->_raw_string), true);
+      return self::purify($this);
     }
   }
 
diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php
index 73d82c34..0fc7f6f3 100644
--- a/modules/gallery/tests/SafeString_Test.php
+++ b/modules/gallery/tests/SafeString_Test.php
@@ -25,8 +25,7 @@ class SafeString_Test extends Unit_Test_Case {
   }
 
   public function toString_for_safe_string_test() {
-    $safe_string = new SafeString("hello <p>world</p>");
-    $safe_string->mark_html_safe();
+    $safe_string = SafeString::of_safe_html("hello <p>world</p>");
     $this->assert_equal("hello <p>world</p>",
 			$safe_string);
   }
@@ -62,7 +61,7 @@ class SafeString_Test extends Unit_Test_Case {
   }
 
   public function for_html_attr_with_safe_html_test() {
-    $safe_string = SafeString::of('"<em>Foo</em>\'s bar"')->mark_html_safe();
+    $safe_string = SafeString::of_safe_html('"<em>Foo</em>\'s bar"');
     $attr_string = $safe_string->for_html_attr();
     $this->assert_equal('&quot;<em>Foo</em>&#039;s bar&quot;',
 			$attr_string);
@@ -86,25 +85,29 @@ class SafeString_Test extends Unit_Test_Case {
   }
 
   public function of_safe_html_test() {
-    $safe_string = SafeString::of("hello <p>world</p>")->mark_html_safe();
+    $safe_string = SafeString::of_safe_html("hello <p>world</p>");
     $this->assert_equal("hello <p>world</p>", $safe_string->for_html());
   }
 
+  public function purify_test() {
+    $safe_string = SafeString::purify("hello <p  >world</p>");
+    $this->assert_equal("hello <p>world</p>", $safe_string);
+  }
+
   public function of_fluid_api_test() {
     $escaped_string = SafeString::of("Foo's bar")->for_js();
     $this->assert_equal("Foo\\'s bar", $escaped_string);
   }
 
   public function safestring_of_safestring_preserves_safe_status_test() {
-    $safe_string = SafeString::of("hello's <p>world</p>")->mark_html_safe();
+    $safe_string = SafeString::of_safe_html("hello's <p>world</p>");
     $safe_string_2 = new SafeString($safe_string);
     $this->assert_equal("hello's <p>world</p>", $safe_string_2);
     $this->assert_equal("hello\\'s <p>world<\\/p>", $safe_string_2->for_js());
   }
 
   public function safestring_of_safestring_preserves_html_safe_status_test() {
-    $safe_string = SafeString::of("hello's <p>world</p>")
-      ->mark_html_safe();
+    $safe_string = SafeString::of_safe_html("hello's <p>world</p>");
     $safe_string_2 = new SafeString($safe_string);
     $this->assert_equal("hello's <p>world</p>", $safe_string_2);
     $this->assert_equal("hello\\'s <p>world<\\/p>", $safe_string_2->for_js());
@@ -112,7 +115,7 @@ class SafeString_Test extends Unit_Test_Case {
 
   public function safestring_of_safestring_safe_status_override_test() {
     $safe_string = new SafeString("hello <p>world</p>");
-    $safe_string_2 = SafeString::of($safe_string)->mark_html_safe();
+    $safe_string_2 = SafeString::of_safe_html($safe_string);
     $this->assert_equal("hello <p>world</p>", $safe_string_2);
   }
 }
diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php
index e0e5bb86..fd596c69 100644
--- a/modules/gallery/tests/Xss_Security_Test.php
+++ b/modules/gallery/tests/Xss_Security_Test.php
@@ -110,10 +110,13 @@ class Xss_Security_Test extends Unit_Test_Case {
 	  } else if ($token[1] == "SafeString") {
 	    // Looking for SafeString::of(...
 	    if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) &&
-		self::_token_matches(array(T_STRING, "of"), $tokens, $token_number + 2)	&&
+		self::_token_matches(array(T_STRING), $tokens, $token_number + 2) &&
+		in_array($tokens[$token_number + 2][1], array("of", "of_safe_html", "purify")) &&
 		self::_token_matches("(", $tokens, $token_number + 3)) {
 	      $frame->is_safestring(true);
-	      $frame->expr_append("::of(");
+
+	      $method = $tokens[$token_number + 2][1];
+	      $frame->expr_append("::$method(");
 
 	      $token_number += 3;
 	      $token = $tokens[$token_number];