From af7b6475ca6ab12df6b91beae1bb5b1c29a5782b Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Thu, 14 Aug 2008 03:57:18 +0000 Subject: [PATCH] Added guaranteed atomic creation of new session objects. Slightly backwards incompatible for custom session backends. Whilst we were in the neighbourhood, use a larger range of session key values to save a small amount of time and use the hardware-base random numbers where available (transparently falls back to pseudo-RNG otherwise). Fixed #1080 git-svn-id: http://code.djangoproject.com/svn/django/trunk@8340 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/sessions/backends/base.py | 32 +++++++++++++-- django/contrib/sessions/backends/cache.py | 27 +++++++++++-- django/contrib/sessions/backends/db.py | 48 ++++++++++++++++------- django/contrib/sessions/backends/file.py | 42 +++++++++++++------- 4 files changed, 112 insertions(+), 37 deletions(-) diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index 7153b8a267..ed1311764c 100644 --- a/django/contrib/sessions/backends/base.py +++ b/django/contrib/sessions/backends/base.py @@ -13,6 +13,19 @@ from django.conf import settings from django.core.exceptions import SuspiciousOperation from django.utils.hashcompat import md5_constructor +# Use the system (hardware-based) random number generator if it exists. +if hasattr(random, 'SystemRandom'): + randint = random.SystemRandom().randint +else: + randint = random.randint +MAX_SESSION_KEY = 18446744073709551616L # 2 << 63 + +class CreateError(Exception): + """ + Used internally as a consistent exception type to catch from save (see the + docstring for SessionBase.save() for details). + """ + pass class SessionBase(object): """ @@ -117,8 +130,9 @@ class SessionBase(object): # No getpid() in Jython, for example pid = 1 while 1: - session_key = md5_constructor("%s%s%s%s" % (random.randint(0, sys.maxint - 1), - pid, time.time(), settings.SECRET_KEY)).hexdigest() + session_key = md5_constructor("%s%s%s%s" + % (random.randrange(0, MAX_SESSION_KEY), pid, time.time(), + settings.SECRET_KEY)).hexdigest() if not self.exists(session_key): break return session_key @@ -213,9 +227,19 @@ class SessionBase(object): """ raise NotImplementedError - def save(self): + def create(self): """ - Saves the session data. + Creates a new session instance. Guaranteed to create a new object with + a unique key and will have saved the result once (with empty data) + before the method returns. + """ + raise NotImplementedError + + def save(self, must_create=False): + """ + Saves the session data. If 'must_create' is True, a new session object + is created (otherwise a CreateError exception is raised). Otherwise, + save() can update an existing object with the same key. """ raise NotImplementedError diff --git a/django/contrib/sessions/backends/cache.py b/django/contrib/sessions/backends/cache.py index 5ffb5a136b..5729c85af8 100644 --- a/django/contrib/sessions/backends/cache.py +++ b/django/contrib/sessions/backends/cache.py @@ -1,4 +1,4 @@ -from django.contrib.sessions.backends.base import SessionBase +from django.contrib.sessions.backends.base import SessionBase, CreateError from django.core.cache import cache class SessionStore(SessionBase): @@ -11,10 +11,28 @@ class SessionStore(SessionBase): def load(self): session_data = self._cache.get(self.session_key) - return session_data or {} + if session_data is not None: + return session_data + self.create() - def save(self): - self._cache.set(self.session_key, self._session, self.get_expiry_age()) + def create(self): + while True: + self.session_key = self._get_new_session_key() + try: + self.save(must_create=True) + except CreateError: + continue + self.modified = True + return + + def save(self, must_create=False): + if must_create: + func = self._cache.add + else: + func = self._cache.set + result = func(self.session_key, self._session, self.get_expiry_age()) + if must_create and not result: + raise CreateError def exists(self, session_key): if self._cache.get(session_key): @@ -23,3 +41,4 @@ class SessionStore(SessionBase): def delete(self, session_key): self._cache.delete(session_key) + diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py index add3d70074..b1dc6f49e3 100644 --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -1,15 +1,13 @@ import datetime from django.contrib.sessions.models import Session -from django.contrib.sessions.backends.base import SessionBase +from django.contrib.sessions.backends.base import SessionBase, CreateError from django.core.exceptions import SuspiciousOperation +from django.db import IntegrityError, transaction class SessionStore(SessionBase): """ Implements database session store. """ - def __init__(self, session_key=None): - super(SessionStore, self).__init__(session_key) - def load(self): try: s = Session.objects.get( @@ -18,15 +16,7 @@ class SessionStore(SessionBase): ) return self.decode(s.session_data) except (Session.DoesNotExist, SuspiciousOperation): - - # Create a new session_key for extra security. - self.session_key = self._get_new_session_key() - self._session_cache = {} - - # Save immediately to minimize collision - self.save() - # Ensure the user is notified via a new cookie. - self.modified = True + self.create() return {} def exists(self, session_key): @@ -36,12 +26,40 @@ class SessionStore(SessionBase): return False return True - def save(self): - Session.objects.create( + def create(self): + while True: + self.session_key = self._get_new_session_key() + try: + # Save immediately to ensure we have a unique entry in the + # database. + self.save(must_create=True) + except CreateError: + # Key wasn't unique. Try again. + continue + self.modified = True + self._session_cache = {} + return + + def save(self, must_create=False): + """ + Saves the current session data to the database. If 'must_create' is + True, a database error will be raised if the saving operation doesn't + create a *new* entry (as opposed to possibly updating an existing + entry). + """ + obj = Session( session_key = self.session_key, session_data = self.encode(self._session), expire_date = self.get_expiry_date() ) + sid = transaction.savepoint() + try: + obj.save(force_insert=must_create) + except IntegrityError: + if must_create: + transaction.savepoint_rollback(sid) + raise CreateError + raise def delete(self, session_key): try: diff --git a/django/contrib/sessions/backends/file.py b/django/contrib/sessions/backends/file.py index 49a7045244..00f374edfd 100644 --- a/django/contrib/sessions/backends/file.py +++ b/django/contrib/sessions/backends/file.py @@ -2,7 +2,7 @@ import os import tempfile from django.conf import settings -from django.contrib.sessions.backends.base import SessionBase +from django.contrib.sessions.backends.base import SessionBase, CreateError from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured @@ -48,26 +48,40 @@ class SessionStore(SessionBase): try: try: session_data = self.decode(session_file.read()) - except(EOFError, SuspiciousOperation): - self._session_key = self._get_new_session_key() - self._session_cache = {} - self.save() - # Ensure the user is notified via a new cookie. - self.modified = True + except (EOFError, SuspiciousOperation): + self.create() finally: session_file.close() - except(IOError): + except IOError: pass return session_data - def save(self): - try: - f = open(self._key_to_file(self.session_key), "wb") + def create(self): + while True: + self._session_key = self._get_new_session_key() try: - f.write(self.encode(self._session)) + self.save(must_create=True) + except CreateError: + continue + self.modified = True + self._session_cache = {} + return + + def save(self, must_create=False): + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | getattr(os, 'O_BINARY', 0) + if must_create: + flags |= os.O_EXCL + try: + fd = os.open(self._key_to_file(self.session_key), flags) + try: + os.write(fd, self.encode(self._session)) finally: - f.close() - except(IOError, EOFError): + os.close(fd) + except OSError, e: + if must_create and e.errno == errno.EEXIST: + raise CreateError + raise + except (IOError, EOFError): pass def exists(self, session_key):