From aeda55e6bffc3cfbf53698af398a19c1a0f02d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Thu, 5 Jul 2012 18:09:48 +0300 Subject: [PATCH] Fixed #3881 -- skip saving session when response status is 500 Saving session data is somewhat likely to lead into error when the status code is 500. It is guaranteed to lead into error if the reason for the 500 code is query error on PostgreSQL. --- django/contrib/sessions/middleware.py | 16 +++++++++------- django/contrib/sessions/tests.py | 16 ++++++++++++++++ docs/releases/1.5.txt | 6 ++++++ docs/topics/http/sessions.txt | 3 +++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/django/contrib/sessions/middleware.py b/django/contrib/sessions/middleware.py index 68cb77f7e1..9f65255f47 100644 --- a/django/contrib/sessions/middleware.py +++ b/django/contrib/sessions/middleware.py @@ -33,11 +33,13 @@ class SessionMiddleware(object): expires_time = time.time() + max_age expires = cookie_date(expires_time) # Save the session data and refresh the client cookie. - 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) + # 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 92ea6bbd91..328b085f1e 100644 --- a/django/contrib/sessions/tests.py +++ b/django/contrib/sessions/tests.py @@ -409,6 +409,22 @@ class SessionMiddlewareTests(unittest.TestCase): self.assertNotIn('httponly', str(response.cookies[settings.SESSION_COOKIE_NAME])) + def test_session_save_on_500(self): + request = RequestFactory().get('/') + response = HttpResponse('Horrible error') + response.status_code = 500 + middleware = SessionMiddleware() + + # Simulate a request the modifies the session + middleware.process_request(request) + request.session['hello'] = 'world' + + # Handle the response through the middleware + response = middleware.process_response(request, response) + + # Check that the value wasn't saved above. + self.assertNotIn('hello', request.session.load()) + class CookieSessionTests(SessionTestsMixin, TestCase): diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 4275fbae52..b863d102ec 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -177,6 +177,12 @@ autocommit behavior was never restored. This bug is now fixed in 1.5. While this is only a bug fix, it is worth checking your applications behavior if you are using PostgreSQL together with the autocommit option. +Session not saved on 500 responses +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Django's session middleware will skip saving the session data if the +response's status code is 500. + Miscellaneous ~~~~~~~~~~~~~ diff --git a/docs/topics/http/sessions.txt b/docs/topics/http/sessions.txt index a32d9b54dd..1f55293413 100644 --- a/docs/topics/http/sessions.txt +++ b/docs/topics/http/sessions.txt @@ -423,6 +423,9 @@ cookie will be sent on every request. Similarly, the ``expires`` part of a session cookie is updated each time the session cookie is sent. +.. versionchanged:: 1.5 + The session is not saved if the response's status code is 500. + Browser-length sessions vs. persistent sessions ===============================================