-
Notifications
You must be signed in to change notification settings - Fork 761
Avoid Django's ALLOWED_HOSTS
check
#3651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
acea889
cb1c60e
71c7a46
3569d26
9a8e195
63a1662
8c30ec0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -194,7 +194,7 @@ def process_request(self, request): | |
# Read more about request.META here: | ||
# https://docs.djangoproject.com/en/3.0/ref/request-response/#django.http.HttpRequest.META | ||
|
||
if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): | ||
if self._url_is_disabled(request): | ||
return | ||
|
||
is_asgi_request = _is_asgi_request(request) | ||
|
@@ -305,7 +305,7 @@ def process_request(self, request): | |
def process_view(self, request, view_func, *args, **kwargs): | ||
# Process view is executed before the view function, here we get the | ||
# route template from request.resolver_match. It is not set yet in process_request | ||
if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): | ||
if self._url_is_disabled(request): | ||
return | ||
|
||
if ( | ||
|
@@ -330,7 +330,7 @@ def process_view(self, request, view_func, *args, **kwargs): | |
duration_attrs[HTTP_ROUTE] = route | ||
|
||
def process_exception(self, request, exception): | ||
if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): | ||
if self._url_is_disabled(request): | ||
return | ||
|
||
if self._environ_activation_key in request.META.keys(): | ||
|
@@ -340,7 +340,7 @@ def process_exception(self, request, exception): | |
# pylint: disable=too-many-locals | ||
# pylint: disable=too-many-statements | ||
def process_response(self, request, response): | ||
if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): | ||
if self._url_is_disabled(request): | ||
return response | ||
|
||
is_asgi_request = _is_asgi_request(request) | ||
|
@@ -453,6 +453,15 @@ def process_response(self, request, response): | |
|
||
return response | ||
|
||
def _url_is_disabled(self, request): | ||
""" | ||
Avoid `request.get_host` to bypass Django's ALLOWED_HOST check | ||
""" | ||
url = "{}://{}{}".format( | ||
request.scheme, request._get_raw_host(), request.path | ||
) | ||
Comment on lines
+460
to
+462
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this break anyone who was relying on the previous behavior of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my experience (not proof of anything, more as an example), host checks are intentional, not an accidental feature of any particular lib (reason why I'm proposing this). The consequence of these changes is that the host won't be checked anymore, everything else works normally. I can put these behind an environment variable, Django settings or just the current code under that method abstraction so that it can more easily be overridden, without having to copy over the whole code. Let me know what you think! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I defer to the python maintainers on how they would want this configurable, if they do. Thanks for the explanation! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not clear to me why the previous implementation appended a |
||
return self._excluded_urls.url_disabled(url) | ||
|
||
|
||
def _parse_duration_attrs( | ||
req_attrs, sem_conv_opt_in_mode=_StabilityMode.DEFAULT | ||
|
Uh oh!
There was an error while loading. Please reload this page.