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

This commit is contained in:
Hasan Ramezani 2020-09-07 13:33:47 +02:00 committed by Mariusz Felisiak
parent fc1446073e
commit 2808cdc8fb
13 changed files with 108 additions and 10 deletions

View File

@ -1,4 +1,4 @@
from django.core.exceptions import SuspiciousOperation from django.core.exceptions import BadRequest, SuspiciousOperation
class InvalidSessionKey(SuspiciousOperation): class InvalidSessionKey(SuspiciousOperation):
@ -9,3 +9,8 @@ class InvalidSessionKey(SuspiciousOperation):
class SuspiciousSession(SuspiciousOperation): class SuspiciousSession(SuspiciousOperation):
"""The session may be tampered with""" """The session may be tampered with"""
pass pass
class SessionInterrupted(BadRequest):
"""The session was interrupted."""
pass

View File

@ -3,7 +3,7 @@ from importlib import import_module
from django.conf import settings from django.conf import settings
from django.contrib.sessions.backends.base import UpdateError 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.cache import patch_vary_headers
from django.utils.deprecation import MiddlewareMixin from django.utils.deprecation import MiddlewareMixin
from django.utils.http import http_date from django.utils.http import http_date
@ -60,7 +60,7 @@ class SessionMiddleware(MiddlewareMixin):
try: try:
request.session.save() request.session.save()
except UpdateError: except UpdateError:
raise SuspiciousOperation( raise SessionInterrupted(
"The request's session was deleted before the " "The request's session was deleted before the "
"request completed. The user may have logged " "request completed. The user may have logged "
"out in a concurrent request, for example." "out in a concurrent request, for example."

View File

@ -71,6 +71,11 @@ class RequestAborted(Exception):
pass pass
class BadRequest(Exception):
"""The request is malformed and cannot be processed."""
pass
class PermissionDenied(Exception): class PermissionDenied(Exception):
"""The user did not have permission to do that""" """The user did not have permission to do that"""
pass pass

View File

@ -8,7 +8,7 @@ from asgiref.sync import sync_to_async
from django.conf import settings from django.conf import settings
from django.core import signals from django.core import signals
from django.core.exceptions import ( from django.core.exceptions import (
PermissionDenied, RequestDataTooBig, SuspiciousOperation, BadRequest, PermissionDenied, RequestDataTooBig, SuspiciousOperation,
TooManyFieldsSent, TooManyFieldsSent,
) )
from django.http import Http404 from django.http import Http404
@ -76,6 +76,17 @@ def response_for_exception(request, exc):
exc_info=sys.exc_info(), 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): elif isinstance(exc, SuspiciousOperation):
if isinstance(exc, (RequestDataTooBig, TooManyFieldsSent)): if isinstance(exc, (RequestDataTooBig, TooManyFieldsSent)):
# POST data can't be accessed again, otherwise the original # POST data can't be accessed again, otherwise the original

View File

@ -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 as a key in dictionaries that otherwise map fields to their respective
list of errors. 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`` ``RequestAborted``
------------------ ------------------
@ -271,6 +283,24 @@ Http exceptions may be imported from ``django.http``.
:exc:`UnreadablePostError` is raised when a user cancels an upload. :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 Transaction Exceptions
====================== ======================

View File

@ -489,6 +489,11 @@ Miscellaneous
:py:meth:`~unittest.TestCase.setUp` method are called before :py:meth:`~unittest.TestCase.setUp` method are called before
``TestContextDecorator.disable()``. ``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: .. _deprecated-features-3.2:
Features deprecated in 3.2 Features deprecated in 3.2

View File

@ -176,6 +176,10 @@ class HandlerRequestTests(SimpleTestCase):
response = self.client.get('/suspicious/') response = self.client.get('/suspicious/')
self.assertEqual(response.status_code, 400) 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): def test_invalid_urls(self):
response = self.client.get('~%A9helloworld') response = self.client.get('~%A9helloworld')
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 404)
@ -259,6 +263,10 @@ class AsyncHandlerRequestTests(SimpleTestCase):
response = await self.async_client.get('/suspicious/') response = await self.async_client.get('/suspicious/')
self.assertEqual(response.status_code, 400) 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): async def test_no_response(self):
msg = ( msg = (
"The view handlers.views.no_response didn't return an " "The view handlers.views.no_response didn't return an "

View File

@ -10,6 +10,7 @@ urlpatterns = [
path('streaming/', views.streaming), path('streaming/', views.streaming),
path('in_transaction/', views.in_transaction), path('in_transaction/', views.in_transaction),
path('not_in_transaction/', views.not_in_transaction), path('not_in_transaction/', views.not_in_transaction),
path('bad_request/', views.bad_request),
path('suspicious/', views.suspicious), path('suspicious/', views.suspicious),
path('malformed_post/', views.malformed_post), path('malformed_post/', views.malformed_post),
path('httpstatus_enum/', views.httpstatus_enum), path('httpstatus_enum/', views.httpstatus_enum),

View File

@ -1,7 +1,7 @@
import asyncio import asyncio
from http import HTTPStatus 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.db import connection, transaction
from django.http import HttpResponse, StreamingHttpResponse from django.http import HttpResponse, StreamingHttpResponse
from django.views.decorators.csrf import csrf_exempt from django.views.decorators.csrf import csrf_exempt
@ -33,6 +33,10 @@ def not_in_transaction(request):
return HttpResponse(str(connection.in_atomic_block)) return HttpResponse(str(connection.in_atomic_block))
def bad_request(request):
raise BadRequest()
def suspicious(request): def suspicious(request):
raise SuspiciousOperation('dubious') raise SuspiciousOperation('dubious')

View File

@ -19,7 +19,9 @@ from django.contrib.sessions.backends.file import SessionStore as FileSession
from django.contrib.sessions.backends.signed_cookies import ( from django.contrib.sessions.backends.signed_cookies import (
SessionStore as CookieSession, 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.middleware import SessionMiddleware
from django.contrib.sessions.models import Session from django.contrib.sessions.models import Session
from django.contrib.sessions.serializers import ( from django.contrib.sessions.serializers import (
@ -28,7 +30,7 @@ from django.contrib.sessions.serializers import (
from django.core import management from django.core import management
from django.core.cache import caches from django.core.cache import caches
from django.core.cache.backends.base import InvalidCacheBackendError 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.http import HttpResponse
from django.test import ( from django.test import (
RequestFactory, SimpleTestCase, TestCase, ignore_warnings, RequestFactory, SimpleTestCase, TestCase, ignore_warnings,
@ -746,10 +748,10 @@ class SessionMiddlewareTests(TestCase):
"The request's session was deleted before the request completed. " "The request's session was deleted before the request completed. "
"The user may have logged out in a concurrent request, for example." "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 # Handle the response through the middleware. It will try to save
# the deleted session which will cause an UpdateError that's caught # the deleted session which will cause an UpdateError that's caught
# and raised as a SuspiciousOperation. # and raised as a SessionInterrupted.
middleware(request) middleware(request)
def test_session_delete_on_end(self): def test_session_delete_on_end(self):

View File

@ -86,6 +86,16 @@ class DebugViewTests(SimpleTestCase):
response = self.client.get('/raises400/') response = self.client.get('/raises400/')
self.assertContains(response, '<div class="context" id="', status_code=400) 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. # Ensure no 403.html template exists to test the default case.
@override_settings(TEMPLATES=[{ @override_settings(TEMPLATES=[{
'BACKEND': 'django.template.backends.django.DjangoTemplates', 'BACKEND': 'django.template.backends.django.DjangoTemplates',
@ -321,6 +331,16 @@ class NonDjangoTemplatesDebugViewTests(SimpleTestCase):
response = self.client.get('/raises400/') response = self.client.get('/raises400/')
self.assertContains(response, '<div class="context" id="', status_code=400) 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): def test_403(self):
response = self.client.get('/raises403/') response = self.client.get('/raises403/')
self.assertContains(response, '<h1>403 Forbidden</h1>', status_code=403) self.assertContains(response, '<h1>403 Forbidden</h1>', status_code=403)

View File

@ -23,6 +23,7 @@ urlpatterns = [
path('raises/', views.raises), path('raises/', views.raises),
path('raises400/', views.raises400), path('raises400/', views.raises400),
path('raises400_bad_request/', views.raises400_bad_request),
path('raises403/', views.raises403), path('raises403/', views.raises403),
path('raises404/', views.raises404), path('raises404/', views.raises404),
path('raises500/', views.raises500), path('raises500/', views.raises500),

View File

@ -3,7 +3,9 @@ import decimal
import logging import logging
import sys 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.http import Http404, HttpResponse, JsonResponse
from django.shortcuts import render from django.shortcuts import render
from django.template import TemplateDoesNotExist from django.template import TemplateDoesNotExist
@ -50,6 +52,10 @@ def raises400(request):
raise SuspiciousOperation raise SuspiciousOperation
def raises400_bad_request(request):
raise BadRequest('Malformed request syntax')
def raises403(request): def raises403(request):
raise PermissionDenied("Insufficient Permissions") raise PermissionDenied("Insufficient Permissions")