fix(django): handle disallowed hosts (#1235)

Author: majorgreys

ddtrace/contrib/django/patch.py

@@ -13,7 +13,6 @@
 
 from ddtrace import config, Pin
 from ddtrace.vendor import debtcollector, wrapt
-from ddtrace.compat import parse
 from ddtrace.constants import ANALYTICS_SAMPLE_RATE_KEY
 from ddtrace.contrib import func_name, dbapi
 from ddtrace.ext import http, sql as sqlx, SpanTypes
@@ -394,21 +393,7 @@ def traced_get_response(django, pin, func, instance, args, kwargs):
                 span.set_tag("http.route", route)
 
             # Set HTTP Request tags
-            # Build `http.url` tag value from request info
-            # DEV: We are explicitly omitting query strings since they may contain sensitive information
-            span.set_tag(
-                http.URL,
-                parse.urlunparse(
-                    parse.ParseResult(
-                        scheme=request.scheme,
-                        netloc=request.get_host(),  # this will include `host:port`
-                        path=request.path,
-                        params="",
-                        query="",
-                        fragment="",
-                    )
-                ),
-            )
+            span.set_tag(http.URL, utils.get_request_uri(request))
 
             response = func(*args, **kwargs)
 

ddtrace/contrib/django/utils.py

@@ -1,3 +1,4 @@
+from ...compat import parse
 from ...internal.logger import get_logger
 
 
@@ -57,3 +58,36 @@ def set_tag_array(span, prefix, value):
     else:
         for i, v in enumerate(value, start=0):
             span.set_tag("{0}.{1}".format(prefix, i), v)
+
+
+def get_request_uri(request):
+    """
+    Helper to rebuild the original request url
+
+    query string or fragments are not included.
+    """
+    # DEV: Use django.http.request.HttpRequest._get_raw_host() when available
+    # otherwise back-off to PEP 333 as done in django 1.8.x
+    if hasattr(request, "_get_raw_host"):
+        host = request._get_raw_host()
+    else:
+        try:
+            # Try to build host how Django would have
+            # https://github.com/django/django/blob/e8d0d2a5efc8012dcc8bf1809dec065ebde64c81/django/http/request.py#L85-L102
+            if "HTTP_HOST" in request.META:
+                host = request.META["HTTP_HOST"]
+            else:
+                host = request.META["SERVER_NAME"]
+                port = str(request.META["SERVER_PORT"])
+                if port != ("443" if request.is_secure() else "80"):
+                    host = "{0}:{1}".format(host, port)
+        except Exception:
+            # This really shouldn't ever happen, but lets guard here just in case
+            log.debug("Failed to build Django request host", exc_info=True)
+            host = "unknown"
+
+    # Build request url from the information available
+    # DEV: We are explicitly omitting query strings since they may contain sensitive information
+    return parse.urlunparse(
+        parse.ParseResult(scheme=request.scheme, netloc=host, path=request.path, params="", query="", fragment="",)
+    )

tests/contrib/django/test_django.py

@@ -1,5 +1,5 @@
 import django
-from django.test import modify_settings
+from django.test import modify_settings, override_settings
 import os
 import pytest
 
@@ -118,6 +118,17 @@ def test_v1XX_middleware(client, test_spans):
     assert span_resources == expected_resources
 
 
+def test_disallowed_host(client, test_spans):
+    with override_settings(ALLOWED_HOSTS="not-testserver"):
+        resp = client.get("/")
+        assert resp.status_code == 400
+        assert b"Bad Request (400)" in resp.content
+
+    root_span = test_spans.get_root_span()
+    assert root_span.get_tag("http.status_code") == "400"
+    assert root_span.get_tag("http.url") == "http://testserver/"
+
+
 """
 Middleware tests
 """