fix(wsgi): Respect HTTP_X_FORWARDED_PROTO in request.url construction (#5963)

#### Issues

* resolves: #5955
* resolves: PY-2283

Author: sl0thentr0py

sentry_sdk/_werkzeug.py

@@ -38,6 +38,7 @@
     from typing import Dict
     from typing import Iterator
     from typing import Tuple
+    from typing import Optional
 
 
 #
@@ -62,35 +63,41 @@ def _get_headers(environ: "Dict[str, str]") -> "Iterator[Tuple[str, str]]":
             yield key.replace("_", "-").title(), value
 
 
-#
+def _strip_default_port(host: str, scheme: "Optional[str]") -> str:
+    """Strip the port from the host if it's the default for the scheme."""
+    if scheme == "http" and host.endswith(":80"):
+        return host[:-3]
+    if scheme == "https" and host.endswith(":443"):
+        return host[:-4]
+    return host
+
+
 # `get_host` comes from `werkzeug.wsgi.get_host`
 # https://github.com/pallets/werkzeug/blob/1.0.1/src/werkzeug/wsgi.py#L145
-#
+
+
 def get_host(environ: "Dict[str, str]", use_x_forwarded_for: bool = False) -> str:
     """
     Return the host for the given WSGI environment.
     """
+    scheme = environ.get("wsgi.url_scheme")
+    if use_x_forwarded_for:
+        scheme = environ.get("HTTP_X_FORWARDED_PROTO", scheme)
+
     if use_x_forwarded_for and "HTTP_X_FORWARDED_HOST" in environ:
-        rv = environ["HTTP_X_FORWARDED_HOST"]
-        if environ["wsgi.url_scheme"] == "http" and rv.endswith(":80"):
-            rv = rv[:-3]
-        elif environ["wsgi.url_scheme"] == "https" and rv.endswith(":443"):
-            rv = rv[:-4]
+        return _strip_default_port(environ["HTTP_X_FORWARDED_HOST"], scheme)
     elif environ.get("HTTP_HOST"):
-        rv = environ["HTTP_HOST"]
-        if environ["wsgi.url_scheme"] == "http" and rv.endswith(":80"):
-            rv = rv[:-3]
-        elif environ["wsgi.url_scheme"] == "https" and rv.endswith(":443"):
-            rv = rv[:-4]
+        return _strip_default_port(environ["HTTP_HOST"], scheme)
     elif environ.get("SERVER_NAME"):
+        # SERVER_NAME/SERVER_PORT describe the internal server, so use
+        # wsgi.url_scheme (not the forwarded scheme) for port decisions.
         rv = environ["SERVER_NAME"]
         if (environ["wsgi.url_scheme"], environ["SERVER_PORT"]) not in (
             ("https", "443"),
             ("http", "80"),
         ):
             rv += ":" + environ["SERVER_PORT"]
+        return rv
     else:
         # In spite of the WSGI spec, SERVER_NAME might not be present.
-        rv = "unknown"
-
-    return rv
+        return "unknown"

sentry_sdk/integrations/wsgi.py

@@ -57,8 +57,12 @@ def get_request_url(
     path_info = environ.get("PATH_INFO", "").lstrip("/")
     path = f"{script_name}/{path_info}"
 
+    scheme = environ.get("wsgi.url_scheme")
+    if use_x_forwarded_for:
+        scheme = environ.get("HTTP_X_FORWARDED_PROTO", scheme)
+
     return "%s://%s/%s" % (
-        environ.get("wsgi.url_scheme"),
+        scheme,
         get_host(environ, use_x_forwarded_for),
         wsgi_decoding_dance(path).lstrip("/"),
     )

tests/integrations/wsgi/test_wsgi.py

@@ -6,7 +6,11 @@
 
 import sentry_sdk
 from sentry_sdk import capture_message
-from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware, _ScopedResponse
+from sentry_sdk.integrations.wsgi import (
+    SentryWsgiMiddleware,
+    _ScopedResponse,
+    get_request_url,
+)
 
 
 @pytest.fixture
@@ -547,3 +551,94 @@ def app(environ, start_response):
         assert isinstance(result, _ScopedResponse)
     else:
         assert result is response_mock
+
+
+@pytest.mark.parametrize(
+    "environ,use_x_forwarded_for,expected_url",
+    [
+        # Without use_x_forwarded_for, wsgi.url_scheme is used
+        (
+            {
+                "wsgi.url_scheme": "http",
+                "SERVER_NAME": "example.com",
+                "SERVER_PORT": "80",
+                "PATH_INFO": "/test",
+                "HTTP_X_FORWARDED_PROTO": "https",
+            },
+            False,
+            "http://example.com/test",
+        ),
+        # With use_x_forwarded_for, HTTP_X_FORWARDED_PROTO is respected
+        (
+            {
+                "wsgi.url_scheme": "http",
+                "SERVER_NAME": "example.com",
+                "SERVER_PORT": "80",
+                "PATH_INFO": "/test",
+                "HTTP_X_FORWARDED_PROTO": "https",
+            },
+            True,
+            "https://example.com/test",
+        ),
+        # With use_x_forwarded_for but no forwarded proto, wsgi.url_scheme is used
+        (
+            {
+                "wsgi.url_scheme": "http",
+                "SERVER_NAME": "example.com",
+                "SERVER_PORT": "80",
+                "PATH_INFO": "/test",
+            },
+            True,
+            "http://example.com/test",
+        ),
+        # Forwarded host with default https port is stripped using forwarded proto
+        (
+            {
+                "wsgi.url_scheme": "http",
+                "SERVER_NAME": "internal",
+                "SERVER_PORT": "80",
+                "PATH_INFO": "/test",
+                "HTTP_X_FORWARDED_PROTO": "https",
+                "HTTP_X_FORWARDED_HOST": "example.com:443",
+            },
+            True,
+            "https://example.com/test",
+        ),
+        # Forwarded host with non-default port is preserved
+        (
+            {
+                "wsgi.url_scheme": "http",
+                "SERVER_NAME": "internal",
+                "SERVER_PORT": "80",
+                "PATH_INFO": "/test",
+                "HTTP_X_FORWARDED_PROTO": "https",
+                "HTTP_X_FORWARDED_HOST": "example.com:8443",
+            },
+            True,
+            "https://example.com:8443/test",
+        ),
+        # Forwarded proto with HTTP_HOST (no forwarded host) strips default port
+        (
+            {
+                "wsgi.url_scheme": "http",
+                "HTTP_HOST": "example.com:443",
+                "SERVER_NAME": "internal",
+                "SERVER_PORT": "80",
+                "PATH_INFO": "/test",
+                "HTTP_X_FORWARDED_PROTO": "https",
+            },
+            True,
+            "https://example.com/test",
+        ),
+    ],
+    ids=[
+        "ignores_forwarded_proto_when_disabled",
+        "respects_forwarded_proto_when_enabled",
+        "falls_back_to_url_scheme_when_no_forwarded_proto",
+        "strips_default_https_port_from_forwarded_host",
+        "preserves_non_default_port_on_forwarded_host",
+        "strips_default_port_from_http_host_with_forwarded_proto",
+    ],
+)
+def test_get_request_url_x_forwarded_proto(environ, use_x_forwarded_for, expected_url):
+    assert get_request_url(environ, use_x_forwarded_for) == expected_url