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

@@ -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),
+            session_key = md5_constructor("%s%s%s%s"
-                                          pid, time.time(), settings.SECRET_KEY)).hexdigest()
+                    % (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
 

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):
+    def create(self):
-        self._cache.set(self.session_key, self._session, self.get_expiry_age())
+        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)
+

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):
-
+            self.create()
-            # 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
             return {}
 
     def exists(self, session_key):
@@ -36,12 +26,40 @@ class SessionStore(SessionBase):
             return False
         return True
 
-    def save(self):
+    def create(self):
-        Session.objects.create(
+        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:

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
 
 
@@ -49,24 +49,38 @@ class SessionStore(SessionBase):
                 try:
                     session_data = self.decode(session_file.read())
                 except (EOFError, SuspiciousOperation):
-                    self._session_key = self._get_new_session_key()
+                    self.create()
-                    self._session_cache = {}
-                    self.save()
-                    # Ensure the user is notified via a new cookie.
-                    self.modified = True
             finally:
                 session_file.close()
-        except(IOError):
+        except IOError:
             pass
         return session_data
 
-    def save(self):
+    def create(self):
+        while True:
+            self._session_key = self._get_new_session_key()
             try:
-            f = open(self._key_to_file(self.session_key), "wb")
+                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:
-                f.write(self.encode(self._session))
+            fd = os.open(self._key_to_file(self.session_key), flags)
+            try:
+                os.write(fd, self.encode(self._session))
             finally:
-                f.close()
+                os.close(fd)
+        except OSError, e:
+            if must_create and e.errno == errno.EEXIST:
+                raise CreateError
+            raise
         except (IOError, EOFError):
             pass