diff --git a/backend/__tests__/unit/services/webAddressProvenance.test.js b/backend/__tests__/unit/services/webAddressProvenance.test.js index 13993e1dac..e2d3368e05 100644 --- a/backend/__tests__/unit/services/webAddressProvenance.test.js +++ b/backend/__tests__/unit/services/webAddressProvenance.test.js @@ -13,7 +13,7 @@ */ const { - addressKey, addressesIn, addressWasGiven, GivenAddresses, + addressKey, addressesIn, addressWasGiven, GivenAddresses, textScanCount, } = require('../../../services/webAddressProvenance'); describe('webAddressProvenance', () => { @@ -144,14 +144,27 @@ describe('webAddressProvenance', () => { expect(given.resolve('')).toBeNull(); }); + // Counted, not timed. This assertion was a 50 ms budget for 1,000 lookups, + // which measures the runner as much as the code: it flaked at 62.47 ms on + // #1768 and again on #2446 (TASK-347). A rescan is a count, so count it — + // ONE rescan fails this now, where the clock needed enough of them to + // exceed a budget, and a cold or loaded machine no longer fails it at all. test('each text is scanned once: refused attempts against three hostile pages cost no rescan', () => { const given = new GivenAddresses(); - const page = Array.from({ length: 400 }, (_, i) => `https://h${i}.example/p${'!'.repeat(50)}x`).join(' '); - [page, page, page].forEach((text) => given.add(text)); + const address = (i) => `https://h${i}.example/p${'!'.repeat(50)}x`; + const page = Array.from({ length: 400 }, (_, i) => address(i)).join(' '); - const started = process.hrtime.bigint(); - for (let i = 0; i < 1000; i += 1) given.has(`https://attacker.example/c?d=${i}`); - expect(Number(process.hrtime.bigint() - started) / 1e6).toBeLessThan(50); + const beforeAdds = textScanCount(); + [page, page, page].forEach((text) => given.add(text)); + expect(textScanCount() - beforeAdds).toBe(3); + + const afterAdds = textScanCount(); + for (let i = 0; i < 1000; i += 1) { + expect(given.has(`https://attacker.example/c?d=${i}`)).toBe(false); + } + // An accepted attempt is a lookup too, so it may not rescan either. + expect(given.has(address(7))).toBe(true); + expect(textScanCount()).toBe(afterAdds); }); }); }); diff --git a/backend/services/webAddressProvenance.ts b/backend/services/webAddressProvenance.ts index e629472f19..827612dcb9 100644 --- a/backend/services/webAddressProvenance.ts +++ b/backend/services/webAddressProvenance.ts @@ -75,8 +75,22 @@ const parseAddress = (raw: string): { key: string; url: string } | null => { /** The comparable form of one written address, or null when it is not one. */ export const addressKey = (raw: string): string | null => parseAddress(raw)?.key ?? null; +/** + * How many texts have been scanned for addresses, ever, in this process. + * + * Instrumentation, not behaviour: the no-rescan guarantee below ("each given + * text is scanned once, and a refused attempt costs no rescan") used to be + * asserted with a wall clock, which measures the shared CI runner as much as + * the code and flaked at 62 ms against a 50 ms budget (TASK-347). A rescan is + * a count, so count it: one rescan fails the test, and a slow machine does + * not. Monotonic and never read by anything here. + */ +let textScans = 0; +export const textScanCount = (): number => textScans; + /** Every address written in `text`, keyed by its comparable form, first writing kept. */ const writtenAddresses = (text: string): Map => { + textScans += 1; const found = new Map(); for (const token of String(text || '').split(TOKEN_SEPARATORS)) { if (!token.includes('.')) continue;