From 0a8f328947bc3015e3c545f00b4c3e323f87d920 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 28 May 2015 10:55:18 +1200 Subject: [PATCH] Fix merge / test regressions --- control/Controller.php | 2 +- forms/Form.php | 2 +- tests/control/ControllerTest.php | 9 ++++--- tests/control/DirectorTest.php | 3 +++ .../ParameterConfirmationTokenTest.php | 4 +++ tests/security/SecurityTest.php | 25 +++++++++++++++---- thirdparty/simpletest/url.php | 7 ++++-- 7 files changed, 40 insertions(+), 12 deletions(-) diff --git a/control/Controller.php b/control/Controller.php index 32d876990..12aeb09ec 100644 --- a/control/Controller.php +++ b/control/Controller.php @@ -509,7 +509,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { // absolute redirection URLs not located on this site may cause phishing if(Director::is_site_url($url)) { - $url = Director::absoluteURL($url); + $url = Director::absoluteURL($url, true); return $this->redirect($url); } else { return false; diff --git a/forms/Form.php b/forms/Form.php index c872c1d26..0826d2f17 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -432,7 +432,7 @@ class Form extends RequestHandler { if(Director::is_site_url($pageURL)) { // Remove existing pragmas $pageURL = preg_replace('/(#.*)/', '', $pageURL); - $pageURL = Director::absoluteURL($pageURL); + $pageURL = Director::absoluteURL($pageURL, true); return $this->controller->redirect($pageURL . '#' . $this->FormName()); } } diff --git a/tests/control/ControllerTest.php b/tests/control/ControllerTest.php index a50be5af7..3a8427142 100644 --- a/tests/control/ControllerTest.php +++ b/tests/control/ControllerTest.php @@ -329,14 +329,15 @@ class ControllerTest extends FunctionalTest { public function testRedirectBackByReferer() { $internalRelativeUrl = '/some-url'; + $internalAbsoluteUrl = Controller::join_links(Director::absoluteBaseURL(), '/some-url'); + $response = $this->get('ControllerTest_Controller/redirectbacktest', null, array('Referer' => $internalRelativeUrl)); $this->assertEquals(302, $response->getStatusCode()); - $this->assertEquals($internalRelativeUrl, $response->getHeader('Location'), + $this->assertEquals($internalAbsoluteUrl, $response->getHeader('Location'), "Redirects on internal relative URLs" ); - $internalAbsoluteUrl = Director::absoluteBaseURL() . '/some-url'; $response = $this->get('ControllerTest_Controller/redirectbacktest', null, array('Referer' => $internalAbsoluteUrl)); $this->assertEquals(302, $response->getStatusCode()); @@ -354,9 +355,11 @@ class ControllerTest extends FunctionalTest { public function testRedirectBackByBackUrl() { $internalRelativeUrl = '/some-url'; + $internalAbsoluteUrl = Controller::join_links(Director::absoluteBaseURL(), '/some-url'); + $response = $this->get('ControllerTest_Controller/redirectbacktest?BackURL=' . urlencode($internalRelativeUrl)); $this->assertEquals(302, $response->getStatusCode()); - $this->assertEquals($internalRelativeUrl, $response->getHeader('Location'), + $this->assertEquals($internalAbsoluteUrl, $response->getHeader('Location'), "Redirects on internal relative URLs" ); diff --git a/tests/control/DirectorTest.php b/tests/control/DirectorTest.php index 14da5d970..55a271879 100644 --- a/tests/control/DirectorTest.php +++ b/tests/control/DirectorTest.php @@ -402,6 +402,9 @@ class DirectorTest extends SapphireTest { } public function testIsHttps() { + if(!TRUSTED_PROXY) { + $this->markTestSkipped('Test cannot be run without trusted proxy'); + } // nothing available $headers = array( 'HTTP_X_FORWARDED_PROTOCOL', 'HTTPS', 'SSL' diff --git a/tests/core/startup/ParameterConfirmationTokenTest.php b/tests/core/startup/ParameterConfirmationTokenTest.php index eb4d034ed..cffa14cbc 100644 --- a/tests/core/startup/ParameterConfirmationTokenTest.php +++ b/tests/core/startup/ParameterConfirmationTokenTest.php @@ -31,9 +31,12 @@ class ParameterConfirmationTokenTest extends SapphireTest { return array($answer, $slash); } + + protected $oldHost = null; public function setUp() { parent::setUp(); + $this->oldHost = $_SERVER['HTTP_HOST']; $_GET['parameterconfirmationtokentest_notoken'] = 'value'; $_GET['parameterconfirmationtokentest_empty'] = ''; $_GET['parameterconfirmationtokentest_withtoken'] = '1'; @@ -48,6 +51,7 @@ class ParameterConfirmationTokenTest extends SapphireTest { foreach($_GET as $param) { if(stripos($param, 'parameterconfirmationtokentest_') === 0) unset($_GET[$param]); } + $_SERVER['HTTP_HOST'] = $this->oldHost; parent::tearDown(); } diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 91e842f55..f9f564847 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -248,13 +248,19 @@ class SecurityTest extends FunctionalTest { /* UNEXPIRED PASSWORD GO THROUGH WITHOUT A HITCH */ $goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , '1nitialPassword'); $this->assertEquals(302, $goodResponse->getStatusCode()); - $this->assertEquals(Director::baseURL() . 'test/link', $goodResponse->getHeader('Location')); + $this->assertEquals( + Controller::join_links(Director::absoluteBaseURL(), 'test/link'), + $goodResponse->getHeader('Location') + ); $this->assertEquals($this->idFromFixture('Member', 'test'), $this->session()->inst_get('loggedInAs')); /* EXPIRED PASSWORDS ARE SENT TO THE CHANGE PASSWORD FORM */ $expiredResponse = $this->doTestLoginForm('expired@silverstripe.com' , '1nitialPassword'); $this->assertEquals(302, $expiredResponse->getStatusCode()); - $this->assertEquals(Director::baseURL() . 'Security/changepassword', $expiredResponse->getHeader('Location')); + $this->assertEquals( + Controller::join_links(Director::baseURL(), 'Security/changepassword'), + $expiredResponse->getHeader('Location') + ); $this->assertEquals($this->idFromFixture('Member', 'expiredpassword'), $this->session()->inst_get('loggedInAs')); @@ -262,7 +268,10 @@ class SecurityTest extends FunctionalTest { $this->mainSession->followRedirection(); $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword'); $this->assertEquals(302, $changedResponse->getStatusCode()); - $this->assertEquals(Director::baseURL() . 'test/link', $changedResponse->getHeader('Location')); + $this->assertEquals( + Controller::join_links(Director::absoluteBaseURL(), 'test/link'), + $changedResponse->getHeader('Location') + ); } public function testChangePasswordForLoggedInUsers() { @@ -272,13 +281,19 @@ class SecurityTest extends FunctionalTest { $this->get('Security/changepassword?BackURL=test/back'); $changedResponse = $this->doTestChangepasswordForm('1nitialPassword', 'changedPassword'); $this->assertEquals(302, $changedResponse->getStatusCode()); - $this->assertEquals(Director::baseURL() . 'test/back', $changedResponse->getHeader('Location')); + $this->assertEquals( + Controller::join_links(Director::absoluteBaseURL(), 'test/back'), + $changedResponse->getHeader('Location') + ); $this->assertEquals($this->idFromFixture('Member', 'test'), $this->session()->inst_get('loggedInAs')); // Check if we can login with the new password $goodResponse = $this->doTestLoginForm('sam@silverstripe.com' , 'changedPassword'); $this->assertEquals(302, $goodResponse->getStatusCode()); - $this->assertEquals(Director::baseURL() . 'test/link', $goodResponse->getHeader('Location')); + $this->assertEquals( + Controller::join_links(Director::absoluteBaseURL(), 'test/link'), + $goodResponse->getHeader('Location') + ); $this->assertEquals($this->idFromFixture('Member', 'test'), $this->session()->inst_get('loggedInAs')); } diff --git a/thirdparty/simpletest/url.php b/thirdparty/simpletest/url.php index d3461657a..cdd725079 100644 --- a/thirdparty/simpletest/url.php +++ b/thirdparty/simpletest/url.php @@ -410,7 +410,7 @@ class SimpleUrl { */ function asString() { $path = $this->_path; - $scheme = $identity = $host = $encoded = $fragment = ''; + $scheme = $identity = $host = $port = $encoded = $fragment = ''; if ($this->_username && $this->_password) { $identity = $this->_username . ':' . $this->_password . '@'; } @@ -419,13 +419,16 @@ class SimpleUrl { $scheme .= "://"; $host = $this->getHost(); } + if ($this->getPort() && $this->getPort() != 80 ) { + $port = ':'.$this->getPort(); + } if (substr($this->_path, 0, 1) == '/') { $path = $this->normalisePath($this->_path); } $encoded = $this->getEncodedRequest(); $fragment = $this->getFragment() ? '#'. $this->getFragment() : ''; $coords = $this->getX() === false ? '' : '?' . $this->getX() . ',' . $this->getY(); - return "$scheme$identity$host$path$encoded$fragment$coords"; + return "$scheme$identity$host$port$path$encoded$fragment$coords"; } /**