From bda21e2b9d19f435d841a5ff9f8f636ae843842d Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 27 Nov 2011 17:52:24 +0000 Subject: [PATCH] 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 --- django/contrib/sessions/backends/base.py | 30 +++++++++---------- django/contrib/sessions/backends/cache.py | 18 ++++++----- django/contrib/sessions/backends/cached_db.py | 18 +++++++---- django/contrib/sessions/backends/db.py | 12 ++++---- django/contrib/sessions/backends/file.py | 6 ++-- .../sessions/backends/signed_cookies.py | 2 +- django/contrib/sessions/tests.py | 12 ++++++-- .../test_client_regress/session.py | 6 ++-- 8 files changed, 59 insertions(+), 45 deletions(-) diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index 4ddb35a8328..2d45e71f1c9 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 41d203e9181..ea9e926cfa3 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 465472d29c4..580e907c30d 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 7d4c535fd4a..219d97d3684 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 0c60fff3899..8ffddc4903a 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 178cc32c931..2a0f261441b 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 afcba2c27fa..f94753cbcfd 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 070584d32df..665729cf00e 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: