From 04050b27538b6c3050ea4b66a6664a23f618f3b6 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 22 Jan 2018 17:19:42 +1300 Subject: [PATCH] API Director::host() now formally includes port in host API Add Director::hostName() and Director::port() Fixes #7685 --- src/Control/Director.php | 89 +++++++++++---- tests/php/Control/DirectorTest.php | 177 ++++++++++++++++------------- 2 files changed, 167 insertions(+), 99 deletions(-) diff --git a/src/Control/Director.php b/src/Control/Director.php index 8af295743..4d6005dc9 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -237,13 +237,10 @@ class Director implements TemplateGlobalProvider $url = strtok($url, '#'); // Handle absolute URLs - if (parse_url($url, PHP_URL_HOST)) { - $bits = parse_url($url); - - // If a port is mentioned in the absolute URL, be sure to add that into the HTTP host - $newVars['_SERVER']['HTTP_HOST'] = isset($bits['port']) - ? $bits['host'] . ':' . $bits['port'] - : $bits['host']; + // If a port is mentioned in the absolute URL, be sure to add that into the HTTP host + $urlHostPort = static::parseHost($url); + if ($urlHostPort) { + $newVars['_SERVER']['HTTP_HOST'] = $urlHostPort; } // Ensure URL is properly made relative. @@ -448,6 +445,29 @@ class Director implements TemplateGlobalProvider return Controller::join_links($parent, $url); } + /** + * Return only host (and optional port) part of a url + * + * @param string $url + * @return string|null Hostname, and optional port, or null if not a valid host + */ + protected static function parseHost($url) + { + // Get base hostname + $host = parse_url($url, PHP_URL_HOST); + if (!$host) { + return null; + } + + // Include port + $port = parse_url($url, PHP_URL_PORT); + if ($port) { + $host .= ':' . $port; + } + + return $host; + } + /** * A helper to determine the current hostname used to access the site. * The following are used to determine the host (in order) @@ -459,14 +479,14 @@ class Director implements TemplateGlobalProvider * - gethostname() * * @param HTTPRequest $request - * @return string + * @return string Host name, including port (if present) */ public static function host(HTTPRequest $request = null) { // Check if overridden by alternate_base_url if ($baseURL = self::config()->get('alternate_base_url')) { $baseURL = Injector::inst()->convertServiceProperty($baseURL); - $host = parse_url($baseURL, PHP_URL_HOST); + $host = static::parseHost($baseURL); if ($host) { return $host; } @@ -485,7 +505,7 @@ class Director implements TemplateGlobalProvider // Check base url if ($baseURL = self::config()->uninherited('default_base_url')) { $baseURL = Injector::inst()->convertServiceProperty($baseURL); - $host = parse_url($baseURL, PHP_URL_HOST); + $host = static::parseHost($baseURL); if ($host) { return $host; } @@ -495,6 +515,32 @@ class Director implements TemplateGlobalProvider return isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : gethostname(); } + /** + * Return port used for the base URL. + * Note, this will be null if not specified, in which case you should assume the default + * port for the current protocol. + * + * @param HTTPRequest $request + * @return int|null + */ + public static function port(HTTPRequest $request = null) + { + $host = static::host($request); + return (int)parse_url($host, PHP_URL_PORT) ?: null; + } + + /** + * Return host name without port + * + * @param HTTPRequest|null $request + * @return string|null + */ + public static function hostName(HTTPRequest $request = null) + { + $host = static::host($request); + return parse_url($host, PHP_URL_HOST) ?: null; + } + /** * Returns the domain part of the URL 'http://www.mysite.com'. Returns FALSE is this environment * variable isn't set. @@ -760,13 +806,14 @@ class Director implements TemplateGlobalProvider */ public static function is_site_url($url) { - $urlHost = parse_url($url, PHP_URL_HOST); - $actualHost = parse_url(self::protocolAndHost(), PHP_URL_HOST); - if ($urlHost && $actualHost && $urlHost == $actualHost) { + // Validate host[:port] + $urlHost = static::parseHost($url); + if ($urlHost && $urlHost === static::host()) { return true; - } else { - return self::is_relative_url($url); } + + // Relative urls always are site urls + return self::is_relative_url($url); } /** @@ -831,10 +878,13 @@ class Director implements TemplateGlobalProvider */ public static function absoluteBaseURLWithAuth(HTTPRequest $request = null) { - $login = ""; - - if (isset($_SERVER['PHP_AUTH_USER'])) { - $login = "$_SERVER[PHP_AUTH_USER]:$_SERVER[PHP_AUTH_PW]@"; + // Detect basic auth + $user = $request->getHeader('PHP_AUTH_USER'); + if ($user) { + $password = $request->getHeader('PHP_AUTH_PW'); + $login = sprintf("%s:%s@", $user, $password) ; + } else { + $login = ''; } return Director::protocol($request) . $login . static::host($request) . Director::baseURL(); @@ -883,6 +933,7 @@ class Director implements TemplateGlobalProvider * * @param array $patterns Array of regex patterns to match URLs that should be HTTPS. * @param string $secureDomain Secure domain to redirect to. Defaults to the current domain. + * Can include port number. * @param HTTPRequest|null $request Request object to check */ public static function forceSSL($patterns = null, $secureDomain = null, HTTPRequest $request = null) diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index 3cd32386b..e2a4ff1e0 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -28,7 +28,7 @@ class DirectorTest extends SapphireTest protected function setUp() { parent::setUp(); - Director::config()->set('alternate_base_url', 'http://www.mysite.com/'); + Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/'); // Ensure redirects enabled on all environments CanonicalURLMiddleware::singleton()->setEnabledEnvs(true); @@ -76,53 +76,53 @@ class DirectorTest extends SapphireTest public function testAbsoluteURL() { - Director::config()->set('alternate_base_url', 'http://www.mysite.com/mysite/'); - $_SERVER['REQUEST_URI'] = "http://www.mysite.com/mysite/sub-page/"; + Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/mysite/'); + $_SERVER['REQUEST_URI'] = "http://www.mysite.com:9090/mysite/sub-page/"; //test empty / local urls foreach (array('', './', '.') as $url) { - $this->assertEquals("http://www.mysite.com/mysite/", Director::absoluteURL($url, Director::BASE)); - $this->assertEquals("http://www.mysite.com/", Director::absoluteURL($url, Director::ROOT)); - $this->assertEquals("http://www.mysite.com/mysite/sub-page/", Director::absoluteURL($url, Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/mysite/", Director::absoluteURL($url, Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL($url, Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/", Director::absoluteURL($url, Director::REQUEST)); } // Test site root url - $this->assertEquals("http://www.mysite.com/", Director::absoluteURL('/')); + $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL('/')); // Test Director::BASE - $this->assertEquals('http://www.mysite.com/', Director::absoluteURL('http://www.mysite.com/', Director::BASE)); + $this->assertEquals('http://www.mysite.com:9090/', Director::absoluteURL('http://www.mysite.com:9090/', Director::BASE)); $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::BASE)); - $this->assertEquals("http://www.mysite.com/test", Director::absoluteURL("http://www.mysite.com/test", Director::BASE)); - $this->assertEquals("http://www.mysite.com/root", Director::absoluteURL("/root", Director::BASE)); - $this->assertEquals("http://www.mysite.com/root/url", Director::absoluteURL("/root/url", Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("http://www.mysite.com:9090/test", Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/root/url", Director::absoluteURL("/root/url", Director::BASE)); // Test Director::ROOT - $this->assertEquals('http://www.mysite.com/', Director::absoluteURL('http://www.mysite.com/', Director::ROOT)); + $this->assertEquals('http://www.mysite.com:9090/', Director::absoluteURL('http://www.mysite.com:9090/', Director::ROOT)); $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::ROOT)); - $this->assertEquals("http://www.mysite.com/test", Director::absoluteURL("http://www.mysite.com/test", Director::ROOT)); - $this->assertEquals("http://www.mysite.com/root", Director::absoluteURL("/root", Director::ROOT)); - $this->assertEquals("http://www.mysite.com/root/url", Director::absoluteURL("/root/url", Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("http://www.mysite.com:9090/test", Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/root/url", Director::absoluteURL("/root/url", Director::ROOT)); // Test Director::REQUEST - $this->assertEquals('http://www.mysite.com/', Director::absoluteURL('http://www.mysite.com/', Director::REQUEST)); + $this->assertEquals('http://www.mysite.com:9090/', Director::absoluteURL('http://www.mysite.com:9090/', Director::REQUEST)); $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::REQUEST)); - $this->assertEquals("http://www.mysite.com/test", Director::absoluteURL("http://www.mysite.com/test", Director::REQUEST)); - $this->assertEquals("http://www.mysite.com/root", Director::absoluteURL("/root", Director::REQUEST)); - $this->assertEquals("http://www.mysite.com/root/url", Director::absoluteURL("/root/url", Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("http://www.mysite.com:9090/test", Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/root/url", Director::absoluteURL("/root/url", Director::REQUEST)); // Test evaluating relative urls relative to base (default) - $this->assertEquals("http://www.mysite.com/mysite/test", Director::absoluteURL("test")); - $this->assertEquals("http://www.mysite.com/mysite/test/url", Director::absoluteURL("test/url")); - $this->assertEquals("http://www.mysite.com/mysite/test", Director::absoluteURL("test", Director::BASE)); - $this->assertEquals("http://www.mysite.com/mysite/test/url", Director::absoluteURL("test/url", Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/mysite/test", Director::absoluteURL("test")); + $this->assertEquals("http://www.mysite.com:9090/mysite/test/url", Director::absoluteURL("test/url")); + $this->assertEquals("http://www.mysite.com:9090/mysite/test", Director::absoluteURL("test", Director::BASE)); + $this->assertEquals("http://www.mysite.com:9090/mysite/test/url", Director::absoluteURL("test/url", Director::BASE)); // Test evaluting relative urls relative to root - $this->assertEquals("http://www.mysite.com/test", Director::absoluteURL("test", Director::ROOT)); - $this->assertEquals("http://www.mysite.com/test/url", Director::absoluteURL("test/url", Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("test", Director::ROOT)); + $this->assertEquals("http://www.mysite.com:9090/test/url", Director::absoluteURL("test/url", Director::ROOT)); // Test relative to requested page - $this->assertEquals("http://www.mysite.com/mysite/sub-page/test", Director::absoluteURL("test", Director::REQUEST)); - $this->assertEquals("http://www.mysite.com/mysite/sub-page/test/url", Director::absoluteURL("test/url", Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/test", Director::absoluteURL("test", Director::REQUEST)); + $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/test/url", Director::absoluteURL("test/url", Director::REQUEST)); // Test that javascript links are not left intact $this->assertStringStartsNotWith('javascript', Director::absoluteURL('javascript:alert("attack")')); @@ -228,6 +228,13 @@ class DirectorTest extends SapphireTest $this->assertTrue(Director::is_absolute_url('//http://test.com')); } + public function testHostPortParts() + { + $this->assertEquals('www.mysite.com:9090', Director::host()); + $this->assertEquals('www.mysite.com', Director::hostName()); + $this->assertEquals('9090', Director::port()); + } + public function testIsRelativeUrl() { $this->assertFalse(Director::is_relative_url('http://test.com')); @@ -249,110 +256,110 @@ class DirectorTest extends SapphireTest return [ // Resilience to slash position [ - 'http://www.mysite.com/base/folder', - 'http://www.mysite.com/base/folder', + 'http://www.mysite.com:9090/base/folder', + 'http://www.mysite.com:9090/base/folder', '' ], [ - 'http://www.mysite.com/base/folder', - 'http://www.mysite.com/base/folder/', + 'http://www.mysite.com:9090/base/folder', + 'http://www.mysite.com:9090/base/folder/', '' ], [ - 'http://www.mysite.com/base/folder/', - 'http://www.mysite.com/base/folder', + 'http://www.mysite.com:9090/base/folder/', + 'http://www.mysite.com:9090/base/folder', '' ], [ - 'http://www.mysite.com/', - 'http://www.mysite.com/', + 'http://www.mysite.com:9090/', + 'http://www.mysite.com:9090/', '' ], [ - 'http://www.mysite.com/', + 'http://www.mysite.com:9090/', 'http://www.mysite.com', '' ], [ 'http://www.mysite.com', - 'http://www.mysite.com/', + 'http://www.mysite.com:9090/', '' ], [ - 'http://www.mysite.com/base/folder', - 'http://www.mysite.com/base/folder/page', + 'http://www.mysite.com:9090/base/folder', + 'http://www.mysite.com:9090/base/folder/page', 'page' ], [ - 'http://www.mysite.com/', - 'http://www.mysite.com/page/', + 'http://www.mysite.com:9090/', + 'http://www.mysite.com:9090/page/', 'page/' ], // Parsing protocol safely [ - 'http://www.mysite.com/base/folder', - 'https://www.mysite.com/base/folder', + 'http://www.mysite.com:9090/base/folder', + 'https://www.mysite.com:9090/base/folder', '' ], [ - 'https://www.mysite.com/base/folder', - 'http://www.mysite.com/base/folder/testpage', + 'https://www.mysite.com:9090/base/folder', + 'http://www.mysite.com:9090/base/folder/testpage', 'testpage' ], [ - 'http://www.mysite.com/base/folder', - '//www.mysite.com/base/folder/testpage', + 'http://www.mysite.com:9090/base/folder', + '//www.mysite.com:9090/base/folder/testpage', 'testpage' ], // Dirty input [ - 'http://www.mysite.com/base/folder', - ' https://www.mysite.com/base/folder/testpage ', + 'http://www.mysite.com:9090/base/folder', + ' https://www.mysite.com:9090/base/folder/testpage ', 'testpage' ], [ - 'http://www.mysite.com/base/folder', - '//www.mysite.com/base//folder/testpage//subpage', + 'http://www.mysite.com:9090/base/folder', + '//www.mysite.com:9090/base//folder/testpage//subpage', 'testpage/subpage' ], // Non-http protocol isn't modified [ - 'http://www.mysite.com/base/folder', + 'http://www.mysite.com:9090/base/folder', 'ftp://test.com', 'ftp://test.com' ], // Alternate hostnames are redirected [ - 'https://www.mysite.com/base/folder', - 'http://mysite.com/base/folder/testpage', + 'https://www.mysite.com:9090/base/folder', + 'http://mysite.com:9090/base/folder/testpage', 'testpage' ], [ 'http://www.otherdomain.com/base/folder', - '//www.mysite.com/base/folder/testpage', + '//www.mysite.com:9090/base/folder/testpage', 'testpage' ], // Base folder is found [ - 'http://www.mysite.com/base/folder', + 'http://www.mysite.com:9090/base/folder', BASE_PATH . '/some/file.txt', 'some/file.txt', ], // public folder is found [ - 'http://www.mysite.com/base/folder', + 'http://www.mysite.com:9090/base/folder', PUBLIC_PATH . '/some/file.txt', 'some/file.txt', ], // querystring is protected [ - 'http://www.mysite.com/base/folder', - '//www.mysite.com/base//folder/testpage//subpage?args=hello', + 'http://www.mysite.com:9090/base/folder', + '//www.mysite.com:9090/base//folder/testpage//subpage?args=hello', 'testpage/subpage?args=hello' ], [ - 'http://www.mysite.com/base/folder', - '//www.mysite.com/base//folder/?args=hello', + 'http://www.mysite.com:9090/base/folder', + '//www.mysite.com:9090/base//folder/?args=hello', '?args=hello' ], ]; @@ -528,11 +535,11 @@ class DirectorTest extends SapphireTest public function testForceWWW() { - $this->expectExceptionRedirect('http://www.mysite.com/some-url'); + $this->expectExceptionRedirect('http://www.mysite.com:9090/some-url'); Director::mockRequest(function ($request) { Injector::inst()->registerService($request, HTTPRequest::class); Director::forceWWW(); - }, 'http://mysite.com/some-url'); + }, 'http://mysite.com:9090/some-url'); } public function testPromisedForceWWW() @@ -549,20 +556,20 @@ class DirectorTest extends SapphireTest return $middleware->process($request, function ($request) { return null; }); - }, 'http://mysite.com/some-url'); + }, 'http://mysite.com:9090/some-url'); // Middleware returns non-exception redirect - $this->assertEquals('http://www.mysite.com/some-url', $response->getHeader('Location')); + $this->assertEquals('http://www.mysite.com:9090/some-url', $response->getHeader('Location')); $this->assertEquals(301, $response->getStatusCode()); } public function testForceSSLProtectsEntireSite() { - $this->expectExceptionRedirect('https://www.mysite.com/some-url'); + $this->expectExceptionRedirect('https://www.mysite.com:9090/some-url'); Director::mockRequest(function ($request) { Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(); - }, 'http://www.mysite.com/some-url'); + }, 'http://www.mysite.com:9090/some-url'); } public function testPromisedForceSSL() @@ -579,31 +586,31 @@ class DirectorTest extends SapphireTest return $middleware->process($request, function ($request) { return null; }); - }, 'http://www.mysite.com/some-url'); + }, 'http://www.mysite.com:9090/some-url'); // Middleware returns non-exception redirect - $this->assertEquals('https://www.mysite.com/some-url', $response->getHeader('Location')); + $this->assertEquals('https://www.mysite.com:9090/some-url', $response->getHeader('Location')); $this->assertEquals(301, $response->getStatusCode()); } public function testForceSSLOnTopLevelPagePattern() { // Expect admin to trigger redirect - $this->expectExceptionRedirect('https://www.mysite.com/admin'); + $this->expectExceptionRedirect('https://www.mysite.com:9090/admin'); Director::mockRequest(function (HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(array('/^admin/')); - }, 'http://www.mysite.com/admin'); + }, 'http://www.mysite.com:9090/admin'); } public function testForceSSLOnSubPagesPattern() { // Expect to redirect to security login page - $this->expectExceptionRedirect('https://www.mysite.com/Security/login'); + $this->expectExceptionRedirect('https://www.mysite.com:9090/Security/login'); Director::mockRequest(function (HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(array('/^Security/')); - }, 'http://www.mysite.com/Security/login'); + }, 'http://www.mysite.com:9090/Security/login'); } public function testForceSSLWithPatternDoesNotMatchOtherPages() @@ -612,14 +619,14 @@ class DirectorTest extends SapphireTest $response = Director::mockRequest(function (HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(array('/^admin/')); - }, 'http://www.mysite.com/normal-page'); + }, 'http://www.mysite.com:9090/normal-page'); $this->assertNull($response, 'Non-matching patterns do not trigger redirect'); // nested url should not triger redirect either $response = Director::mockRequest(function (HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); Director::forceSSL(array('/^admin/', '/^Security/')); - }, 'http://www.mysite.com/just-another-page/sub-url'); + }, 'http://www.mysite.com:9090/just-another-page/sub-url'); $this->assertNull($response, 'Non-matching patterns do not trigger redirect'); } @@ -630,7 +637,17 @@ class DirectorTest extends SapphireTest Director::mockRequest(function (HTTPRequest $request) { Injector::inst()->registerService($request, HTTPRequest::class); return Director::forceSSL(array('/^admin/'), 'secure.mysite.com'); - }, 'http://www.mysite.com/admin'); + }, 'http://www.mysite.com:9090/admin'); + } + + public function testForceSSLAlternateDomainWithPort() + { + // Ensure that forceSSL throws the appropriate exception + $this->expectExceptionRedirect('https://secure.mysite.com:81/admin'); + Director::mockRequest(function (HTTPRequest $request) { + Injector::inst()->registerService($request, HTTPRequest::class); + return Director::forceSSL(array('/^admin/'), 'secure.mysite.com:81'); + }, 'http://www.mysite.com:9090/admin'); } /** @@ -652,10 +669,10 @@ class DirectorTest extends SapphireTest return $middleware->process($request, function ($request) { return null; }); - }, 'http://mysite.com/some-url'); + }, 'http://mysite.com:9090/some-url'); // Middleware returns non-exception redirect - $this->assertEquals('https://www.mysite.com/some-url', $response->getHeader('Location')); + $this->assertEquals('https://www.mysite.com:9090/some-url', $response->getHeader('Location')); $this->assertEquals(301, $response->getStatusCode()); } @@ -938,7 +955,7 @@ class DirectorTest extends SapphireTest public function testMockRequest() { - Director::config()->set('alternate_base_url', 'http://www.mysite.com/some-subdir/'); + Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/some-subdir/'); // Can handle root-relative $url Director::mockRequest(function (HTTPRequest $request) { @@ -952,7 +969,7 @@ class DirectorTest extends SapphireTest $this->assertEquals('some-page/nested', $request->getURL()); $this->assertEquals(1, $request->getVar('query')); $this->assertEquals('/some-subdir/some-page/nested', $_SERVER['REQUEST_URI']); - }, 'http://www.mysite.com/some-subdir/some-page/nested?query=1'); + }, 'http://www.mysite.com:9090/some-subdir/some-page/nested?query=1'); // Can handle relative $url Director::mockRequest(function (HTTPRequest $request) {