sess_destroy_sid() does not remove the session cookie from the user's browser

i have noticed this also, however, the code you posted is pressflow's core not drupal's. It'd be great to see this code added since most that use memcache are likely to be using pressflow.

I agree - eat those cookies whenever possible!

Yep, we've seen this one on Economist.com.

The Pressflow code is more or less a straight backport from D7. Seems sensible to copy the whole thing across here, so here's a patch to do that.

whitespace.

#6 is working for me, thanks. It kills the Drupal/Pressflow session cookie on logout.

One small issue I'm trying to work out is that the "normal" session cookie is destroyed, but I'm using Securepages Prevent Hijack, which also sets a secure cookie on the page. This SSL session cookie hangs around on logout since this code only kills the regular cookie.

The Pressflow module cookie_cache_bypass is another example, at least in my case. That module defines a cookie called NO_CACHE on form submit. In my case that cookie hangs around after logout as well, although it's only valid for cache_lifetime or 5 minutes.

How do you feel about defining a hook here for other modules to kill their cookies? module_invoke_all() so that the other modules can jump in and get rid of whatever other cookies they decided to define. I'm open to other options, but who knows how many other places people have defined cookies.

For what it's worth, here is what works for SecurePages Prevent Hijack cookies.

1) Make sure to configure SecurePages so the /logout path is secure. Otherwise the cookies aren't in $_COOKIE.

2) Add this block of code with the #6 patch above.

// Unset the Securepages Prevent Hijack cookie.
if (isset($_COOKIE[SECUREPAGES_SESSID]) || drupal_valid_token($_COOKIE[SECUREPAGES_SESSID], 'securepages_prevent_hijack')) {
  if (!isset($params)) {
    $params = session_get_cookie_params();
  }
  setcookie(SECUREPAGES_SESSID, '', $_SERVER['REQUEST_TIME'] - 3600, $params['path'], $params['domain'], 1);
  unset($_COOKIE[SECUREPAGES_SESSID]);
 }

For good measure I clear out any NO_CACHE cookies from cookie_cache_bypass as well.

// Pressflow cookie_cache_bypass
if (isset($_COOKIE['NO_CACHE'])) {
  global $cookie_domain;
  setcookie('NO_CACHE', '', $_SERVER['REQUEST_TIME'] - 3600, '/', $cookie_domain);
}

That's it for me, then all of the cookies are gone except the Google Analytics stuff which is properly configured in the upstream proxy.

+1 for #6, seems to work well for me.

Patch from #6 works. While working with Pressflow and Varnish I independently came to the same conclusion.

I'm still left questioning how/why/if Pressflow should or can be changed to take care of this. Lazy session creation should clean up after itself instead of needing to reimplement it in every session.inc out there. Any comments anyone?

@cedarm, core in Drupal 7 has the same code in session.inc, so I'd post that to a core issue for 8.x. This logic should definitely happen outside of session.inc, it's independent of where sessions are stored.

Committed this to 6.x-1.x, needs to be forward ported to 7.x, so moving there.

I've had problems with the patch from #6.
Some users could log in fine and everything worked, but others would get kicked out immediately after log in. Not sure what the cause was.

Running:
Pressflow 6.20.97
Memcache 1.8
3 memcache servers and 1 default bin.

I recently setup a new server using Memcached and Varnish and noticed a very odd behavior after moving the sites over to pressflow and dropping in this module. I would get random occurrences of people visiting example1.com and getting content from example2.com. In addition there would be situations where someone would login correctly to example1.com and get the admin backend for example2.com.

A) Is this behavior likely a result of the issue discussed here?

For now, we have shut down memcache until this is resolved.

B) Should this issue be marked as D7? Seems all the discussion here is for D6.

@rjmackay: if you upgrade to 6.19 the login issue with pressflow should be fixed, that's nothing to do with this patch.

@jlmeredith - this is against 7.x because the patch was already committed to 6.x.

example1.com vs. example2.com has nothing to do with this issue. You should check your memcache configuration to make sure it's not pointing both sites at the same memcache bin without using prefixes - otherwise each site will request and set the same cache items. You can either separate the bins or use prefixes to avoid this. Please open another issue for this since it's not related to this one.

Logic is already in 7.x branch copied from 7.x core so moving back to 6.x and fixed.