[Django] #34073: Refactor session middleware to allow easier overrides

37 views
Skip to first unread message

Django

unread,
Oct 6, 2022, 3:06:48 PM10/6/22
to django-...@googlegroups.com
#34073: Refactor session middleware to allow easier overrides
-------------------------------------+-------------------------------------
Reporter: Michael | Owner: nobody
Gisi |
Type: | Status: new
Cleanup/optimization |
Component: | Version: 4.1
contrib.sessions |
Severity: Normal | Keywords: middleware
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I've recently needed to manipulate the session cookie domain per-request.
In order to do so, I needed to create a new middleware class inheriting
from `SessionMiddleware`, overriding the `process_response` method.

Because the middleware logic reads the domain directly from settings, the
result is a largely copy-pasted method just to change the domain being set
on the cookie. This override is also liable to break if the middleware or
settings change in future Django releases.

In contrast, `SecurityMiddleware` was much easier to override, since any
settings are loaded as instance attributes in `__init__`.

The proposed solution would consist of loading settings in the session
middleware `__init__` e.g. `self.cookie_domain =
settings.SESSION_COOKIE_DOMAIN`.

Happy to submit a PR if this seems reasonable.

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

Django

unread,
Oct 7, 2022, 2:13:54 AM10/7/22
to django-...@googlegroups.com
#34073: Refactor session middleware to allow easier overrides
--------------------------------------+------------------------------------
Reporter: Michael Gisi | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: 4.1
Severity: Normal | Resolution:
Keywords: middleware | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):

* cc: Adam Johnson (added)
* stage: Unreviewed => Accepted


Comment:

Sounds reasonable.

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

Django

unread,
Oct 7, 2022, 4:54:42 AM10/7/22
to django-...@googlegroups.com
#34073: Refactor session middleware to allow easier overrides
--------------------------------------+------------------------------------
Reporter: Michael Gisi | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.sessions | Version: 4.1
Severity: Normal | Resolution: wontfix

Keywords: middleware | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Adam Johnson):

* status: new => closed
* resolution: => wontfix


Comment:

Copying from settings in `__init__` will mean that tests using
`override_settings` to replace the values will no longer work.

You can manipulate the domain of a cookie after it's set:

{{{
In [11]: from django.http import HttpResponse

In [12]: r = HttpResponse()

In [13]: r.set_cookie("session", "123", domain="example.com")

In [14]: r.cookies["session"]["domain"] = "example.org"
}}}

Cookies in `response.cookies` are `http.cookies.Morsel` objects:
https://docs.python.org/3.10/library/http.cookies.html#http.cookies.Morsel

So you can subclass the existing middleware and override
`process_response` to call `super()`, then manipulate the cookie before
returning the response.

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

Django

unread,
Oct 7, 2022, 6:00:02 AM10/7/22
to django-...@googlegroups.com
#34073: Refactor session middleware to allow easier overrides
-------------------------------------+-------------------------------------

Reporter: Michael Gisi | Owner: nobody
Type: | Status: closed
Cleanup/optimization |

Component: contrib.sessions | Version: 4.1
Severity: Normal | Resolution: wontfix
Keywords: middleware | Triage Stage:
| Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Unreviewed


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

Django

unread,
Oct 7, 2022, 1:49:48 PM10/7/22
to django-...@googlegroups.com
#34073: Refactor session middleware to allow easier overrides
-------------------------------------+-------------------------------------
Reporter: Michael Gisi | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: contrib.sessions | Version: 4.1
Severity: Normal | Resolution: wontfix
Keywords: middleware | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Michael Gisi):

Replying to [comment:2 Adam Johnson]:

Got it, I hadn't considered the effect on tests or the ability to modify
cookies after calling `set_cookie`. Thank you for your detailed response,
cheers.

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

Reply all
Reply to author
Forward
0 new messages