diff options
-rw-r--r-- | includes/batch.inc | 8 | ||||
-rw-r--r-- | includes/bootstrap.inc | 107 | ||||
-rw-r--r-- | includes/common.inc | 44 | ||||
-rw-r--r-- | includes/form.inc | 5 | ||||
-rw-r--r-- | includes/locale.inc | 8 | ||||
-rw-r--r-- | includes/session.inc | 141 | ||||
-rw-r--r-- | install.php | 2 | ||||
-rw-r--r-- | modules/book/book.install | 6 | ||||
-rw-r--r-- | modules/dblog/dblog.admin.inc | 15 | ||||
-rw-r--r-- | modules/node/node.admin.inc | 13 | ||||
-rw-r--r-- | modules/openid/openid.inc | 8 | ||||
-rw-r--r-- | modules/openid/openid.module | 6 | ||||
-rw-r--r-- | modules/simpletest/tests/session.test | 113 | ||||
-rw-r--r-- | modules/simpletest/tests/session_test.module | 22 | ||||
-rw-r--r-- | modules/system/system.admin.inc | 4 | ||||
-rw-r--r-- | modules/system/system.install | 8 | ||||
-rw-r--r-- | modules/user/user.admin.inc | 10 | ||||
-rw-r--r-- | modules/user/user.module | 4 | ||||
-rw-r--r-- | update.php | 8 |
19 files changed, 242 insertions, 290 deletions
diff --git a/includes/batch.inc b/includes/batch.inc index ed29b2b23..42d615926 100644 --- a/includes/batch.inc +++ b/includes/batch.inc @@ -416,6 +416,12 @@ function _batch_finished() { $_batch = $batch; $batch = NULL; + // Clean-up the session. + unset($_SESSION['batches'][$batch['id']]); + if (empty($_SESSION['batches'])) { + unset($_SESSION['batches']); + } + // Redirect if needed. if ($_batch['progressive']) { // Revert the 'destination' that was saved in batch_process(). @@ -443,7 +449,7 @@ function _batch_finished() { // We get here if $form['#redirect'] was FALSE, or if the form is a // multi-step form. We save the final $form_state value to be retrieved // by drupal_get_form(), and redirect to the originating page. - drupal_set_session('batch_form_state', $_batch['form_state']); + $_SESSION['batch_form_state'] = $_batch['form_state']; drupal_goto($_batch['source_page']); } } diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 90c2da466..1566f48d9 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -671,7 +671,6 @@ function variable_del($name) { unset($conf[$name]); } - /** * Retrieve the current page from the cache. * @@ -680,36 +679,33 @@ function variable_del($name) { * from a form submission, the contents of a shopping cart, or other user- * specific content that should not be cached and displayed to other users. * - * @param $retrieve - * If TRUE, look up and return the current page in the cache, or start output - * buffering if the conditions for caching are satisfied. If FALSE, only - * return a boolean value indicating whether the current request may be - * cached. * @return - * The cache object, if the page was found in the cache; TRUE if the page was - * not found, but output buffering was started in order to possibly cache the - * current request; FALSE if the page was not found, and the current request - * may not be cached (e.g. because it belongs to an authenticated user). If - * $retrieve is TRUE, only return either TRUE or FALSE. + * The cache object, if the page was found in the cache, NULL otherwise. */ -function page_get_cache($retrieve) { - global $user, $base_root; - static $ob_started = FALSE; +function drupal_page_get_cache() { + global $base_root; - if ($user->uid || ($_SERVER['REQUEST_METHOD'] != 'GET' && $_SERVER['REQUEST_METHOD'] != 'HEAD') || count(drupal_get_messages(NULL, FALSE)) || $_SERVER['SERVER_SOFTWARE'] === 'PHP CLI') { - return FALSE; + if (drupal_page_is_cacheable()) { + return cache_get($base_root . request_uri(), 'cache_page'); } - if ($retrieve) { - $cache = cache_get($base_root . request_uri(), 'cache_page'); - if ($cache) { - return $cache; - } - else { - ob_start(); - $ob_started = TRUE; - } +} + +/** + * Determine the cacheability of the current page. + * + * @param $allow_caching + * Set to FALSE if you want to prevent this page to get cached. + * @return + * TRUE if the current page can be cached, FALSE otherwise. + */ +function drupal_page_is_cacheable($allow_caching = NULL) { + $allow_caching_static = &drupal_static(__FUNCTION__, TRUE); + if (isset($allow_caching)) { + $allow_caching_static = $allow_caching; } - return $ob_started; + + return $allow_caching_static && ($_SERVER['REQUEST_METHOD'] == 'GET' || $_SERVER['REQUEST_METHOD'] == 'HEAD') + && $_SERVER['SERVER_SOFTWARE'] !== 'PHP CLI'; } /** @@ -885,7 +881,7 @@ function drupal_send_headers($default_headers = array(), $only_default = FALSE) * the client think that the anonymous and authenticated pageviews are * identical. * - * @see page_set_cache() + * @see drupal_page_set_cache() */ function drupal_page_header() { $headers_sent = &drupal_static(__FUNCTION__, FALSE); @@ -914,7 +910,7 @@ function drupal_page_header() { * and the conditions match those currently in the cache, a 304 Not Modified * response is sent. */ -function drupal_page_cache_header(stdClass $cache) { +function drupal_serve_page_from_cache(stdClass $cache) { // Negotiate whether to use compression. $page_compression = variable_get('page_compression', TRUE) && extension_loaded('zlib'); $return_compressed = $page_compression && isset($_SERVER['HTTP_ACCEPT_ENCODING']) && strpos($_SERVER['HTTP_ACCEPT_ENCODING'], 'gzip') !== FALSE; @@ -1169,10 +1165,6 @@ function watchdog($type, $message, $variables = array(), $severity = WATCHDOG_NO */ function drupal_set_message($message = NULL, $type = 'status', $repeat = TRUE) { if ($message) { - if (!isset($_SESSION['messages'])) { - drupal_set_session('messages', array()); - } - if (!isset($_SESSION['messages'][$type])) { $_SESSION['messages'][$type] = array(); } @@ -1180,6 +1172,9 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = TRUE) { if ($repeat || !in_array($message, $_SESSION['messages'][$type])) { $_SESSION['messages'][$type][] = $message; } + + // Mark this page has being not cacheable. + drupal_page_is_cacheable(FALSE); } // Messages not set when DB connection fails. @@ -1364,17 +1359,7 @@ function _drupal_bootstrap($phase) { case DRUPAL_BOOTSTRAP_SESSION: require_once DRUPAL_ROOT . '/' . variable_get('session_inc', 'includes/session.inc'); - session_set_save_handler('_sess_open', '_sess_close', '_sess_read', '_sess_write', '_sess_destroy_sid', '_sess_gc'); - // If a session cookie exists, initialize the session. Otherwise the - // session is only started on demand in drupal_session_start(), making - // anonymous users not use a session cookie unless something is stored in - // $_SESSION. This allows HTTP proxies to cache anonymous pageviews. - if (isset($_COOKIE[session_name()])) { - drupal_session_start(); - } - else { - $user = drupal_anonymous_user(); - } + drupal_session_initialize(); break; case DRUPAL_BOOTSTRAP_VARIABLES: @@ -1384,22 +1369,26 @@ function _drupal_bootstrap($phase) { case DRUPAL_BOOTSTRAP_LATE_PAGE_CACHE: $cache_mode = variable_get('cache', CACHE_DISABLED); + // Get the page from the cache. - $cache = $cache_mode == CACHE_DISABLED ? FALSE : page_get_cache(TRUE); + if ($cache_mode != CACHE_DISABLED) { + $cache = drupal_page_get_cache(); + } + else { + $cache = FALSE; + } + // If the skipping of the bootstrap hooks is not enforced, call hook_boot. - if (!is_object($cache) || $cache_mode != CACHE_AGGRESSIVE) { + if (!$cache || $cache_mode != CACHE_AGGRESSIVE) { // Load module handling. require_once DRUPAL_ROOT . '/includes/module.inc'; module_invoke_all('boot'); } + // If there is a cached page, display it. - if (is_object($cache)) { - // Destroy empty anonymous sessions. - if (drupal_session_is_started() && empty($_SESSION)) { - session_destroy(); - } + if ($cache) { header('X-Drupal-Cache: HIT'); - drupal_page_cache_header($cache); + drupal_serve_page_from_cache($cache); // If the skipping of the bootstrap hooks is not enforced, call hook_exit. if ($cache_mode != CACHE_AGGRESSIVE) { module_invoke_all('exit'); @@ -1408,19 +1397,13 @@ function _drupal_bootstrap($phase) { exit; } - // Prepare for non-cached page workflow. - - // If the session has not already been started and output buffering is - // not enabled, the HTTP headers must be sent now, including the session - // cookie. If output buffering is enabled, the session may be started - // at any time using drupal_session_start(). - if ($cache === FALSE) { - drupal_page_header(); - drupal_session_start(); - } - else { + if (!$cache && drupal_page_is_cacheable()) { header('X-Drupal-Cache: MISS'); } + + // Prepare for non-cached page workflow. + ob_start(); + drupal_page_header(); break; case DRUPAL_BOOTSTRAP_LANGUAGE: diff --git a/includes/common.inc b/includes/common.inc index e110e517a..a8a9a2466 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -325,11 +325,9 @@ function drupal_goto($path = '', $query = NULL, $fragment = NULL, $http_response module_invoke_all('exit', $url); } - if (drupal_session_is_started()) { - // Even though session_write_close() is registered as a shutdown function, - // we need all session data written to the database before redirecting. - session_write_close(); - } + // Commit the session, if necessary. We need all session data written to the + // database before redirecting. + drupal_session_commit(); header('Location: ' . $url, TRUE, $http_response_code); @@ -2156,19 +2154,17 @@ function l($text, $path, array $options = array()) { function drupal_page_footer() { global $user; - // Destroy empty anonymous sessions if possible. - if (!headers_sent() && drupal_session_is_started() && empty($_SESSION) && !$user->uid) { - session_destroy(); - } - elseif (!empty($_SESSION) && !drupal_session_is_started()) { - watchdog('session', '$_SESSION is non-empty yet no code has called drupal_session_start().', array(), WATCHDOG_NOTICE); - } + module_invoke_all('exit'); - if (variable_get('cache', CACHE_DISABLED) != CACHE_DISABLED) { - page_set_cache(); - } + // Commit the user session, if needed. + drupal_session_commit(); - module_invoke_all('exit'); + if (variable_get('cache', CACHE_DISABLED) != CACHE_DISABLED && ($cache = drupal_page_set_cache())) { + drupal_serve_page_from_cache($cache); + } + else { + ob_flush(); + } module_implements(MODULE_IMPLEMENTS_WRITE_CACHE); _registry_check_code(REGISTRY_WRITE_LOOKUP_CACHE); @@ -3282,11 +3278,12 @@ function _drupal_bootstrap_full() { * * @see drupal_page_header */ -function page_set_cache() { - global $user, $base_root; +function drupal_page_set_cache() { + global $base_root; - if (page_get_cache(FALSE)) { + if (drupal_page_is_cacheable()) { $cache_page = TRUE; + $cache = (object) array( 'cid' => $base_root . request_uri(), 'data' => ob_get_clean(), @@ -3294,12 +3291,14 @@ function page_set_cache() { 'created' => REQUEST_TIME, 'headers' => array(), ); + // Restore preferred header names based on the lower-case names returned // by drupal_get_header(). $header_names = _drupal_set_preferred_header_name(); foreach (drupal_get_header() as $name_lower => $value) { $cache->headers[$header_names[$name_lower]] = $value; } + if (variable_get('page_compression', TRUE) && function_exists('gzencode')) { // We do not store the data in case the zlib mode is deflate. This should // be rarely happening. @@ -3315,12 +3314,7 @@ function page_set_cache() { if ($cache_page && $cache->data) { cache_set($cache->cid, $cache->data, 'cache_page', $cache->expire, $cache->headers); } - drupal_page_cache_header($cache); - } - else { - // If output buffering was enabled during bootstrap, and the headers were - // not sent in the DRUPAL_BOOTSTRAP_LATE_PAGE_CACHE phase, send them now. - drupal_page_header(); + return $cache; } } diff --git a/includes/form.inc b/includes/form.inc index 7a05010f1..51c84f381 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -2794,7 +2794,7 @@ function form_clean_id($id = NULL, $flush = FALSE) { * foreach ($results as $result) { * $items[] = t('Loaded node %title.', array('%title' => $result)); * } - * drupal_set_session('my_batch_results', $items); + * $_SESSION['my_batch_results'] = $items; * } * @endcode */ @@ -2952,6 +2952,9 @@ function batch_process($redirect = NULL, $url = NULL) { )) ->execute(); + // Set the batch number in the session to guarantee that it will stay alive. + $_SESSION['batches'][$batch['id']] = TRUE; + drupal_goto($batch['url'], 'op=start&id=' . $batch['id']); } else { diff --git a/includes/locale.inc b/includes/locale.inc index 8241ce9ba..79613f933 100644 --- a/includes/locale.inc +++ b/includes/locale.inc @@ -613,8 +613,6 @@ function locale_translation_filters() { * @ingroup forms */ function locale_translation_filter_form() { - $session = &$_SESSION['locale_translation_filter']; - $session = is_array($session) ? $session : array(); $filters = locale_translation_filters(); $form['filters'] = array( @@ -642,8 +640,8 @@ function locale_translation_filter_form() { '#options' => $filter['options'], ); } - if (!empty($session[$key])) { - $form['filters']['status'][$key]['#default_value'] = $session[$key]; + if (!empty($_SESSION['locale_translation_filter'][$key])) { + $form['filters']['status'][$key]['#default_value'] = $_SESSION['locale_translation_filter'][$key]; } } @@ -651,7 +649,7 @@ function locale_translation_filter_form() { '#type' => 'submit', '#value' => t('Filter'), ); - if (!empty($session)) { + if (!empty($_SESSION['locale_translation_filter'])) { $form['filters']['buttons']['reset'] = array( '#type' => 'submit', '#value' => t('Reset') diff --git a/includes/session.inc b/includes/session.inc index e6bcde173..90b0b6ebc 100644 --- a/includes/session.inc +++ b/includes/session.inc @@ -128,7 +128,7 @@ function _sess_write($key, $value) { // 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() || ($user->uid == 0 && empty($_COOKIE[session_name()]) && empty($value))) { + if (!drupal_save_session() || empty($user) || (empty($user->uid) && empty($_COOKIE[session_name()]) && empty($value))) { return TRUE; } @@ -158,86 +158,117 @@ function _sess_write($key, $value) { } /** - * Propagate $_SESSION and set session cookie if not already set. This function - * should be called before writing to $_SESSION, usually via - * drupal_set_session(). - * - * @param $start - * If FALSE, the session is not actually started. This is only used by - * drupal_session_is_started(). - * @return - * TRUE if session has already been started, or FALSE if it has not. + * Initialize the session handler, starting a session if needed. */ -function drupal_session_start($start = TRUE) { - $started = &drupal_static(__FUNCTION__, FALSE); - if ($start && !$started) { - $started = TRUE; - session_start(); +function drupal_session_initialize() { + global $user; + + session_set_save_handler('_sess_open', '_sess_close', '_sess_read', '_sess_write', '_sess_destroy_sid', '_sess_gc'); + + if (isset($_COOKIE[session_name()])) { + // If a session cookie exists, initialize the session. Otherwise the + // session is only started on demand in drupal_session_commit(), making + // anonymous users not use a session cookie unless something is stored in + // $_SESSION. This allows HTTP proxies to cache anonymous pageviews. + drupal_session_start(); + if (!empty($user->uid) || !empty($_SESSION)) { + drupal_page_is_cacheable(FALSE); + } + } + else { + // Set a session identifier for this request. This is necessary because + // we lazyly start sessions at the end of this request, and some + // processes (like drupal_get_token()) needs to know the future + // session ID in advance. + $user = drupal_anonymous_user(); + session_id(md5(uniqid('', TRUE))); } - return $started; } /** - * Return whether a session has been started and the $_SESSION variable is - * available. + * Forcefully start a session, preserving already set session data. */ -function drupal_session_is_started() { - return drupal_session_start(FALSE); +function drupal_session_start() { + if (!drupal_session_started()) { + // Save current session data before starting it, as PHP will destroy it. + $session_data = isset($_SESSION) ? $_SESSION : NULL; + + session_start(); + drupal_session_started(TRUE); + + // Restore session data. + if (!empty($session_data)) { + $_SESSION += $session_data; + } + } } /** - * Get a session variable. + * Commit the current session, if necessary. * - * @param $name - * The name of the variable to get. If not supplied, all variables are returned. - * @return - * The value of the variable, or FALSE if the variable is not set. + * If an anonymous user already have an empty session, destroy it. */ -function drupal_get_session($name = NULL) { - if (is_null($name)) { - return $_SESSION; - } - elseif (isset($_SESSION[$name])) { - return $_SESSION[$name]; +function drupal_session_commit() { + global $user; + + if (empty($user->uid) && empty($_SESSION)) { + if (drupal_session_started()) { + // Destroy empty anonymous sessions. + session_destroy(); + } } else { - return FALSE; + if (!drupal_session_started()) { + drupal_session_start(); + } + // Write the session data. + session_write_close(); } } /** - * Set a session variable. The variable becomes accessible via $_SESSION[$name] - * in the current and later requests. If there is no active PHP session prior - * to the call, one is started automatically. - * - * Anonymous users generate less server load if their $_SESSION variable is - * empty, so unused entries should be unset using unset($_SESSION['foo']). - * - * @param $name - * The name of the variable to set. - * @param $value - * The value to set. + * Return whether a session has been started. */ -function drupal_set_session($name, $value) { - drupal_session_start(); - $_SESSION[$name] = $value; +function drupal_session_started($set = NULL) { + static $session_started = FALSE; + if (isset($set)) { + $session_started = $set; + } + return $session_started && session_id(); } /** * Called when an anonymous user becomes authenticated or vice-versa. */ function drupal_session_regenerate() { - $old_session_id = session_id(); + global $user; + + // Set the session cookie "httponly" flag to reduce the risk of session + // stealing via XSS. extract(session_get_cookie_params()); - // Set "httponly" to TRUE to reduce the risk of session stealing via XSS. session_set_cookie_params($lifetime, $path, $domain, $secure, TRUE); - session_regenerate_id(); - db_update('sessions') - ->fields(array( - 'sid' => session_id() - )) - ->condition('sid', $old_session_id) - ->execute(); + + if (drupal_session_started()) { + $old_session_id = session_id(); + session_regenerate_id(); + } + 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. + $account = $user; + drupal_session_start(); + $user = $account; + } + + if (isset($old_session_id)) { + db_update('sessions') + ->fields(array( + 'sid' => session_id() + )) + ->condition('sid', $old_session_id) + ->execute(); + } } /** diff --git a/install.php b/install.php index ec158f3e6..d122cbab8 100644 --- a/install.php +++ b/install.php @@ -625,7 +625,7 @@ function install_tasks($profile, $task) { drupal_install_init_database(); drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL); - drupal_set_session('messages', $messages); + $_SESSION['messages'] = $messages; // URL used to direct page requests. $url = $base_url . '/install.php?locale=' . $install_locale . '&profile=' . $profile; diff --git a/modules/book/book.install b/modules/book/book.install index 3dc117282..c0ea1be01 100644 --- a/modules/book/book.install +++ b/modules/book/book.install @@ -134,8 +134,8 @@ function book_update_6000() { db_create_table($ret, 'book', $schema['book']); - drupal_set_session('book_update_6000_orphans', array('from' => 0)); - drupal_set_session('book_update_6000', array()); + $_SESSION['book_update_6000_orphans'] = array('from' => 0); + $_SESSION['book_update_6000'] = array(); $result = db_query("SELECT * from {book_temp} WHERE parent = 0"); // Collect all books - top-level nodes. @@ -152,7 +152,7 @@ function book_update_6000() { return $ret; } } - elseif ($_SESSION['book_update_6000_orphans']) { + elseif (isset($_SESSION['book_update_6000_orphans'])) { // Do the first batched part of the update - collect orphans. $update_count = 400; // Update this many at a time. diff --git a/modules/dblog/dblog.admin.inc b/modules/dblog/dblog.admin.inc index 945a85a0e..53389586d 100644 --- a/modules/dblog/dblog.admin.inc +++ b/modules/dblog/dblog.admin.inc @@ -284,10 +284,6 @@ function _dblog_format_message($dblog) { * @see dblog_filter_form_validate() */ function dblog_filter_form() { - if (!isset($_SESSION['dblog_overview_filter'])) { - drupal_set_session('dblog_overview_filter', array()); - } - $session = &$_SESSION['dblog_overview_filter']; $filters = dblog_filters(); $form['filters'] = array( @@ -305,8 +301,8 @@ function dblog_filter_form() { '#size' => 8, '#options' => $filter['options'], ); - if (!empty($session[$key])) { - $form['filters']['status'][$key]['#default_value'] = $session[$key]; + if (!empty($_SESSION['dblog_overview_filter'][$key])) { + $form['filters']['status'][$key]['#default_value'] = $_SESSION['dblog_overview_filter'][$key]; } } @@ -314,7 +310,7 @@ function dblog_filter_form() { '#type' => 'submit', '#value' => t('Filter'), ); - if (!empty($session)) { + if (!empty($_SESSION['dblog_overview_filter'])) { $form['filters']['buttons']['reset'] = array( '#type' => 'submit', '#value' => t('Reset') @@ -343,15 +339,12 @@ function dblog_filter_form_submit($form, &$form_state) { case t('Filter'): foreach ($filters as $name => $filter) { if (isset($form_state['values'][$name])) { - if (!isset($_SESSION['dblog_overview_filter'])) { - drupal_set_session('dblog_overview_filter', array()); - } $_SESSION['dblog_overview_filter'][$name] = $form_state['values'][$name]; } } break; case t('Reset'): - drupal_set_session('dblog_overview_filter', array()); + $_SESSION['dblog_overview_filter'] = array(); break; } return 'admin/reports/dblog'; diff --git a/modules/node/node.admin.inc b/modules/node/node.admin.inc index 74d41146b..67790b1f4 100644 --- a/modules/node/node.admin.inc +++ b/modules/node/node.admin.inc @@ -171,7 +171,8 @@ function node_build_filter_query() { // Build query $where = $args = array(); $join = ''; - foreach ($_SESSION['node_overview_filter'] as $index => $filter) { + $filter_data = isset($_SESSION['node_overview_filter']) ? $_SESSION['node_overview_filter'] : array(); + foreach ($filter_data as $index => $filter) { list($key, $value) = $filter; switch ($key) { case 'status': @@ -202,10 +203,7 @@ function node_build_filter_query() { * Return form for node administration filters. */ function node_filter_form() { - if (!isset($_SESSION['node_overview_filter'])) { - drupal_set_session('node_overview_filter', array()); - } - $session = &$_SESSION['node_overview_filter']; + $session = isset($_SESSION['node_overview_filter']) ? $_SESSION['node_overview_filter'] : array(); $filters = node_filters(); $i = 0; @@ -320,9 +318,6 @@ function node_filter_form_submit($form, &$form_state) { $flat_options = form_options_flatten($filters[$filter]['options']); if (isset($flat_options[$form_state['values'][$filter]])) { - if (!isset($_SESSION['node_overview_filter'])) { - drupal_set_session('node_overview_filter', array()); - } $_SESSION['node_overview_filter'][] = array($filter, $form_state['values'][$filter]); } } @@ -331,7 +326,7 @@ function node_filter_form_submit($form, &$form_state) { array_pop($_SESSION['node_overview_filter']); break; case t('Reset'): - drupal_set_session('node_overview_filter', array()); + $_SESSION['node_overview_filter'] = array(); break; } } diff --git a/modules/openid/openid.inc b/modules/openid/openid.inc index 9bc4de789..debf16169 100644 --- a/modules/openid/openid.inc +++ b/modules/openid/openid.inc @@ -61,6 +61,10 @@ function openid_redirect_http($url, $message) { $sep = (strpos($url, '?') === FALSE) ? '?' : '&'; header('Location: ' . $url . $sep . implode('&', $query), TRUE, 302); + + // Commit session data before redirecting. + drupal_session_commit(); + exit; } @@ -73,6 +77,10 @@ function openid_redirect($url, $message) { $output .= '<script type="text/javascript">document.getElementById("openid-redirect-form").submit();</script>'; $output .= "</body></html>\n"; print $output; + + // Commit session data before redirecting. + drupal_session_commit(); + exit; } diff --git a/modules/openid/openid.module b/modules/openid/openid.module index 03053333f..a879ccdb7 100644 --- a/modules/openid/openid.module +++ b/modules/openid/openid.module @@ -174,9 +174,6 @@ function openid_begin($claimed_id, $return_to = '', $form_values = array()) { } // Store discovered information in the users' session so we don't have to rediscover. - if (!isset($_SESSION['openid'])) { - drupal_set_session('openid', array()); - } $_SESSION['openid']['service'] = $services[0]; // Store the claimed id $_SESSION['openid']['claimed_id'] = $claimed_id; @@ -434,9 +431,6 @@ function openid_authentication($response) { // We were unable to register a valid new user, redirect to standard // user/register and prefill with the values we received. drupal_set_message(t('OpenID registration failed for the reasons listed. You may register now, or if you already have an account you can <a href="@login">log in</a> now and add your OpenID under "My Account"', array('@login' => url('user/login'))), 'error'); - if (!isset($_SESSION['openid'])) { - drupal_set_session('openid', array()); - } $_SESSION['openid']['values'] = $form_state['values']; // We'll want to redirect back to the same place. $destination = drupal_get_destination(); diff --git a/modules/simpletest/tests/session.test b/modules/simpletest/tests/session.test index d713cbb38..d53c14c20 100644 --- a/modules/simpletest/tests/session.test +++ b/modules/simpletest/tests/session.test @@ -31,12 +31,15 @@ class SessionTestCase extends DrupalWebTestCase { // Test session hardening code from SA-2008-044. $user = $this->drupalCreateUser(array('access content')); + // Enable sessions. $this->sessionReset($user->uid); + // Make sure the session cookie is set as HttpOnly. $this->drupalLogin($user); $this->assertTrue(preg_match('/HttpOnly/i', $this->drupalGetHeader('Set-Cookie', TRUE)), t('Session cookie is set as HttpOnly.')); $this->drupalLogout(); + // Verify that the session is regenerated if a module calls exit // in hook_user_login(). user_save($user, array('name' => 'session_test_user')); @@ -46,6 +49,7 @@ class SessionTestCase extends DrupalWebTestCase { preg_match('/\s*session_id:(.*)\n/', $this->drupalGetContent(), $matches); $this->assertTrue(!empty($matches[1]) , t('Found session ID before logging in.')); $original_session = $matches[1]; + // We cannot use $this->drupalLogin($user); because we exit in // session_test_user_login() which breaks a normal assertion. $edit = array( @@ -69,12 +73,16 @@ class SessionTestCase extends DrupalWebTestCase { * drupal_session_count() since session data is already generated here. */ function testDataPersistence() { + // At the very start, we have no session. + $expected_anonymous = 0; + $expected_authenticated = 0; + $user = $this->drupalCreateUser(array('access content')); // Enable sessions. $this->sessionReset($user->uid); $this->drupalLogin($user); - $this->session_count_authenticated = $this->session_count++; + $expected_authenticated++; $value_1 = $this->randomName(); $this->drupalGet('session-test/set/' . $value_1); @@ -97,51 +105,56 @@ class SessionTestCase extends DrupalWebTestCase { // Logout the user and make sure the stored value no longer persists. $this->drupalLogout(); + $expected_authenticated--; + $this->sessionReset(); $this->drupalGet('session-test/get'); - // Session count should go up since we're accessing anonymously now. - $this->session_count_anonymous = $this->session_count++; $this->assertNoText($value_1, t("After logout, previous user's session data is not available."), t('Session')); + // Now try to store some data as an anonymous user. $value_3 = $this->randomName(); $this->drupalGet('session-test/set/' . $value_3); $this->assertText($value_3, t('Session data stored for anonymous user.'), t('Session')); $this->drupalGet('session-test/get'); $this->assertText($value_3, t('Session correctly returned the stored data for an anonymous user.'), t('Session')); + // Session count should go up since we have started an anonymous session now. + $expected_anonymous++; + // Try to store data when drupal_save_session(FALSE). $value_4 = $this->randomName(); $this->drupalGet('session-test/no-set/' . $value_4); $this->assertText($value_4, t('The session value was correctly passed to session-test/no-set.'), t('Session')); $this->drupalGet('session-test/get'); $this->assertText($value_3, t('Session data is not saved for drupal_save_session(FALSE).'), t('Session')); - // Logout and get first user back in. Sessions shouldn't persist through - // logout, so the data won't be on the page. + // Login, the data should persist. $this->drupalLogin($user); + $expected_anonymous--; + $expected_authenticated++; $this->sessionReset($user->uid); $this->drupalGet('session-test/get'); $this->assertNoText($value_1, t('Session has persisted for an authenticated user after logging out and then back in.'), t('Session')); - // Logout and create another user. + // Change session and create another user. $user2 = $this->drupalCreateUser(array('access content')); $this->sessionReset($user2->uid); $this->drupalLogin($user2); - $this->session_count_authenticated = $this->session_count++; + $expected_authenticated++; // Perform drupal_session_count tests here in order to use the session data already generated. // Test absolute count. $anonymous = drupal_session_count(0, TRUE); $authenticated = drupal_session_count(0, FALSE); - $this->assertEqual($anonymous + $authenticated, $this->session_count, t('Correctly counted @count total sessions.', array('@count' => $this->session_count)), t('Session')); + $this->assertEqual($anonymous + $authenticated, $expected_anonymous + $expected_authenticated, t('@count total sessions (expected @expected).', array('@count' => $anonymous + $authenticated, '@expected' => $expected_anonymous + $expected_authenticated)), t('Session')); // Test anonymous count. - $this->assertEqual($anonymous, $this->session_count_anonymous, t('Correctly counted @count anonymous sessions.', array('@count' => $anonymous)), t('Session')); + $this->assertEqual($anonymous, $expected_anonymous, t('@count anonymous sessions (expected @expected).', array('@count' => $anonymous, '@expected' => $expected_anonymous)), t('Session')); // Test authenticated count. - $this->assertEqual($authenticated, $this->session_count_authenticated, t('Correctly counted @count authenticated sessions.', array('@count' => $authenticated)), t('Session')); + $this->assertEqual($authenticated, $expected_authenticated, t('@count authenticated sessions (expected @expected).', array('@count' => $authenticated, '@expected' => $expected_authenticated)), t('Session')); // Should return 0 sessions from 1 second from now. - $this->assertEqual(drupal_session_count(time() + 1), 0, t('Correctly returned 0 sessions newer than the current time.'), t('Session')); + $this->assertEqual(drupal_session_count(time() + 1), 0, t('0 sessions newer than the current time.'), t('Session')); } @@ -149,83 +162,39 @@ class SessionTestCase extends DrupalWebTestCase { * Test that empty anonymous sessions are destroyed. */ function testEmptyAnonymousSession() { - // With caching disabled, a session is always started. + // Verify that no session is automatically created for anonymous user. $this->drupalGet(''); $this->assertSessionCookie(FALSE); - $this->assertSessionStarted(TRUE); $this->assertSessionEmpty(TRUE); + // The same behavior is expected when caching is enabled. variable_set('cache', CACHE_NORMAL); - - // During this request the session is destroyed in drupal_page_footer(), - // and the session cookie is unset. $this->drupalGet(''); - $this->assertSessionCookie(TRUE); - $this->assertSessionStarted(TRUE); + $this->assertSessionCookie(FALSE); $this->assertSessionEmpty(TRUE); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', t('Page was not cached.')); - // When PHP deletes a cookie, it sends "Set-Cookie: cookiename=deleted; - // expires=..." - $this->assertTrue(preg_match('/SESS\w+=deleted/', $this->drupalGetHeader('Set-Cookie')), t('Session cookie was deleted.')); - - // Verify that the session cookie was actually deleted. - $this->drupalGet(''); - $this->assertSessionCookie(FALSE); - $this->assertSessionStarted(FALSE); - $this->assertFalse($this->drupalGetHeader('Set-Cookie'), t('New session was not started.')); // Start a new session by setting a message. $this->drupalGet('session-test/set-message'); - $this->assertSessionCookie(FALSE); - $this->assertSessionStarted(FALSE); + $this->assertSessionCookie(TRUE); $this->assertTrue($this->drupalGetHeader('Set-Cookie'), t('New session was started.')); - // Display the message. + // Display the message, during the same request the session is destroyed + // and the session cookie is unset. $this->drupalGet(''); - $this->assertSessionCookie(TRUE); - $this->assertSessionStarted(TRUE); + $this->assertSessionCookie(FALSE); $this->assertSessionEmpty(FALSE); $this->assertFalse($this->drupalGetHeader('X-Drupal-Cache'), t('Caching was bypassed.')); $this->assertText(t('This is a dummy message.'), t('Message was displayed.')); - - // During this request the session is destroyed in _drupal_bootstrap(), - // and the session cookie is unset. - $this->drupalGet(''); - $this->assertSessionCookie(TRUE); - $this->assertSessionStarted(TRUE); - $this->assertSessionEmpty(TRUE); - $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', t('Page was cached.')); - $this->assertNoText(t('This is a dummy message.'), t('Message was not cached.')); $this->assertTrue(preg_match('/SESS\w+=deleted/', $this->drupalGetHeader('Set-Cookie')), t('Session cookie was deleted.')); // Verify that session was destroyed. $this->drupalGet(''); $this->assertSessionCookie(FALSE); - $this->assertSessionStarted(FALSE); + $this->assertSessionEmpty(TRUE); + $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 modifying $_SESSION without having started a session - // generates a watchdog message, and that no messages have been generated - // so far. - $this->assertEqual($this->getWarningCount(), 0, t('No watchdog messages have been generated')); - $this->drupalGet('/session-test/set-not-started'); - $this->assertSessionCookie(FALSE); - $this->assertSessionStarted(FALSE); - $this->assertEqual($this->getWarningCount(), 1, t('1 watchdog messages has been generated')); - } - - /** - * Count watchdog messages about modifying $_SESSION without having started a - * session. - */ - function getWarningCount() { - return db_select('watchdog') - ->condition('type', 'session') - ->condition('message', '$_SESSION is non-empty yet no code has called drupal_session_start().') - ->countQuery() - ->execute() - ->fetchField(); } /** @@ -250,22 +219,10 @@ class SessionTestCase extends DrupalWebTestCase { */ function assertSessionCookie($sent) { if ($sent) { - $this->assertIdentical($this->drupalGetHeader('X-Session-Cookie'), '1', t('Session cookie was sent.')); - } - else { - $this->assertIdentical($this->drupalGetHeader('X-Session-Cookie'), '0', t('Session cookie was not sent.')); - } - } - - /** - * Assert whether session was started during the bootstrap process. - */ - function assertSessionStarted($started) { - if ($started) { - $this->assertIdentical($this->drupalGetHeader('X-Session-Started'), '1', t('Session was started.')); + $this->assertNotNull($this->session_id, t('Session cookie was sent.')); } else { - $this->assertIdentical($this->drupalGetHeader('X-Session-Started'), '0', t('Session was not started.')); + $this->assertNull($this->session_id, t('Session cookie was not sent.')); } } diff --git a/modules/simpletest/tests/session_test.module b/modules/simpletest/tests/session_test.module index 352a21e25..eb656ce80 100644 --- a/modules/simpletest/tests/session_test.module +++ b/modules/simpletest/tests/session_test.module @@ -51,22 +51,10 @@ function session_test_menu() { * Implement hook_boot(). */ function session_test_boot() { - header('X-Session-Cookie: ' . intval(isset($_COOKIE[session_name()]))); - header('X-Session-Started: ' . intval(drupal_session_is_started())); header('X-Session-Empty: ' . intval(empty($_SESSION))); } /** - * Implement hook_init(). - */ -function session_test_init() { - // hook_init() is called later in the bootstrap process, but not in cached - // requests. Here the header set in hook_boot() is overwritten, so the - // session state is reported as late in the bootstrap process as possible. - header('X-Session-Started: ' . intval(drupal_session_is_started())); -} - -/** * Page callback, prints the stored session value to the screen. */ function _session_test_get() { @@ -82,7 +70,7 @@ function _session_test_get() { * Page callback, stores a value in $_SESSION['session_test_value']. */ function _session_test_set($value) { - drupal_set_session('session_test_value', $value); + $_SESSION['session_test_value'] = $value; return t('The current value of the stored session variable has been set to %val', array('%val' => $value)); } @@ -100,6 +88,12 @@ function _session_test_no_set($value) { * Menu callback: print the current session ID. */ function _session_test_id() { + // Set a value in $_SESSION, so that drupal_session_commit() will start + // a session. + $_SESSION['test'] = 'test'; + + drupal_session_commit(); + return 'session_id:' . session_id() . "\n"; } @@ -119,7 +113,7 @@ function _session_test_set_message() { * having started the session in advance. */ function _session_test_set_not_started() { - if (!drupal_session_is_started()) { + if (!drupal_session_will_start()) { $_SESSION['session_test_value'] = t('Session was not started'); } } diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc index 83cca4ad2..6fd477674 100644 --- a/modules/system/system.admin.inc +++ b/modules/system/system.admin.inc @@ -1734,7 +1734,7 @@ function system_clean_url_settings() { // When accessing this form using a non-clean URL, allow a re-check to make // sure clean URLs can be disabled at all times. $available = FALSE; - if (strpos(request_uri(), '?q=') === FALSE || drupal_get_session('clean_url')) { + if (strpos(request_uri(), '?q=') === FALSE || !empty($_SESSION['clean_url'])) { $available = TRUE; } else { @@ -1745,7 +1745,7 @@ function system_clean_url_settings() { } if ($available) { - drupal_set_session('clean_url', TRUE); + $_SESSION['clean_url'] = TRUE; $form['clean_url'] = array( '#type' => 'checkbox', diff --git a/modules/system/system.install b/modules/system/system.install index 2bfe95db7..1f535c586 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -1983,10 +1983,10 @@ function system_update_6021() { // Multi-part update if (!isset($_SESSION['system_update_6021'])) { db_add_field($ret, 'menu', 'converted', array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE, 'default' => 0, 'size' => 'tiny')); - drupal_set_session('system_update_6021_max', db_result(db_query('SELECT COUNT(*) FROM {menu}'))); - drupal_set_session('menu_menu_map', array(1 => 'navigation')); + $_SESSION['system_update_6021_max'] = db_result(db_query('SELECT COUNT(*) FROM {menu}')); + $_SESSION['menu_menu_map'] = array(1 => 'navigation'); // 0 => FALSE is for new menus, 1 => FALSE is for the navigation. - drupal_set_session('menu_item_map', array(0 => FALSE, 1 => FALSE)); + $_SESSION['menu_item_map'] = array(0 => FALSE, 1 => FALSE); $table = array( 'fields' => array( 'menu_name' => array('type' => 'varchar', 'length' => 32, 'not null' => TRUE, 'default' => ''), @@ -1997,7 +1997,7 @@ function system_update_6021() { ); db_create_table($ret, 'menu_custom', $table); db_query("INSERT INTO {menu_custom} (menu_name, title, description) VALUES ('%s', '%s', '%s')", $menus['navigation']); - drupal_set_session('system_update_6021', 0); + $_SESSION['system_update_6021'] = 0; } $limit = 50; diff --git a/modules/user/user.admin.inc b/modules/user/user.admin.inc index 08f9b1ba5..b53f9b927 100644 --- a/modules/user/user.admin.inc +++ b/modules/user/user.admin.inc @@ -33,10 +33,7 @@ function user_admin($callback_arg = '') { * @see user_filter_form_submit() */ function user_filter_form() { - if (!isset($_SESSION['user_overview_filter'])) { - drupal_set_session('user_overview_filter', array()); - } - $session = &$_SESSION['user_overview_filter']; + $session = isset($_SESSION['user_overview_filter']) ? $_SESSION['user_overview_filter'] : array(); $filters = user_filters(); $i = 0; @@ -103,9 +100,6 @@ function user_filter_form_submit($form, &$form_state) { // Merge an array of arrays into one if necessary. $options = $filter == 'permission' ? call_user_func_array('array_merge', $filters[$filter]['options']) : $filters[$filter]['options']; if (isset($options[$form_state['values'][$filter]])) { - if (!isset($_SESSION['user_overview_filter'])) { - drupal_set_session('user_overview_filter', array()); - } $_SESSION['user_overview_filter'][] = array($filter, $form_state['values'][$filter]); } } @@ -114,7 +108,7 @@ function user_filter_form_submit($form, &$form_state) { array_pop($_SESSION['user_overview_filter']); break; case t('Reset'): - drupal_set_session('user_overview_filter', array()); + $_SESSION['user_overview_filter'] = array(); break; case t('Update'): return; diff --git a/modules/user/user.module b/modules/user/user.module index 460d6858e..089f121ab 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1715,10 +1715,12 @@ function user_authenticate_finalize(&$edit) { ->fields(array('login' => $user->login)) ->condition('uid', $user->uid) ->execute(); + // Regenerate the session ID to prevent against session fixation attacks. // This is called before hook_user in case one of those functions fails // or incorrectly does a redirect which would leave the old session in place. drupal_session_regenerate(); + user_module_invoke('login', $edit, $user); } @@ -2482,7 +2484,7 @@ function user_build_filter_query(SelectQuery $query) { $filters = user_filters(); // Extend Query with filter conditions. - foreach ($_SESSION['user_overview_filter'] as $filter) { + foreach (isset($_SESSION['user_overview_filter']) ? $_SESSION['user_overview_filter'] : array() as $filter) { list($key, $value) = $filter; // This checks to see if this permission filter is an enabled permission for // the authenticated role. If so, then all users would be listed, and we can diff --git a/update.php b/update.php index 89c33b9d9..a0fb6d924 100644 --- a/update.php +++ b/update.php @@ -292,7 +292,7 @@ function update_batch() { // During the update, bring the site offline so that schema changes do not // affect visiting users. - drupal_set_session('site_offline', variable_get('site_offline', FALSE)); + $_SESSION['site_offline'] = variable_get('site_offline', FALSE); if ($_SESSION['site_offline'] == FALSE) { variable_set('site_offline', TRUE); } @@ -326,9 +326,9 @@ function update_finished($success, $results, $operations) { // clear the caches in case the data has been updated. drupal_flush_all_caches(); - drupal_set_session('update_results', $results); - drupal_set_session('update_success', $success); - drupal_set_session('updates_remaining', $operations); + $_SESSION['update_results'] = $results; + $_SESSION['update_success'] = $success; + $_SESSION['updates_remaining'] = $operations; // Now that the update is done, we can put the site back online if it was // previously turned off. |