From 5fec97b9df6ea075483276de159e522a29437773 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 27 Oct 2012 23:12:08 +0200 Subject: [PATCH] Fixed #18194 -- Expiration of file-based sessions * Prevented stale session files from being loaded * Added removal of stale session files in django-admin.py clearsessions Thanks ej for the report, crodjer and Elvard for their inputs. --- django/contrib/sessions/backends/base.py | 12 +++- django/contrib/sessions/backends/cache.py | 4 ++ django/contrib/sessions/backends/db.py | 5 ++ django/contrib/sessions/backends/file.py | 71 +++++++++++++++---- .../sessions/backends/signed_cookies.py | 4 ++ .../management/commands/clearsessions.py | 14 ++-- django/contrib/sessions/tests.py | 61 ++++++++++++++++ docs/ref/django-admin.txt | 2 - docs/topics/http/sessions.txt | 32 ++++++--- 9 files changed, 176 insertions(+), 29 deletions(-) diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index 1f63c3b45a..ff8ab7f677 100644 --- a/django/contrib/sessions/backends/base.py +++ b/django/contrib/sessions/backends/base.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals import base64 -import time from datetime import datetime, timedelta try: from django.utils.six.moves import cPickle as pickle @@ -309,3 +308,14 @@ class SessionBase(object): Loads the session data and returns a dictionary. """ raise NotImplementedError + + @classmethod + def clear_expired(cls): + """ + Remove expired sessions from the session store. + + If this operation isn't possible on a given backend, it should raise + NotImplementedError. If it isn't necessary, because the backend has + a built-in expiration mechanism, it should be a no-op. + """ + raise NotImplementedError diff --git a/django/contrib/sessions/backends/cache.py b/django/contrib/sessions/backends/cache.py index b66123b915..0c7eb8d2cb 100644 --- a/django/contrib/sessions/backends/cache.py +++ b/django/contrib/sessions/backends/cache.py @@ -65,3 +65,7 @@ class SessionStore(SessionBase): return session_key = self.session_key self._cache.delete(KEY_PREFIX + session_key) + + @classmethod + def clear_expired(cls): + pass diff --git a/django/contrib/sessions/backends/db.py b/django/contrib/sessions/backends/db.py index 4dacc96000..47e89b66e5 100644 --- a/django/contrib/sessions/backends/db.py +++ b/django/contrib/sessions/backends/db.py @@ -71,6 +71,11 @@ class SessionStore(SessionBase): except Session.DoesNotExist: pass + @classmethod + def clear_expired(cls): + Session.objects.filter(expire_date__lt=timezone.now()).delete() + transaction.commit_unless_managed() + # At bottom to avoid circular import from django.contrib.sessions.models import Session diff --git a/django/contrib/sessions/backends/file.py b/django/contrib/sessions/backends/file.py index 20ac2c2087..f3a71f8271 100644 --- a/django/contrib/sessions/backends/file.py +++ b/django/contrib/sessions/backends/file.py @@ -1,3 +1,4 @@ +import datetime import errno import os import tempfile @@ -5,27 +6,36 @@ import tempfile from django.conf import settings from django.contrib.sessions.backends.base import SessionBase, CreateError from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured - +from django.utils import timezone class SessionStore(SessionBase): """ Implements a file based session store. """ def __init__(self, session_key=None): - self.storage_path = getattr(settings, "SESSION_FILE_PATH", None) - if not self.storage_path: - self.storage_path = tempfile.gettempdir() - - # Make sure the storage path is valid. - if not os.path.isdir(self.storage_path): - raise ImproperlyConfigured( - "The session storage path %r doesn't exist. Please set your" - " SESSION_FILE_PATH setting to an existing directory in which" - " Django can store session data." % self.storage_path) - + self.storage_path = type(self)._get_storage_path() self.file_prefix = settings.SESSION_COOKIE_NAME super(SessionStore, self).__init__(session_key) + @classmethod + def _get_storage_path(cls): + try: + return cls._storage_path + except AttributeError: + storage_path = getattr(settings, "SESSION_FILE_PATH", None) + if not storage_path: + storage_path = tempfile.gettempdir() + + # Make sure the storage path is valid. + if not os.path.isdir(storage_path): + raise ImproperlyConfigured( + "The session storage path %r doesn't exist. Please set your" + " SESSION_FILE_PATH setting to an existing directory in which" + " Django can store session data." % storage_path) + + cls._storage_path = storage_path + return storage_path + VALID_KEY_CHARS = set("abcdef0123456789") def _key_to_file(self, session_key=None): @@ -44,6 +54,18 @@ class SessionStore(SessionBase): return os.path.join(self.storage_path, self.file_prefix + session_key) + def _last_modification(self): + """ + Return the modification time of the file storing the session's content. + """ + modification = os.stat(self._key_to_file()).st_mtime + if settings.USE_TZ: + modification = datetime.datetime.utcfromtimestamp(modification) + modification = modification.replace(tzinfo=timezone.utc) + else: + modification = datetime.datetime.fromtimestamp(modification) + return modification + def load(self): session_data = {} try: @@ -56,6 +78,15 @@ class SessionStore(SessionBase): session_data = self.decode(file_data) except (EOFError, SuspiciousOperation): self.create() + + # Remove expired sessions. + expiry_age = self.get_expiry_age( + modification=self._last_modification(), + expiry=session_data.get('_session_expiry')) + if expiry_age < 0: + session_data = {} + self.delete() + self.create() except IOError: self.create() return session_data @@ -142,3 +173,19 @@ class SessionStore(SessionBase): def clean(self): pass + + @classmethod + def clear_expired(cls): + storage_path = getattr(settings, "SESSION_FILE_PATH", tempfile.gettempdir()) + file_prefix = settings.SESSION_COOKIE_NAME + + for session_file in os.listdir(storage_path): + if not session_file.startswith(file_prefix): + continue + session_key = session_file[len(file_prefix):] + session = cls(session_key) + # When an expired session is loaded, its file is removed, and a + # new file is immediately created. Prevent this by disabling + # the create() method. + session.create = lambda: None + session.load() diff --git a/django/contrib/sessions/backends/signed_cookies.py b/django/contrib/sessions/backends/signed_cookies.py index 23915cf98c..c2b7a3123f 100644 --- a/django/contrib/sessions/backends/signed_cookies.py +++ b/django/contrib/sessions/backends/signed_cookies.py @@ -92,3 +92,7 @@ class SessionStore(SessionBase): return signing.dumps(session_cache, compress=True, salt='django.contrib.sessions.backends.signed_cookies', serializer=PickleSerializer) + + @classmethod + def clear_expired(cls): + pass diff --git a/django/contrib/sessions/management/commands/clearsessions.py b/django/contrib/sessions/management/commands/clearsessions.py index fb7666f85a..8eb23dfee0 100644 --- a/django/contrib/sessions/management/commands/clearsessions.py +++ b/django/contrib/sessions/management/commands/clearsessions.py @@ -1,11 +1,15 @@ +from django.conf import settings from django.core.management.base import NoArgsCommand -from django.utils import timezone +from django.utils.importlib import import_module + class Command(NoArgsCommand): help = "Can be run as a cronjob or directly to clean out expired sessions (only with the database backend at the moment)." def handle_noargs(self, **options): - from django.db import transaction - from django.contrib.sessions.models import Session - Session.objects.filter(expire_date__lt=timezone.now()).delete() - transaction.commit_unless_managed() + engine = import_module(settings.SESSION_ENGINE) + try: + engine.SessionStore.clear_expired() + except NotImplementedError: + self.stderr.write("Session engine '%s' doesn't support clearing " + "expired sessions.\n" % settings.SESSION_ENGINE) diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py index 21e8d845b8..129cd21735 100644 --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -1,4 +1,5 @@ from datetime import timedelta +import os import shutil import string import tempfile @@ -12,6 +13,7 @@ from django.contrib.sessions.backends.file import SessionStore as FileSession from django.contrib.sessions.backends.signed_cookies import SessionStore as CookieSession from django.contrib.sessions.models import Session from django.contrib.sessions.middleware import SessionMiddleware +from django.core import management from django.core.cache import DEFAULT_CACHE_ALIAS from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation from django.http import HttpResponse @@ -319,6 +321,30 @@ class DatabaseSessionTests(SessionTestsMixin, TestCase): del self.session._session_cache self.assertEqual(self.session['y'], 2) + @override_settings(SESSION_ENGINE="django.contrib.sessions.backends.db") + def test_clearsessions_command(self): + """ + Test clearsessions command for clearing expired sessions. + """ + self.assertEqual(0, Session.objects.count()) + + # One object in the future + self.session['foo'] = 'bar' + self.session.set_expiry(3600) + self.session.save() + + # One object in the past + other_session = self.backend() + other_session['foo'] = 'bar' + other_session.set_expiry(-3600) + other_session.save() + + # Two sessions are in the database before clearsessions... + self.assertEqual(2, Session.objects.count()) + management.call_command('clearsessions') + # ... and one is deleted. + self.assertEqual(1, Session.objects.count()) + @override_settings(USE_TZ=True) class DatabaseSessionWithTimeZoneTests(DatabaseSessionTests): @@ -358,6 +384,9 @@ class FileSessionTests(SessionTestsMixin, unittest.TestCase): # Do file session tests in an isolated directory, and kill it after we're done. self.original_session_file_path = settings.SESSION_FILE_PATH self.temp_session_store = settings.SESSION_FILE_PATH = tempfile.mkdtemp() + # Reset the file session backend's internal caches + if hasattr(self.backend, '_storage_path'): + del self.backend._storage_path super(FileSessionTests, self).setUp() def tearDown(self): @@ -368,6 +397,7 @@ class FileSessionTests(SessionTestsMixin, unittest.TestCase): @override_settings( SESSION_FILE_PATH="/if/this/directory/exists/you/have/a/weird/computer") def test_configuration_check(self): + del self.backend._storage_path # Make sure the file backend checks for a good storage dir self.assertRaises(ImproperlyConfigured, self.backend) @@ -381,6 +411,37 @@ class FileSessionTests(SessionTestsMixin, unittest.TestCase): self.assertRaises(SuspiciousOperation, self.backend("a/b/c").load) + @override_settings(SESSION_ENGINE="django.contrib.sessions.backends.file") + def test_clearsessions_command(self): + """ + Test clearsessions command for clearing expired sessions. + """ + storage_path = self.backend._get_storage_path() + file_prefix = settings.SESSION_COOKIE_NAME + + def count_sessions(): + return len([session_file for session_file in os.listdir(storage_path) + if session_file.startswith(file_prefix)]) + + self.assertEqual(0, count_sessions()) + + # One object in the future + self.session['foo'] = 'bar' + self.session.set_expiry(3600) + self.session.save() + + # One object in the past + other_session = self.backend() + other_session['foo'] = 'bar' + other_session.set_expiry(-3600) + other_session.save() + + # Two sessions are in the filesystem before clearsessions... + self.assertEqual(2, count_sessions()) + management.call_command('clearsessions') + # ... and one is deleted. + self.assertEqual(1, count_sessions()) + class CacheSessionTests(SessionTestsMixin, unittest.TestCase): diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index 833db0839c..e0b08450e9 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -1200,8 +1200,6 @@ clearsessions Can be run as a cron job or directly to clean out expired sessions. -This is only supported by the database backend at the moment. - ``django.contrib.sitemaps`` --------------------------- diff --git a/docs/topics/http/sessions.txt b/docs/topics/http/sessions.txt index 1e043405f4..d9c472d092 100644 --- a/docs/topics/http/sessions.txt +++ b/docs/topics/http/sessions.txt @@ -272,6 +272,13 @@ You can edit it multiple times. Returns either ``True`` or ``False``, depending on whether the user's session cookie will expire when the user's Web browser is closed. + .. method:: SessionBase.clear_expired + + .. versionadded:: 1.5 + + Removes expired sessions from the session store. This class method is + called by :djadmin:`clearsessions`. + Session object guidelines ------------------------- @@ -458,22 +465,29 @@ This setting is a global default and can be overwritten at a per-session level by explicitly calling the :meth:`~backends.base.SessionBase.set_expiry` method of ``request.session`` as described above in `using sessions in views`_. -Clearing the session table +Clearing the session store ========================== -If you're using the database backend, note that session data can accumulate in -the ``django_session`` database table and Django does *not* provide automatic -purging. Therefore, it's your job to purge expired sessions on a regular basis. +As users create new sessions on your website, session data can accumulate in +your session store. If you're using the database backend, the +``django_session`` database table will grow. If you're using the file backend, +your temporary directory will contain an increasing number of files. -To understand this problem, consider what happens when a user uses a session. +To understand this problem, consider what happens with the database backend. When a user logs in, Django adds a row to the ``django_session`` database table. Django updates this row each time the session data changes. If the user logs out manually, Django deletes the row. But if the user does *not* log out, -the row never gets deleted. +the row never gets deleted. A similar process happens with the file backend. -Django provides a sample clean-up script: ``django-admin.py clearsessions``. -That script deletes any session in the session table whose ``expire_date`` is -in the past -- but your application may have different requirements. +Django does *not* provide automatic purging of expired sessions. Therefore, +it's your job to purge expired sessions on a regular basis. Django provides a +clean-up management command for this purpose: :djadmin:`clearsessions`. It's +recommended to call this command on a regular basis, for example as a daily +cron job. + +Note that the cache backend isn't vulnerable to this problem, because caches +automatically delete stale data. Neither is the cookie backend, because the +session data is stored by the users' browsers. Settings ========