Fixed #11555 -- Made SessionBase.session_key read-only. Cleaned up code slightly. Refs #13478.

This also removes the implicit initialization of the session key on the first access in favor of explicit initialization.



git-svn-id: http://code.djangoproject.com/svn/django/trunk@17155 bcc190cf-cafb-0310-a4f2-bffc1f526a37

8 files changed, 59 insertions, 45 deletions

diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py
index 4ddb35a832..2d45e71f1c 100644
--- a/django/contrib/sessions/backends/base.py
+++ b/django/contrib/sessions/backends/base.py
@@ -54,12 +54,6 @@ class SessionBase(object):
         del self._session[key]
         self.modified = True
 
-    def keys(self):
-        return self._session.keys()
-
-    def items(self):
-        return self._session.items()
-
     def get(self, key, default=None):
         return self._session.get(key, default)
 
@@ -116,9 +110,15 @@ class SessionBase(object):
     def has_key(self, key):
         return key in self._session
 
+    def keys(self):
+        return self._session.keys()
+
     def values(self):
         return self._session.values()
 
+    def items(self):
+        return self._session.items()
+
     def iterkeys(self):
         return self._session.iterkeys()
 
@@ -145,7 +145,7 @@ class SessionBase(object):
         except AttributeError:
             # No getpid() in Jython, for example
             pid = 1
-        while 1:
+        while True:
             session_key = hashlib.md5("%s%s%s%s"
                     % (randrange(0, MAX_SESSION_KEY), pid, time.time(),
                        settings.SECRET_KEY)).hexdigest()
@@ -153,17 +153,15 @@ class SessionBase(object):
                 break
         return session_key
 
-    def _get_session_key(self):
-        if self._session_key:
-            return self._session_key
-        else:
+    def _get_or_create_session_key(self):
+        if self._session_key is None:
             self._session_key = self._get_new_session_key()
-            return self._session_key
+        return self._session_key
 
-    def _set_session_key(self, session_key):
-        self._session_key = session_key
+    def _get_session_key(self):
+        return self._session_key
 
-    session_key = property(_get_session_key, _set_session_key)
+    session_key = property(_get_session_key)
 
     def _get_session(self, no_load=False):
         """
@@ -174,7 +172,7 @@ class SessionBase(object):
         try:
             return self._session_cache
         except AttributeError:
-            if self._session_key is None or no_load:
+            if self.session_key is None or no_load:
                 self._session_cache = {}
             else:
                 self._session_cache = self.load()
diff --git a/django/contrib/sessions/backends/cache.py b/django/contrib/sessions/backends/cache.py
index 41d203e918..ea9e926cfa 100644
--- a/django/contrib/sessions/backends/cache.py
+++ b/django/contrib/sessions/backends/cache.py
@@ -11,8 +11,12 @@ class SessionStore(SessionBase):
         self._cache = cache
         super(SessionStore, self).__init__(session_key)
 
+    @property
+    def cache_key(self):
+        return KEY_PREFIX + self._get_or_create_session_key()
+
     def load(self):
-        session_data = self._cache.get(KEY_PREFIX + self.session_key)
+        session_data = self._cache.get(self.cache_key)
         if session_data is not None:
             return session_data
         self.create()
@@ -25,7 +29,7 @@ class SessionStore(SessionBase):
         # and then raise an exception. That's the risk you shoulder if using
         # cache backing.
         for i in xrange(10000):
-            self.session_key = self._get_new_session_key()
+            self._session_key = self._get_new_session_key()
             try:
                 self.save(must_create=True)
             except CreateError:
@@ -39,8 +43,9 @@ class SessionStore(SessionBase):
             func = self._cache.add
         else:
             func = self._cache.set
-        result = func(KEY_PREFIX + self.session_key, self._get_session(no_load=must_create),
-                self.get_expiry_age())
+        result = func(self.cache_key,
+                      self._get_session(no_load=must_create),
+                      self.get_expiry_age())
         if must_create and not result:
             raise CreateError
 
@@ -49,8 +54,7 @@ class SessionStore(SessionBase):
 
     def delete(self, session_key=None):
         if session_key is None:
-            if self._session_key is None:
+            if self.session_key is None:
                 return
-            session_key = self._session_key
+            session_key = self.session_key
         self._cache.delete(KEY_PREFIX + session_key)
-
diff --git a/django/contrib/sessions/backends/cached_db.py b/django/contrib/sessions/backends/cached_db.py
index 465472d29c..580e907c30 100644
--- a/django/contrib/sessions/backends/cached_db.py
+++ b/django/contrib/sessions/backends/cached_db.py
@@ -16,12 +16,15 @@ class SessionStore(DBStore):
     def __init__(self, session_key=None):
         super(SessionStore, self).__init__(session_key)
 
+    @property
+    def cache_key(self):
+        return KEY_PREFIX + self._get_or_create_session_key()
+
     def load(self):
-        data = cache.get(KEY_PREFIX + self.session_key, None)
+        data = cache.get(self.cache_key, None)
         if data is None:
             data = super(SessionStore, self).load()
-            cache.set(KEY_PREFIX + self.session_key, data, 
-                      settings.SESSION_COOKIE_AGE)
+            cache.set(self.cache_key, data, settings.SESSION_COOKIE_AGE)
         return data
 
     def exists(self, session_key):
@@ -29,12 +32,15 @@ class SessionStore(DBStore):
 
     def save(self, must_create=False):
         super(SessionStore, self).save(must_create)
-        cache.set(KEY_PREFIX + self.session_key, self._session, 
-                  settings.SESSION_COOKIE_AGE)
+        cache.set(self.cache_key, self._session, settings.SESSION_COOKIE_AGE)
 
     def delete(self, session_key=None):
         super(SessionStore, self).delete(session_key)
-        cache.delete(KEY_PREFIX + (session_key or self.session_key))
+        if session_key is None:
+            if self.session_key is None:
+                return
+            session_key = self.session_key
+        cache.delete(KEY_PREFIX + session_key)
 
     def flush(self):
         """
diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py
index 7d4c535fd4..219d97d368 100644
--- a/django/contrib/sessions/backends/db.py
+++ b/django/contrib/sessions/backends/db.py
@@ -32,7 +32,7 @@ class SessionStore(SessionBase):
 
     def create(self):
         while True:
-            self.session_key = self._get_new_session_key()
+            self._session_key = self._get_new_session_key()
             try:
                 # Save immediately to ensure we have a unique entry in the
                 # database.
@@ -52,9 +52,9 @@ class SessionStore(SessionBase):
         entry).
         """
         obj = Session(
-            session_key = self.session_key,
-            session_data = self.encode(self._get_session(no_load=must_create)),
-            expire_date = self.get_expiry_date()
+            session_key=self._get_or_create_session_key(),
+            session_data=self.encode(self._get_session(no_load=must_create)),
+            expire_date=self.get_expiry_date()
         )
         using = router.db_for_write(Session, instance=obj)
         sid = transaction.savepoint(using=using)
@@ -68,9 +68,9 @@ class SessionStore(SessionBase):
 
     def delete(self, session_key=None):
         if session_key is None:
-            if self._session_key is None:
+            if self.session_key is None:
                 return
-            session_key = self._session_key
+            session_key = self.session_key
         try:
             Session.objects.get(session_key=session_key).delete()
         except Session.DoesNotExist:
diff --git a/django/contrib/sessions/backends/file.py b/django/contrib/sessions/backends/file.py
index 0c60fff389..8ffddc4903 100644
--- a/django/contrib/sessions/backends/file.py
+++ b/django/contrib/sessions/backends/file.py
@@ -33,7 +33,7 @@ class SessionStore(SessionBase):
         Get the file associated with this session key.
         """
         if session_key is None:
-            session_key = self.session_key
+            session_key = self._get_or_create_session_key()
 
         # Make sure we're not vulnerable to directory traversal. Session keys
         # should always be md5s, so they should never contain directory
@@ -135,9 +135,9 @@ class SessionStore(SessionBase):
 
     def delete(self, session_key=None):
         if session_key is None:
-            if self._session_key is None:
+            if self.session_key is None:
                 return
-            session_key = self._session_key
+            session_key = self.session_key
         try:
             os.unlink(self._key_to_file(session_key))
         except OSError:
diff --git a/django/contrib/sessions/backends/signed_cookies.py b/django/contrib/sessions/backends/signed_cookies.py
index 178cc32c93..2a0f261441 100644
--- a/django/contrib/sessions/backends/signed_cookies.py
+++ b/django/contrib/sessions/backends/signed_cookies.py
@@ -30,7 +30,7 @@ class SessionStore(SessionBase):
         raises BadSignature if signature fails.
         """
         try:
-            return signing.loads(self._session_key,
+            return signing.loads(self.session_key,
                 serializer=PickleSerializer,
                 max_age=settings.SESSION_COOKIE_AGE,
                 salt='django.contrib.sessions.backends.signed_cookies')
diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py
index afcba2c27f..f94753cbcf 100644
--- a/django/contrib/sessions/tests.py
+++ b/django/contrib/sessions/tests.py
@@ -136,6 +136,7 @@ class SessionTestsMixin(object):
         self.assertTrue(self.session.exists(self.session.session_key))
 
     def test_delete(self):
+        self.session.save()
         self.session.delete(self.session.session_key)
         self.assertFalse(self.session.exists(self.session.session_key))
 
@@ -175,6 +176,11 @@ class SessionTestsMixin(object):
             # session key; make sure that entry is manually deleted
             session.delete('1')
 
+    def test_session_key_is_read_only(self):
+        def set_session_key(session):
+            session.session_key = session._get_new_session_key()
+        self.assertRaises(AttributeError, set_session_key, self.session)
+
     # Custom session expiry
     def test_default_expiry(self):
         # A normal session has a max age equal to settings
@@ -360,7 +366,7 @@ class SessionMiddlewareTests(unittest.TestCase):
         response = middleware.process_response(request, response)
         self.assertTrue(
             response.cookies[settings.SESSION_COOKIE_NAME]['httponly'])
-        self.assertIn('httponly', 
+        self.assertIn('httponly',
             str(response.cookies[settings.SESSION_COOKIE_NAME]))
 
     @override_settings(SESSION_COOKIE_HTTPONLY=False)
@@ -375,12 +381,12 @@ class SessionMiddlewareTests(unittest.TestCase):
 
         # Handle the response through the middleware
         response = middleware.process_response(request, response)
-        #if it isn't in the cookie, that's fine (Python 2.5). 
+        # If it isn't in the cookie, that's fine (Python 2.5)
         if 'httponly' in settings.SESSION_COOKIE_NAME:
             self.assertFalse(
                response.cookies[settings.SESSION_COOKIE_NAME]['httponly'])
 
-        self.assertNotIn('httponly', 
+        self.assertNotIn('httponly',
                          str(response.cookies[settings.SESSION_COOKIE_NAME]))
 
 class CookieSessionTests(SessionTestsMixin, TestCase):
diff --git a/tests/regressiontests/test_client_regress/session.py b/tests/regressiontests/test_client_regress/session.py
index 070584d32d..665729cf00 100644
--- a/tests/regressiontests/test_client_regress/session.py
+++ b/tests/regressiontests/test_client_regress/session.py
@@ -14,13 +14,13 @@ class SessionStore(SessionBase):
         return False
 
     def create(self):
-        self.session_key = self.encode({})
+        self._session_key = self.encode({})
 
     def save(self, must_create=False):
-        self.session_key = self.encode(self._session)
+        self._session_key = self.encode(self._session)
 
     def delete(self, session_key=None):
-        self.session_key = self.encode({})
+        self._session_key = self.encode({})
 
     def load(self):
         try: