From 06364af3c379c931889c4cc34dd920fee3db204a Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 19 Jun 2017 16:59:34 +1200 Subject: [PATCH] Fix controller namespace Move states to sub namespace --- _config/tests.yml | 11 +- src/Control/Director.php | 46 ++- src/Core/CoreKernel.php | 2 +- src/Dev/FunctionalTest.php | 6 +- src/Dev/SapphireTest.php | 2 + src/Dev/{ => State}/ExtensionTestState.php | 11 +- src/Dev/{ => State}/FlushableTestState.php | 3 +- src/Dev/State/GlobalsTestState.php | 44 +++ src/Dev/{ => State}/KernelTestState.php | 3 +- src/Dev/{ => State}/SapphireTestState.php | 4 +- src/Dev/{ => State}/TestState.php | 5 +- src/View/Dev/RequirementsTestState.php | 2 +- tests/php/Control/ControllerTest.php | 19 +- .../Control/ControllerTest/SubController.php | 6 +- tests/php/Control/DirectorTest.php | 275 +++++++++--------- .../Control/DirectorTest/TestController.php | 20 +- tests/php/Control/FlushRequestFilterTest.php | 2 - .../FlushRequestFilterTest/TestFlushable.php | 1 - tests/php/Control/SessionTest.php | 11 +- 19 files changed, 284 insertions(+), 189 deletions(-) rename src/Dev/{ => State}/ExtensionTestState.php (93%) rename src/Dev/{ => State}/FlushableTestState.php (93%) create mode 100644 src/Dev/State/GlobalsTestState.php rename src/Dev/{ => State}/KernelTestState.php (96%) rename src/Dev/{ => State}/SapphireTestState.php (93%) rename src/Dev/{ => State}/TestState.php (88%) diff --git a/_config/tests.yml b/_config/tests.yml index 91f3a6a60..439f76a2e 100644 --- a/_config/tests.yml +++ b/_config/tests.yml @@ -2,18 +2,19 @@ Name: sapphiretest --- SilverStripe\Core\Injector\Injector: - SilverStripe\Dev\SapphireTestState: + SilverStripe\Dev\State\SapphireTestState: properties: States: - extensions: %$SilverStripe\Dev\ExtensionTestState - flushable: %$SilverStripe\Dev\FlushableTestState + globals: %$SilverStripe\Dev\State\GlobalsTestState + extensions: %$SilverStripe\Dev\State\ExtensionTestState + flushable: %$SilverStripe\Dev\State\FlushableTestState requirements: %$SilverStripe\View\Dev\RequirementsTestState --- Name: kerneltest Before: '*' --- SilverStripe\Core\Injector\Injector: - SilverStripe\Dev\SapphireTestState: + SilverStripe\Dev\State\SapphireTestState: properties: States: - kernel: %$SilverStripe\Dev\KernelTestState + kernel: %$SilverStripe\Dev\State\KernelTestState diff --git a/src/Control/Director.php b/src/Control/Director.php index e7dbf651c..d0768d18d 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -176,6 +176,49 @@ class Director implements TemplateGlobalProvider $headers = array(), $cookies = array(), &$request = null + ) { + return static::mockRequest( + function (HTTPRequest $request) { + return static::direct($request); + }, + $url, + $postVars, + $session, + $httpMethod, + $body, + $headers, + $cookies, + $request + ); + } + + /** + * Mock a request, passing this to the given callback, before resetting. + * + * @param callable $callback Action to pass the HTTPRequst object + * @param string $url The URL to build + * @param array $postVars The $_POST & $_FILES variables. + * @param array|Session $session The {@link Session} object representing the current session. + * By passing the same object to multiple calls of Director::test(), you can simulate a persisted + * session. + * @param string $httpMethod The HTTP method, such as GET or POST. It will default to POST if + * postVars is set, GET otherwise. Overwritten by $postVars['_method'] if present. + * @param string $body The HTTP body. + * @param array $headers HTTP headers with key-value pairs. + * @param array|Cookie_Backend $cookies to populate $_COOKIE. + * @param HTTPRequest $request The {@see SS_HTTP_Request} object generated as a part of this request. + * @return mixed Result of callback + */ + public static function mockRequest( + $callback, + $url, + $postVars = [], + $session = [], + $httpMethod = null, + $body = null, + $headers = [], + $cookies = [], + &$request = null ) { // Build list of cleanup promises $finally = []; @@ -261,6 +304,7 @@ class Director implements TemplateGlobalProvider $newVars['_GET'] = []; } $newVars['_SERVER']['REQUEST_URI'] = Director::baseURL() . $url; + $newVars['_REQUEST'] = array_merge($newVars['_GET'], $newVars['_POST']); // Create new request $request = HTTPRequest::createFromVariables($newVars, $body); @@ -275,7 +319,7 @@ class Director implements TemplateGlobalProvider try { // Normal request handling - return static::direct($request); + return call_user_func($callback, $request); } finally { // Restore state in reverse order to assignment foreach (array_reverse($finally) as $callback) { diff --git a/src/Core/CoreKernel.php b/src/Core/CoreKernel.php index 8fbae8bb3..7b73fb7b4 100644 --- a/src/Core/CoreKernel.php +++ b/src/Core/CoreKernel.php @@ -132,7 +132,7 @@ class CoreKernel implements Kernel public function setEnvironment($environment) { - if (!in_array($environment, [self::DEV, self::TEST, self::LIVE])) { + if (!in_array($environment, [self::DEV, self::TEST, self::LIVE, null])) { throw new InvalidArgumentException( "Director::set_environment_type passed '$environment'. It should be passed dev, test, or live" ); diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php index d27193896..8e7cd4d7c 100644 --- a/src/Dev/FunctionalTest.php +++ b/src/Dev/FunctionalTest.php @@ -85,12 +85,13 @@ class FunctionalTest extends SapphireTest protected function setUp() { + parent::setUp(); + // Skip calling FunctionalTest directly. if (static::class == __CLASS__) { $this->markTestSkipped(sprintf('Skipping %s ', static::class)); } - parent::setUp(); $this->mainSession = new TestSession(); // Disable theme, if necessary @@ -115,9 +116,8 @@ class FunctionalTest extends SapphireTest protected function tearDown() { SecurityToken::enable(); - - parent::tearDown(); unset($this->mainSession); + parent::tearDown(); } /** diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index 957eabac5..5e796fa41 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -20,6 +20,8 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Injector\InjectorLoader; use SilverStripe\Core\Manifest\ClassLoader; use SilverStripe\Core\TestKernel; +use SilverStripe\Dev\State\SapphireTestState; +use SilverStripe\Dev\State\TestState; use SilverStripe\i18n\i18n; use SilverStripe\ORM\DataExtension; use SilverStripe\ORM\DataObject; diff --git a/src/Dev/ExtensionTestState.php b/src/Dev/State/ExtensionTestState.php similarity index 93% rename from src/Dev/ExtensionTestState.php rename to src/Dev/State/ExtensionTestState.php index eefe8d484..cf3fca16f 100644 --- a/src/Dev/ExtensionTestState.php +++ b/src/Dev/State/ExtensionTestState.php @@ -1,10 +1,12 @@ extensionsToReapply || $this->extensionsToRemove; + $isAltered = false; + $this->extensionsToReapply = []; + $this->extensionsToRemove = []; /** @var string|SapphireTest $class */ /** @var string|DataObject $dataClass */ @@ -68,7 +72,6 @@ class ExtensionTestState implements TestState if (!class_exists($dataClass)) { throw new LogicException("Test {$class} requires dataClass {$dataClass} which doesn't exist"); } - $this->extensionsToRemove[$dataClass] = array(); foreach ($extensions as $extension) { $extension = Extension::get_classname_without_arguments($extension); if (!class_exists($extension)) { @@ -76,7 +79,7 @@ class ExtensionTestState implements TestState } if (!$dataClass::has_extension($extension)) { if (!isset($this->extensionsToRemove[$dataClass])) { - $this->extensionsToReapply[$dataClass] = []; + $this->extensionsToRemove[$dataClass] = []; } $this->extensionsToRemove[$dataClass][] = $extension; $dataClass::add_extension($extension); diff --git a/src/Dev/FlushableTestState.php b/src/Dev/State/FlushableTestState.php similarity index 93% rename from src/Dev/FlushableTestState.php rename to src/Dev/State/FlushableTestState.php index f1eaaaf2c..23ed07f3f 100644 --- a/src/Dev/FlushableTestState.php +++ b/src/Dev/State/FlushableTestState.php @@ -1,10 +1,11 @@ vars = Director::envToVars(); + } + + public function tearDown(SapphireTest $test) + { + Director::varsToEnv($this->vars); + } + + public function setUpOnce($class) + { + $this->staticVars = Director::envToVars(); + } + + public function tearDownOnce($class) + { + Director::varsToEnv($this->staticVars); + } +} diff --git a/src/Dev/KernelTestState.php b/src/Dev/State/KernelTestState.php similarity index 96% rename from src/Dev/KernelTestState.php rename to src/Dev/State/KernelTestState.php index e8e7c2e45..14e78aa61 100644 --- a/src/Dev/KernelTestState.php +++ b/src/Dev/State/KernelTestState.php @@ -1,10 +1,11 @@ get("ContainerController/subcontroller/subaction"); $this->assertEquals('subaction', $response->getBody()); - $request = new HTTPRequest( - 'GET', - 'ContainerController/subcontroller/substring/subvieweraction' - ); - /* Shift to emulate the director selecting the controller */ - $request->shift(); - /* Handle the request to create conditions where improperly passing the action to the viewer might fail */ - $controller = new ControllerTest\ContainerController(); - try { - $controller->handleRequest($request); - } catch (ControllerTest\SubController_Exception $e) { - $this->fail($e->getMessage()); - } + // Handle nested action + $response = $this->get('ContainerController/subcontroller/substring/subvieweraction'); + $this->assertEquals('Hope this works', $response->getBody()); } } diff --git a/tests/php/Control/ControllerTest/SubController.php b/tests/php/Control/ControllerTest/SubController.php index 8b87d1d02..001dd53dd 100644 --- a/tests/php/Control/ControllerTest/SubController.php +++ b/tests/php/Control/ControllerTest/SubController.php @@ -34,10 +34,6 @@ class SubController extends Controller implements TestOnly public function subvieweraction() { - return $this->customise( - array( - 'Thoughts' => 'Hope this works', - ) - ); + return 'Hope this works'; } } diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index c7afcb0e8..e3c1e81fe 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -3,31 +3,21 @@ namespace SilverStripe\Control\Tests; use SilverStripe\Control\Cookie_Backend; +use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse_Exception; -use SilverStripe\Control\Tests\DirectorTest\TestController; -use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Injector\Injector; -use SilverStripe\Dev\SapphireTest; -use SilverStripe\Control\Director; use SilverStripe\Control\RequestProcessor; -use SilverStripe\Security\Security; +use SilverStripe\Control\Tests\DirectorTest\TestController; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Kernel; +use SilverStripe\Dev\SapphireTest; /** * @todo test Director::alternateBaseFolder() */ class DirectorTest extends SapphireTest { - - protected static $originalRequestURI; - - protected $originalProtocolHeaders = array(); - - protected $originalGet = array(); - - protected $originalSession = array(); - protected static $extra_controllers = [ TestController::class, ]; @@ -35,28 +25,8 @@ class DirectorTest extends SapphireTest protected function setUp() { parent::setUp(); - - // Hold the original request URI once so it doesn't get overwritten - if (!self::$originalRequestURI) { - self::$originalRequestURI = $_SERVER['REQUEST_URI']; - } - $_SERVER['REQUEST_URI'] = 'http://www.mysite.com'; - - $this->originalGet = $_GET; - $this->originalSession = $_SESSION; - $_SESSION = array(); - - $headers = array( - 'HTTP_X_FORWARDED_PROTOCOL', 'HTTPS', 'SSL' - ); - - foreach ($headers as $header) { - if (isset($_SERVER[$header])) { - $this->originalProtocolHeaders[$header] = $_SERVER[$header]; - } - } - - Config::modify()->set(Director::class, 'alternate_base_url', '/'); + Director::config()->set('alternate_base_url', 'http://www.mysite.com/'); + $this->expectedRedirect = null; } protected function getExtraRoutes() @@ -77,24 +47,6 @@ class DirectorTest extends SapphireTest Director::config()->set('rules', $this->getExtraRoutes()); } - protected function tearDown() - { - $_GET = $this->originalGet; - $_SESSION = $this->originalSession; - - // Reinstate the original REQUEST_URI after it was modified by some tests - $_SERVER['REQUEST_URI'] = self::$originalRequestURI; - - if ($this->originalProtocolHeaders) { - foreach ($this->originalProtocolHeaders as $header => $value) { - $_SERVER[$header] = $value; - } - } - - - parent::tearDown(); - } - public function testFileExists() { $tempFileName = 'DirectorTest_testFileExists.tmp'; @@ -118,55 +70,53 @@ class DirectorTest extends SapphireTest public function testAbsoluteURL() { - - $rootURL = Director::protocolAndHost(); - $_SERVER['REQUEST_URI'] = "$rootURL/mysite/sub-page/"; - Director::config()->set('alternate_base_url', '/mysite/'); + Director::config()->set('alternate_base_url', 'http://www.mysite.com/mysite/'); + $_SERVER['REQUEST_URI'] = "http://www.mysite.com/mysite/sub-page/"; //test empty / local urls foreach (array('', './', '.') as $url) { - $this->assertEquals("$rootURL/mysite/", Director::absoluteURL($url, Director::BASE)); - $this->assertEquals("$rootURL/", Director::absoluteURL($url, Director::ROOT)); - $this->assertEquals("$rootURL/mysite/sub-page/", Director::absoluteURL($url, Director::REQUEST)); + $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)); } // Test site root url - $this->assertEquals("$rootURL/", Director::absoluteURL('/')); + $this->assertEquals("http://www.mysite.com/", Director::absoluteURL('/')); // Test Director::BASE - $this->assertEquals($rootURL, Director::absoluteURL($rootURL, Director::BASE)); + $this->assertEquals('http://www.mysite.com/', Director::absoluteURL('http://www.mysite.com/', Director::BASE)); $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::BASE)); - $this->assertEquals("$rootURL/test", Director::absoluteURL("$rootURL/test", Director::BASE)); - $this->assertEquals("$rootURL/root", Director::absoluteURL("/root", Director::BASE)); - $this->assertEquals("$rootURL/root/url", Director::absoluteURL("/root/url", 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)); // Test Director::ROOT - $this->assertEquals($rootURL, Director::absoluteURL($rootURL, Director::ROOT)); + $this->assertEquals('http://www.mysite.com/', Director::absoluteURL('http://www.mysite.com/', Director::ROOT)); $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::ROOT)); - $this->assertEquals("$rootURL/test", Director::absoluteURL("$rootURL/test", Director::ROOT)); - $this->assertEquals("$rootURL/root", Director::absoluteURL("/root", Director::ROOT)); - $this->assertEquals("$rootURL/root/url", Director::absoluteURL("/root/url", 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)); // Test Director::REQUEST - $this->assertEquals($rootURL, Director::absoluteURL($rootURL, Director::REQUEST)); + $this->assertEquals('http://www.mysite.com/', Director::absoluteURL('http://www.mysite.com/', Director::REQUEST)); $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::REQUEST)); - $this->assertEquals("$rootURL/test", Director::absoluteURL("$rootURL/test", Director::REQUEST)); - $this->assertEquals("$rootURL/root", Director::absoluteURL("/root", Director::REQUEST)); - $this->assertEquals("$rootURL/root/url", Director::absoluteURL("/root/url", 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)); // Test evaluating relative urls relative to base (default) - $this->assertEquals("$rootURL/mysite/test", Director::absoluteURL("test")); - $this->assertEquals("$rootURL/mysite/test/url", Director::absoluteURL("test/url")); - $this->assertEquals("$rootURL/mysite/test", Director::absoluteURL("test", Director::BASE)); - $this->assertEquals("$rootURL/mysite/test/url", Director::absoluteURL("test/url", Director::BASE)); + $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)); // Test evaluting relative urls relative to root - $this->assertEquals("$rootURL/test", Director::absoluteURL("test", Director::ROOT)); - $this->assertEquals("$rootURL/test/url", Director::absoluteURL("test/url", Director::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)); // Test relative to requested page - $this->assertEquals("$rootURL/mysite/sub-page/test", Director::absoluteURL("test", Director::REQUEST)); - $this->assertEquals("$rootURL/mysite/sub-page/test/url", Director::absoluteURL("test/url", Director::REQUEST)); + $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)); // Test that javascript links are not left intact $this->assertStringStartsNotWith('javascript', Director::absoluteURL('javascript:alert("attack")')); @@ -177,17 +127,15 @@ class DirectorTest extends SapphireTest public function testAlternativeBaseURL() { - // Get original protocol and hostname - $rootURL = Director::protocolAndHost(); - // relative base URLs - you should end them in a / Director::config()->set('alternate_base_url', '/relativebase/'); - $_SERVER['REQUEST_URI'] = "$rootURL/relativebase/sub-page/"; + $_SERVER['HTTP_HOST'] = 'www.somesite.com'; + $_SERVER['REQUEST_URI'] = "/relativebase/sub-page/"; $this->assertEquals('/relativebase/', Director::baseURL()); - $this->assertEquals($rootURL . '/relativebase/', Director::absoluteBaseURL()); + $this->assertEquals('http://www.somesite.com/relativebase/', Director::absoluteBaseURL()); $this->assertEquals( - $rootURL . '/relativebase/subfolder/test', + 'http://www.somesite.com/relativebase/subfolder/test', Director::absoluteURL('subfolder/test') ); @@ -336,6 +284,10 @@ class DirectorTest extends SapphireTest unset($_GET['isTest']); unset($_GET['isDev']); + /** @var Kernel $kernel */ + $kernel = Injector::inst()->get(Kernel::class); + $kernel->setEnvironment(null); + // Test isDev=1 $_GET['isDev'] = '1'; $this->assertTrue(Director::isDev()); @@ -399,28 +351,40 @@ class DirectorTest extends SapphireTest ); } - public function testTestRequestCarriesGlobals() + public function providerTestTestRequestCarriesGlobals() { - $fixture = array('somekey' => 'sometestvalue'); + $tests = []; + $fixture = [ 'somekey' => 'sometestvalue' ]; foreach (array('get', 'post') as $method) { foreach (array('return%sValue', 'returnRequestValue', 'returnCookieValue') as $testfunction) { $url = 'TestController/' . sprintf($testfunction, ucfirst($method)) . '?' . http_build_query($fixture); - - $getresponse = Director::test( - $url, - $fixture, - null, - strtoupper($method), - null, - null, - Injector::inst()->createWithArgs(Cookie_Backend::class, array($fixture)) - ); - - $this->assertInstanceOf(HTTPResponse::class, $getresponse, 'Director::test() returns HTTPResponse'); - $this->assertEquals($fixture['somekey'], $getresponse->getBody(), 'Director::test() ' . $testfunction); + $tests[] = [$url, $fixture, $method]; } } + return $tests; + } + + /** + * @dataProvider providerTestTestRequestCarriesGlobals + * @param $url + * @param $fixture + * @param $method + */ + public function testTestRequestCarriesGlobals($url, $fixture, $method) + { + $getresponse = Director::test( + $url, + $fixture, + null, + strtoupper($method), + null, + null, + Injector::inst()->createWithArgs(Cookie_Backend::class, array($fixture)) + ); + + $this->assertInstanceOf(HTTPResponse::class, $getresponse, 'Director::test() returns HTTPResponse'); + $this->assertEquals($fixture['somekey'], $getresponse->getBody(), "Director::test({$url}, {$method})"); } /** @@ -446,46 +410,87 @@ class DirectorTest extends SapphireTest public function testForceSSLProtectsEntireSite() { - $_SERVER['REQUEST_URI'] = '/admin'; - $output = Director::forceSSL(); - $this->assertEquals($output, 'https://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']); - - $_SERVER['REQUEST_URI'] = Director::baseURL() . 'some-url'; - $output = Director::forceSSL(); - $this->assertEquals($output, 'https://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']); + $this->expectExceptionRedirect('https://www.mysite.com/some-url'); + Director::mockRequest(function () { + Director::forceSSL(); + }, '/some-url'); } public function testForceSSLOnTopLevelPagePattern() { - $_SERVER['REQUEST_URI'] = Director::baseURL() . 'admin'; - $output = Director::forceSSL(array('/^admin/')); - $this->assertEquals($output, 'https://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']); + // Expect admin to trigger redirect + $this->expectExceptionRedirect('https://www.mysite.com/admin'); + Director::mockRequest(function () { + Director::forceSSL(array('/^admin/')); + }, '/admin'); } public function testForceSSLOnSubPagesPattern() { - $_SERVER['REQUEST_URI'] = Director::baseURL() . Config::inst()->get(Security::class, 'login_url'); - $output = Director::forceSSL(array('/^Security/')); - $this->assertEquals($output, 'https://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']); + // Expect to redirect to security login page + $this->expectExceptionRedirect('https://www.mysite.com/Security/login'); + Director::mockRequest(function () { + Director::forceSSL(array('/^Security/')); + }, '/Security/login'); } public function testForceSSLWithPatternDoesNotMatchOtherPages() { - $_SERVER['REQUEST_URI'] = Director::baseURL() . 'normal-page'; - $output = Director::forceSSL(array('/^admin/')); - $this->assertFalse($output); + // Not on same url should not trigger redirect + Director::mockRequest(function () { + $this->assertFalse(Director::forceSSL(array('/^admin/'))); + }, Director::baseURL() . 'normal-page'); - $_SERVER['REQUEST_URI'] = Director::baseURL() . 'just-another-page/sub-url'; - $output = Director::forceSSL(array('/^admin/', '/^Security/')); - $this->assertFalse($output); + // nested url should not triger redirect either + Director::mockRequest(function () { + $this->assertFalse(Director::forceSSL(array('/^admin/', '/^Security/'))); + }, Director::baseURL() . 'just-another-page/sub-url'); } public function testForceSSLAlternateDomain() { - Director::config()->set('alternate_base_url', '/'); - $_SERVER['REQUEST_URI'] = Director::baseURL() . 'admin'; - $output = Director::forceSSL(array('/^admin/'), 'secure.mysite.com'); - $this->assertEquals($output, 'https://secure.mysite.com/admin'); + // Ensure that forceSSL throws the appropriate exception + $this->expectExceptionRedirect('https://secure.mysite.com/admin'); + Director::mockRequest(function (HTTPRequest $request) { + return Director::forceSSL(array('/^admin/'), 'secure.mysite.com'); + }, Director::baseURL() . 'admin'); + } + + /** + * Set url to redirect to + * + * @var string + */ + protected $expectedRedirect = null; + + /** + * Expects this test to throw a HTTPResponse_Exception with the given redirect + * + * @param string $url + */ + protected function expectExceptionRedirect($url) + { + $this->expectedRedirect = $url; + } + + protected function runTest() + { + try { + $result = parent::runTest(); + if ($this->expectedRedirect) { + $this->fail("Expected to redirect to {$this->expectedRedirect} but no redirect found"); + } + return $result; + } catch (HTTPResponse_Exception $exception) { + // Check URL + if ($this->expectedRedirect) { + $url = $exception->getResponse()->getHeader('Location'); + $this->assertEquals($this->expectedRedirect, $url, "Expected to redirect to {$this->expectedRedirect}"); + return null; + } else { + throw $exception; + } + } } /** @@ -614,28 +619,30 @@ class DirectorTest extends SapphireTest $processor = new RequestProcessor(array($filter)); Injector::inst()->registerService($processor, RequestProcessor::class); - - Director::test('some-dummy-url'); + $response = Director::test('some-dummy-url'); + $this->assertEquals(404, $response->getStatusCode()); $this->assertEquals(1, $filter->preCalls); $this->assertEquals(1, $filter->postCalls); $filter->failPost = true; - $this->expectException(HTTPResponse_Exception::class); - - Director::test('some-dummy-url'); + $response = Director::test('some-dummy-url'); + $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals(_t(Director::class.'.REQUEST_ABORTED', 'Request aborted'), $response->getBody()); $this->assertEquals(2, $filter->preCalls); $this->assertEquals(2, $filter->postCalls); $filter->failPre = true; - Director::test('some-dummy-url'); + $response = Director::test('some-dummy-url'); + $this->assertEquals(400, $response->getStatusCode()); + $this->assertEquals(_t(Director::class.'.INVALID_REQUEST', 'Invalid request'), $response->getBody()); $this->assertEquals(3, $filter->preCalls); - // preCall 'false' will trigger an exception and prevent post call execution + // preCall 'true' will trigger an exception and prevent post call execution $this->assertEquals(2, $filter->postCalls); } } diff --git a/tests/php/Control/DirectorTest/TestController.php b/tests/php/Control/DirectorTest/TestController.php index 713831d17..e3e4400be 100644 --- a/tests/php/Control/DirectorTest/TestController.php +++ b/tests/php/Control/DirectorTest/TestController.php @@ -26,21 +26,33 @@ class TestController extends Controller implements TestOnly public function returnGetValue($request) { - return $_GET['somekey']; + if (isset($_GET['somekey'])) { + return $_GET['somekey']; + } + return null; } public function returnPostValue($request) { - return $_POST['somekey']; + if (isset($_POST['somekey'])) { + return $_POST['somekey']; + } + return null; } public function returnRequestValue($request) { - return $_REQUEST['somekey']; + if (isset($_REQUEST['somekey'])) { + return $_REQUEST['somekey']; + } + return null; } public function returnCookieValue($request) { - return $_COOKIE['somekey']; + if (isset($_COOKIE['somekey'])) { + return $_COOKIE['somekey']; + } + return null; } } diff --git a/tests/php/Control/FlushRequestFilterTest.php b/tests/php/Control/FlushRequestFilterTest.php index c32de20db..8644cc014 100644 --- a/tests/php/Control/FlushRequestFilterTest.php +++ b/tests/php/Control/FlushRequestFilterTest.php @@ -13,9 +13,7 @@ class FlushRequestFilterTest extends FunctionalTest public function testImplementorsAreCalled() { TestFlushable::$flushed = false; - $this->get('?flush=1'); - $this->assertTrue(TestFlushable::$flushed); } } diff --git a/tests/php/Control/FlushRequestFilterTest/TestFlushable.php b/tests/php/Control/FlushRequestFilterTest/TestFlushable.php index e3d68aa3b..c867696f0 100644 --- a/tests/php/Control/FlushRequestFilterTest/TestFlushable.php +++ b/tests/php/Control/FlushRequestFilterTest/TestFlushable.php @@ -7,7 +7,6 @@ use SilverStripe\Dev\TestOnly; class TestFlushable implements Flushable, TestOnly { - public static $flushed = false; public static function flush() diff --git a/tests/php/Control/SessionTest.php b/tests/php/Control/SessionTest.php index 145e189b8..afdda1013 100644 --- a/tests/php/Control/SessionTest.php +++ b/tests/php/Control/SessionTest.php @@ -3,7 +3,6 @@ namespace SilverStripe\Control\Tests; use SilverStripe\Control\Session; -use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; /** @@ -105,14 +104,6 @@ class SessionTest extends SapphireTest $this->assertEquals(array('something' => array('does' => null)), $result); } - public function testNonStandardPath() - { - Session::config()->set('store_path', (realpath(dirname($_SERVER['DOCUMENT_ROOT']) . '/../session'))); - $this->session->start(); - - $this->assertEquals(Config::inst()->get('SilverStripe\\Control\\Session', 'store_path'), ''); - } - public function testUserAgentLockout() { // Set a user agent @@ -120,6 +111,7 @@ class SessionTest extends SapphireTest // Generate our session $s = new Session(array()); + $s->init(); $s->set('val', 123); $s->finalize(); @@ -128,6 +120,7 @@ class SessionTest extends SapphireTest // Verify the new session reset our values $s2 = new Session($s); + $s2->init(); $this->assertNotEquals($s2->get('val'), 123); } }