From 393c0e24223c701edeb8ce7dc9d0f852f0c081ad Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Sun, 18 Aug 2013 22:36:36 -0700 Subject: [PATCH] Fixed #20936 -- When logging out/ending a session, don't create a new, empty session. Previously, when logging out, the existing session was overwritten by a new sessionid instead of deleting the session altogether. This behavior added overhead by creating a new session record in whichever backend was in use: db, cache, etc. This extra session is unnecessary at the time since no session data is meant to be preserved when explicitly logging out. --- django/contrib/sessions/backends/base.py | 9 +++- django/contrib/sessions/backends/cached_db.py | 2 +- django/contrib/sessions/middleware.py | 49 +++++++++++-------- django/contrib/sessions/tests.py | 21 ++++++++ docs/releases/1.8.txt | 3 +- docs/topics/http/sessions.txt | 13 +++-- 6 files changed, 69 insertions(+), 28 deletions(-) diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index a77a25bb31..c7819b220d 100644 --- a/django/contrib/sessions/backends/base.py +++ b/django/contrib/sessions/backends/base.py @@ -142,6 +142,13 @@ class SessionBase(object): self.accessed = True self.modified = True + def is_empty(self): + "Returns True when there is no session_key and the session is empty" + try: + return not bool(self._session_key) and not self._session_cache + except AttributeError: + return True + def _get_new_session_key(self): "Returns session key that isn't being used." while True: @@ -268,7 +275,7 @@ class SessionBase(object): """ self.clear() self.delete() - self.create() + self._session_key = None def cycle_key(self): """ diff --git a/django/contrib/sessions/backends/cached_db.py b/django/contrib/sessions/backends/cached_db.py index f5c14b0e1e..71bf40d9c4 100644 --- a/django/contrib/sessions/backends/cached_db.py +++ b/django/contrib/sessions/backends/cached_db.py @@ -79,7 +79,7 @@ class SessionStore(DBStore): """ self.clear() self.delete(self.session_key) - self.create() + self._session_key = '' # At bottom to avoid circular import diff --git a/django/contrib/sessions/middleware.py b/django/contrib/sessions/middleware.py index 211ef57d45..d1595b1ae4 100644 --- a/django/contrib/sessions/middleware.py +++ b/django/contrib/sessions/middleware.py @@ -18,32 +18,39 @@ class SessionMiddleware(object): def process_response(self, request, response): """ If request.session was modified, or if the configuration is to save the - session every time, save the changes and set a session cookie. + session every time, save the changes and set a session cookie or delete + the session cookie if the session has been emptied. """ try: accessed = request.session.accessed modified = request.session.modified + empty = request.session.is_empty() except AttributeError: pass else: - if accessed: - patch_vary_headers(response, ('Cookie',)) - if modified or settings.SESSION_SAVE_EVERY_REQUEST: - if request.session.get_expire_at_browser_close(): - max_age = None - expires = None - else: - max_age = request.session.get_expiry_age() - expires_time = time.time() + max_age - expires = cookie_date(expires_time) - # Save the session data and refresh the client cookie. - # Skip session save for 500 responses, refs #3881. - if response.status_code != 500: - request.session.save() - response.set_cookie(settings.SESSION_COOKIE_NAME, - request.session.session_key, max_age=max_age, - expires=expires, domain=settings.SESSION_COOKIE_DOMAIN, - path=settings.SESSION_COOKIE_PATH, - secure=settings.SESSION_COOKIE_SECURE or None, - httponly=settings.SESSION_COOKIE_HTTPONLY or None) + # First check if we need to delete this cookie. + # The session should be deleted only if the session is entirely empty + if settings.SESSION_COOKIE_NAME in request.COOKIES and empty: + response.delete_cookie(settings.SESSION_COOKIE_NAME) + else: + if accessed: + patch_vary_headers(response, ('Cookie',)) + if modified or settings.SESSION_SAVE_EVERY_REQUEST: + if request.session.get_expire_at_browser_close(): + max_age = None + expires = None + else: + max_age = request.session.get_expiry_age() + expires_time = time.time() + max_age + expires = cookie_date(expires_time) + # Save the session data and refresh the client cookie. + # Skip session save for 500 responses, refs #3881. + if response.status_code != 500: + request.session.save() + response.set_cookie(settings.SESSION_COOKIE_NAME, + request.session.session_key, max_age=max_age, + expires=expires, domain=settings.SESSION_COOKIE_DOMAIN, + path=settings.SESSION_COOKIE_PATH, + secure=settings.SESSION_COOKIE_SECURE or None, + httponly=settings.SESSION_COOKIE_HTTPONLY or None) return response diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py index 8cda7ea05b..144b709166 100644 --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -568,6 +568,27 @@ class SessionMiddlewareTests(unittest.TestCase): # Check that the value wasn't saved above. self.assertNotIn('hello', request.session.load()) + def test_session_delete_on_end(self): + request = RequestFactory().get('/') + response = HttpResponse('Session test') + middleware = SessionMiddleware() + + # Before deleting, there has to be an existing cookie + request.COOKIES[settings.SESSION_COOKIE_NAME] = 'abc' + + # Simulate a request that ends the session + middleware.process_request(request) + request.session.flush() + + # Handle the response through the middleware + response = middleware.process_response(request, response) + + # Check that the cookie was deleted, not recreated. + # A deleted cookie header looks like: + # Set-Cookie: sessionid=; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Path=/ + self.assertEqual('Set-Cookie: {0}=; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Path=/'.format(settings.SESSION_COOKIE_NAME), + str(response.cookies[settings.SESSION_COOKIE_NAME])) + class CookieSessionTests(SessionTestsMixin, TestCase): diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index c4e0ade59e..cd2317df42 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -61,7 +61,8 @@ Minor features :mod:`django.contrib.sessions` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -* ... +* Session cookie is now deleted after + :meth:`~django.contrib.sessions.backends.base.SessionBase.flush()` is called. :mod:`django.contrib.sitemaps` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/topics/http/sessions.txt b/docs/topics/http/sessions.txt index 864777bfe4..4188655329 100644 --- a/docs/topics/http/sessions.txt +++ b/docs/topics/http/sessions.txt @@ -231,12 +231,17 @@ You can edit it multiple times. .. method:: flush() - Delete the current session data from the session and regenerate the - session key value that is sent back to the user in the cookie. This is - used if you want to ensure that the previous session data can't be - accessed again from the user's browser (for example, the + Delete the current session data from the session and delete the session + cookie. This is used if you want to ensure that the previous session data + can't be accessed again from the user's browser (for example, the :func:`django.contrib.auth.logout()` function calls it). + .. versionchanged:: 1.8 + + Deletion of the session cookie is a behavior new in Django 1.8. + Previously, the behavior was to regenerate the session key value that + was sent back to the user in the cookie. + .. method:: set_test_cookie() Sets a test cookie to determine whether the user's browser supports