Handle request.get_host() raising DisallowedHost in CsrfViewMiddleware._origin_verified()

[Chris Jerdonek]

Currently, on this line, CsrfViewMiddleware._origin_verified() doesn't handle request.get_host() raising DisallowedHost:
​https://github.com/django/django/blob/41e6b2a3c5e723256506b9ff49437d52a1f3bf43/django/middleware/csrf.py#L229-L231

Since Django was previously fixed to handle request.get_host() raising DisallowedHost elsewhere in CsrfViewMiddleware.process_view() (see ticket #28693), it seems like it should be handled here, too.

[Tim Graham]

It might be better to perform host validation elsewhere in Django as suggested in #27575 so that DisallowedHost doesn't need to be caught everywhere.

[Chris Jerdonek]

Another option would be for get_host() to accept an argument that causes it to return e.g. None on a disallowed host instead of raising DisallowedHost. That would make people more aware of that possibility and give callers another option aside from a try-except for handling that case.

[Mariusz Felisiak]

Thanks for the report. I attached a test.

[Chris Jerdonek]

Thanks, Mariusz. However, do you know for sure that's testing the right code path? _origin_verified() only gets called when if request.method not in ('GET', 'HEAD', 'OPTIONS', 'TRACE'), but the test appears to be GET. I could be wrong though since my observation is based on inspection rather than running the test.

[Mariusz Felisiak]

Regression test.

[Mariusz Felisiak]

Replying to Chris Jerdonek:

Thanks, Mariusz. However, do you know for sure that's testing the right code path? _origin_verified() only gets called when if request.method not in ('GET', 'HEAD', 'OPTIONS', 'TRACE'), but the test appears to be GET. I could be wrong though since my observation is based on inspection rather than running the test.

Test crashes because method is not set in this case. I updated attached test to use the POST method.

---

Replying to Tim Graham:

It might be better to perform host validation elsewhere in Django as suggested in #27575 so that DisallowedHost doesn't need to be caught everywhere.

This can be tricky, so I'd fix this case independently and discuss the options in #27575.

[Chris Jerdonek]

Test crashes because method is not set in this case. I updated attached test to use the POST method.

It might be good to fix _get_GET_no_csrf_cookie_request() so that the method is indeed set to GET.

[Mariusz Felisiak]

Replying to Chris Jerdonek:

It might be good to fix _get_GET_no_csrf_cookie_request() so that the method is indeed set to GET.

Agreed, ​PR.

[Chris Jerdonek]

PR: ​https://github.com/django/django/pull/14179

[Mariusz Felisiak <felisiak.mariusz@…>]

In ff514309:

Fixed #32578 -- Fixed crash in CsrfViewMiddleware when a request with Origin header has an invalid host.