Fixed #31962 -- Made SessionMiddleware raise SessionInterrupted when session destroyed while request is processing.

13 files changed, 108 insertions, 10 deletions

diff --git a/django/contrib/sessions/exceptions.py b/django/contrib/sessions/exceptions.py
index 4f4dc6b048..8dbca3d04f 100644
--- a/django/contrib/sessions/exceptions.py
+++ b/django/contrib/sessions/exceptions.py
@@ -1,4 +1,4 @@
-from django.core.exceptions import SuspiciousOperation
+from django.core.exceptions import BadRequest, SuspiciousOperation
 
 
 class InvalidSessionKey(SuspiciousOperation):
@@ -9,3 +9,8 @@ class InvalidSessionKey(SuspiciousOperation):
 class SuspiciousSession(SuspiciousOperation):
     """The session may be tampered with"""
     pass
+
+
+class SessionInterrupted(BadRequest):
+    """The session was interrupted."""
+    pass
diff --git a/django/contrib/sessions/middleware.py b/django/contrib/sessions/middleware.py
index cb8c1ff45b..50627a663b 100644
--- a/django/contrib/sessions/middleware.py
+++ b/django/contrib/sessions/middleware.py
@@ -3,7 +3,7 @@ from importlib import import_module
 
 from django.conf import settings
 from django.contrib.sessions.backends.base import UpdateError
-from django.core.exceptions import SuspiciousOperation
+from django.contrib.sessions.exceptions import SessionInterrupted
 from django.utils.cache import patch_vary_headers
 from django.utils.deprecation import MiddlewareMixin
 from django.utils.http import http_date
@@ -60,7 +60,7 @@ class SessionMiddleware(MiddlewareMixin):
                     try:
                         request.session.save()
                     except UpdateError:
-                        raise SuspiciousOperation(
+                        raise SessionInterrupted(
                             "The request's session was deleted before the "
                             "request completed. The user may have logged "
                             "out in a concurrent request, for example."
diff --git a/django/core/exceptions.py b/django/core/exceptions.py
index 2b3e55fe49..5780ffdb4a 100644
--- a/django/core/exceptions.py
+++ b/django/core/exceptions.py
@@ -71,6 +71,11 @@ class RequestAborted(Exception):
     pass
 
 
+class BadRequest(Exception):
+    """The request is malformed and cannot be processed."""
+    pass
+
+
 class PermissionDenied(Exception):
     """The user did not have permission to do that"""
     pass
diff --git a/django/core/handlers/exception.py b/django/core/handlers/exception.py
index 98fb46083a..70b23edb62 100644
--- a/django/core/handlers/exception.py
+++ b/django/core/handlers/exception.py
@@ -8,7 +8,7 @@ from asgiref.sync import sync_to_async
 from django.conf import settings
 from django.core import signals
 from django.core.exceptions import (
-    PermissionDenied, RequestDataTooBig, SuspiciousOperation,
+    BadRequest, PermissionDenied, RequestDataTooBig, SuspiciousOperation,
     TooManyFieldsSent,
 )
 from django.http import Http404
@@ -76,6 +76,17 @@ def response_for_exception(request, exc):
             exc_info=sys.exc_info(),
         )
 
+    elif isinstance(exc, BadRequest):
+        if settings.DEBUG:
+            response = debug.technical_500_response(request, *sys.exc_info(), status_code=400)
+        else:
+            response = get_exception_response(request, get_resolver(get_urlconf()), 400, exc)
+        log_response(
+            '%s: %s', str(exc), request.path,
+            response=response,
+            request=request,
+            exc_info=sys.exc_info(),
+        )
     elif isinstance(exc, SuspiciousOperation):
         if isinstance(exc, (RequestDataTooBig, TooManyFieldsSent)):
             # POST data can't be accessed again, otherwise the original
diff --git a/docs/ref/exceptions.txt b/docs/ref/exceptions.txt
index ae2180ff9f..d4af1de79d 100644
--- a/docs/ref/exceptions.txt
+++ b/docs/ref/exceptions.txt
@@ -162,6 +162,18 @@ or model are classified as ``NON_FIELD_ERRORS``. This constant is used
 as a key in dictionaries that otherwise map fields to their respective
 list of errors.
 
+``BadRequest``
+--------------
+
+.. exception:: BadRequest
+
+    .. versionadded:: 3.2
+
+    The :exc:`BadRequest` exception is raised when the request cannot be
+    processed due to a client error. If a ``BadRequest`` exception reaches the
+    ASGI/WSGI handler level it results in a
+    :class:`~django.http.HttpResponseBadRequest`.
+
 ``RequestAborted``
 ------------------
 
@@ -271,6 +283,24 @@ Http exceptions may be imported from ``django.http``.
 
     :exc:`UnreadablePostError` is raised when a user cancels an upload.
 
+.. currentmodule:: django.contrib.sessions.exceptions
+
+Sessions Exceptions
+===================
+
+Sessions exceptions are defined in ``django.contrib.sessions.exceptions``.
+
+``SessionInterrupted``
+----------------------
+
+.. exception:: SessionInterrupted
+
+    .. versionadded:: 3.2
+
+    :exc:`SessionInterrupted` is raised when a session is destroyed in a
+    concurrent request. It's a subclass of
+    :exc:`~django.core.exceptions.BadRequest`.
+
 Transaction Exceptions
 ======================
 
diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt
index a7bd5575d1..63d6f0700d 100644
--- a/docs/releases/3.2.txt
+++ b/docs/releases/3.2.txt
@@ -489,6 +489,11 @@ Miscellaneous
   :py:meth:`~unittest.TestCase.setUp` method are called before
   ``TestContextDecorator.disable()``.
 
+* ``SessionMiddleware`` now raises a
+  :exc:`~django.contrib.sessions.exceptions.SessionInterrupted` exception
+  instead of :exc:`~django.core.exceptions.SuspiciousOperation` when a session
+  is destroyed in a concurrent request.
+
 .. _deprecated-features-3.2:
 
 Features deprecated in 3.2
diff --git a/tests/handlers/tests.py b/tests/handlers/tests.py
index 3b655a91c2..1d445cd38c 100644
--- a/tests/handlers/tests.py
+++ b/tests/handlers/tests.py
@@ -176,6 +176,10 @@ class HandlerRequestTests(SimpleTestCase):
         response = self.client.get('/suspicious/')
         self.assertEqual(response.status_code, 400)
 
+    def test_bad_request_in_view_returns_400(self):
+        response = self.client.get('/bad_request/')
+        self.assertEqual(response.status_code, 400)
+
     def test_invalid_urls(self):
         response = self.client.get('~%A9helloworld')
         self.assertEqual(response.status_code, 404)
@@ -259,6 +263,10 @@ class AsyncHandlerRequestTests(SimpleTestCase):
         response = await self.async_client.get('/suspicious/')
         self.assertEqual(response.status_code, 400)
 
+    async def test_bad_request_in_view_returns_400(self):
+        response = await self.async_client.get('/bad_request/')
+        self.assertEqual(response.status_code, 400)
+
     async def test_no_response(self):
         msg = (
             "The view handlers.views.no_response didn't return an "
diff --git a/tests/handlers/urls.py b/tests/handlers/urls.py
index a438da55b4..e287e3734e 100644
--- a/tests/handlers/urls.py
+++ b/tests/handlers/urls.py
@@ -10,6 +10,7 @@ urlpatterns = [
     path('streaming/', views.streaming),
     path('in_transaction/', views.in_transaction),
     path('not_in_transaction/', views.not_in_transaction),
+    path('bad_request/', views.bad_request),
     path('suspicious/', views.suspicious),
     path('malformed_post/', views.malformed_post),
     path('httpstatus_enum/', views.httpstatus_enum),
diff --git a/tests/handlers/views.py b/tests/handlers/views.py
index 6d573c5df8..739aa2de40 100644
--- a/tests/handlers/views.py
+++ b/tests/handlers/views.py
@@ -1,7 +1,7 @@
 import asyncio
 from http import HTTPStatus
 
-from django.core.exceptions import SuspiciousOperation
+from django.core.exceptions import BadRequest, SuspiciousOperation
 from django.db import connection, transaction
 from django.http import HttpResponse, StreamingHttpResponse
 from django.views.decorators.csrf import csrf_exempt
@@ -33,6 +33,10 @@ def not_in_transaction(request):
     return HttpResponse(str(connection.in_atomic_block))
 
 
+def bad_request(request):
+    raise BadRequest()
+
+
 def suspicious(request):
     raise SuspiciousOperation('dubious')
 
diff --git a/tests/sessions_tests/tests.py b/tests/sessions_tests/tests.py
index 29b58073e1..11adef9d75 100644
--- a/tests/sessions_tests/tests.py
+++ b/tests/sessions_tests/tests.py
@@ -19,7 +19,9 @@ 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.exceptions import InvalidSessionKey
+from django.contrib.sessions.exceptions import (
+    InvalidSessionKey, SessionInterrupted,
+)
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.contrib.sessions.models import Session
 from django.contrib.sessions.serializers import (
@@ -28,7 +30,7 @@ from django.contrib.sessions.serializers import (
 from django.core import management
 from django.core.cache import caches
 from django.core.cache.backends.base import InvalidCacheBackendError
-from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
+from django.core.exceptions import ImproperlyConfigured
 from django.http import HttpResponse
 from django.test import (
     RequestFactory, SimpleTestCase, TestCase, ignore_warnings,
@@ -746,10 +748,10 @@ class SessionMiddlewareTests(TestCase):
             "The request's session was deleted before the request completed. "
             "The user may have logged out in a concurrent request, for example."
         )
-        with self.assertRaisesMessage(SuspiciousOperation, msg):
+        with self.assertRaisesMessage(SessionInterrupted, msg):
             # Handle the response through the middleware. It will try to save
             # the deleted session which will cause an UpdateError that's caught
-            # and raised as a SuspiciousOperation.
+            # and raised as a SessionInterrupted.
             middleware(request)
 
     def test_session_delete_on_end(self):
diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py
index 6d1b3e6f48..2337d3ed3d 100644
--- a/tests/view_tests/tests/test_debug.py
+++ b/tests/view_tests/tests/test_debug.py
@@ -86,6 +86,16 @@ class DebugViewTests(SimpleTestCase):
             response = self.client.get('/raises400/')
         self.assertContains(response, '<div class="context" id="', status_code=400)
 
+    def test_400_bad_request(self):
+        # When DEBUG=True, technical_500_template() is called.
+        with self.assertLogs('django.request', 'WARNING') as cm:
+            response = self.client.get('/raises400_bad_request/')
+        self.assertContains(response, '<div class="context" id="', status_code=400)
+        self.assertEqual(
+            cm.records[0].getMessage(),
+            'Malformed request syntax: /raises400_bad_request/',
+        )
+
     # Ensure no 403.html template exists to test the default case.
     @override_settings(TEMPLATES=[{
         'BACKEND': 'django.template.backends.django.DjangoTemplates',
@@ -321,6 +331,16 @@ class NonDjangoTemplatesDebugViewTests(SimpleTestCase):
             response = self.client.get('/raises400/')
         self.assertContains(response, '<div class="context" id="', status_code=400)
 
+    def test_400_bad_request(self):
+        # When DEBUG=True, technical_500_template() is called.
+        with self.assertLogs('django.request', 'WARNING') as cm:
+            response = self.client.get('/raises400_bad_request/')
+        self.assertContains(response, '<div class="context" id="', status_code=400)
+        self.assertEqual(
+            cm.records[0].getMessage(),
+            'Malformed request syntax: /raises400_bad_request/',
+        )
+
     def test_403(self):
         response = self.client.get('/raises403/')
         self.assertContains(response, '<h1>403 Forbidden</h1>', status_code=403)
diff --git a/tests/view_tests/urls.py b/tests/view_tests/urls.py
index 6c6f73467a..f394934f4d 100644
--- a/tests/view_tests/urls.py
+++ b/tests/view_tests/urls.py
@@ -23,6 +23,7 @@ urlpatterns = [
     path('raises/', views.raises),
 
     path('raises400/', views.raises400),
+    path('raises400_bad_request/', views.raises400_bad_request),
     path('raises403/', views.raises403),
     path('raises404/', views.raises404),
     path('raises500/', views.raises500),
diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py
index 36c7bda4b4..3d7d0908b0 100644
--- a/tests/view_tests/views.py
+++ b/tests/view_tests/views.py
@@ -3,7 +3,9 @@ import decimal
 import logging
 import sys
 
-from django.core.exceptions import PermissionDenied, SuspiciousOperation
+from django.core.exceptions import (
+    BadRequest, PermissionDenied, SuspiciousOperation,
+)
 from django.http import Http404, HttpResponse, JsonResponse
 from django.shortcuts import render
 from django.template import TemplateDoesNotExist
@@ -50,6 +52,10 @@ def raises400(request):
     raise SuspiciousOperation
 
 
+def raises400_bad_request(request):
+    raise BadRequest('Malformed request syntax')
+
+
 def raises403(request):
     raise PermissionDenied("Insufficient Permissions")