diff --git a/routstr/core/middleware.py b/routstr/core/middleware.py index cff7d39a..228cd68a 100644 --- a/routstr/core/middleware.py +++ b/routstr/core/middleware.py @@ -2,6 +2,7 @@ import time import uuid from contextvars import ContextVar from typing import Callable +from urllib.parse import urlsplit from fastapi import Request, Response from starlette.datastructures import Headers @@ -39,6 +40,15 @@ def client_app_from_headers(headers: Headers) -> str: if raw is None: continue cleaned = "".join(ch for ch in raw if ch.isprintable()).strip() + if header in ("http-referer", "referer"): + try: + url = urlsplit(cleaned) + if url.scheme not in ("http", "https") or not url.hostname: + continue + except ValueError: + continue + # Attribution needs the origin, not credentials or private page URLs. + cleaned = f"{url.scheme}://{url.netloc.rsplit('@', 1)[-1]}" if cleaned: return cleaned[:_CLIENT_APP_MAX_LENGTH] return UNKNOWN_CLIENT_APP diff --git a/tests/unit/test_client_app_logging.py b/tests/unit/test_client_app_logging.py index 3f527083..a76d411c 100644 --- a/tests/unit/test_client_app_logging.py +++ b/tests/unit/test_client_app_logging.py @@ -1,9 +1,10 @@ """Tests for client-app identification in request logging.""" +import asyncio import logging import pytest -from fastapi import FastAPI +from fastapi import FastAPI, Request, Response from fastapi.testclient import TestClient from starlette.datastructures import Headers @@ -66,6 +67,27 @@ def test_client_app_from_headers(headers: dict[str, str], expected: str) -> None assert client_app_from_headers(Headers(headers)) == expected +@pytest.mark.parametrize("header", ["http-referer", "referer"]) +@pytest.mark.parametrize( + ("url", "expected"), + [ + ( + "https://alice:password@app.example:8443/private/chat?token=secret#access_token=secret", + "https://app.example:8443", + ), + ("http://[::1]:3000/chat?key=secret", "http://[::1]:3000"), + ("https://app.example/" + "a" * 200, "https://app.example"), + ("https://[invalid", "curl/8.4.0"), + ("/private/chat?token=secret", "curl/8.4.0"), + ("javascript:secret", "curl/8.4.0"), + ("https:///private", "curl/8.4.0"), + ], +) +def test_referrer_only_identifies_origin(header: str, url: str, expected: str) -> None: + headers = Headers({header: url, "user-agent": "curl/8.4.0"}) + assert client_app_from_headers(headers) == expected + + def test_value_is_truncated_to_120_chars() -> None: assert client_app_from_headers(Headers({"x-title": "a" * 500})) == "a" * 120 @@ -86,6 +108,69 @@ def test_filter_reads_context_variable() -> None: client_app_context.reset(token) +@pytest.mark.parametrize("fail", [False, True]) +async def test_context_is_restored_after_request(fail: bool) -> None: + middleware = LoggingMiddleware(FastAPI()) + request = Request( + { + "type": "http", + "method": "GET", + "path": "/test", + "query_string": b"", + "headers": [], + } + ) + + async def call_next(request: Request) -> Response: + assert client_app_context.get() == UNKNOWN_CLIENT_APP + if fail: + raise RuntimeError("handler failed") + return Response() + + token = client_app_context.set("outer") + try: + if fail: + with pytest.raises(RuntimeError, match="handler failed"): + await middleware.dispatch(request, call_next) + else: + await middleware.dispatch(request, call_next) + assert client_app_context.get() == "outer" + finally: + client_app_context.reset(token) + + +async def test_concurrent_requests_keep_their_own_client_app() -> None: + middleware = LoggingMiddleware(FastAPI()) + ready = asyncio.Event() + apps: list[str] = [] + + async def call_next(request: Request) -> Response: + apps.append(request.headers["x-title"]) + if len(apps) == 2: + ready.set() + await asyncio.wait_for(ready.wait(), timeout=5) + assert client_app_context.get() == request.headers["x-title"] + return Response() + + await asyncio.gather( + *( + middleware.dispatch( + Request( + { + "type": "http", + "method": "GET", + "path": "/test", + "query_string": b"", + "headers": [(b"x-title", app)], + } + ), + call_next, + ) + for app in (b"Goose", b"Pi") + ) + ) + + def test_filter_defaults_to_unknown_outside_request_context() -> None: record = _record() assert ClientAppFilter().filter(record) is True