Commit 5962cc5a authored by Dries's avatar Dries

- Patch #477944 by DamZ: more streamlining and clean-up of session handling code.

parent 907faab2
......@@ -436,6 +436,8 @@ function drupal_initialize_variables() {
ini_set('session.use_trans_sid', '0');
// Don't send HTTP headers using PHP's session handler.
ini_set('session.cache_limiter', 'none');
// Use httponly session cookies.
ini_set('session.cookie_httponly', '1');
}
/**
......
......@@ -351,7 +351,7 @@ protected function prepareItem($cache) {
// If enforcing a minimum cache lifetime, validate that the data is
// currently valid for this user before we return it by making sure the cache
// entry was created before the timestamp in the current session's cache
// timer. The cache variable is loaded into the $user object by _sess_read()
// timer. The cache variable is loaded into the $user object by _drupal_session_read()
// in session.inc. If the data is permanent or we're not enforcing a minimum
// cache lifetime always return the cached data.
if ($cache->expire != CACHE_PERMANENT && variable_get('cache_lifetime', 0) && $user->cache > $cache->created) {
......@@ -394,7 +394,7 @@ function clear($cid = NULL, $wildcard = FALSE) {
if (empty($cid)) {
if (variable_get('cache_lifetime', 0)) {
// We store the time in the current user's $user->cache variable which
// will be saved into the sessions bin by _sess_write(). We then
// will be saved into the sessions bin by _drupal_session_write(). We then
// simulate that the cache was flushed for this user by not returning
// cached data that was cached before the timestamp.
$user->cache = REQUEST_TIME;
......
......@@ -6,12 +6,12 @@
* User session handling functions.
*
* The user-level session storage handlers:
* - _sess_open()
* - _sess_close()
* - _sess_read()
* - _sess_write()
* - _sess_destroy_sid()
* - _sess_gc()
* - _drupal_session_open()
* - _drupal_session_close()
* - _drupal_session_read()
* - _drupal_session_write()
* - _drupal_session_destroy()
* - _drupal_session_garbage_collection()
* are assigned by session_set_save_handler() in bootstrap.inc and are called
* automatically by PHP. These functions should not be called directly. Session
* data should instead be accessed via the $_SESSION superglobal.
......@@ -29,7 +29,7 @@
* @return
* This function will always return TRUE.
*/
function _sess_open() {
function _drupal_session_open() {
return TRUE;
}
......@@ -45,7 +45,7 @@ function _sess_open() {
* @return
* This function will always return TRUE.
*/
function _sess_close() {
function _drupal_session_close() {
return TRUE;
}
......@@ -59,13 +59,13 @@ function _sess_close() {
* This function should not be called directly. Session data should
* instead be accessed via the $_SESSION superglobal.
*
* @param $key
* @param $sid
* Session ID.
* @return
* Either an array of the session data, or an empty string, if no data
* was found or the user is anonymous.
*/
function _sess_read($key) {
function _drupal_session_read($sid) {
global $user;
// Write and Close handlers are called after destructing objects
......@@ -83,7 +83,7 @@ function _sess_read($key) {
// Otherwise, if the session is still active, we have a record of the
// client's session in the database.
$user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = :sid", array(':sid' => $key))->fetchObject();
$user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = :sid", array(':sid' => $sid))->fetchObject();
// We found the client's session record and they are an authenticated user.
if ($user && $user->uid > 0) {
......@@ -114,26 +114,23 @@ function _sess_read($key) {
* This function should not be called directly. Session data should
* instead be accessed via the $_SESSION superglobal.
*
* @param $key
* @param $sid
* Session ID.
* @param $value
* Serialized array of the session data.
* @return
* This function will always return TRUE.
*/
function _sess_write($key, $value) {
function _drupal_session_write($sid, $value) {
global $user;
// If saving of session data is disabled, or if a new empty anonymous session
// has been started, do nothing. This keeps anonymous users, including
// crawlers, out of the session table, unless they actually have something
// stored in $_SESSION.
if (!drupal_save_session() || empty($user) || (empty($user->uid) && empty($_COOKIE[session_name()]) && empty($value))) {
return TRUE;
if (!drupal_save_session()) {
// We don't have anything to do if we are not allowed to save the session.
return;
}
db_merge('sessions')
->key(array('sid' => $key))
->key(array('sid' => $sid))
->fields(array(
'uid' => $user->uid,
'cache' => isset($user->cache) ? $user->cache : 0,
......@@ -163,7 +160,7 @@ function _sess_write($key, $value) {
function drupal_session_initialize() {
global $user;
session_set_save_handler('_sess_open', '_sess_close', '_sess_read', '_sess_write', '_sess_destroy_sid', '_sess_gc');
session_set_save_handler('_drupal_session_open', '_drupal_session_close', '_drupal_session_read', '_drupal_session_write', '_drupal_session_destroy', '_drupal_session_garbage_collection');
if (isset($_COOKIE[session_name()])) {
// If a session cookie exists, initialize the session. Otherwise the
......@@ -211,13 +208,21 @@ function drupal_session_start() {
function drupal_session_commit() {
global $user;
if (!drupal_save_session()) {
// We don't have anything to do if we are not allowed to save the session.
return;
}
if (empty($user->uid) && empty($_SESSION)) {
// There is no session data to store, destroy the session if it was
// previously started.
if (drupal_session_started()) {
// Destroy empty anonymous sessions.
session_destroy();
}
}
else {
// There is session data to store. Start the session if it is not already
// started.
if (!drupal_session_started()) {
drupal_session_start();
}
......@@ -243,11 +248,6 @@ function drupal_session_started($set = NULL) {
function drupal_session_regenerate() {
global $user;
// Set the session cookie "httponly" flag to reduce the risk of session
// stealing via XSS.
extract(session_get_cookie_params());
session_set_cookie_params($lifetime, $path, $domain, $secure, TRUE);
if (drupal_session_started()) {
$old_session_id = session_id();
session_regenerate_id();
......@@ -255,7 +255,7 @@ function drupal_session_regenerate() {
else {
// Start the session when it doesn't exist yet.
// Preserve the logged in user, as it will be reset to anonymous
// by _sess_read.
// by _drupal_session_read.
$account = $user;
drupal_session_start();
$user = $account;
......@@ -303,13 +303,25 @@ function drupal_session_count($timestamp = 0, $anonymous = TRUE) {
* @param string $sid
* Session ID.
*/
function _sess_destroy_sid($sid) {
function _drupal_session_destroy($sid) {
global $user;
// Delete session data.
db_delete('sessions')
->condition('sid', $sid)
->execute();
// Unset cookie.
extract(session_get_cookie_params());
setcookie(session_name(), '', time() - 3600, $path, $domain, $secure, $httponly);
// Reset $_SESSION and $user to prevent a new session from being started
// in drupal_session_commit().
$_SESSION = array();
$user = drupal_anonymous_user();
// Unset the session cookie.
if (isset($_COOKIE[session_name()])) {
$params = session_get_cookie_params();
setcookie(session_name(), '', REQUEST_TIME - 3600, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
unset($_COOKIE[session_name()]);
}
}
/**
......@@ -333,7 +345,7 @@ function drupal_session_destroy_uid($uid) {
* The value of session.gc_maxlifetime, passed by PHP.
* Sessions not updated for more than $lifetime seconds will be removed.
*/
function _sess_gc($lifetime) {
function _drupal_session_garbage_collection($lifetime) {
// Be sure to adjust 'php_value session.gc_maxlifetime' to a large enough
// value. For example, if you want user sessions to stay in your database
// for three weeks before deleting them, you need to set gc_maxlifetime
......
......@@ -948,11 +948,10 @@ protected function drupalGetToken($value = '') {
* Logs a user out of the internal browser, then check the login page to confirm logout.
*/
protected function drupalLogout() {
// Make a request to the logout page.
$this->drupalGet('user/logout');
// Load the user page, the idea being if you were properly logged out you should be seeing a login screen.
$this->drupalGet('user');
// Make a request to the logout page, and redirect to the user page, the
// idea being if you were properly logged out you should be seeing a login
// screen.
$this->drupalGet('user/logout', array('query' => 'destination=user'));
$pass = $this->assertField('name', t('Username field found.'), t('Logout'));
$pass = $pass && $this->assertField('pass', t('Password field found.'), t('Logout'));
......
......@@ -195,6 +195,17 @@ class SessionTestCase extends DrupalWebTestCase {
$this->assertNoText(t('This is a dummy message.'), t('Message was not cached.'));
$this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', t('Page was cached.'));
$this->assertFalse($this->drupalGetHeader('Set-Cookie'), t('New session was not started.'));
// Verify that no session is created if drupal_save_session(FALSE) is called.
$this->drupalGet('session-test/set-message-but-dont-save');
$this->assertSessionCookie(FALSE);
$this->assertSessionEmpty(TRUE);
// Verify that no message is displayed.
$this->drupalGet('');
$this->assertSessionCookie(FALSE);
$this->assertSessionEmpty(TRUE);
$this->assertNoText(t('This is a dummy message.'), t('The message was not saved.'));
}
/**
......@@ -205,6 +216,7 @@ class SessionTestCase extends DrupalWebTestCase {
function sessionReset($uid = 0) {
// Close the internal browser.
$this->curlClose();
$this->loggedInUser = FALSE;
// Change cookie file for user.
$this->cookieFile = file_directory_temp() . '/cookie.' . $uid . '.txt';
......
......@@ -37,6 +37,12 @@ function session_test_menu() {
'access arguments' => array('access content'),
'type' => MENU_CALLBACK,
);
$items['session-test/set-message-but-dont-save'] = array(
'title' => t('Session value'),
'page callback' => '_session_test_set_message_but_dont_save',
'access arguments' => array('access content'),
'type' => MENU_CALLBACK,
);
$items['session-test/set-not-started'] = array(
'title' => t('Session value'),
'page callback' => '_session_test_set_not_started',
......@@ -108,6 +114,14 @@ function _session_test_set_message() {
// instead.
}
/**
* Menu callback, sets a message but call drupal_save_session(FALSE).
*/
function _session_test_set_message_but_dont_save() {
drupal_save_session(FALSE);
_session_test_set_message();
}
/**
* Menu callback, stores a value in $_SESSION['session_test_value'] without
* having started the session in advance.
......
......@@ -1970,10 +1970,8 @@ function _user_cancel($edit, $account, $method) {
// After cancelling account, ensure that user is logged out.
if ($account->uid == $user->uid) {
// Destroy the current session.
// Destroy the current session, and reset $user to the anonymous user.
session_destroy();
// Load the anonymous user.
$user = drupal_anonymous_user();
}
else {
drupal_session_destroy_uid($account->uid);
......
......@@ -136,12 +136,10 @@ function user_logout() {
watchdog('user', 'Session closed for %name.', array('%name' => $user->name));
// Destroy the current session:
session_destroy();
module_invoke_all('user_logout', NULL, $user);
// Load the anonymous user
$user = drupal_anonymous_user();
// Destroy the current session, and reset $user to the anonymous user.
session_destroy();
drupal_goto();
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment