Opened 4 years ago

Closed 4 years ago

#32579 closed Cleanup/optimization (fixed)

Two outdated code comments in CsrfViewMiddleware.process_view()

Reported by: Chris Jerdonek Owned by: Chris Jerdonek
Component: CSRF Version: dev
Severity: Normal Keywords: CsrfViewMiddleware
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I noticed that a couple code comments in CsrfViewMiddleware.process_view() are outdated:

First, there's this one:
https://github.com/django/django/blob/41e6b2a3c5e723256506b9ff49437d52a1f3bf43/django/middleware/csrf.py#L333-L334
which wasn't updated here:
https://github.com/django/django/commit/b0c56b895fd2694d7f5d4595bdbbc41916607f45

There's also this one:
https://github.com/django/django/blob/41e6b2a3c5e723256506b9ff49437d52a1f3bf43/django/middleware/csrf.py#L314-L316
which wasn't updated quite correctly here:
https://github.com/django/django/commit/ddf169cdaca91e92dd5bfe6796bb6f38369ecb68

Something like this would be better for the second one:

- # If there isn't a CSRF_COOKIE_DOMAIN, require an exact match
- # match on host:port. If not, obey the cookie rules (or those
- # for the session cookie, if CSRF_USE_SESSIONS).
  good_referer = (
      settings.SESSION_COOKIE_DOMAIN
      if settings.CSRF_USE_SESSIONS
      else settings.CSRF_COOKIE_DOMAIN
  )
- if good_referer is not None:
-     server_port = request.get_port()
-     if server_port not in ('443', '80'):
-         good_referer = '%s:%s' % (good_referer, server_port)
- else:
+ if good_referer is None:
+     # If no cookie domain is configured, allow matching the
+     # current host:port.
      try:
          # request.get_host() includes the port.
          good_referer = request.get_host()
      except DisallowedHost:
          pass
+ else:
+     server_port = request.get_port()
+     if server_port not in ('443', '80'):
+         good_referer = '%s:%s' % (good_referer, server_port)

Change History (7)

comment:1 by Mariusz Felisiak, 4 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Chris Jerdonek, 4 years ago

Owner: changed from nobody to Chris Jerdonek
Status: newassigned

comment:4 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 70332e6c:

Refs #32579 -- Optimized good_hosts creation in CsrfViewMiddleware.process_view().

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In f382524:

Refs #32579 -- Fixed cookie domain comment in CsrfViewMiddleware.process_view().

comment:7 by Mariusz Felisiak, 4 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top