From e0505406a753b7e9686ccae2a706a74a43ad3579 Mon Sep 17 00:00:00 2001
From: Simon Welsh <simon@simon.geek.nz>
Date: Fri, 29 Jun 2012 17:14:28 +1200
Subject: [PATCH] FIX Director::is_absolute_url() now ignores query and
 fragment strings

Director::is_absolute_url() checks for //. It used to include the
entire URI, now it ignores the query and fragment strings.
---
 control/Director.php           | 35 +++++++++++++++++++---------------
 tests/control/DirectorTest.php |  5 ++++-
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/control/Director.php b/control/Director.php
index f3540abe1..be0785b3c 100644
--- a/control/Director.php
+++ b/control/Director.php
@@ -548,24 +548,29 @@ class Director implements TemplateGlobalProvider {
 	 * @return boolean
 	 */
 	public static function is_absolute_url($url) {
+		// Strip off the query and fragment parts of the URL before checking
+		if(($queryPosition = strpos($url, '?')) !== false) {
+			$url = substr($url, 0, $queryPosition-1);
+		}
+		if(($hashPosition = strpos($url, '#')) !== false) {
+			$url = substr($url, 0, $hashPosition-1);
+		}
 		$colonPosition = strpos($url, ':');
-	  return (
-	  	// Base check for existence of a host on a compliant URL
-	  	parse_url($url, PHP_URL_HOST)
-	  	// Check for more than one leading slash without a protocol.
+		$slashPosition = strpos($url, '/');
+		return (
+			// Base check for existence of a host on a compliant URL
+			parse_url($url, PHP_URL_HOST)
+			// Check for more than one leading slash without a protocol.
 			// While not a RFC compliant absolute URL, it is completed to a valid URL by some browsers,
 			// and hence a potential security risk. Single leading slashes are not an issue though.
-	  	|| preg_match('/\s*[\/]{2,}/', $url)
-	  	|| (
-	  		// If a colon is found, check if it's part of a valid scheme definition
-		  	// (meaning its not preceded by a slash, hash or questionmark).
-		  	// URLs in query parameters are assumed to be correctly urlencoded based on RFC3986,
-		  	// in which case no colon should be present in the parameters.
-	  		$colonPosition !== FALSE 
-	  		&& !preg_match('![/?#]!', substr($url, 0, $colonPosition))
-	  	)
-	  	
-	  );
+			|| preg_match('/\s*[\/]{2,}/', $url)
+			|| (
+				// If a colon is found, check if it's part of a valid scheme definition
+				// (meaning its not preceded by a slash).
+				$colonPosition !== FALSE 
+				&& ($slashPosition === FALSE || $colonPosition < $slashPosition)
+			)
+		);
 	}
 	
 	/**
diff --git a/tests/control/DirectorTest.php b/tests/control/DirectorTest.php
index 3d68179f3..a176b0195 100644
--- a/tests/control/DirectorTest.php
+++ b/tests/control/DirectorTest.php
@@ -98,6 +98,8 @@ class DirectorTest extends SapphireTest {
 		$this->assertFalse(Director::is_absolute_url('test.com/testpage'));
 		$this->assertFalse(Director::is_absolute_url('/relative'));
 		$this->assertFalse(Director::is_absolute_url('relative'));
+		$this->assertFalse(Director::is_absolute_url("/relative/?url=http://foo.com"));
+		$this->assertFalse(Director::is_absolute_url("/relative/#http://foo.com"));
 		$this->assertTrue(Director::is_absolute_url("https://test.com/?url=http://foo.com"));
 		$this->assertTrue(Director::is_absolute_url("trickparseurl:http://test.com"));
 		$this->assertTrue(Director::is_absolute_url('//test.com'));
@@ -116,7 +118,8 @@ class DirectorTest extends SapphireTest {
 		$this->assertFalse(Director::is_relative_url('ftp://test.com'));
 		$this->assertTrue(Director::is_relative_url('/relative'));
 		$this->assertTrue(Director::is_relative_url('relative'));
-		// $this->assertTrue(Director::is_relative_url('/relative/?url=http://test.com'));
+		$this->assertTrue(Director::is_relative_url('/relative/?url=http://test.com'));
+		$this->assertTrue(Director::is_relative_url('/relative/#=http://test.com'));
 	}
 	
 	public function testMakeRelative() {