From aa7d2449d9573134fde707a671fe6165bbd1f5df Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Mon, 4 Oct 2010 04:16:47 +0000
Subject: [PATCH] BUGFIX Produce XHTML compliant URLs in HTTP::setGetVar() by
 default (regression from r98373, see #5101) (from r100904)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@111538 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/HTTP.php      | 6 +++++-
 tests/HTTPTest.php | 6 +++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/core/HTTP.php b/core/HTTP.php
index b2194cd44..a5fca604e 100644
--- a/core/HTTP.php
+++ b/core/HTTP.php
@@ -77,6 +77,9 @@ class HTTP {
 	 * Will try to include a GET parameter for an existing URL,
 	 * preserving existing parameters and fragments.
 	 * If no URL is given, falls back to $_SERVER['REQUEST_URI'].
+	 * Uses parse_url() to dissect the URL, and http_build_query() to reconstruct it
+	 * with the additional parameter. Converts any '&' (ampersand)
+	 * URL parameter separators to the more XHTML compliant '&amp;'.
 	 * 
 	 * CAUTION: If the URL is determined to be relative,
 	 * it is prepended with Director::absoluteBaseURL().
@@ -117,7 +120,8 @@ class HTTP {
 				: ''
 			) . (
 				($params)
-				? '?' . http_build_query($params)
+				// XHTML compliant by default
+				? '?' . http_build_query($params, null, '&amp;')
 				: ''
 			) . (
 				isset($parts['fragment']) && $parts['fragment'] != ''
diff --git a/tests/HTTPTest.php b/tests/HTTPTest.php
index 60d761ca6..210bc78c9 100644
--- a/tests/HTTPTest.php
+++ b/tests/HTTPTest.php
@@ -71,7 +71,7 @@ class HTTPTest extends SapphireTest {
 			);
 		}
 		
-		foreach(array($expectedBasePath, '/relative/url?baz=buz&foo=bar') as $e) {
+		foreach(array($expectedBasePath, '/relative/url?baz=buz&amp;foo=bar') as $e) {
 			$this->assertContains(
 				$e,
 				HTTP::setGetVar('foo', 'bar', '/relative/url?baz=buz'),
@@ -80,7 +80,7 @@ class HTTPTest extends SapphireTest {
 		}
 
 		$this->assertEquals(
-			'http://test.com/?foo=new&buz=baz',
+			'http://test.com/?foo=new&amp;buz=baz',
 			HTTP::setGetVar('foo', 'new', 'http://test.com/?foo=old&buz=baz'),
 			'Absolute URL without path and multipe existing query params, overwriting an existing parameter'
 		);
@@ -93,7 +93,7 @@ class HTTPTest extends SapphireTest {
 
 		$this->assertEquals(
 			// http_build_query() escapes angular brackets, they should be correctly urldecoded by the browser client
-			'http://test.com/?foo%5Btest%5D=one&foo%5Btest%5D=two',
+			'http://test.com/?foo%5Btest%5D=one&amp;foo%5Btest%5D=two',
 			HTTP::setGetVar('foo[test]', 'two', 'http://test.com/?foo[test]=one'),
 			'Absolute URL and PHP array query string notation'
 		);