diff --git a/.sampo/changesets/async-capture-redirect-path-prefix.md b/.sampo/changesets/async-capture-redirect-path-prefix.md new file mode 100644 index 000000000..06a135521 --- /dev/null +++ b/.sampo/changesets/async-capture-redirect-path-prefix.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Follow async capture redirects without duplicating or incorrectly retaining a configured host path prefix. diff --git a/posthog/_async_request.py b/posthog/_async_request.py index d50f17ead..ae750de9a 100644 --- a/posthog/_async_request.py +++ b/posthog/_async_request.py @@ -76,14 +76,17 @@ def _origin(url: str) -> tuple[str, str, Optional[int]]: return parsed.scheme.lower(), (parsed.hostname or "").lower(), port -def _same_origin_redirect_path( - base_url: str, current_path: str, location: str +def _same_origin_redirect_url( + base_url: str, current_url: str, location: str ) -> Optional[str]: - target = urlsplit(urljoin(urljoin(f"{base_url}/", current_path), location)) + target = urlsplit(urljoin(current_url, location)) if _origin(target.geturl()) != _origin(base_url): return None - path = target.path or "/" - return f"{path}?{target.query}" if target.query else path + return ( + urlsplit(base_url) + ._replace(path=target.path or "/", query=target.query, fragment="") + .geturl() + ) def _serialize_flags_body( @@ -223,10 +226,11 @@ async def async_batch_post( try: logging.getLogger("posthog").debug("making async capture request") base_url = remove_trailing_slash(normalize_host(host)) - request_path = path + # Absolute URLs avoid reapplying an HTTPX base_url path on redirects. + request_url = f"{base_url}{path}" for redirect_count in range(6): response = await http_client.post( - request_path, content=data, headers=headers, timeout=timeout + request_url, content=data, headers=headers, timeout=timeout ) if response.status_code not in (307, 308): _process_response(response) @@ -235,16 +239,16 @@ async def async_batch_post( location = response.headers.get("Location") or response.headers.get( "location" ) - redirect_path = ( - _same_origin_redirect_path(base_url, request_path, location) + redirect_url = ( + _same_origin_redirect_url(base_url, request_url, location) if location else None ) - if redirect_path is None: + if redirect_url is None: raise APIError(400, "Cross-origin or invalid redirect blocked") if redirect_count >= 5: raise APIError(400, "Too many capture redirects") - request_path = redirect_path + request_url = redirect_url finally: if owns_client: await http_client.aclose() diff --git a/posthog/test/test_async_request.py b/posthog/test/test_async_request.py index 0d941dd19..d95da855a 100644 --- a/posthog/test/test_async_request.py +++ b/posthog/test/test_async_request.py @@ -85,7 +85,7 @@ def test_build_client_scopes_requests_to_host_without_following_redirects(): @pytest.mark.asyncio -async def test_async_batch_post_uses_relative_path_and_sanitized_logs(caplog): +async def test_async_batch_post_uses_configured_host_and_sanitized_logs(caplog): caplog.set_level(logging.DEBUG, logger="posthog") client = FakeAsyncClient() @@ -97,7 +97,7 @@ async def test_async_batch_post_uses_relative_path_and_sanitized_logs(caplog): client=client, ) - assert client.calls[0][1] == ("/batch/",) + assert client.calls[0][1] == ("https://example.com/batch/",) assert "super-secret" not in caplog.text assert "test-secret-key" not in caplog.text assert "https://example.com" not in caplog.text @@ -121,11 +121,58 @@ async def test_async_batch_post_follows_same_origin_temporary_redirect(): ) assert [call[1] for call in client.calls] == [ - ("/batch/",), - ("/redirected-batch/",), + ("https://example.com/batch/",), + ("https://example.com/redirected-batch/",), ] +@pytest.mark.asyncio +@pytest.mark.parametrize("status", [307, 308]) +@pytest.mark.parametrize( + "host", ["https://example.com/proxy", "https://example.com/proxy/"] +) +@pytest.mark.parametrize( + ("location", "redirected_path"), + [ + ("/proxy/redirected-batch/", "/proxy/redirected-batch/"), + ("https://example.com/proxy/redirected-batch/", "/proxy/redirected-batch/"), + ("/redirected-batch/", "/redirected-batch/"), + ("../redirected-batch/", "/proxy/redirected-batch/"), + ("?accepted=1", "/proxy/batch/?accepted=1"), + ], +) +async def test_async_batch_post_redirects_with_host_path_prefix( + status, host, location, redirected_path +): + requests = [] + responses = [ + httpx.Response(status, headers={"Location": location}), + httpx.Response(status, headers={"Location": "?attempt=2"}), + httpx.Response(200), + ] + + def handle_request(request): + requests.append(request) + return responses.pop(0) + + batch = [{"event": "test", "distinct_id": "test-user"}] + async with httpx.AsyncClient( + base_url=host, transport=httpx.MockTransport(handle_request) + ) as client: + await async_batch_post( + "test-key", host, batch=batch, path="/batch/", client=client + ) + + assert [str(request.url) for request in requests] == [ + "https://example.com/proxy/batch/", + f"https://example.com{redirected_path}", + f"https://example.com{redirected_path.split('?')[0]}?attempt=2", + ] + assert all(request.method == "POST" for request in requests) + assert all(request.content == requests[0].content for request in requests) + assert json.loads(requests[0].content)["batch"] == batch + + @pytest.mark.asyncio async def test_async_batch_post_rejects_cross_origin_temporary_redirect(): client = FakeAsyncClient(