From 6cb08cd9d46fd45648ff1ba2d6f39555e64402ce Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:22:19 +0200 Subject: [PATCH 1/4] fix(invite): one list for a file-looking segment, one rule per path, two tests that prove something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four follow-ups the reviews of #34 and #37 left open. The gate that decides whether this pass owns a path and the parser that reads the code out of it both asked whether a suffix names a file, with two different sets: the gate carried jpg|jpeg|webp|ico|txt|xml that the parser lacked, the parser carried html that the gate lacked. So /invite/AB.HTML passed the gate and then lost its code, while /invite/AB.JSON never reached the gate at all. Both read one constant now, and a case asks the two as a single question across every suffix — the sweep includes html, a suffix that names no file type, and a mixed-case spelling — so a set that grows on one side and not the other fails there rather than in production. The two still ask it of different things, deliberately: the gate of the whole path, because /invite/AB12CD/logo.png is an asset request whoever owns the first segment, and the parser of the code segment alone. A case pins that difference as well as the agreement. html is not in the list for one reason: a code that happens to end in .HTML stays a code. The shell needs no help from the list — the parser names index.html outright — and the 308 that canonicalises it comes from the platform and is handed on either way. public/_headers had a rule for /invite/ and another for /invite/*, and the first matches the second. Every answer that matched both carried the value twice: measured on the deploy, /invite/ answered cache-control: public, max-age=60, public, max-age=60 Pages concatenates what every matching rule says rather than letting one win, and a parser takes the first. The rules the wildcards already cover are gone, the bare paths keep theirs, and /invite/invite.js keeps its own rule above the wildcard — the restatement below it, added when the match order was thought to be uncertain, only appended a third value nobody reads. check-site.mjs now requires the wildcard, rejects a rule the wildcard covers, and rejects the same exact path declared twice, which its own header parser had been merging out of sight. Two tests could not tell a working build from a broken one. The clipboard stub resolved and discarded what it was handed, so the case named after copying the canonical invite URL passed on any string, or on none; it records now, and the case requires exactly one write, equal to the href the canonical link names. And the fifteen-second lookup budget was asserted against its own constant and nowhere else: a request that never answers now has to leave the loading state by itself, and the case measures how long that took against the budget the page itself reports. --- functions/lib/itunes-banner.js | 27 +++++++++- public/_headers | 29 +++++------ scripts/check-site.mjs | 34 ++++++++++--- test/itunes-banner-function.test.mjs | 73 ++++++++++++++++++++++++++++ tests/behavior.spec.mjs | 48 +++++++++++++++++- 5 files changed, 183 insertions(+), 28 deletions(-) diff --git a/functions/lib/itunes-banner.js b/functions/lib/itunes-banner.js index 8458f47..e3518c0 100644 --- a/functions/lib/itunes-banner.js +++ b/functions/lib/itunes-banner.js @@ -107,9 +107,32 @@ export function isLandingShell(html) { return typeof html === 'string' && LANDING_MARKS.every((mark) => html.includes(mark)); } +/** + * A suffix that names a file rather than a campaign code. + * + * One list, used by both questions that ask it. They used to ask it with two + * different sets: the gate carried `jpg|jpeg|webp|ico|txt|xml` that the parser + * lacked, the parser carried `html` that the gate lacked. So `/invite/AB.HTML` + * passed the gate and then lost its code, while `/invite/AB.JSON` never + * reached the gate at all. + * + * The two still ask it of different things, and that is deliberate: the gate + * asks it of the whole path, because `/invite/AB12CD/logo.png` is an asset + * request whoever owns the first segment, and the parser asks it of the code + * segment alone, because that is the part that would become a code. For the + * two-segment shape a shared link actually has, the two therefore agree, and a + * case in test/itunes-banner-function.test.mjs holds that. + * + * `html` is deliberately not in the list, for one reason: a code that happens + * to end in `.HTML` stays a code. The shell needs no help from the list — + * parseLandingFromUrl names `index.html` outright — and the `308` that + * canonicalises it comes from the platform and is handed on either way. + */ +const ASSET_SUFFIX = /\.(js|css|map|png|svg|json|jpg|jpeg|webp|ico|txt|xml)$/i; + export function shouldRewriteItunesBanner(pathname) { const path = String(pathname || ''); - if (/\.(js|css|map|png|svg|json|jpg|jpeg|webp|ico|txt|xml)$/i.test(path)) { + if (ASSET_SUFFIX.test(path)) { return false; } return ( @@ -328,7 +351,7 @@ export function parseLandingFromUrl(urlLike) { const kind = (parts[0] || '').toLowerCase(); if (kind !== 'invite' && kind !== 'promo') return null; const segment = parts[1]; - if (segment && /\.(js|css|map|png|svg|json|html)$/i.test(segment)) { + if (segment && ASSET_SUFFIX.test(segment)) { return { kind, code: null }; } let code = segment && segment.toLowerCase() !== 'index.html' ? capCode(segment) : null; diff --git a/public/_headers b/public/_headers index b3afc0b..6a56f84 100644 --- a/public/_headers +++ b/public/_headers @@ -26,35 +26,30 @@ # Invite/promo HTML (SPA shells), including /invite/{code}. Short max-age # so copy-code / CTA markup rolls out without waiting on the zone Browser -# Cache TTL. /invite/invite.js is restated after the wildcard so the 1h -# script cache wins whether first- or last-match applies. +# Cache TTL. +# +# The bare path needs its own rule; the wildcard covers everything under it, +# /invite/ and /invite/index.html included. Rules for those two used to stand +# beside the wildcard, and every answer that matched both carried the value +# twice: measured on the deploy, /invite/ answered +# `cache-control: public, max-age=60, public, max-age=60`. Pages concatenates +# what every matching rule says rather than letting one win, and a parser takes +# the first, so /invite/invite.js keeps its own rule above the wildcard and is +# not restated below it — the restatement only added a third value nobody +# reads. On production the zone's Browser Cache TTL replaces the lot with +# max-age=14400 anyway. /invite Cache-Control: public, max-age=60 -/invite/ - Cache-Control: public, max-age=60 - -/invite/index.html - Cache-Control: public, max-age=60 - /invite/* Cache-Control: public, max-age=60 /promo Cache-Control: public, max-age=60 -/promo/ - Cache-Control: public, max-age=60 - -/promo/index.html - Cache-Control: public, max-age=60 - /promo/* Cache-Control: public, max-age=60 -/invite/invite.js - Cache-Control: public, max-age=3600 - /.well-known/apple-app-site-association Content-Type: application/json Cache-Control: public, max-age=3600 diff --git a/scripts/check-site.mjs b/scripts/check-site.mjs index b927ecf..d71e48e 100644 --- a/scripts/check-site.mjs +++ b/scripts/check-site.mjs @@ -545,20 +545,38 @@ if (!existsSync(headersPath)) { 'must be a non-immutable max-age', ); requireHeader(blocks, '/js/*', 'cache-control', notImmutable, 'must be a non-immutable max-age'); + // The wildcards cover the shells and every code-bearing path under them; the + // bare paths need their own rule because a wildcard does not match them. A + // rule for /invite/index.html beside the wildcard would match twice and the + // answer would carry the value twice, which is what it used to do. requireHeader( blocks, - '/invite/index.html', - 'cache-control', - notImmutable, - 'must be a non-immutable max-age', - ); - requireHeader( - blocks, - '/promo/index.html', + '/invite', 'cache-control', notImmutable, 'must be a non-immutable max-age', ); + requireHeader(blocks, '/promo', 'cache-control', notImmutable, 'must be a non-immutable max-age'); + for (const path of ['/invite/', '/invite/index.html', '/promo/', '/promo/index.html']) { + if (blocks.has(path)) { + fail( + `_headers: ${path} is already covered by the wildcard; two matches send the value twice`, + ); + } + } + // Two blocks for the same exact path send the value twice as surely as a + // path and a wildcard do, and parseCfHeaders merges them into one entry, so + // the map cannot show it. Count the block openers instead. + const declared = new Set(); + for (const raw of read(headersPath).split(/\r?\n/)) { + const line = raw.trimEnd(); + if (!line || line.startsWith(' ') || line.startsWith('\t')) continue; + if (line.trimStart().startsWith('#')) continue; + const path = line.trim(); + if (declared.has(path)) + fail(`_headers: ${path} is declared twice; both matches send the value`); + declared.add(path); + } requireHeader( blocks, '/invite/*', diff --git a/test/itunes-banner-function.test.mjs b/test/itunes-banner-function.test.mjs index 6350db0..118ddf7 100644 --- a/test/itunes-banner-function.test.mjs +++ b/test/itunes-banner-function.test.mjs @@ -40,6 +40,79 @@ describe('shouldRewriteItunesBanner', () => { }); }); +describe('the asset-suffix rule', () => { + test('the two questions about a file-looking segment give the same answer', () => { + // They used to be asked with two different sets: the gate carried + // jpg|jpeg|webp|ico|txt|xml that the parser lacked, the parser carried html + // that the gate lacked. So /invite/AB.HTML passed the gate and then lost + // its code, and /invite/AB.JSON never reached the gate. For the two-segment + // shape a shared link has, the answer has to be the same on both sides. + const suffixes = [ + 'js', + 'css', + 'map', + 'png', + 'svg', + 'json', + 'jpg', + 'jpeg', + 'webp', + 'ico', + 'txt', + 'xml', + ]; + // Asked as one question, so a set that grows on one side and not the other + // fails here rather than in production. `html` is in the sweep too: the two + // have to agree on it as well, and they agree that it is a code. + // + // Two segments, which is the shape a shared link has. Deeper paths are a + // different question by design — the gate reads the whole path, so + // /invite/AB12CD/logo.png is an asset request, while the parser reads the + // code segment and still finds AB12CD there. + const owns = (path) => shouldRewriteItunesBanner(path); + const reads = (path) => parseLandingFromUrl(`https://realunit.app${path}`).code !== null; + for (const suffix of [...suffixes, 'html', 'pdf', 'HTML', 'JsOn']) { + const path = `/invite/AB12CD.${suffix}`; + expect([suffix, owns(path)]).toEqual([suffix, reads(path)]); + } + for (const suffix of suffixes) { + const path = `/invite/AB12CD.${suffix.toUpperCase()}`; + expect([suffix, shouldRewriteItunesBanner(path)]).toEqual([suffix, false]); + expect([suffix, parseLandingFromUrl(`https://realunit.app${path}`)]).toEqual([ + suffix, + { kind: 'invite', code: null }, + ]); + } + // And a dot that names no file type is a code like any other, on both + // sides — otherwise this case would pass on a list that swallows every dot. + expect(shouldRewriteItunesBanner('/invite/AB12CD.PDF')).toBe(true); + expect(parseLandingFromUrl('https://realunit.app/invite/AB12CD.PDF')).toEqual({ + kind: 'invite', + code: 'AB12CD.PDF', + }); + // html is not in the list, so a code that happens to end in it stays a + // code. The shell needs no help from the list: parseLandingFromUrl names + // index.html outright, which is why it still reads as no code at all. + expect(shouldRewriteItunesBanner('/invite/index.html')).toBe(true); + expect(parseLandingFromUrl('https://realunit.app/invite/index.html')).toEqual({ + kind: 'invite', + code: null, + }); + expect(shouldRewriteItunesBanner('/invite/AB12CD.HTML')).toBe(true); + expect(parseLandingFromUrl('https://realunit.app/invite/AB12CD.HTML')).toEqual({ + kind: 'invite', + code: 'AB12CD.HTML', + }); + // And the deeper path the two answer differently, on purpose: an asset + // request the pass does not own, whose code segment is still a code. + expect(shouldRewriteItunesBanner('/invite/AB12CD/logo.png')).toBe(false); + expect(parseLandingFromUrl('https://realunit.app/invite/AB12CD/logo.png')).toEqual({ + kind: 'invite', + code: 'AB12CD', + }); + }); +}); + describe('parseLandingFromUrl', () => { test('path, query, hash, and nested URL', () => { expect(parseLandingFromUrl('https://realunit.app/invite/AB12CD')).toEqual({ diff --git a/tests/behavior.spec.mjs b/tests/behavior.spec.mjs index 6cb9b4b..b9391a2 100644 --- a/tests/behavior.spec.mjs +++ b/tests/behavior.spec.mjs @@ -759,6 +759,36 @@ test.describe('invite and promo landing', () => { test.skip(testInfo.project.name !== 'desktop-chromium', 'desktop-only invite-flow checks'); }); + test('a lookup that never answers gives up after the fifteen-second budget', async ({ page }) => { + // The budget existed only as an exported number, asserted against itself in + // the unit suite. What it is for is this: a request that never answers must + // not leave the visitor on the loading state for ever. + // Real time, not the fake clock: the budget's timer is armed while the page + // loads, and a clock installed before that leaves the page in a state this + // case is not about. + test.setTimeout(60_000); + let pending = 0; + await page.route(REFERRAL_CODE_ENDPOINT, () => { + pending += 1; // never fulfilled, never aborted + }); + const started = Date.now(); + await page.goto('/invite/AB12CD'); + // The page shows the code straight away and says it is checking it; the + // spinner section is only the step before that. + await expect(page.locator('#ok-code-hint')).toHaveText('Code wird geprüft…'); + await expect(page.locator('#state-unavailable')).toBeVisible({ timeout: 25_000 }); + // The budget itself, read from the page rather than restated here, and + // measured rather than bracketed: a case that only required the state to + // arrive within some wide window would stay green on a budget of five + // seconds or of twenty. + const budget = await page.evaluate(() => window.RealUnitInvite.LOOKUP_TIMEOUT_MS); + expect(budget).toBe(15_000); + const elapsed = Date.now() - started; + expect(elapsed).toBeGreaterThan(budget - 2_000); + expect(elapsed).toBeLessThan(budget + 6_000); + expect(pending).toBe(1); + }); + test('an invite path without a code is invalid and does not call the API', async ({ page }) => { const calls = []; await page.route(REFERRAL_CODE_ENDPOINT, (route) => { @@ -1191,10 +1221,18 @@ test.describe('invite and promo landing', () => { }), ); await page.goto('/invite/AB12CD'); + // The stub records what it was handed. Resolving and discarding it would + // let this case pass on any string at all, including none. await page.evaluate(() => { + window.__written = []; Object.defineProperty(navigator, 'clipboard', { configurable: true, - value: { writeText: () => Promise.resolve() }, + value: { + writeText: (text) => { + window.__written.push(text); + return Promise.resolve(); + }, + }, }); }); await expect(page.locator('#ok-copy-link')).toBeVisible(); @@ -1207,6 +1245,14 @@ test.describe('invite and promo landing', () => { await expect(page.locator('#ok-copy-link')).not.toHaveAttribute('aria-live'); await page.locator('#ok-copy-link').click(); await expect(page.locator('#ok-copy-link')).toHaveText('Kopiert'); + // The clipboard itself, not the label beside it: the label is written from + // the DOM and would read the same whatever was copied. Exactly one write, + // and the same href the canonical link names — the host differs between + // the deploy and this server, the path with the code does not. + await expect.poll(() => page.evaluate(() => window.__written.length)).toBe(1); + const canonical = await page.locator('link[rel="canonical"]').getAttribute('href'); + expect(canonical).toMatch(/\/invite\/AB12CD$/); + expect(await page.evaluate(() => window.__written[0])).toBe(canonical); await expect(page.locator('#ok-copy-link')).toHaveAttribute( 'aria-label', /Kopiert .*\/invite\/AB12CD$/, From c353e198126cfa57cf00b80bf884526b60193e64 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:55:16 +0200 Subject: [PATCH 2/4] test(invite): say which of the two claims does which job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment said the budget is read from the page rather than restated here, and the line under it restates it. Both belong there — the size is pinned outright, because bounds computed from the page's own number would follow it anywhere, and the elapsed time is measured against that number so another timer cannot pass for this one. The comment now says that instead of denying half of it. --- tests/behavior.spec.mjs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/behavior.spec.mjs b/tests/behavior.spec.mjs index b9391a2..42401c6 100644 --- a/tests/behavior.spec.mjs +++ b/tests/behavior.spec.mjs @@ -777,10 +777,12 @@ test.describe('invite and promo landing', () => { // spinner section is only the step before that. await expect(page.locator('#ok-code-hint')).toHaveText('Code wird geprüft…'); await expect(page.locator('#state-unavailable')).toBeVisible({ timeout: 25_000 }); - // The budget itself, read from the page rather than restated here, and - // measured rather than bracketed: a case that only required the state to - // arrive within some wide window would stay green on a budget of five - // seconds or of twenty. + // Two separate claims, because either alone would let something through. + // The size is pinned outright: without it, a budget moved to five seconds + // or to twenty would still satisfy bounds computed from itself. The + // elapsed time is then measured against it, so a page that gave up for + // some other reason, or on some other timer, does not pass for the one + // this case is named after. const budget = await page.evaluate(() => window.RealUnitInvite.LOOKUP_TIMEOUT_MS); expect(budget).toBe(15_000); const elapsed = Date.now() - started; From 7a1545a0f8410f5986ef4aecc63fba562e7b091a Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:57:32 +0200 Subject: [PATCH 3/4] test(invite): three comments that claimed a little more than the code The sweep's comment implied it catches any divergence between the two sides, where it walks a list of its own and catches a divergence on what it enumerates. The .PDF case called it a dot that names no file type, where PDF is a file type this rule simply does not call an asset. And the timeout case said the budget was asserted against itself, where the unit suite pinned it against a literal and went no further. --- test/itunes-banner-function.test.mjs | 11 +++++++---- tests/behavior.spec.mjs | 7 ++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/test/itunes-banner-function.test.mjs b/test/itunes-banner-function.test.mjs index 118ddf7..33aae63 100644 --- a/test/itunes-banner-function.test.mjs +++ b/test/itunes-banner-function.test.mjs @@ -61,8 +61,10 @@ describe('the asset-suffix rule', () => { 'txt', 'xml', ]; - // Asked as one question, so a set that grows on one side and not the other - // fails here rather than in production. `html` is in the sweep too: the two + // Asked as one question, so a suffix that moves to one side and not the + // other fails here rather than in production. The sweep is a list of its + // own, not the rule's: it catches a divergence on what it enumerates, and + // a suffix neither side knows is nobody's disagreement. `html` is in the sweep too: the two // have to agree on it as well, and they agree that it is a code. // // Two segments, which is the shape a shared link has. Deeper paths are a @@ -83,8 +85,9 @@ describe('the asset-suffix rule', () => { { kind: 'invite', code: null }, ]); } - // And a dot that names no file type is a code like any other, on both - // sides — otherwise this case would pass on a list that swallows every dot. + // And a suffix this rule does not call an asset is a code like any other, + // on both sides — otherwise the case would pass on a rule that swallowed + // every dot. expect(shouldRewriteItunesBanner('/invite/AB12CD.PDF')).toBe(true); expect(parseLandingFromUrl('https://realunit.app/invite/AB12CD.PDF')).toEqual({ kind: 'invite', diff --git a/tests/behavior.spec.mjs b/tests/behavior.spec.mjs index 42401c6..b039e72 100644 --- a/tests/behavior.spec.mjs +++ b/tests/behavior.spec.mjs @@ -760,9 +760,10 @@ test.describe('invite and promo landing', () => { }); test('a lookup that never answers gives up after the fifteen-second budget', async ({ page }) => { - // The budget existed only as an exported number, asserted against itself in - // the unit suite. What it is for is this: a request that never answers must - // not leave the visitor on the loading state for ever. + // The unit suite pinned the exported number against a literal and stopped + // there; nothing exercised what the number is for. This does: a request + // that never answers must not leave the visitor on the loading state for + // ever. // Real time, not the fake clock: the budget's timer is armed while the page // loads, and a clock installed before that leaves the page in a state this // case is not about. From 4a578919dbd5fb999f7419beae509df00b940749 Mon Sep 17 00:00:00 2001 From: TaprootFreakAI <315477232+TaprootFreakAI@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:58:05 +0200 Subject: [PATCH 4/4] fix(invite): catch any covered rule, and start the clock where the wait does The covered-path check listed the four rules that happened to be there, so a rule added later would have slipped past it. It now rejects any exact path a landing wildcard covers, with the script's own rule kept on purpose: it wants a longer cache than the shells, and a parser takes the first of the two values. Proven by adding a rule for /invite/foo and watching it fail. The budget case took its timestamp before the navigation, so a slow page load counted against the budget and could push the upper bound over on a busy machine. It starts when the page says it is checking, which is when the timer the case is about was armed. --- scripts/check-site.mjs | 18 +++++++++++++----- tests/behavior.spec.mjs | 5 ++++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/scripts/check-site.mjs b/scripts/check-site.mjs index d71e48e..cb5b8aa 100644 --- a/scripts/check-site.mjs +++ b/scripts/check-site.mjs @@ -557,11 +557,19 @@ if (!existsSync(headersPath)) { 'must be a non-immutable max-age', ); requireHeader(blocks, '/promo', 'cache-control', notImmutable, 'must be a non-immutable max-age'); - for (const path of ['/invite/', '/invite/index.html', '/promo/', '/promo/index.html']) { - if (blocks.has(path)) { - fail( - `_headers: ${path} is already covered by the wildcard; two matches send the value twice`, - ); + // Any exact path a wildcard already covers, not a list of the ones that + // happened to be there: a rule added later would otherwise slip past. The + // script keeps its own rule on purpose — it wants a longer cache than the + // shells, and a parser takes the first value of the two. + const wildcardKeep = new Set(['/invite/invite.js']); + for (const path of blocks.keys()) { + if (wildcardKeep.has(path) || path.endsWith('*')) continue; + for (const prefix of ['/invite/', '/promo/']) { + if (path === prefix || path.startsWith(prefix)) { + fail( + `_headers: ${path} is already covered by ${prefix}*; two matches send the value twice`, + ); + } } } // Two blocks for the same exact path send the value twice as surely as a diff --git a/tests/behavior.spec.mjs b/tests/behavior.spec.mjs index b039e72..71f594d 100644 --- a/tests/behavior.spec.mjs +++ b/tests/behavior.spec.mjs @@ -772,11 +772,14 @@ test.describe('invite and promo landing', () => { await page.route(REFERRAL_CODE_ENDPOINT, () => { pending += 1; // never fulfilled, never aborted }); - const started = Date.now(); await page.goto('/invite/AB12CD'); // The page shows the code straight away and says it is checking it; the // spinner section is only the step before that. await expect(page.locator('#ok-code-hint')).toHaveText('Code wird geprüft…'); + // The clock starts here, not at the navigation: a slow page load would + // otherwise be counted against the budget and could push the upper bound + // over on a busy machine. + const started = Date.now(); await expect(page.locator('#state-unavailable')).toBeVisible({ timeout: 25_000 }); // Two separate claims, because either alone would let something through. // The size is pinned outright: without it, a budget moved to five seconds