- Patch #477944 by Damien Tournoud: fix and streamline page cache and session handling.

6 files changed, 164 insertions, 149 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();
+  }
 }
 
 /**