Skip to content

Make the tracking snippet cache safe#8

Open
loevgaard wants to merge 2 commits into
masterfrom
fix/cache-safe-tracking-snippet
Open

Make the tracking snippet cache safe#8
loevgaard wants to merge 2 commits into
masterfrom
fix/cache-safe-tracking-snippet

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Fixes #7

The problem

The injected tracking snippet was built server side at render time, with per-visitor data baked into the HTML: the client id, referrer and matched source were embedded as a literal JSON payload, and the injection decision itself depended on per-visitor state (bot check, session-timeout cookie check). Under a full page cache (Varnish, CDN, HttpCache), whichever visitor populated the cache froze their identity into the shared page — every visitor served from that cache entry tracked as the cached visitor. Worse, a mid-session or bot visitor populating the cache stripped the snippet for everyone after them.

The fix

The snippet is now byte-identical for every visitor — only the track route URL and the session_timeout value are interpolated, both cache-safe constants. Everything per-visitor moved to where per-visitor state actually exists:

  • Session throttle → browser. The snippet keeps a rolling last-seen timestamp in localStorage and skips the POST within session_timeout, unless the URL has a query string or the referrer is cross-host (campaign hints — the server-side matcher decides definitively). Storage failures fail open to tracking.
  • Client id → POST time. The snippet posts only {page, referrer}. The same-origin fetch() carries the setono_client_id cookie (deliberately HttpOnly=false), and TrackAction resolves the id via the existing ClientContext chain. First-ever visits work too: the id is minted server side and the cookie is set on the POST response.
  • Source matching → POST time. TrackAction builds a synthetic Request from the posted page/referrer (seeded with the real IP/UA) and reuses ClientInformationResolver unchanged — matcher chain, default source and client context all work as-is.
  • Bot filtering → POST time only. A render-time bot check was itself cache-unsafe (a bot populating the cache would strip the snippet for humans). Most bots never POST anyway since they don't execute JS.

Backwards compatibility

  • Pages cached with the pre-1.2 snippet keep POSTing full payloads for their TTL. Posted values always win over resolved ones, and legacy payloads (which always contain clientId and source) skip resolution entirely — byte-for-byte legacy behaviour. The README recommends purging the page cache after upgrading.
  • AddJavascriptSubscriber's constructor signature is unchanged; the four now-unused arguments are kept and marked @deprecated unused, to be removed in 2.0. Its previous $sourceMatcher deprecation (unreleased) was dropped since the argument is no longer used.
  • TrackAction gains a nullable-last ClientInformationResolverInterface argument with the documented trigger_deprecation() shim (version 1.2; the stale unreleased 1.1 strings from Fix security, correctness, and robustness issues found in audit #6 were bumped too), plus a ?Request parameter appended to __invoke that the framework injects.

Tests

  • New regression test: two visitors with different cookies, referrers, user agents and campaigns receive assertSame-identical snippets, with all per-visitor services mocked as never-consulted at render time.
  • TrackAction: minimal-payload resolution (asserting the synthetic request carries the posted page/query/referrer and the real IP/UA), legacy-payload passthrough, per-field overlay, malformed-URL tolerance (version-adaptive for lowest deps), bot ordering, and both deprecation shims.
  • ClientInformationResolver got its first direct test file (it is now load-bearing at POST time).
  • The rendered snippet was additionally syntax-checked with Node and behaviourally exercised with stubbed browser globals (throttle, campaign bypass, expiry, storage failure, unparsable referrer).

Verification

  • Psalm, ECS, PHPUnit (52 tests, 227 assertions) all green
  • Infection: Covered Code MSI 100 % (CI-enforced threshold)
  • Test app lint:container / lint:yaml / lint:twig pass
  • composer normalize clean; dependency-analyser output identical to master

The injected snippet was built server side at render time with per-visitor
data baked into the HTML (client id, referrer, matched source), and the
injection decision itself depended on per-visitor state (bot check, session
cookie). Under a full page cache, whichever visitor populated the cache froze
their identity — or the absence of the snippet — into the page for everyone.

The snippet is now byte-identical for every visitor: only the track route URL
and the session timeout are interpolated. Everything per-visitor moved to
where per-visitor state is actually available:

- The session throttle runs in the browser against a rolling localStorage
  timestamp; a query string or cross-host referrer bypasses it.
- The client id is resolved in TrackAction from the setono_client_id cookie
  riding the same-origin fetch() POST.
- Source matching runs in TrackAction against a synthetic request built from
  the posted page and referrer, reusing ClientInformationResolver unchanged.
- Bot filtering happens solely in TrackAction.

Posted payload values always win over resolved ones, so pages cached with the
pre-1.2 snippet (which POST full payloads) keep behaving exactly as before
until they expire.

AddJavascriptSubscriber keeps its now-unused constructor arguments for
signature compatibility; TrackAction gains a nullable-last resolver argument
with the usual deprecation shim.

Fixes #7
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.99%. Comparing base (14fd7c8) to head (54726cb).

Additional details and impacted files
@@             Coverage Diff              @@
##             master       #8      +/-   ##
============================================
+ Coverage     63.98%   68.99%   +5.01%     
- Complexity      137      142       +5     
============================================
  Files            23       23              
  Lines           472      487      +15     
============================================
+ Hits            302      336      +34     
+ Misses          170      151      -19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Automated browsers (Selenium, Puppeteer, Playwright, headless Chrome)
execute JavaScript, so they pass the "bots don't run JS" filter, and many
use a real browser User-Agent that the server side bot detection on /track
cannot flag. Guard on the standardized navigator.webdriver flag instead —
real browsers leave it false or undefined. This also keeps host apps' own
end-to-end test runs out of the attribution data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking snippet embeds the client id server-side, which breaks under full-page caching

1 participant