From 67743c8079cf5698d06e8730807b6f431f16b3a3 Mon Sep 17 00:00:00 2001
From: mark_story <mark@mark-story.com>
Date: Mon, 9 Apr 2012 21:20:03 -0400
Subject: [PATCH] Fix missing Html escaping on string urls for assets.

Add HTML escaping for asset paths provided as strings.
Split existing tests up.

Fixes #2766
---
 .../Test/Case/View/Helper/HtmlHelperTest.php  | 13 ++++++++++++
 lib/Cake/Test/Case/View/HelperTest.php        | 21 +++++++++++++++++--
 lib/Cake/View/Helper.php                      | 10 ++++-----
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/lib/Cake/Test/Case/View/Helper/HtmlHelperTest.php b/lib/Cake/Test/Case/View/Helper/HtmlHelperTest.php
index 60699d00f..922ab36a0 100644
--- a/lib/Cake/Test/Case/View/Helper/HtmlHelperTest.php
+++ b/lib/Cake/Test/Case/View/Helper/HtmlHelperTest.php
@@ -355,6 +355,9 @@ class HtmlHelperTest extends CakeTestCase {
 		$result = $this->Html->image('/test/view/1.gif');
 		$this->assertTags($result, array('img' => array('src' => '/test/view/1.gif', 'alt' => '')));
 
+		$result = $this->Html->image('test.gif?one=two&three=four');
+		$this->assertTags($result, array('img' => array('src' => 'img/test.gif?one=two&amp;three=four', 'alt' => '')));
+
 		$result = $this->Html->image('test.gif', array('fullBase' => true));
 		$here = $this->Html->url('/', true);
 		$this->assertTags($result, array('img' => array('src' => $here . 'img/test.gif', 'alt' => '')));
@@ -515,6 +518,10 @@ class HtmlHelperTest extends CakeTestCase {
 		$expected['link']['href'] = 'preg:/.*css\/screen\.css\?1234/';
 		$this->assertTags($result, $expected);
 
+		$result = $this->Html->css('screen.css?with=param&other=param');
+		$expected['link']['href'] = 'css/screen.css?with=param&amp;other=param';
+		$this->assertTags($result, $expected);
+
 		$result = $this->Html->css('http://whatever.com/screen.css?1234');
 		$expected['link']['href'] = 'preg:/http:\/\/.*\/screen\.css\?1234/';
 		$this->assertTags($result, $expected);
@@ -787,6 +794,12 @@ class HtmlHelperTest extends CakeTestCase {
 		);
 		$this->assertTags($result, $expected);
 
+		$result = $this->Html->script('test.json.js?foo=bar&other=test');
+		$expected = array(
+			'script' => array('type' => 'text/javascript', 'src' => 'js/test.json.js?foo=bar&amp;other=test')
+		);
+		$this->assertTags($result, $expected);
+
 		$result = $this->Html->script('foo');
 		$this->assertNull($result, 'Script returned upon duplicate inclusion %s');
 
diff --git a/lib/Cake/Test/Case/View/HelperTest.php b/lib/Cake/Test/Case/View/HelperTest.php
index 9205805a2..26daf5918 100644
--- a/lib/Cake/Test/Case/View/HelperTest.php
+++ b/lib/Cake/Test/Case/View/HelperTest.php
@@ -625,6 +625,17 @@ class HelperTest extends CakeTestCase {
 		$result = $this->Helper->assetUrl('style', array('ext' => '.css'));
 		$this->assertEqual('style.css', $result);
 
+		$result = $this->Helper->assetUrl('foo.jpg?one=two&three=four');
+		$this->assertEquals('foo.jpg?one=two&amp;three=four', $result);
+	}
+
+/**
+ * Test assetUrl with plugins.
+ *
+ * @return void
+ */
+	public function testAssetUrlPlugin() {
+		$this->Helper->webroot = '';
 		CakePlugin::load('TestPlugin');
 
 		$result = $this->Helper->assetUrl('TestPlugin.style', array('ext' => '.css'));
@@ -634,13 +645,19 @@ class HelperTest extends CakeTestCase {
 		$this->assertEqual('TestPlugin.style.css', $result);
 
 		CakePlugin::unload('TestPlugin');
+	}
 
+/**
+ * test assetUrl and Asset.timestamp = force
+ *
+ * @return void
+ */
+	public function testAssetUrlTimestampForce() {
+		$this->Helper->webroot = '';
 		Configure::write('Asset.timestamp', 'force');
 
 		$result = $this->Helper->assetUrl('cake.generic.css', array('pathPrefix' => CSS_URL));
 		$this->assertRegExp('/' . preg_quote(CSS_URL . 'cake.generic.css?', '/') . '[0-9]+/', $result);
-
-		Configure::write('Asset.timestamp', $_timestamp);
 	}
 
 /**
diff --git a/lib/Cake/View/Helper.php b/lib/Cake/View/Helper.php
index 284087e2c..f8f9d81d3 100644
--- a/lib/Cake/View/Helper.php
+++ b/lib/Cake/View/Helper.php
@@ -270,10 +270,10 @@ class Helper extends Object {
  *
  * @param string|array Path string or url array
  * @param array $options Options array. Possible keys:
- * 	`fullBase` Return full url with domain name
- * 	`pathPrefix` Path prefix for relative urls
- * 	`ext` Asset extension to append
- * 	`plugin` False value will prevent parsing path as a plugin
+ *   `fullBase` Return full url with domain name
+ *   `pathPrefix` Path prefix for relative urls
+ *   `ext` Asset extension to append
+ *   `plugin` False value will prevent parsing path as a plugin
  * @return string Generated url
  */
 	public function assetUrl($path, $options = array()) {
@@ -296,7 +296,7 @@ class Helper extends Object {
 			if (isset($plugin)) {
 				$path = Inflector::underscore($plugin) . '/' . $path;
 			}
-			$path = $this->assetTimestamp($this->webroot($path));
+			$path = h($this->assetTimestamp($this->webroot($path)));
 
 			if (!empty($options['fullBase'])) {
 				$path = $this->url('/', true) . $path;