From c7a98407b1f8b46224c3615edfbfae4d29ffb4e0 Mon Sep 17 00:00:00 2001
From: Sean Harvey <sean@silverstripe.com>
Date: Thu, 16 Dec 2010 22:55:17 +0000
Subject: [PATCH] ENHANCEMENT #5055 Convert unpredictability and replacing
 inconsistent conversion. Use htmlspecialchars() and html_entity_decode()
 wherever possible which are faster than str_replace()

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@115140 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/Convert.php              | 34 +++++++++++++++++-----------------
 tests/ConvertTest.php         | 14 ++++++++++----
 tests/fieldtypes/TextTest.php |  2 +-
 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/core/Convert.php b/core/Convert.php
index c2c0c01a9..228369b25 100755
--- a/core/Convert.php
+++ b/core/Convert.php
@@ -29,14 +29,19 @@ class Convert {
 	 * @return array|string
 	 */
 	static function raw2att($val) {
-		if(is_array($val)) {
-			foreach($val as $k => $v) $val[$k] = self::raw2att($v);
-			return $val;
-		} else {
-			return str_replace(array('&','"',"'",'<','>'), array('&amp;','&quot;','&#39;','&lt;','&gt;'), $val);
-		}
+		return self::raw2xml($val);
 	}
-	
+
+	/**
+	 * Convert a value to be suitable for an HTML attribute.
+	 * 
+	 * @param string|array $val String to escape, or array of strings
+	 * @return array|string
+	 */
+	static function raw2htmlatt($val) {
+		return self::raw2att($val);
+	}
+
 	/**
 	 * Convert a value to be suitable for an HTML attribute.
 	 * 
@@ -48,14 +53,12 @@ class Convert {
 	 * @param array|string $val String to escape, or array of strings
 	 * @return array|string
 	 */
-	static function raw2htmlatt($val) {
+	static function raw2htmlname($val) {
 		if(is_array($val)) {
-			foreach($val as $k => $v) $val[$k] = self::raw2htmlatt($v);
+			foreach($val as $k => $v) $val[$k] = self::raw2htmlname($v);
 			return $val;
 		} else {
-			$val = self::raw2att($val);
-			$val = preg_replace('/[^a-zA-Z0-9\-_]*/', '', $val);
-			return $val;
+			return preg_replace('/[^a-zA-Z0-9\-_:.]+/','', $val);
 		}
 	}
 	
@@ -71,7 +74,7 @@ class Convert {
 			foreach($val as $k => $v) $val[$k] = self::raw2xml($v);
 			return $val;
 		} else {
-			return str_replace(array('&','<','>',"\n",'"',"'"), array('&amp;','&lt;','&gt;','<br />','&quot;','&#39;'), $val);
+			return htmlspecialchars($val, ENT_QUOTES, 'UTF-8');
 		}
 	}
 	
@@ -132,10 +135,7 @@ class Convert {
 		} else {
 			// More complex text needs to use html2raw instead
 			if(strpos($val,'<') !== false) return self::html2raw($val);
-			
-			$converted = str_replace(array('&amp;','&lt;','&gt;','&quot;','&apos;', '&#39;'), array('&','<','>','"',"'", "'"), $val);
-			$converted = ereg_replace('&#[0-9]+;', '', $converted);
-			return $converted;
+			else return html_entity_decode($val, ENT_QUOTES, 'UTF-8');
 		}
 	}
 	
diff --git a/tests/ConvertTest.php b/tests/ConvertTest.php
index dca24fad9..d34326e7f 100644
--- a/tests/ConvertTest.php
+++ b/tests/ConvertTest.php
@@ -22,10 +22,10 @@ class ConvertTest extends SapphireTest {
 	 */
 	function testRaw2HtmlAtt() {
 		$val1 = '<input type="text">';
-		$this->assertEquals('ltinputtypequottextquotgt', Convert::raw2htmlatt($val1), 'Special characters are escaped');
+		$this->assertEquals('&lt;input type=&quot;text&quot;&gt;', Convert::raw2htmlatt($val1), 'Special characters are escaped');
 		
 		$val2 = 'This is some normal text.';
-		$this->assertEquals('Thisissomenormaltext', Convert::raw2htmlatt($val2), 'Normal text is not escaped');
+		$this->assertEquals('This is some normal text.', Convert::raw2htmlatt($val2), 'Normal text is not escaped');
 	}
 	
 	function testHtml2raw() {
@@ -37,8 +37,6 @@ class ConvertTest extends SapphireTest {
 		
 		$val2 = 'This has a <strong class="test" style="font-weight: bold">strong tag with attributes</STRONG>.'; 
 		$this->assertEquals('This has a *strong tag with attributes*.', Convert::xml2raw($val2), 'Strong tags with attributes are replaced with asterisks');
-	
-		
 	}
 	
 	/**
@@ -50,6 +48,14 @@ class ConvertTest extends SapphireTest {
 		
 		$val2 = 'This is some normal text.';
 		$this->assertEquals('This is some normal text.', Convert::raw2xml($val2), 'Normal text is not escaped');
+
+		$val3 = "This is test\nNow on a new line.";
+		$this->assertEquals("This is test\nNow on a new line.", Convert::raw2xml($val3), 'Newlines are retained. They should not be replaced with <br /> as it is not XML valid');
+	}
+	
+	function testRaw2HtmlName() {
+		$val1 = 'test test 123';
+		$this->assertEquals('testtest123', Convert::raw2htmlname($val1));
 	}
 	
 	/**
diff --git a/tests/fieldtypes/TextTest.php b/tests/fieldtypes/TextTest.php
index b4e29b6c3..f48d1e4d8 100644
--- a/tests/fieldtypes/TextTest.php
+++ b/tests/fieldtypes/TextTest.php
@@ -54,7 +54,7 @@ class TextTest extends SapphireTest {
 	function testLimitWordCountXML() {
 		$cases = array(
 			'<p>Stuff & stuff</p>' => 'Stuff &amp;...',
-			"Stuff\nBlah Blah Blah" => "Stuff<br />Blah Blah...",
+			"Stuff\nBlah Blah Blah" => "Stuff\nBlah Blah...",
 			"Stuff<Blah Blah" => "Stuff&lt;Blah Blah",
 			"Stuff>Blah Blah" => "Stuff&gt;Blah Blah"
 		);