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

@@ -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

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

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

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()

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

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)

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):
 

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``
 ---------------------------
 

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 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.
 
-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.
+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
 ========