lgtm
This commit is contained in:
parent
a08b5d2ae0
commit
b98f0e3904
38 changed files with 3732 additions and 483 deletions
|
|
@ -7,6 +7,10 @@ const NAVIGATION_TIMEOUT = 15_000;
|
|||
const READY_TIMEOUT = 15_000;
|
||||
const RENDER_BUFFER_MS = 500;
|
||||
const POOL_SIZE = 3;
|
||||
// Abort screenshots whose main frame navigates more than this many times after
|
||||
// the initial load. Protects against redirect loops triggered by malicious
|
||||
// invite codes (e.g. client-side `location.href` chains).
|
||||
const MAX_POST_LOAD_NAVIGATIONS = 5;
|
||||
|
||||
let browser: Browser | null = null;
|
||||
let sharedContext: BrowserContext | null = null;
|
||||
|
|
@ -124,7 +128,7 @@ async function ensureContext(): Promise<BrowserContext> {
|
|||
body: entry.body,
|
||||
});
|
||||
} catch {
|
||||
await route.continue().catch(() => {});
|
||||
await route.continue().catch(() => { });
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -152,11 +156,11 @@ async function warmPool(): Promise<void> {
|
|||
async function acquirePage(): Promise<Page> {
|
||||
const page = pagePool.shift();
|
||||
if (page && !page.isClosed()) {
|
||||
warmPool().catch(() => {});
|
||||
warmPool().catch(() => { });
|
||||
return page;
|
||||
}
|
||||
const newPage = await createPage();
|
||||
warmPool().catch(() => {});
|
||||
warmPool().catch(() => { });
|
||||
return newPage;
|
||||
}
|
||||
|
||||
|
|
@ -165,14 +169,14 @@ async function releasePage(page: Page): Promise<void> {
|
|||
if (page.isClosed()) return;
|
||||
// Navigate to blank to free rendered page resources (DOM, WebGL context)
|
||||
// while keeping the page object alive for V8 code cache reuse
|
||||
await page.goto('about:blank', { timeout: 5000 }).catch(() => {});
|
||||
await page.goto('about:blank', { timeout: 5000 }).catch(() => { });
|
||||
if (!page.isClosed() && pagePool.length < POOL_SIZE) {
|
||||
pagePool.push(page);
|
||||
} else {
|
||||
await page.close().catch(() => {});
|
||||
await page.close().catch(() => { });
|
||||
}
|
||||
} catch {
|
||||
await page.close().catch(() => {});
|
||||
await page.close().catch(() => { });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -206,11 +210,11 @@ export async function initialize(appUrl: string): Promise<void> {
|
|||
// Non-fatal — cache will still have JS/CSS/tiles from the partial load
|
||||
}
|
||||
console.log(`Pre-warm complete. Cache: ${networkCache.stats()}`);
|
||||
await page.close().catch(() => {});
|
||||
await page.close().catch(() => { });
|
||||
await warmPool();
|
||||
return;
|
||||
} catch (err) {
|
||||
await page.close().catch(() => {});
|
||||
await page.close().catch(() => { });
|
||||
if (attempt === MAX_RETRIES) {
|
||||
console.warn(`Pre-warm failed after ${MAX_RETRIES} attempts (non-fatal):`, err);
|
||||
} else {
|
||||
|
|
@ -230,6 +234,17 @@ export async function takeScreenshot(url: string, authHeader?: string): Promise<
|
|||
const page = await acquirePage();
|
||||
const t0 = performance.now();
|
||||
|
||||
let postLoadNavigations = 0;
|
||||
let navigationLoopDetected = false;
|
||||
const mainFrame = page.mainFrame();
|
||||
const onNavigated = (frame: ReturnType<typeof page.mainFrame>) => {
|
||||
if (frame !== mainFrame) return;
|
||||
postLoadNavigations += 1;
|
||||
if (postLoadNavigations > MAX_POST_LOAD_NAVIGATIONS) {
|
||||
navigationLoopDetected = true;
|
||||
}
|
||||
};
|
||||
|
||||
try {
|
||||
// Inject Authorization header on API requests so the headless browser
|
||||
// is authenticated (required for licensed users outside the free zone).
|
||||
|
|
@ -250,6 +265,10 @@ export async function takeScreenshot(url: string, authHeader?: string): Promise<
|
|||
if (response) {
|
||||
console.log(` Navigate: ${(t1 - t0).toFixed(0)}ms (status ${response.status()})`);
|
||||
}
|
||||
// Start counting only AFTER the initial navigation completes — the
|
||||
// initial `goto` itself fires framenavigated, so we'd double-count.
|
||||
postLoadNavigations = 0;
|
||||
page.on('framenavigated', onNavigated);
|
||||
|
||||
// Wait for the frontend to signal that data is loaded and layers created
|
||||
try {
|
||||
|
|
@ -259,6 +278,12 @@ export async function takeScreenshot(url: string, authHeader?: string): Promise<
|
|||
} catch {
|
||||
console.warn(' Timed out waiting for __screenshot_ready');
|
||||
}
|
||||
|
||||
if (navigationLoopDetected) {
|
||||
throw new Error(
|
||||
`Navigation loop detected (${postLoadNavigations} post-load navigations)`,
|
||||
);
|
||||
}
|
||||
const t2 = performance.now();
|
||||
console.log(` Ready: ${(t2 - t1).toFixed(0)}ms`);
|
||||
|
||||
|
|
@ -277,10 +302,11 @@ export async function takeScreenshot(url: string, authHeader?: string): Promise<
|
|||
|
||||
return Buffer.from(screenshot);
|
||||
} finally {
|
||||
page.off('framenavigated', onNavigated);
|
||||
// Remove page-level auth route before returning page to pool
|
||||
// so the next screenshot doesn't inherit stale credentials
|
||||
if (authHeader) {
|
||||
await page.unrouteAll({ behavior: 'wait' }).catch(() => {});
|
||||
await page.unrouteAll({ behavior: 'wait' }).catch(() => { });
|
||||
}
|
||||
await releasePage(page);
|
||||
}
|
||||
|
|
@ -311,17 +337,17 @@ export async function checkWebGL(): Promise<Record<string, unknown>> {
|
|||
|
||||
export async function closeBrowser(): Promise<void> {
|
||||
for (const page of pagePool) {
|
||||
await page.close().catch(() => {});
|
||||
await page.close().catch(() => { });
|
||||
}
|
||||
pagePool.length = 0;
|
||||
|
||||
if (sharedContext) {
|
||||
await sharedContext.close().catch(() => {});
|
||||
await sharedContext.close().catch(() => { });
|
||||
sharedContext = null;
|
||||
}
|
||||
|
||||
if (browser) {
|
||||
await browser.close().catch(() => {});
|
||||
await browser.close().catch(() => { });
|
||||
browser = null;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,8 +16,17 @@ test('buildScreenshotRequest accepts supported screenshot parameters', () => {
|
|||
crime: ['Burglary (avg/yr):0:5', 'Vehicle crime (avg/yr):0:10'],
|
||||
voteShare: ['% Labour:30:55', '% Conservative:10:35'],
|
||||
ethnicity: ['% White:10:80', '% South Asian:5:35'],
|
||||
amenityDistance: [
|
||||
'Distance%20to%20nearest%20amenity%20(Park)%20(km):0:0.4',
|
||||
'Distance%20to%20nearest%20amenity%20(Caf%C3%A9)%20(km):0:1.5',
|
||||
],
|
||||
transportDistance: 'Distance%20to%20nearest%20amenity%20(Bus%20stop)%20(km):0:0.3',
|
||||
amenityCount2km: 'Number%20of%20amenities%20(Cafe)%20within%202km:2:8',
|
||||
amenityCount5km: 'Number%20of%20amenities%20(Park)%20within%205km:1:20',
|
||||
poi: 'supermarket',
|
||||
tt: 'transit:kings-cross:Kings Cross:b:0:30',
|
||||
share: 'abc123',
|
||||
pc: 'SW1A 1AA',
|
||||
});
|
||||
|
||||
assert.equal(result.pagePath, '/invite/abc123');
|
||||
|
|
@ -36,6 +45,31 @@ test('buildScreenshotRequest accepts supported screenshot parameters', () => {
|
|||
]);
|
||||
assert.deepEqual(result.qs.getAll('voteShare'), ['% Labour:30:55', '% Conservative:10:35']);
|
||||
assert.deepEqual(result.qs.getAll('ethnicity'), ['% White:10:80', '% South Asian:5:35']);
|
||||
assert.deepEqual(result.qs.getAll('amenityDistance'), [
|
||||
'Distance%20to%20nearest%20amenity%20(Park)%20(km):0:0.4',
|
||||
'Distance%20to%20nearest%20amenity%20(Caf%C3%A9)%20(km):0:1.5',
|
||||
]);
|
||||
assert.deepEqual(result.qs.getAll('transportDistance'), [
|
||||
'Distance%20to%20nearest%20amenity%20(Bus%20stop)%20(km):0:0.3',
|
||||
]);
|
||||
assert.deepEqual(result.qs.getAll('amenityCount2km'), [
|
||||
'Number%20of%20amenities%20(Cafe)%20within%202km:2:8',
|
||||
]);
|
||||
assert.deepEqual(result.qs.getAll('amenityCount5km'), [
|
||||
'Number%20of%20amenities%20(Park)%20within%205km:1:20',
|
||||
]);
|
||||
assert.equal(result.qs.get('share'), 'abc123');
|
||||
assert.equal(result.qs.get('pc'), 'SW1A 1AA');
|
||||
});
|
||||
|
||||
test('buildScreenshotRequest safely passes through future dashboard parameters', () => {
|
||||
const result = buildScreenshotRequest({
|
||||
futureFilter: ['alpha:1:2', 'beta:3:4'],
|
||||
viewMode_2: 'postcode',
|
||||
});
|
||||
|
||||
assert.deepEqual(result.qs.getAll('futureFilter'), ['alpha:1:2', 'beta:3:4']);
|
||||
assert.equal(result.qs.get('viewMode_2'), 'postcode');
|
||||
});
|
||||
|
||||
test('buildScreenshotRequest rejects invalid numeric values', () => {
|
||||
|
|
@ -67,3 +101,12 @@ test('buildScreenshotRequest limits repeated parameters', () => {
|
|||
test('buildScreenshotRequest rejects control characters', () => {
|
||||
assert.throws(() => buildScreenshotRequest({ filter: 'Feature:\u0000:1' }), ValidationError);
|
||||
});
|
||||
|
||||
test('buildScreenshotRequest rejects reserved screenshot service parameters', () => {
|
||||
assert.throws(() => buildScreenshotRequest({ screenshot: '0' }), ValidationError);
|
||||
assert.throws(() => buildScreenshotRequest({ _auth: '1' }), ValidationError);
|
||||
});
|
||||
|
||||
test('buildScreenshotRequest rejects unsafe passthrough parameter names', () => {
|
||||
assert.throws(() => buildScreenshotRequest({ 'filter[]': 'Feature:0:1' }), ValidationError);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,11 +8,27 @@ export interface ValidatedScreenshotRequest {
|
|||
}
|
||||
|
||||
const MAX_REPEATED_PARAMS = 40;
|
||||
const MAX_QUERY_KEYS = 80;
|
||||
const MAX_VALUE_LENGTH = 500;
|
||||
const NUMERIC_RE = /^-?(?:\d+|\d*\.\d+)$/;
|
||||
const PATH_RE = /^\/(?:invite\/[A-Za-z0-9]{1,20})?$/;
|
||||
const QUERY_KEY_RE = /^[A-Za-z][A-Za-z0-9_-]{0,63}$/;
|
||||
const SAFE_VALUE_RE = /^[^\u0000-\u001f\u007f]+$/;
|
||||
const REPEATED_KEYS = ['filter', 'school', 'crime', 'voteShare', 'ethnicity', 'poi', 'tt'] as const;
|
||||
const REPEATED_KEYS = [
|
||||
'filter',
|
||||
'school',
|
||||
'crime',
|
||||
'voteShare',
|
||||
'ethnicity',
|
||||
'amenityDistance',
|
||||
'transportDistance',
|
||||
'amenityCount2km',
|
||||
'amenityCount5km',
|
||||
'poi',
|
||||
'tt',
|
||||
] as const;
|
||||
const PASSTHROUGH_SINGLE_KEYS = ['share', 'pc'] as const;
|
||||
const RESERVED_QUERY_KEYS = new Set(['path', 'screenshot', '_auth']);
|
||||
|
||||
type Query = Record<string, unknown>;
|
||||
|
||||
|
|
@ -56,6 +72,20 @@ function assertSafeValue(key: string, value: string): void {
|
|||
}
|
||||
}
|
||||
|
||||
function assertSafeKey(key: string): void {
|
||||
if (!QUERY_KEY_RE.test(key)) {
|
||||
validationError(`${key} is invalid`);
|
||||
}
|
||||
}
|
||||
|
||||
function appendSafeValues(qs: URLSearchParams, query: Query, key: string): void {
|
||||
assertSafeKey(key);
|
||||
for (const value of repeatedStrings(query, key)) {
|
||||
assertSafeValue(key, value);
|
||||
qs.append(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
function appendBoundedNumber(
|
||||
qs: URLSearchParams,
|
||||
query: Query,
|
||||
|
|
@ -76,7 +106,22 @@ function appendBoundedNumber(
|
|||
}
|
||||
|
||||
export function buildScreenshotRequest(query: Query): ValidatedScreenshotRequest {
|
||||
const queryKeys = Object.keys(query);
|
||||
if (queryKeys.length > MAX_QUERY_KEYS) {
|
||||
validationError('query has too many parameters');
|
||||
}
|
||||
|
||||
const qs = new URLSearchParams();
|
||||
const handledKeys = new Set<string>([
|
||||
'lat',
|
||||
'lon',
|
||||
'zoom',
|
||||
'tab',
|
||||
'og',
|
||||
'path',
|
||||
...PASSTHROUGH_SINGLE_KEYS,
|
||||
...REPEATED_KEYS,
|
||||
]);
|
||||
|
||||
appendBoundedNumber(qs, query, 'lat', -90, 90);
|
||||
appendBoundedNumber(qs, query, 'lon', -180, 180);
|
||||
|
|
@ -98,11 +143,23 @@ export function buildScreenshotRequest(query: Query): ValidatedScreenshotRequest
|
|||
qs.set('og', og);
|
||||
}
|
||||
|
||||
for (const key of PASSTHROUGH_SINGLE_KEYS) {
|
||||
const value = firstString(query, key);
|
||||
if (value == null) continue;
|
||||
assertSafeValue(key, value);
|
||||
qs.set(key, value);
|
||||
}
|
||||
|
||||
for (const key of REPEATED_KEYS) {
|
||||
for (const value of repeatedStrings(query, key)) {
|
||||
assertSafeValue(key, value);
|
||||
qs.append(key, value);
|
||||
appendSafeValues(qs, query, key);
|
||||
}
|
||||
|
||||
for (const key of queryKeys) {
|
||||
if (handledKeys.has(key)) continue;
|
||||
if (RESERVED_QUERY_KEYS.has(key)) {
|
||||
validationError(`${key} is reserved`);
|
||||
}
|
||||
appendSafeValues(qs, query, key);
|
||||
}
|
||||
|
||||
const pagePath = firstString(query, 'path') ?? '/';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue