Opened 13 years ago

Closed 13 years ago

#17810 closed Bug (fixed)

Switching from cookie-based sessions to memcached-based sessions raises exception

Reported by: Gabriel Hurley Owned by: nobody
Component: contrib.sessions Version: 1.4-beta-1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using the cookie-based session backend, the entire session cookie is stuffed into the session name. If you then switch the session backend to a backend with a length limit (such as memcached, or some DBs) you'll get very obtuse errors such as "Key too long" from memcached.

While arbitrarily switching session backends isn't generally supported behavior, this could be a huge problem for a production site if they ever decided cookie-based sessions weren't ideal for their uses and switched. It would immediately break for every user who had an existing session on the site.

Ideally, if the session backend is unable to retrieve the session due to an error during retrieval, a new session should be generated.

Attachments (1)

17810-catch-all.patch (1.6 KB ) - added by Aymeric Augustin 13 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Paul McMillan, 13 years ago

Triage Stage: UnreviewedAccepted
Version: 1.31.4-beta-1

Marking as accepted, and a release blocker. We don't need to support transitioning between one session database and another, but switching from one session backend to another should never completely block users from accessing a website until they clear their cookies.

Version 0, edited 13 years ago by Paul McMillan (next)

comment:2 by Jannis Leidel, 13 years ago

Severity: Release blockerNormal

This is exactly that: "arbitrarily switching session backends isn't generally supported behavior". Removing the release blocker again.

comment:3 by Paul McMillan, 13 years ago

Resolution: fixed
Status: newclosed

In [17795]:

Fixed #17810. Catch session key errors.

Catches memcached session key errors related to overly long session keys.
This is a long-standing bug, but severity was exacerbated by the addition
of cookie-backed session storage, which generates long session values. If
an installation switched from cookie-backed session store to memcached,
users would not be able to log in because of the server error from overly
long memcached keys.

comment:4 by Aymeric Augustin, 13 years ago

Since this is a bit sensitive and we're very close to 1.4 final, I confirmed manually that the fix works.

I started with a website running on Django 1.4c2.

I added this code to a view to generate a very long cookie:

    if request.user.is_superuser:   # mess only with myself, not with users
        with open('/dev/urandom') as stuff:
            request.session['stuff'] = stuff.read(1024)

I set SESSION_ENGINE = 'django.contrib.sessions.backends.signed_cookies' and restarted the server. The value for sessionid in the cookie is 1658 characters long after encoding.

I set SESSION_ENGINE = 'django.contrib.sessions.backends.cache' and restarted the server again (my cache backend is memcached).

I encountered an exception: MemcachedKeyLengthError: Key length is > 250.

I updated to trunk, performed the same steps and didn't encounter the exception.

comment:5 by Aymeric Augustin, 13 years ago

Resolution: fixed
Status: closedreopened

Note that two things are wrong in the patch:

  • the except clause doesn't work under Python 2.5 — this is trivial to fix);
  • checking an exception by name is ugly, I'm reopening the ticket to figure out a better solution (if possible) after 1.4 is out.

comment:6 by Jannis Leidel, 13 years ago

In [17796]:

Fixed an incompatibility with Python 2.5 in the changes done in r17795. Refs #17810.

comment:7 by Aymeric Augustin, 13 years ago

I tried with 'django.core.cache.backends.memcached.PyLibMCCache' as cache backend instead of 'django.core.cache.backends.memcached.MemcachedCache' and the problem still exists: ValueError: key too long, max is 251.

MemcachedKeyLengthError inherits MemcachedKeyError, which inherits Exception, so the first shared ancestor is Exception :(

comment:8 by Aymeric Augustin, 13 years ago

Severity: NormalRelease blocker

by Aymeric Augustin, 13 years ago

Attachment: 17810-catch-all.patch added

comment:9 by Aymeric Augustin, 13 years ago

Resolution: fixed
Status: reopenedclosed

In [17797]:

Fixed #17810 (again). Catch session key errors.

The previous commit didn't work with PyLibMC.
This solution appears to be the best compromise
at this point in the 1.4 release cycle.

Note: See TracTickets for help on using tickets.
Back to Top