Merge pull request #371 from halkyon/sapphire

---

Dont start the session until its actually necessary, which is to say there is a cookie available with the current PHP session name (or a request variable with the session_name() - typically PHPSESSID.) The latter allows for passing session ID through as an alternative to cookies.
This commit is contained in:
Ingo Schommer 2012-05-03 14:44:36 +02:00
commit 68051fdb96
6 changed files with 93 additions and 32 deletions

View File

@ -93,10 +93,15 @@ class Director implements TemplateGlobalProvider {
$req->addHeader($header, $value); $req->addHeader($header, $value);
} }
// Load the session into the controller // Only resume a session if its not started already, and a session identifier exists
if(!isset($_SESSION) && (isset($_COOKIE[session_name()]) || isset($_REQUEST[session_name()]))) Session::start();
// Initiate an empty session - doesn't initialize an actual PHP session until saved (see belwo)
$session = new Session(isset($_SESSION) ? $_SESSION : null); $session = new Session(isset($_SESSION) ? $_SESSION : null);
// Main request handling
$result = Director::handleRequest($req, $session, $model); $result = Director::handleRequest($req, $session, $model);
// Save session data (and start/resume it if required)
$session->inst_save(); $session->inst_save();
// Return code for a redirection request // Return code for a redirection request

View File

@ -102,6 +102,17 @@ class Session {
protected $changedData = array(); protected $changedData = array();
/**
* 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.
*/
function __construct($data) {
if($data instanceof Session) $data = $data->inst_getAll();
$this->data = $data;
}
/** /**
* Cookie domain, for example 'www.php.net'. * Cookie domain, for example 'www.php.net'.
* *
@ -176,17 +187,6 @@ class Session {
return self::$session_store_path; return self::$session_store_path;
} }
/**
* Create a new session object, with the given starting data
*
* @param $data Can be an array of data (such as $_SESSION) or another Session object to clone.
*/
function __construct($data) {
if($data instanceof Session) $data = $data->inst_getAll();
$this->data = $data;
}
/** /**
* Provide an <code>array</code> of rules specifing timeouts for IPv4 address ranges or * Provide an <code>array</code> of rules specifing timeouts for IPv4 address ranges or
* individual IPv4 addresses. The key is an IP address or range and the value is the time * individual IPv4 addresses. The key is an IP address or range and the value is the time
@ -300,12 +300,14 @@ class Session {
$var = &$var[$n]; $var = &$var[$n];
$diffVar = &$diffVar[$n]; $diffVar = &$diffVar[$n];
} }
$var = $val; if($var !== $val) {
$diffVar = $val; $var = $val;
$diffVar = $val;
}
} }
} }
public function inst_addToArray($name, $val) { public function inst_addToArray($name, $val) {
$names = explode('.', $name); $names = explode('.', $name);
@ -355,14 +357,22 @@ class Session {
$diffVar = &$this->changedData; $diffVar = &$this->changedData;
foreach($names as $n) { foreach($names as $n) {
// don't clear a record that doesn't exist
if(!isset($var[$n])) return;
$var = &$var[$n]; $var = &$var[$n];
}
// only loop to find data within diffVar if var is proven to exist in the above loop
foreach($names as $n) {
$diffVar = &$diffVar[$n]; $diffVar = &$diffVar[$n];
} }
$var = null; if($var !== null) {
$diffVar = null; $var = null;
$diffVar = null;
}
} }
public function inst_clearAll() { public function inst_clearAll() {
if($this->data && is_array($this->data)) { if($this->data && is_array($this->data)) {
foreach(array_keys($this->data) as $key) { foreach(array_keys($this->data) as $key) {
@ -370,7 +380,7 @@ class Session {
} }
} }
} }
public function inst_getAll() { public function inst_getAll() {
return $this->data; return $this->data;
} }
@ -380,7 +390,10 @@ class Session {
* Only save the changes, so that anyone manipulating $_SESSION directly doesn't get burned. * Only save the changes, so that anyone manipulating $_SESSION directly doesn't get burned.
*/ */
public function inst_save() { public function inst_save() {
$this->recursivelyApply($this->changedData, $_SESSION); if($this->changedData) {
if(!isset($_SESSION)) Session::start();
$this->recursivelyApply($this->changedData, $_SESSION);
}
} }
/** /**
@ -397,7 +410,15 @@ class Session {
} }
} }
} }
/**
* Return the changed data, for debugging purposes.
* @return array
*/
public function inst_changedData() {
return $this->changedData;
}
/** /**
* Sets the appropriate form message in session, with type. This will be shown once, * Sets the appropriate form message in session, with type. This will be shown once,
* for the form specified. * for the form specified.
@ -431,7 +452,7 @@ class Session {
// Allow storing the session in a non standard location // Allow storing the session in a non standard location
if($session_path) session_save_path($session_path); if($session_path) session_save_path($session_path);
// @ is to supress win32 warnings/notices when session wasn't cleaned up properly // @ 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! // There's nothing we can do about this, because it's an operating system function!
if($sid) session_id($sid); if($sid) session_id($sid);

View File

@ -59,9 +59,7 @@ if (version_compare(phpversion(), '5.3.2', '<')) {
/** /**
* Include SilverStripe's core code * Include SilverStripe's core code
*/ */
require_once("core/Core.php"); require_once('core/Core.php');
Session::start();
// IIS will sometimes generate this. // IIS will sometimes generate this.
if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) { if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) {

View File

@ -791,9 +791,17 @@ class Versioned extends DataExtension {
if(!headers_sent() && !Director::is_cli()) { if(!headers_sent() && !Director::is_cli()) {
if(Versioned::current_stage() == 'Live') { if(Versioned::current_stage() == 'Live') {
Cookie::set('bypassStaticCache', null, 0, null, null, false, true /* httponly */); // clear the cookie if it's set
if(!empty($_COOKIE['bypassStaticCache'])) {
Cookie::set('bypassStaticCache', null, 0, null, null, false, true /* httponly */);
unset($_COOKIE['bypassStaticCache']);
}
} else { } else {
Cookie::set('bypassStaticCache', '1', 0, null, null, false, true /* httponly */); // set the cookie if it's cleared
if(empty($_COOKIE['bypassStaticCache'])) {
Cookie::set('bypassStaticCache', '1', 0, null, null, false, true /* httponly */);
$_COOKIE['bypassStaticCache'] = 1;
}
} }
} }
} }

View File

@ -55,9 +55,6 @@ class SecurityToken extends Object implements TemplateGlobalProvider {
*/ */
function __construct($name = null) { function __construct($name = null) {
$this->name = ($name) ? $name : self::get_default_name(); $this->name = ($name) ? $name : self::get_default_name();
// only regenerate if the token isn't already set in the session
if(!$this->getValue()) $this->setValue($this->generate());
parent::__construct(); parent::__construct();
} }
@ -132,7 +129,15 @@ class SecurityToken extends Object implements TemplateGlobalProvider {
* @return String * @return String
*/ */
function getValue() { function getValue() {
return Session::get($this->getName()); $value = Session::get($this->getName());
// only regenerate if the token isn't already set in the session
if(!$value) {
$value = $this->generate();
$this->setValue($value);
}
return $value;
} }
/** /**

View File

@ -45,6 +45,30 @@ class SessionTest extends SapphireTest {
$this->assertEquals($session, array('Test' => 'Test', 'Test-2' => 'Test-2')); $this->assertEquals($session, array('Test' => 'Test', 'Test-2' => 'Test-2'));
} }
/**
* Check that changedData isn't populated with junk when clearing non-existent entries.
*/
function testClearElementThatDoesntExist() {
$s = new Session(array('something' => array('does' => 'exist')));
$s->inst_clear('something.doesnt.exist');
$this->assertEquals(array(), $s->inst_changedData());
$s->inst_set('something-else', 'val');
$s->inst_clear('something-new');
$this->assertEquals(array('something-else' => 'val'), $s->inst_changedData());
}
/**
* Check that changedData is populated with clearing data.
*/
function testClearElementThatDoesExist() {
$s = new Session(array('something' => array('does' => 'exist')));
$s->inst_clear('something.does');
$this->assertEquals(array('something' => array('does' => null)), $s->inst_changedData());
}
function testNonStandardPath(){ function testNonStandardPath(){
Session::set_session_store_path(realpath(dirname($_SERVER['DOCUMENT_ROOT']) . '/../session')); Session::set_session_store_path(realpath(dirname($_SERVER['DOCUMENT_ROOT']) . '/../session'));
Session::start(); Session::start();