diff --git a/CHANGELOG.md b/CHANGELOG.md index 05d3b39006..305f2a37d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-instrumentation-django`: Fix `_DjangoMiddleware` to avoid hitting a host check ([#3651](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3651)) + ### Fixed - `opentelemetry-instrumentation`: Fix dependency conflict detection when instrumented packages are not installed by moving check back to before instrumentors are loaded. Add "instruments-any" feature for instrumentations that target multiple packages. diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index f607046959..5d54d32d33 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -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_HOSTS` check + """ + url = "{}://{}{}".format( + request.scheme, request._get_raw_host(), request.path + ) + return self._excluded_urls.url_disabled(url) + def _parse_duration_attrs( req_attrs, sem_conv_opt_in_mode=_StabilityMode.DEFAULT diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 960bf97bc4..c488bf3774 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -116,6 +116,10 @@ def setUpClass(cls): def setUp(self): super().setUp() setup_test_environment() + conf.settings.ALLOWED_HOSTS = [ + # Django adds "testserver" within "setup_test_environment" so this check doesn't break during tests. + "unknown" + ] test_name = "" if hasattr(self, "_testMethodName"): test_name = self._testMethodName @@ -230,6 +234,20 @@ def test_templated_route_get(self): self.assertEqual(span.attributes["http.scheme"], "http") self.assertEqual(span.attributes["http.status_code"], 200) + def test_when_middleware_triggers_host_check_then_returns_bad_request(self): + def return_host(request): + # "HttpRequest.get_host" checks for allowed hosts and raises + # `django.core.DisallowedHost` if it isn't allowed. + return request.get_host() + + with patch.object( + _DjangoMiddleware, "process_request", side_effect=return_host + ) as process_mock: + response = Client().get("/route/2020/template/") + + assert response.status_code == 400 + process_mock.assert_called_once() + def test_traced_get(self): Client().get("/traced/")