From 8063b349c8715b341a4f1fdc1487203381e38f18 Mon Sep 17 00:00:00 2001
From: Sean Harvey <sean@silverstripe.com>
Date: Fri, 22 Aug 2014 14:45:44 +1200
Subject: [PATCH] BUG Fixing Director::test() failing on BASE_URL prefixed URLs

Example: you have a site in a sub-directory off the webroot, you call
->Link() on a SiteTree record, which returns "/[sitedir]/my-page", and
you pass this URL to Director::test(). It's a valid URL, but
Director::test() will throw a 404.

Director::test() should be ensuring that all URLs passed to it are
properly made relative, not just in the case where it thinks the URL
is absolute.
---
 control/Director.php                  |  5 ++++-
 dev/FunctionalTest.php                | 24 ++++++++++++++++++++++++
 tests/control/HTTPTest.php            | 15 ++-------------
 tests/control/RequestHandlingTest.php | 18 ++++++++++++++++--
 4 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/control/Director.php b/control/Director.php
index b0c7cea8f..6688e3a2a 100644
--- a/control/Director.php
+++ b/control/Director.php
@@ -255,9 +255,12 @@ class Director implements TemplateGlobalProvider {
 			} else {
 				$_SERVER['HTTP_HOST'] = $bits['host'];
 			}
-			$url = Director::makeRelative($url);
 		}
 
+		// Ensure URL is properly made relative.
+		// Example: url passed is "/ss31/my-page" (prefixed with BASE_URL), this should be changed to "my-page"
+		$url = self::makeRelative($url);
+
 		$urlWithQuerystring = $url;
 		if(strpos($url, '?') !== false) {
 			list($url, $getVarsEncoded) = explode('?', $url, 2);
diff --git a/dev/FunctionalTest.php b/dev/FunctionalTest.php
index 45a8ebc3d..885f5d425 100644
--- a/dev/FunctionalTest.php
+++ b/dev/FunctionalTest.php
@@ -110,6 +110,30 @@ class FunctionalTest extends SapphireTest {
 		}
 	}
 
+	/**
+	 * Run a test while mocking the base url with the provided value
+	 * @param string $url The base URL to use for this test
+	 * @param callable $callback The test to run
+	 */
+	protected function withBaseURL($url, $callback) {
+		$oldBase = Config::inst()->get('Director', 'alternate_base_url');
+		Config::inst()->update('Director', 'alternate_base_url', $url);
+		$callback($this);
+		Config::inst()->update('Director', 'alternate_base_url', $oldBase);
+	}
+
+	/**
+	 * Run a test while mocking the base folder with the provided value
+	 * @param string $folder The base folder to use for this test
+	 * @param callable $callback The test to run
+	 */
+	protected function withBaseFolder($folder, $callback) {
+		$oldFolder = Config::inst()->get('Director', 'alternate_base_folder');
+		Config::inst()->update('Director', 'alternate_base_folder', $folder);
+		$callback($this);
+		Config::inst()->update('Director', 'alternate_base_folder', $oldFolder);
+	}
+
 	/**
 	 * Submit a get request
 	 * @uses Director::test()
diff --git a/tests/control/HTTPTest.php b/tests/control/HTTPTest.php
index 2d6a08e34..7baaa5e40 100644
--- a/tests/control/HTTPTest.php
+++ b/tests/control/HTTPTest.php
@@ -5,7 +5,7 @@
  * @package framework
  * @subpackage tests
  */
-class HTTPTest extends SapphireTest {
+class HTTPTest extends FunctionalTest {
 	
 	/**
 	 * Tests {@link HTTP::getLinksIn()}
@@ -227,16 +227,5 @@ class HTTPTest extends SapphireTest {
 			);	
 		});
 	}
-	
-	/**
-	 * Run a test while mocking the base url with the provided value
-	 * @param string $url The base URL to use for this test
-	 * @param callable $callback The test to run
-	 */
-	protected function withBaseURL($url, $callback) {
-		$oldBase = Config::inst()->get('Director', 'alternate_base_url');
-		Config::inst()->update('Director', 'alternate_base_url', $url);
-		$callback($this);
-		Config::inst()->update('Director', 'alternate_base_url', $oldBase);
-	}
+
 }
diff --git a/tests/control/RequestHandlingTest.php b/tests/control/RequestHandlingTest.php
index 99e2989bc..f1ebb780a 100644
--- a/tests/control/RequestHandlingTest.php
+++ b/tests/control/RequestHandlingTest.php
@@ -60,10 +60,24 @@ class RequestHandlingTest extends FunctionalTest {
 		$this->assertEquals("MyField requested", $response->getBody());
 		
 		/* We can also make a POST request on a form field, which could be used for in-place editing, for example. */
-		$response = Director::test("testGoodBase1/TestForm/fields/MyField" ,array("MyField" => 5));
+		$response = Director::test("testGoodBase1/TestForm/fields/MyField", array("MyField" => 5));
 		$this->assertEquals("MyField posted, update to 5", $response->getBody());
 	}
-		
+
+	public function testBaseUrlPrefixed() {
+		$this->withBaseFolder('/silverstripe', function($test) {
+			$test->assertEquals(
+				'MyField requested',
+				Director::test('/silverstripe/testGoodBase1/TestForm/fields/MyField')->getBody()
+			);
+
+			$test->assertEquals(
+				'MyField posted, update to 5',
+				Director::test('/silverstripe/testGoodBase1/TestForm/fields/MyField', array('MyField' => 5))->getBody()
+			);
+		});
+	}
+
 	public function testBadBase() {
 		/* We no longer support using hacky attempting to handle URL parsing with broken rules */
 		$response = Director::test("testBadBase/method/1/2");