Messages are lost if set just prior to logout

Session should be regenerated not destroyed. I'm trying to look latter

I believe this is delt with in the (still incomplete #813492: HTTPS sessions use an invalid merge query).

Temporarily un-duplicating and bumping, to keep this on the radar.

Checked manually (the test modified by #5 seems to be gone):

- messages triggered on logout are still lost in oblivion
- messages triggered on login and user insert work

Downgrading. This only affects logout, and looks by-design to me:

  watchdog('user', 'Session closed for %name.', array('%name' => $user->name));

  module_invoke_all('user_logout', $user);

  // Destroy the current session, and reset $user to the anonymous user.
  session_destroy();

  drupal_goto();

We could consider being slightly less heavy-handed here (for example by defining a hook to prevent part of the session from being destroyed). But at least this works as designed.

Just in case anyone Googling this runs across this issue - drupal_set_message stores the message in the database, tied to session. user_logout destroys the session and does a drupal_goto. Users will get a new session as soon as they load the drupal_goto'd page, but there's no way to link that to the message, which was associated with their old session. To get the drupal_set_message functionality without something like including a querystring and checking for it on the destination page, we need to give them their new session *before* the goto. Here's how I did that:

1) Call session_get_cookie_params to get the current session's attributes
2) Call session_destroy to end the current session
3) Call session_set_cookie_params based on the values returned by 1)
4) Call session_start to start a new session
5) Log the user out with $user = drupal_anonymous_user(session_id()); instead of just $user = drupal_anonymous_user();
6) drupal_set_message followed by drupal_goto will now work.

I don't know if this is the safest/sanest approach, regarding e.g. attacks on session ID, but it at least works for my simple use case. Suggestions for refinement would be appreciated.

(This was in D6, by the way, but the logout code is similar.)

in #11.5 did you mean "Log the user IN...", not "Log the user out..." If not, huh?

Yes, good catch, thanks.

This affects Drupal 6 too. Would like to see it resolved for both D6 and D7 as doing a drupal_set_message on hook_user('logout') is beneficial for the user experience.

@eronarn: I tried out your idea with the following code:

  // 1) Call session_get_cookie_params to get the current session's attributes
  $p = session_get_cookie_params();
  // 2) Call session_destroy to end the current session
  session_destroy();
  // 3) Call session_set_cookie_params based on the values returned by 1)
  session_set_cookie_params($p['lifetime'], $p['path'], $p['domain'], $p['secure'], $p['httponly']);
  // 4) Call session_start to start a new session
  session_start();
  // 5) Log the user out with $user = drupal_anonymous_user(session_id());
  // instead of just $user = drupal_anonymous_user();
  $user = drupal_anonymous_user(session_id());
  // 6) drupal_set_message followed by drupal_goto will now work.
  drupal_goto();

Unfortunately it gives the following error:
Fatal error: session_start() [<a href='function.session-start'>function.session-start</a>]: Failed to initialize storage module: user (path: /Applications/MAMP/tmp/php) in /modules/user/user.pages.inc on line 157
Going to dive into it some more.

I've resolved the error by changing the session_start part to:

  session_set_save_handler('sess_open', 'sess_close', 'sess_read', 'sess_write', 'sess_destroy_sid', 'sess_gc');
  session_start();

Now to retain the messages.

Here's a patch against Drupal 6.

Patch sponsored by the Siebel Scholars Foundation.

no one-letter variable names. as for the rest, i have no idea

Changed $p to $params.

Patches for both D6 and D7.

This is still a problem and the scope is larger than originally outlined. Any other modules that set $_SESSION variables which aren't tied to the login lose their data on logout.

The D7 patch is outdated and needs reworking. Not sure about the D6 patch.

Moving to 8.x

+++ modules/user/user.pages.inc	17 Jan 2011 18:27:54 -0000
@@ -170,9 +170,22 @@ function user_logout() {
+  // Obtain the current session's attributes.

This part cares only about session params but other content stored is going to be lost

Powered by Dreditor.

subscribe (sorry if this is the wrong way to do it, but I can't see a monitor/subscribe button anywhere).

Related: #758730: Fatal error on logout.

The route forward seems to be what I asked for in #9: we implement drupal_session_destroy() as described in #758730: Fatal error on logout and we add a hook in there that allow contrib modules to rescue part of $_SESSION.

Adding session tag.

IMO, this is a small problem, not worthy a complex solution. Code that wants to set a message after logout should try to run after the destroy.

Here's a simple solution: don't destroy the session on logout.

Session != login
Drupal seems to make this assumption, however.

Ran into the same problem with Drupal 7.12: after logging out all shopping cart related data (custom module) is gone...

@moshe weitzman:

Code that wants to set a message after logout should try to run after the destroy.

Could you sketch in how this might work? Sample use case: using Rules, log out a user and present them a message about having been logged out. Currently, this would be done with two actions: (a) set a message and (b) redirect (to user/logout). But if the message is set before redirecting, it is never presented to the user as the session ends. If it is set after redirecting, it is never executed because the page execution (and session) have ended. How could a rule action execute after the destroy?

+++ b/modules/user/user.pages.inc
@@ -171,9 +171,22 @@ function user_logout() {
+  session_set_save_handler('sess_open', 'sess_close', 'sess_read', 'sess_write', 'sess_destroy_sid', 'sess_gc');

For D7+ the session calls are renamed, see http://drupal.org/node/224333#session_rename.

I have added so it retains all the old session data. This patch is for drupal 7.

Forgot to set the correct status.

One thing to think about is security as it will retain all the previous session data. So if a module that don't want data to be retained need to implement hook_user_logout and remove the session data there.

@yingtho #39
Agreed! Regenerating the whole session array is hazardous, not what you would expect, and acutely contrary to the concept of being logged in (versus not).
Only the 'messages' bucket - $_SESSION['messages'] - should be regenerated (and that only if non-empty ;-)

Relaying the responsibility of destroying session data to modules is not correct - and would btw never be carried into effect in the real world.

Set Drupal message that survives session destruction (like user logout):
Module State - state_set_message(); documentation.

Do we still need to address this in D8, or have session handling changed to such a degree that this issue only apply to D7?

Is there a steps to reproduce?

Reproduction in D7:

drupal_set_message('You\'ll never see this message...');
module_load_include('pages.inc', 'user');
user_logout();

Thanks @DamienMcKenna for the 9 year old D6 patch.

We would still want to fix this bug in D8 as well. Thanks!

Retitling this one.

At least on D7, core has been refactored a bit since the last patch from 8yrs ago. The solution now (probably similar on D8) is something like this:

      module_load_include('pages.inc', 'user');
      user_logout_current_user();
      drupal_set_message('My message that the user will see after they log out.');
      drupal_goto();

Drupal 8.9.x is now open only to security issues.

I think this is doable in D9+ as follows:

Implement hook_user_logout which gets called before logout, set some sort of flag that a user is being logged out.

Add an event listener for the \Drupal\Core\Session\AccountEvents::SET_USER event, if the flag from above is set and the new set user is anonymous, add the message, if not, unset the flag.

Bit janky to have a hook for before and event for after, but the APIs are there.

Should this not be fixed in core?

Are you meaning that core copies the messages from one session to the next?

Yes.

Ok, then let's keep it open