[Django] #31962: Raise of SuspiciousOperation should use more specific SuspiciousSession class

[Django]

#31962: Raise of SuspiciousOperation should use more specific SuspiciousSession
class
--------------------------------------------+------------------------
Reporter: alexmv | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
--------------------------------------------+------------------------
`django/contrib/sessions/middleware.py` contains the only non-test place
where a bare `SuspiciousOperation` is raised; all other locations use one
of its more specific subclasses. Making this suspicious operation that
involves a session into a `raise SuspiciousSession` would remove this
outlier, and make the exception more specific.

This is also notable because every SuspiciousOperation subclass gets its
own logger, so this is the only place that is logged via the
`django.security.SuspiciousOperation` logger.

This should only require:

{{{
diff --git django/contrib/sessions/middleware.py
django/contrib/sessions/middleware.py
index cb8c1ff45b..f8386a21ce 100644
--- django/contrib/sessions/middleware.py
+++ 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 SuspiciousSession
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 SuspiciousSession(
"The request's session was deleted before the
"
"request completed. The user may have logged
"
"out in a concurrent request, for example."
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31962>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.
--------------------------------------+------------------------------------
Reporter: Alex Vandiver | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by felixxm):

* has_patch: 1 => 0
* type: Uncategorized => Cleanup/optimization
* easy: 1 => 0
* stage: Unreviewed => Accepted

Comment:

`SuspiciousSession` is better than `SuspiciousOperation`, but it's not a
big improvement because this error is not really caused by user and there
is nothing suspicious in it (see #27363). I think we should use a new
exception class as
[https://github.com/django/django/pull/7431#discussion_r85209451 proposed
in the original PR].

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:1>

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.

-------------------------------------+-------------------------------------
Reporter: Alex Vandiver | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

* owner: nobody => Hasan Ramezani
* status: new => assigned

Comment:

@felixxm

> I think we should use a new exception class

What do you think about `SessionInterrupted` as new exception class name?

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:2>

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.
-------------------------------------+-------------------------------------
Reporter: Alex Vandiver | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned

Component: contrib.sessions | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/13395 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:3>

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.
-------------------------------------+-------------------------------------
Reporter: Alex Vandiver | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned

Component: contrib.sessions | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* needs_better_patch: 0 => 1
* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:4>

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.
-------------------------------------+-------------------------------------
Reporter: Alex Vandiver | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned

Component: contrib.sessions | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Hasan Ramezani):

* needs_better_patch: 1 => 0
* needs_tests: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:5>

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.
-------------------------------------+-------------------------------------
Reporter: Alex Vandiver | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned

Component: contrib.sessions | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:6>

[Django]

#31962: Raise a more appriopriate exception when session cannot be updated.
-------------------------------------+-------------------------------------
Reporter: Alex Vandiver | Owner: Hasan
Type: | Ramezani

Cleanup/optimization | Status: closed
Component: contrib.sessions | Version: master
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"2808cdc8fb15ad27f83af3e62db69f5ea7ced29e" 2808cdc]:
{{{
#!CommitTicketReference repository=""
revision="2808cdc8fb15ad27f83af3e62db69f5ea7ced29e"
Fixed #31962 -- Made SessionMiddleware raise SessionInterrupted when
session destroyed while request is processing.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31962#comment:7>