diff --git a/control/Controller.php b/control/Controller.php index 78f1c6e46..e0416661c 100644 --- a/control/Controller.php +++ b/control/Controller.php @@ -439,7 +439,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { if(isset(self::$controller_stack[1])) { $this->session = self::$controller_stack[1]->getSession(); } else { - $this->session = new Session(null); + $this->session = Injector::inst()->create('Session', array()); } } } diff --git a/control/Director.php b/control/Director.php index a8a7aeca3..6d1c636fd 100644 --- a/control/Director.php +++ b/control/Director.php @@ -135,12 +135,13 @@ class Director implements TemplateGlobalProvider { $req->addHeader($header, $value); } + // Initiate an empty session - doesn't initialize an actual PHP session until saved (see below) + $session = Injector::inst()->create('Session', isset($_SESSION) ? $_SESSION : array()); + // Only resume a session if its not started already, and a session identifier exists if(!isset($_SESSION) && Session::request_contains_session_id()) { - Session::start(); + $session->inst_start(); } - // Initiate an empty session - doesn't initialize an actual PHP session until saved (see belwo) - $session = new Session(isset($_SESSION) ? $_SESSION : null); $output = Injector::inst()->get('RequestProcessor')->preRequest($req, $session, $model); @@ -151,7 +152,7 @@ class Director implements TemplateGlobalProvider { $result = Director::handleRequest($req, $session, $model); - // Save session data (and start/resume it if required) + // Save session data. Note that inst_save() will start/resume the session if required. $session->inst_save(); // Return code for a redirection request @@ -229,7 +230,7 @@ class Director implements TemplateGlobalProvider { if(!$httpMethod) $httpMethod = ($postVars || is_array($postVars)) ? "POST" : "GET"; - if(!$session) $session = new Session(null); + if(!$session) $session = Injector::inst()->create('Session', array()); // Back up the current values of the superglobals $existingRequestVars = isset($_REQUEST) ? $_REQUEST : array(); diff --git a/control/Session.php b/control/Session.php index 5940e92aa..71886726e 100644 --- a/control/Session.php +++ b/control/Session.php @@ -139,20 +139,19 @@ class Session { /** * Start PHP session, then create a new Session object with the given start data. * - * @param $data Can be an array of data (such as $_SESSION) or another Session object to clone. + * @param $data array|Session Can be an array of data (such as $_SESSION) or another Session object to clone. */ public function __construct($data) { if($data instanceof Session) $data = $data->inst_getAll(); $this->data = $data; - + if (isset($this->data['HTTP_USER_AGENT'])) { if ($this->data['HTTP_USER_AGENT'] != $this->userAgent()) { // Funny business detected! $this->inst_clearAll(); - - Session::destroy(); - Session::start(); + $this->inst_destroy(); + $this->inst_start(); } } } @@ -347,11 +346,78 @@ class Session { if(Controller::has_curr()) { return Controller::curr()->getSession(); } else { - if(!self::$default_session) self::$default_session = new Session(isset($_SESSION) ? $_SESSION : array()); + if(!self::$default_session) { + self::$default_session = Injector::inst()->create('Session', isset($_SESSION) ? $_SESSION : array()); + } + return self::$default_session; } } + public function inst_start($sid = null) { + $path = Config::inst()->get('Session', 'cookie_path'); + if(!$path) $path = Director::baseURL(); + $domain = Config::inst()->get('Session', 'cookie_domain'); + $secure = Director::is_https() && Config::inst()->get('Session', 'cookie_secure'); + $session_path = Config::inst()->get('Session', 'session_store_path'); + $timeout = Config::inst()->get('Session', 'timeout'); + + if(!session_id() && !headers_sent()) { + if($domain) { + session_set_cookie_params($timeout, $path, $domain, $secure, true); + } else { + session_set_cookie_params($timeout, $path, null, $secure, true); + } + + // Allow storing the session in a non standard location + if($session_path) session_save_path($session_path); + + // If we want a secure cookie for HTTPS, use a seperate session name. This lets us have a + // seperate (less secure) session for non-HTTPS requests + if($secure) session_name('SECSESSID'); + + // @ is to supress win32 warnings/notices when session wasn't cleaned up properly + // There's nothing we can do about this, because it's an operating system function! + if($sid) session_id($sid); + @session_start(); + + $this->data = isset($_SESSION) ? $_SESSION : array(); + } + + // Modify the timeout behaviour so it's the *inactive* time before the session expires. + // By default it's the total session lifetime + if($timeout && !headers_sent()) { + Cookie::set(session_name(), session_id(), $timeout/86400, $path, $domain ? $domain + : null, $secure, true); + } + } + + public function inst_destroy($removeCookie = true) { + if(session_id()) { + if($removeCookie) { + $path = Config::inst()->get('Session', 'cookie_path'); + if(!$path) $path = Director::baseURL(); + $domain = Config::inst()->get('Session', 'cookie_domain'); + $secure = Config::inst()->get('Session', 'cookie_secure'); + + if($domain) { + Cookie::set(session_name(), '', null, $path, $domain, $secure, true); + } else { + Cookie::set(session_name(), '', null, $path, null, $secure, true); + } + + unset($_COOKIE[session_name()]); + } + + session_destroy(); + + // Clean up the superglobal - session_destroy does not do it. + // http://nz1.php.net/manual/en/function.session-destroy.php + unset($_SESSION); + $this->data = array(); + } + } + public function inst_set($name, $val) { // Quicker execution path for "."-free names if(strpos($name,'.') === false) { @@ -472,7 +538,11 @@ class Session { public function inst_save() { if($this->changedData) { $this->inst_finalize(); - if(!isset($_SESSION)) Session::start(); + + if(!isset($_SESSION)) { + $this->inst_start(); + } + $this->recursivelyApply($this->changedData, $_SESSION); } } @@ -529,41 +599,7 @@ class Session { * @param string $sid Start the session with a specific ID */ public static function start($sid = null) { - $path = Config::inst()->get('Session', 'cookie_path'); - if(!$path) $path = Director::baseURL(); - $domain = Config::inst()->get('Session', 'cookie_domain'); - $secure = Director::is_https() && Config::inst()->get('Session', 'cookie_secure'); - $session_path = Config::inst()->get('Session', 'session_store_path'); - $timeout = Config::inst()->get('Session', 'timeout'); - - if(!session_id() && !headers_sent()) { - if($domain) { - session_set_cookie_params($timeout, $path, $domain, - $secure /* secure */, true /* httponly */); - } else { - session_set_cookie_params($timeout, $path, null, - $secure /* secure */, true /* httponly */); - } - - // Allow storing the session in a non standard location - if($session_path) session_save_path($session_path); - - // If we want a secure cookie for HTTPS, use a seperate session name. This lets us have a - // seperate (less secure) session for non-HTTPS requests - if($secure) session_name('SECSESSID'); - - // @ is to supress win32 warnings/notices when session wasn't cleaned up properly - // There's nothing we can do about this, because it's an operating system function! - if($sid) session_id($sid); - @session_start(); - } - - // Modify the timeout behaviour so it's the *inactive* time before the session expires. - // By default it's the total session lifetime - if($timeout && !headers_sent()) { - Cookie::set(session_name(), session_id(), $timeout/86400, $path, $domain ? $domain - : null, $secure, true); - } + self::current_session()->inst_start($sid); } /** @@ -572,29 +608,7 @@ class Session { * @param bool $removeCookie If set to TRUE, removes the user's cookie, FALSE does not remove */ public static function destroy($removeCookie = true) { - if(session_id()) { - if($removeCookie) { - $path = Config::inst()->get('Session', 'cookie_path'); - if(!$path) $path = Director::baseURL(); - $domain = Config::inst()->get('Session', 'cookie_domain'); - $secure = Config::inst()->get('Session', 'cookie_secure'); - - if($domain) { - Cookie::set(session_name(), '', null, $path, $domain, $secure, true); - } - else { - Cookie::set(session_name(), '', null, $path, null, $secure, true); - } - - unset($_COOKIE[session_name()]); - } - - session_destroy(); - - // Clean up the superglobal - session_destroy does not do it. - // http://nz1.php.net/manual/en/function.session-destroy.php - unset($_SESSION); - } + self::current_session()->inst_destroy($removeCookie); } /** diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 4e4c7d9eb..0fd6cb4f1 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -201,7 +201,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase { DataObject::reset(); if(class_exists('SiteTree')) SiteTree::reset(); Hierarchy::reset(); - if(Controller::has_curr()) Controller::curr()->setSession(new Session(array())); + if(Controller::has_curr()) Controller::curr()->setSession(Injector::inst()->create('Session', array())); Security::$database_is_ready = null; $fixtureFile = static::get_fixture_file(); diff --git a/dev/TestSession.php b/dev/TestSession.php index afb71eb7f..398bbfe71 100644 --- a/dev/TestSession.php +++ b/dev/TestSession.php @@ -23,7 +23,7 @@ class TestSession { private $lastUrl; public function __construct() { - $this->session = new Session(array()); + $this->session = Injector::inst()->create('Session', array()); $this->controller = new Controller(); $this->controller->setSession($this->session); $this->controller->pushCurrent(); diff --git a/docs/en/topics/testing/creating-a-functional-test.md b/docs/en/topics/testing/creating-a-functional-test.md index 763c4ca4d..23e963264 100644 --- a/docs/en/topics/testing/creating-a-functional-test.md +++ b/docs/en/topics/testing/creating-a-functional-test.md @@ -18,7 +18,7 @@ URLs. Here is an example from the subsites module: * Return a session that has a user logged in as an administrator */ public function adminLoggedInSession() { - return new Session(array( + return Injector::inst()->create('Session', array( 'loggedInAs' => $this->idFromFixture('Member', 'admin') )); } @@ -66,4 +66,4 @@ the *_t()* method to avoid tests failing when i18n is enabled. Note that for a more highlevel testing approach, SilverStripe also supports [behaviour-driven testing through Behat](https://github.com/silverstripe-labs/silverstripe-behat-extension). It interacts -directly with your website or CMS interface by remote controlling an actual browser, driven by natural language assertions. \ No newline at end of file +directly with your website or CMS interface by remote controlling an actual browser, driven by natural language assertions. diff --git a/tests/FakeController.php b/tests/FakeController.php index 726c2a668..481dab7d0 100644 --- a/tests/FakeController.php +++ b/tests/FakeController.php @@ -5,7 +5,7 @@ class FakeController extends Controller { public function __construct() { parent::__construct(); - $session = new Session(isset($_SESSION) ? $_SESSION : null); + $session = Injector::inst()->create('Session', isset($_SESSION) ? $_SESSION : array()); $this->setSession($session); $this->pushCurrent(); @@ -21,4 +21,4 @@ class FakeController extends Controller { $this->init(); } -} \ No newline at end of file +} diff --git a/tests/api/RestfulServiceTest.php b/tests/api/RestfulServiceTest.php index fc101b7c6..5196965cf 100644 --- a/tests/api/RestfulServiceTest.php +++ b/tests/api/RestfulServiceTest.php @@ -362,7 +362,7 @@ class RestfulServiceTest_MockRestfulService extends RestfulService { public function request($subURL = '', $method = "GET", $data = null, $headers = null, $curlOptions = array()) { if(!$this->session) { - $this->session = new Session(array()); + $this->session = Injector::inst()->create('Session', array()); } $url = $this->baseURL . $subURL; // Url for the request diff --git a/tests/control/SessionTest.php b/tests/control/SessionTest.php index 49f7a427b..42dc39cb0 100644 --- a/tests/control/SessionTest.php +++ b/tests/control/SessionTest.php @@ -47,7 +47,7 @@ class SessionTest extends SapphireTest { } public function testSettingExistingDoesntClear() { - $s = new Session(array('something' => array('does' => 'exist'))); + $s = Injector::inst()->create('Session', array('something' => array('does' => 'exist'))); $s->inst_set('something.does', 'exist'); $result = $s->inst_changedData(); @@ -59,7 +59,7 @@ class SessionTest extends SapphireTest { * Check that changedData isn't populated with junk when clearing non-existent entries. */ public function testClearElementThatDoesntExist() { - $s = new Session(array('something' => array('does' => 'exist'))); + $s = Injector::inst()->create('Session', array('something' => array('does' => 'exist'))); $s->inst_clear('something.doesnt.exist'); $result = $s->inst_changedData(); @@ -77,7 +77,7 @@ class SessionTest extends SapphireTest { * Check that changedData is populated with clearing data. */ public function testClearElementThatDoesExist() { - $s = new Session(array('something' => array('does' => 'exist'))); + $s = Injector::inst()->create('Session', array('something' => array('does' => 'exist'))); $s->inst_clear('something.does'); $result = $s->inst_changedData(); @@ -97,7 +97,7 @@ class SessionTest extends SapphireTest { $_SERVER['HTTP_USER_AGENT'] = 'Test Agent'; // Generate our session - $s = new Session(array()); + $s = Injector::inst()->create('Session', array()); $s->inst_set('val', 123); $s->inst_finalize(); @@ -105,7 +105,7 @@ class SessionTest extends SapphireTest { $_SERVER['HTTP_USER_AGENT'] = 'Fake Agent'; // Verify the new session reset our values - $s2 = new Session($s); + $s2 = Injector::inst()->create('Session', $s); $this->assertNotEquals($s2->inst_get('val'), 123); } } diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index 0b7246cac..f2a1f9183 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -537,7 +537,7 @@ class VersionedTest extends SapphireTest { * Tests that reading mode persists between requests */ public function testReadingPersistent() { - $session = new Session(array()); + $session = Injector::inst()->create('Session', array()); // Set to stage Director::test('/?stage=Stage', null, $session); @@ -568,7 +568,7 @@ class VersionedTest extends SapphireTest { ); // Test that session doesn't redundantly store the default stage if it doesn't need to - $session2 = new Session(array()); + $session2 = Injector::inst()->create('Session', array()); Director::test('/', null, $session2); $this->assertEmpty($session2->inst_changedData()); Director::test('/?stage=Live', null, $session2); @@ -660,4 +660,4 @@ class VersionedTest_SingleStage extends DataObject implements TestOnly { private static $extensions = array( 'Versioned("Stage")' ); -} \ No newline at end of file +}