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

2 files changed, 43 insertions, 92 deletions

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');
   }
 }