sess_write() seems to be broken on login when using Pressflow. I am not experiencing these problems with vanilla Drupal...

When the user first attempts to log in, a cookie gets set in the browser, but nothing gets written to the session bin, and the user is not recognized as an authenticated user.

If the user immediately tries to log in again, i.e. when there is already a cookie set in the browser, at that point, the session is written correctly to the session bin.

Upon further examination of the sess_write() code:

function sess_write($key, $value) {
  global $user;

  // If the client doesn't have a session, and one isn't being created ($value),
  // do nothing. If session saving has been turned off, do nothing.
  // Cases 1a, 1c, and 2c are covered here.
  if ((empty($_COOKIE[session_name()]) && empty($value)) || !session_save_session()) {
    return TRUE;
  }

If I add an additional check for $user->uid == 0 (which is also the way recent versions of Drupal's session.inc code does it)

if (!session_save_session() || ($user->uid == 0 && empty($_COOKIE[session_name()]) && empty($value)))

the problem for Pressflow is resolved.

I am aware that this problem is probably related to Pressflow's changes, however, I think check should be there anyways.

CommentFileSizeAuthor
memcache-session.inc_.patch597 bytesken54671

Comments

spindler’s picture

spindler commented

I've just run into this same problem and I'm not using pressflow. I found the same fix as you by debugging the code. I think this fix needs to be committed if for no other reason that it matches the behavior of the default drupal session cache. OTOH things don't seem to happen around here.

jeremy’s picture

jeremy commented

This looks correct to me. Does anyone want to weight in as to why memcache-session.inc is missing the test for ($user->uid == 0)? Are there any reasons to not commit this?

Flying Drupalist’s picture

Flying Drupalist commented

Please consider a commit.

jcmarco’s picture

jcmarco commented
Status: Active » Reviewed & tested by the community

Tested and working fine

johnthomas00’s picture

johnthomas00 commented

Works for me too. Please commit.

jtsnow’s picture

jtsnow commented

This did not fix the problem for me. However the problem doesn't occur on every login attempt. The problem is sporadic with and without the patch. Could something in the memcached bin configuration be causing this? I the bin is not full and I am nowhere near the max number of connections.

quicksketch’s picture

quicksketch
he/him
commented

Perhaps this is related to #791888: sess_destroy_sid() does not remove the session cookie from the user's browser, where sess_destroy_sid() is broken also.

andreiashu’s picture

andreiashu commented
Priority: Normal » Major

I got here because of the same bug. I managed to replicate the behaviour and indeed the patch fixes the problem.
Raising the priority as this is quite a blocker for pressflow websites (and I would think that a lot of pressflow instances use memcache for session control) that need login functionality.

Anonymous’s picture

Anonymous (not verified) commented

I work on pressflow and the problem is permanent. The only solution is to disable the memcache for the session.
Otherwise the solution is this patch.
Please, commit.

Mark Theunissen’s picture

Mark Theunissen commented

This patch has been live on Economist.com for a while and has fixed the issue. RTBC.

mwillis’s picture

mwillis commented

Recently installed pressflow and began having intermittent failures to login as well when using the memcache-session.inc. This patch worked for me. I'd think that since pressflow "breaks" anonymous user cookies in order to work with external caching mechanisms that the fixed memcache-session file should be an optional part of the pressflow package. If there's no harm to non-pressflow memcache-session users though, a commit here would be appreciated just the same.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed http://drupalcode.org/project/memcache.git/commit/e9228c0 thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.