Skip to content

Commit ea73def

Browse files
committed
fix!: follow capture redirects only to the configured origin
_post_v1 disables automatic redirects and resends the batch only on a 307 or 308 to the origin of host, at most 5 times. Any other redirect returns as a terminal failure. Covers the sync client and the async client, which lost its v0 same-origin guard when v1 became the only path.
1 parent 5718a37 commit ea73def

5 files changed

Lines changed: 172 additions & 10 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
pypi/posthog: major
3+
---
4+
5+
Capture follows a `307` or `308` redirect only to the origin of `host`, at most 5 times, for the sync and async clients. Any other redirect fails the batch and reaches `on_error`. In 7.x the sync client followed redirects to any origin and resent the event batch there.

‎docs/migration-7.x-to-8.0.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ You need to change code if your app does any of these:
1212
- sets `capture_mode`, `POSTHOG_CAPTURE_MODE` or `gzip`
1313
- imports `CaptureV1Error`, `posthog.capture_v1`, `request.batch_post`, `EVENTS_ENDPOINT` or `AI_EVENTS_ENDPOINT`
1414
- sends events to a self-hosted PostHog that does not serve the capture v1 endpoints
15+
- sends events through a proxy that redirects capture requests to another host
1516
- reuses one event `uuid` for more than one event
1617
- sets `$process_person_profile` to turn person processing on for events without a distinct ID
1718
- passes strings such as `"true"` for `$cookieless_mode`, `$ignore_sent_at` or `$process_person_profile`
@@ -31,6 +32,11 @@ You need to change code if your app does any of these:
3132
If you send events to a self-hosted PostHog, check that it serves both endpoints before you upgrade.
3233
An endpoint that is not served drops every event sent to it.
3334

35+
The SDK follows a `307` or `308` redirect only to the origin of `host` (same scheme, host and port), at most 5 times.
36+
Any other redirect fails the batch and reaches `on_error`.
37+
In 7.x the sync client followed redirects to any origin.
38+
If a proxy redirects capture requests to another host, point `host` at the final host.
39+
3440
## SDK identity
3541

3642
PostHog sets `$lib` and `$lib_version` on every event from the `PostHog-Sdk-Info` request header, which is always `posthog-python/<version>`.

‎posthog/capture_send.py‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from gzip import GzipFile
3030
from io import BytesIO
3131
from typing import TYPE_CHECKING, Optional
32+
from urllib.parse import urljoin, urlsplit
3233
from uuid import UUID
3334

3435
from posthog.capture_compression import CaptureCompression, _zstandard
@@ -72,6 +73,9 @@
7273
# HTTP status classification. 429 is terminal in v1 (unlike v0, where it is
7374
# retried) — the backend signals overload via retryable 5xx + Retry-After.
7475
_RETRYABLE_STATUSES = frozenset({408, 500, 502, 503, 504})
76+
# Only these keep the method and body, so a resent batch is unchanged.
77+
_REDIRECT_STATUSES = frozenset({307, 308})
78+
_MAX_REDIRECTS = 5
7579
_TERMINAL_STATUSES = frozenset({400, 401, 402, 413, 415, 429})
7680

7781
# Single ceiling (seconds) for the retry backoff: caps the exponential schedule
@@ -275,6 +279,34 @@ def _compress_v1(
275279
return data, None
276280

277281

282+
def _origin(url: str) -> Optional[tuple[str, str, int]]:
283+
parsed = urlsplit(url)
284+
try:
285+
port = parsed.port
286+
except ValueError:
287+
return None
288+
if port is None:
289+
port = 443 if parsed.scheme.lower() == "https" else 80
290+
return parsed.scheme.lower(), (parsed.hostname or "").lower(), port
291+
292+
293+
def _same_origin_redirect_url(
294+
base_url: str, current_url: str, location: Optional[str]
295+
) -> Optional[str]:
296+
"""Return the redirect target on ``base_url``'s origin, or ``None``."""
297+
if not location:
298+
return None
299+
target = urlsplit(urljoin(current_url, location))
300+
base_origin = _origin(base_url)
301+
if base_origin is None or _origin(target.geturl()) != base_origin:
302+
return None
303+
return (
304+
urlsplit(base_url)
305+
._replace(path=target.path or "/", query=target.query, fragment="")
306+
.geturl()
307+
)
308+
309+
278310
def _post_v1(
279311
api_key: str,
280312
host: Optional[str],
@@ -296,6 +328,11 @@ def _post_v1(
296328
retries. The body is compressed per ``compression`` (advertised via
297329
``Content-Encoding``). Returns the raw response; classification is left to
298330
the caller.
331+
332+
Follows only 307/308 redirects to the origin of ``host``, at most
333+
:data:`_MAX_REDIRECTS` times, resending the same body and headers. Any other
334+
redirect comes back as the response, which the caller treats as a terminal
335+
failure, so a batch never goes to another origin.
299336
"""
300337
trimmed_host = remove_trailing_slash(normalize_host(host))
301338
url = trimmed_host + path
@@ -314,9 +351,25 @@ def _post_v1(
314351
headers["Content-Encoding"] = encoding
315352

316353
log.debug("capture v1 POST %s attempt=%s request_id=%s", url, attempt, request_id)
317-
return (session or _get_session()).post(
318-
url, data=body, headers=headers, timeout=timeout
319-
)
354+
http = session or _get_session()
355+
for _ in range(_MAX_REDIRECTS + 1):
356+
res = http.post(
357+
url, data=body, headers=headers, timeout=timeout, allow_redirects=False
358+
)
359+
if res.status_code not in _REDIRECT_STATUSES:
360+
return res
361+
target = _same_origin_redirect_url(
362+
trimmed_host, url, res.headers.get("Location")
363+
)
364+
if target is None:
365+
log.warning(
366+
"capture v1 did not follow a %s redirect to another origin",
367+
res.status_code,
368+
)
369+
return res
370+
url = target
371+
log.warning("capture v1 stopped after %d redirects", _MAX_REDIRECTS)
372+
return res
320373

321374

322375
def _parse_v1_response(res: "requests.Response") -> _V1ParsedResponse:

‎posthog/test/test_capture_send.py‎

Lines changed: 104 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,23 @@ def json(self):
4949

5050

5151
class _RecordingSession:
52-
"""Captures the args of a single ``.post`` and returns a canned response."""
52+
"""Records each ``.post`` and returns canned responses, repeating the last."""
5353

54-
def __init__(self, response):
55-
self._response = response
54+
def __init__(self, *responses):
55+
self._responses = list(responses)
5656
self.calls = []
5757

58-
def post(self, url, data=None, headers=None, timeout=None):
58+
def post(self, url, data=None, headers=None, timeout=None, allow_redirects=True):
5959
self.calls.append(
60-
{"url": url, "data": data, "headers": headers, "timeout": timeout}
60+
{
61+
"url": url,
62+
"data": data,
63+
"headers": headers,
64+
"timeout": timeout,
65+
"allow_redirects": allow_redirects,
66+
}
6167
)
62-
return self._response
68+
return self._responses[min(len(self.calls), len(self._responses)) - 1]
6369

6470

6571
class _PostV1Stub:
@@ -212,6 +218,90 @@ def test_zstd_without_package_raises_actionable_error(self) -> None:
212218
self._post(_results_response({}), compression=CaptureCompression.ZSTD)
213219
self.assertIn("posthog[zstd]", str(ctx.exception))
214220

221+
@parameterized.expand(
222+
[
223+
(
224+
"relative_location",
225+
"https://us.i.posthog.com",
226+
"/i/v1/analytics/events?retry=1",
227+
"https://us.i.posthog.com/i/v1/analytics/events?retry=1",
228+
),
229+
(
230+
"host_path_prefix",
231+
"https://example.com/ingest",
232+
"https://example.com/ingest/i/v1/analytics/events",
233+
"https://example.com/ingest/i/v1/analytics/events",
234+
),
235+
(
236+
"explicit_default_port",
237+
"https://us.i.posthog.com",
238+
"https://us.i.posthog.com:443/other",
239+
"https://us.i.posthog.com/other",
240+
),
241+
]
242+
)
243+
def test_follows_same_origin_redirect_with_same_body(
244+
self, _name, host, location, expected_url
245+
) -> None:
246+
final = _results_response({})
247+
session = _RecordingSession(
248+
_FakeResponse(307, headers={"Location": location}), final
249+
)
250+
body = _build_v1_batch_body([_to_v1_event(_msg("u-1"))])
251+
res = _post_v1(
252+
"phc_key", host, body, attempt=1, request_id="r", session=session
253+
)
254+
255+
self.assertIs(res, final)
256+
self.assertEqual([c["url"] for c in session.calls][1], expected_url)
257+
self.assertEqual(session.calls[0]["data"], session.calls[1]["data"])
258+
self.assertEqual(session.calls[0]["headers"], session.calls[1]["headers"])
259+
self.assertTrue(all(c["allow_redirects"] is False for c in session.calls))
260+
261+
@parameterized.expand(
262+
[
263+
("other_host", 307, "https://attacker.example.com/collect"),
264+
("loopback", 308, "http://127.0.0.1:8080/collect"),
265+
("https_to_http", 307, "http://us.i.posthog.com/i/v1/analytics/events"),
266+
("other_port", 308, "https://us.i.posthog.com:8443/i/v1/analytics/events"),
267+
("missing_location", 307, None),
268+
("not_307_or_308", 302, "/i/v1/analytics/events"),
269+
]
270+
)
271+
def test_does_not_follow_other_redirects(self, _name, status, location) -> None:
272+
redirect = _FakeResponse(
273+
status, headers={"Location": location} if location else {}
274+
)
275+
session = _RecordingSession(redirect, _results_response({}))
276+
body = _build_v1_batch_body([_to_v1_event(_msg("u-1"))])
277+
res = _post_v1(
278+
"phc_key",
279+
"https://us.i.posthog.com",
280+
body,
281+
attempt=1,
282+
request_id="r",
283+
session=session,
284+
)
285+
286+
self.assertIs(res, redirect)
287+
self.assertEqual(len(session.calls), 1)
288+
289+
def test_stops_after_max_redirects(self) -> None:
290+
loop = _FakeResponse(307, headers={"Location": "/i/v1/analytics/events"})
291+
session = _RecordingSession(loop)
292+
body = _build_v1_batch_body([_to_v1_event(_msg("u-1"))])
293+
res = _post_v1(
294+
"phc_key",
295+
"https://us.i.posthog.com",
296+
body,
297+
attempt=1,
298+
request_id="r",
299+
session=session,
300+
)
301+
302+
self.assertIs(res, loop)
303+
self.assertEqual(len(session.calls), 6)
304+
215305

216306
class TestParseV1Response(unittest.TestCase):
217307
def test_success_parses_results_with_details(self) -> None:
@@ -449,7 +539,14 @@ def test_malformed_2xx_is_terminal(self) -> None:
449539
self.assertEqual(len(stub.calls), 1)
450540
self.assertEqual(exc.status, 200)
451541

452-
@parameterized.expand([("bad_request", 400), ("rate_limited", 429)])
542+
@parameterized.expand(
543+
[
544+
("bad_request", 400),
545+
("rate_limited", 429),
546+
("unfollowed_redirect", 307),
547+
("unfollowed_permanent_redirect", 308),
548+
]
549+
)
453550
def test_terminal_status_raises_immediately(self, _name, status) -> None:
454551
stub, exc = self._run_expecting_error(
455552
[_msg("u-1")],

‎typings/requests/__init__.pyi‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class Session:
3636
headers: dict[str, str],
3737
timeout: float,
3838
stream: bool = ...,
39+
allow_redirects: bool = ...,
3940
) -> Response: ...
4041
def get(
4142
self,

0 commit comments

Comments
 (0)