#32596: Add a method to CsrfViewMiddleware to encapsulate its referer logic
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: CSRF | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
CsrfViewMiddleware,referer | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Chris Jerdonek:
Old description:
> The `CsrfViewMiddleware` class's
> [https://github.com/django/django/blob/cac9ec73db35a6d38d33f271f4724da486c60e9f/django/middleware/csrf.py#L259-L379
> process_view()`] method currently clocks in at 120 lines. If a method
> were added that encapsulates its HTTP referer logic (similar to how
> `CsrfViewMiddleware._origin_verified()` encapsulates its origin logic),
> its length could be cut in half. I think this would make the method a lot
> easier to maintain and understand. For example, in addition to being
> shorter, one optimization I'm thinking of would be easier to implement.
> This is because there's no way to "jump out" of an if-block, whereas with
> a method, you can return early.
>
> Given that the reason string for rejecting a request can vary in the
> referer case, I would suggest a method that raises an exception internal
> to the module, rather than returning a value. The call could then look
> something like this (it would go here:
> https://github.com/django/django/blob/cac9ec73db35a6d38d33f271f4724da486c60e9f/django/middleware/csrf.py#L283):
>
> {{{#!python
> elif request.is_secure():
> try:
> self._check_referer(request)
> except _RejectRequest as exc:
> return self._reject(request, exc.reason)
> }}}
New description:
The `CsrfViewMiddleware` class's
[https://github.com/django/django/blob/cac9ec73db35a6d38d33f271f4724da486c60e9f/django/middleware/csrf.py#L259-L379
process_view()] method currently clocks in at 120 lines. If a method were
added that encapsulates its HTTP referer logic (similar to how
`CsrfViewMiddleware._origin_verified()` encapsulates its origin logic),
its length could be cut in half. I think this would make the method a lot
easier to maintain and understand. For example, in addition to being
shorter, one optimization I'm thinking of would be easier to implement.
This is because there's no way to "jump out" of an if-block, whereas with
a method, you can return early.
Given that the reason string for rejecting a request can vary in the
referer case, I would suggest a method that raises an exception internal
to the module, rather than returning a value. The call could then look
something like this (it would go here:
https://github.com/django/django/blob/cac9ec73db35a6d38d33f271f4724da486c60e9f/django/middleware/csrf.py#L283):
{{{#!python
elif request.is_secure():
try:
self._check_referer(request)
except _RejectRequest as exc:
return self._reject(request, exc.reason)
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32596#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.afb0b0cded3892a092284c8e34f7b912%40djangoproject.com.
