From 04b00b668d0d56c37460cbed19671f4b1b5916c3 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 27 Oct 2012 19:18:03 +0200 Subject: [PATCH] Fixed #19200 -- Session expiry with cached_db Also did a little bit of cleanup. --- django/contrib/sessions/backends/base.py | 10 +++++-- django/contrib/sessions/backends/cached_db.py | 25 +++++++++++++--- django/contrib/sessions/backends/db.py | 2 +- .../sessions/backends/signed_cookies.py | 1 + django/contrib/sessions/tests.py | 29 ++++++++++++++----- 5 files changed, 52 insertions(+), 15 deletions(-) diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index c8393f23c6..65bbe059eb 100644 --- a/django/contrib/sessions/backends/base.py +++ b/django/contrib/sessions/backends/base.py @@ -170,9 +170,13 @@ class SessionBase(object): _session = property(_get_session) - def get_expiry_age(self): - """Get the number of seconds until the session expires.""" - expiry = self.get('_session_expiry') + def get_expiry_age(self, expiry=None): + """Get the number of seconds until the session expires. + + expiry is an optional parameter specifying the datetime of expiry. + """ + if expiry is None: + expiry = self.get('_session_expiry') if not expiry: # Checks both None and 0 cases return settings.SESSION_COOKIE_AGE if not isinstance(expiry, datetime): diff --git a/django/contrib/sessions/backends/cached_db.py b/django/contrib/sessions/backends/cached_db.py index ff6076df77..8e4cf8b7e7 100644 --- a/django/contrib/sessions/backends/cached_db.py +++ b/django/contrib/sessions/backends/cached_db.py @@ -2,9 +2,10 @@ Cached, database-backed sessions. """ -from django.conf import settings from django.contrib.sessions.backends.db import SessionStore as DBStore from django.core.cache import cache +from django.core.exceptions import SuspiciousOperation +from django.utils import timezone KEY_PREFIX = "django.contrib.sessions.cached_db" @@ -28,9 +29,21 @@ class SessionStore(DBStore): # Some backends (e.g. memcache) raise an exception on invalid # cache keys. If this happens, reset the session. See #17810. data = None + if data is None: - data = super(SessionStore, self).load() - cache.set(self.cache_key, data, settings.SESSION_COOKIE_AGE) + # Duplicate DBStore.load, because we need to keep track + # of the expiry date to set it properly in the cache. + try: + s = Session.objects.get( + session_key=self.session_key, + expire_date__gt=timezone.now() + ) + data = self.decode(s.session_data) + cache.set(self.cache_key, data, + self.get_expiry_age(s.expire_date)) + except (Session.DoesNotExist, SuspiciousOperation): + self.create() + data = {} return data def exists(self, session_key): @@ -40,7 +53,7 @@ class SessionStore(DBStore): def save(self, must_create=False): super(SessionStore, self).save(must_create) - cache.set(self.cache_key, self._session, settings.SESSION_COOKIE_AGE) + cache.set(self.cache_key, self._session, self.get_expiry_age()) def delete(self, session_key=None): super(SessionStore, self).delete(session_key) @@ -58,3 +71,7 @@ class SessionStore(DBStore): self.clear() self.delete(self.session_key) self.create() + + +# At bottom to avoid circular import +from django.contrib.sessions.models import Session diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py index babdb72c27..4dacc96000 100644 --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -14,7 +14,7 @@ class SessionStore(SessionBase): def load(self): try: s = Session.objects.get( - session_key = self.session_key, + session_key=self.session_key, expire_date__gt=timezone.now() ) return self.decode(s.session_data) diff --git a/django/contrib/sessions/backends/signed_cookies.py b/django/contrib/sessions/backends/signed_cookies.py index 41ba7af634..23915cf98c 100644 --- a/django/contrib/sessions/backends/signed_cookies.py +++ b/django/contrib/sessions/backends/signed_cookies.py @@ -32,6 +32,7 @@ class SessionStore(SessionBase): try: return signing.loads(self.session_key, serializer=PickleSerializer, + # This doesn't handle non-default expiry dates, see #19201 max_age=settings.SESSION_COOKIE_AGE, salt='django.contrib.sessions.backends.signed_cookies') except (signing.BadSignature, ValueError): diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py index fc2d8753d7..527e8eb331 100644 --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -83,7 +83,7 @@ class SessionTestsMixin(object): self.session['some key'] = 1 self.session.modified = False self.session.accessed = False - self.assertTrue('some key' in self.session) + self.assertIn('some key', self.session) self.assertTrue(self.session.accessed) self.assertFalse(self.session.modified) @@ -200,28 +200,28 @@ class SessionTestsMixin(object): # Using seconds self.session.set_expiry(10) delta = self.session.get_expiry_date() - timezone.now() - self.assertTrue(delta.seconds in (9, 10)) + self.assertIn(delta.seconds, (9, 10)) age = self.session.get_expiry_age() - self.assertTrue(age in (9, 10)) + self.assertIn(age, (9, 10)) def test_custom_expiry_timedelta(self): # Using timedelta self.session.set_expiry(timedelta(seconds=10)) delta = self.session.get_expiry_date() - timezone.now() - self.assertTrue(delta.seconds in (9, 10)) + self.assertIn(delta.seconds, (9, 10)) age = self.session.get_expiry_age() - self.assertTrue(age in (9, 10)) + self.assertIn(age, (9, 10)) def test_custom_expiry_datetime(self): # Using fixed datetime self.session.set_expiry(timezone.now() + timedelta(seconds=10)) delta = self.session.get_expiry_date() - timezone.now() - self.assertTrue(delta.seconds in (9, 10)) + self.assertIn(delta.seconds, (9, 10)) age = self.session.get_expiry_age() - self.assertTrue(age in (9, 10)) + self.assertIn(age, (9, 10)) def test_custom_expiry_reset(self): self.session.set_expiry(None) @@ -258,6 +258,21 @@ class SessionTestsMixin(object): encoded = self.session.encode(data) self.assertEqual(self.session.decode(encoded), data) + def test_actual_expiry(self): + # Regression test for #19200 + old_session_key = None + new_session_key = None + try: + self.session['foo'] = 'bar' + self.session.set_expiry(-timedelta(seconds=10)) + self.session.create() + # With an expiry date in the past, the session expires instantly. + new_session = self.backend(self.session.session_key) + self.assertNotIn('foo', new_session) + finally: + self.session.delete(old_session_key) + self.session.delete(new_session_key) + class DatabaseSessionTests(SessionTestsMixin, TestCase):