From b5535f84c58a9274b68163e0a2eb0ba987ca67ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Klari=C4=87?= Date: Mon, 14 Sep 2026 18:57:15 +0200 Subject: [PATCH] fix: detect redirects without replacing the document Redirect detection fetched the document from Node and fulfilled the navigation with it. A fulfilled document has no remote address, so Chromium places it in the public address space, and its local network access checks then block every request the page makes to an origin on a private address. Rendering a Docker service name or any other internal host over plain http returned the page shell with a 200 and no content. Loopback is exempt from those checks, which is why the integration suite never saw it. The document request is now paused through CDP at the response stage and released untouched unless it is a redirect, so the browser receives it from the network. Redirects Chromium makes itself, such as an HSTS upgrade, are asked of the origin from Node so the origin's own status is still what gets reported. --- .changeset/private-network-origins.md | 9 + src/browser/renderPage.ts | 202 +++++++++++--- tests/integration/fixtureServer.ts | 30 ++- tests/integration/render.test.ts | 77 ++++++ tests/unit/renderPage.test.ts | 362 ++++++++++++++++++-------- tests/unit/renderer.test.ts | 56 ++-- 6 files changed, 556 insertions(+), 180 deletions(-) create mode 100644 .changeset/private-network-origins.md diff --git a/.changeset/private-network-origins.md b/.changeset/private-network-origins.md new file mode 100644 index 0000000..f4c36a3 --- /dev/null +++ b/.changeset/private-network-origins.md @@ -0,0 +1,9 @@ +--- +'renderready': patch +--- + +Render pages from origins on private network addresses, such as a Docker service name over plain +http. Redirect detection used to hand the browser a document fetched from Node, which Chromium +treats as public, so its local network access checks blocked every script and stylesheet the page +loaded from its own origin and the render came back empty. The browser now receives the document +from the network itself, and redirects are still reported without fetching the destination. diff --git a/src/browser/renderPage.ts b/src/browser/renderPage.ts index fe04a06..277994d 100644 --- a/src/browser/renderPage.ts +++ b/src/browser/renderPage.ts @@ -1,4 +1,4 @@ -import type { Page, Route } from 'playwright-core'; +import type { CDPSession, Page, Route } from 'playwright-core'; import type { ResourceType } from '../config.js'; import { RenderError, errorMessage } from '../errors.js'; @@ -47,15 +47,13 @@ export async function renderPage( await page.setViewportSize(options.viewport); - // Registered first so it sits *underneath* the document interceptor below: - // Playwright runs the most recently registered matching handler first. await installResourceBlocking(page, options); - // Populated by the interceptor during navigation when the requested URL - // answers with a 3xx. Absent entirely when following redirects is enabled. + // Populated during navigation when the requested URL answers with a 3xx. + // Absent entirely when following redirects is enabled. const redirect = options.followRedirects ? undefined - : await installRedirectDetection(page, url, logger); + : await installRedirectDetection(page, url, remaining, logger); const asRedirectResult = (): RenderPageResult | undefined => redirect?.status === undefined @@ -126,12 +124,24 @@ export async function renderPage( }; } finally { tracker.stop(); + await redirect?.detach(); } } interface RedirectCapture { status: number | undefined; headers: Record; + /** Stop intercepting. Never throws, including after the page has closed. */ + detach: () => Promise; +} + +/** The parts of a CDP `Fetch.requestPaused` event redirect detection reads. */ +interface PausedRequest { + requestId: string; + request: { url: string }; + /** Present when paused at the response stage, which is the only stage enabled. */ + responseStatusCode?: number; + responseHeaders?: { name: string; value: string }[]; } /** @@ -139,62 +149,170 @@ interface RedirectCapture { * * Crawlers need to see the 3xx so they can update their index, so the default is * not to follow — but Playwright's `goto()` always does. The workaround is to - * intercept the top-level document request and re-fetch it with `maxRedirects: 0`. - * On a 3xx we record it and abort, so the destination is never fetched or - * rendered; otherwise we fulfill the navigation from the response we already - * have, avoiding a second request to the origin. + * pause the top-level document request through the Chrome DevTools Protocol once + * its response headers arrive. On a 3xx we record it and fail the request, so the + * destination is never fetched or rendered; otherwise the browser carries on + * with the response it is already receiving. + * + * Letting that response through matters. Fetching the document from Node and + * fulfilling the navigation with it looks equivalent, but a fulfilled document + * has no remote address, so Chromium places it in the public address space. Its + * local network access checks then block every request the page makes to an + * origin on a private address — a Docker service name, say — and the page never + * loads its own scripts. + * + * Not every redirect that pauses came from the origin. Chromium makes some up + * without sending the request — an HSTS upgrade of `http://` to `https://` is a + * `307 Internal Redirect` — and those are asked of the origin from Node instead, + * so the caller hears what a crawler would. That probe is only read, never handed + * to the page, so it cannot trip the checks above. * - * Scoped to the exact requested URL, so subresources are untouched. + * Scoped to document requests for the exact requested URL; any other document + * that pauses is released untouched. */ async function installRedirectDetection( page: Page, url: string, + remaining: () => number, logger: Logger, ): Promise { - const capture: RedirectCapture = { status: undefined, headers: {} }; - const normalized = new URL(url).href; - - await page.route( - requestUrl => requestUrl.href === normalized, - async (route: Route) => { - if (route.request().resourceType() !== 'document') { - // Same URL but not the navigation itself; let the blocking handler - // registered underneath decide. - return route.fallback(); + const capture: RedirectCapture = { + status: undefined, + headers: {}, + detach: async () => {}, + }; + // Compared without the fragment, which CDP never includes in a request URL. + const target = withoutFragment(url); + + let session: CDPSession | undefined; + try { + session = await page.context().newCDPSession(page); + const cdp = session; + + const originRedirect = async (event: PausedRequest): Promise => { + const status = event.responseStatusCode; + if ( + status === undefined || + !isRedirectStatus(status) || + withoutFragment(event.request.url) !== target + ) { + return undefined; } + const headers = headerRecord(event.responseHeaders ?? []); + if (headers[INTERNAL_REDIRECT_HEADER] === undefined) { + return { status, headers }; + } + const answer = await probeOrigin(page, url, remaining(), logger); + return answer !== undefined && isRedirectStatus(answer.status) ? answer : undefined; + }; - let documentResponse; + const release = async (event: PausedRequest): Promise => { try { - documentResponse = await route.fetch({ maxRedirects: 0 }); + const redirect = await originRedirect(event); + if (redirect) { + capture.status = redirect.status; + capture.headers = redirect.headers; + await cdp.send('Fetch.failRequest', { + requestId: event.requestId, + errorReason: 'Aborted', + }); + return; + } + await cdp.send('Fetch.continueRequest', { requestId: event.requestId }); } catch (error) { - // Could not fetch it ourselves; fall back to an ordinary navigation and - // accept that a redirect will be followed. - logger.debug('Redirect probe failed; navigating normally', { + // The page closed while the request was paused, which ends the render anyway. + logger.debug('Could not release a paused document request', { url, error: errorMessage(error), }); - return route.continue(); } + }; - try { - if (isRedirectStatus(documentResponse.status())) { - capture.status = documentResponse.status(); - capture.headers = documentResponse.headers(); - await route.abort(); - return; - } - await route.fulfill({ response: documentResponse }); - } finally { - // Not optional: without this the response body is retained for the life - // of the context, which leaks steadily under load. - await documentResponse.dispose().catch(() => {}); - } - }, - ); + cdp.on('Fetch.requestPaused', event => void release(event)); + await cdp.send('Fetch.enable', { + patterns: [{ urlPattern: '*', resourceType: 'Document', requestStage: 'Response' }], + }); + capture.detach = () => cdp.detach().catch(() => {}); + } catch (error) { + // Without interception the navigation is ordinary, and a redirect is followed. + logger.debug('Redirect detection unavailable; navigating normally', { + url, + error: errorMessage(error), + }); + await session?.detach().catch(() => {}); + } return capture; } +interface RedirectResponse { + status: number; + headers: Record; +} + +/** Chromium sets this on a redirect it made itself rather than received. */ +const INTERNAL_REDIRECT_HEADER = 'non-authoritative-reason'; + +/** + * How the origin itself answers `url`, without following a redirect. + * + * @returns `undefined` when the origin could not be asked, in which case the + * browser's own redirect is followed, as a navigation without detection would. + */ +async function probeOrigin( + page: Page, + url: string, + timeoutMs: number, + logger: Logger, +): Promise { + let response; + try { + // A timeout of 0 means none at all to Playwright, so a spent budget still gets one. + response = await page.request.fetch(url, { maxRedirects: 0, timeout: Math.max(1, timeoutMs) }); + } catch (error) { + logger.debug('Origin probe failed; following the browser redirect', { + url, + error: errorMessage(error), + }); + return undefined; + } + try { + return { status: response.status(), headers: response.headers() }; + } finally { + // Not optional: without this the response body is retained for the life of + // the context, which leaks steadily under load. + await response.dispose().catch(() => {}); + } +} + +/** + * Never throws: a paused request whose URL cannot be parsed must still be + * released, or its navigation hangs until the render times out. + */ +function withoutFragment(url: string): string { + try { + const parsed = new URL(url); + parsed.hash = ''; + return parsed.href; + } catch { + return url; + } +} + +/** + * CDP header entries as a record. Names are lowercased, as Playwright reports + * them, and repeated headers are joined into one comma-separated value. + */ +function headerRecord(entries: { name: string; value: string }[]): Record { + const headers: Record = {}; + for (const { name, value } of entries) { + const key = name.toLowerCase(); + const existing = headers[key]; + headers[key] = existing === undefined ? value : `${existing}, ${value}`; + } + return headers; +} + /** * Abort requests matching the configured resource types or URL substrings. * diff --git a/tests/integration/fixtureServer.ts b/tests/integration/fixtureServer.ts index 683e985..b2f3056 100644 --- a/tests/integration/fixtureServer.ts +++ b/tests/integration/fixtureServer.ts @@ -54,6 +54,17 @@ const NO_FLAG_PAGE = page( `, ); +/** + * Content comes from a script the page loads from its own origin. Unlike an + * inline script, that is a request the document makes, so it only renders if + * the browser lets the document reach its own origin. + */ +const EXTERNAL_SCRIPT_PAGE = page( + 'External script', + `
loading
+ `, +); + /** Content arrives via fetch, so the quiet window is what decides. */ const XHR_PAGE = page( 'Xhr', @@ -102,7 +113,12 @@ const COOKIE_PAGE = page( const HEADER_ECHO_PAGE = (headerValue: string): string => page('Headers', `
x-renderready:${headerValue}
`); -export async function startFixtureServer(): Promise { +/** + * @param host Address to listen on, and the host in the returned `url`. Loopback + * by default; a private network address puts the origin where Chromium's local + * network access checks apply, which they do not to loopback. + */ +export async function startFixtureServer(host = '127.0.0.1'): Promise { const requests: string[] = []; const server: Server = createServer((request, response) => { @@ -123,6 +139,8 @@ export async function startFixtureServer(): Promise { return html(NO_FLAG_PAGE); case '/xhr': return html(XHR_PAGE); + case '/external-script': + return html(EXTERNAL_SCRIPT_PAGE); case '/ld-json': return html(LD_JSON_PAGE); case '/relative': @@ -162,6 +180,12 @@ export async function startFixtureServer(): Promise { }); return response.end(page('Header', 'ok')); + case '/app.js': + response.writeHead(200, { 'content-type': 'text/javascript' }); + return response.end( + "document.getElementById('app').textContent = 'content from an external script';", + ); + case '/styles.css': response.writeHead(200, { 'content-type': 'text/css' }); return response.end('body{color:red}'); @@ -175,13 +199,13 @@ export async function startFixtureServer(): Promise { }); await new Promise(resolve => { - server.listen(0, '127.0.0.1', resolve); + server.listen(0, host, resolve); }); const { port } = server.address() as AddressInfo; return { - url: `http://127.0.0.1:${port}`, + url: `http://${host}:${port}`, requests, reset: () => { requests.length = 0; diff --git a/tests/integration/render.test.ts b/tests/integration/render.test.ts index 774096f..cdbc8e6 100644 --- a/tests/integration/render.test.ts +++ b/tests/integration/render.test.ts @@ -1,3 +1,5 @@ +import { networkInterfaces } from 'node:os'; + import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { noopLogger } from '../../src/logger.js'; @@ -110,6 +112,81 @@ describe('redirects', () => { expect(result.html).toContain('content from javascript'); expect(origin.requests).toContain('/ready'); }); + + // Resource blocking intercepts every request through Playwright, on top of + // the document interception redirect detection does. + it('still reports a redirect while resource blocking is intercepting too', async () => { + const blocking = createRenderer({ + logger: noopLogger, + blockedResourceTypes: ['image'], + pageDoneCheckInterval: 50, + waitAfterLastRequest: 100, + }); + await blocking.start(); + try { + const redirect = await blocking.render(`${origin.url}/redirect`); + expect(redirect.statusCode).toBe(302); + expect(origin.requests).not.toContain('/ready'); + + const rendered = await blocking.render(`${origin.url}/external-script`); + expect(rendered.html).toContain('content from an external script'); + } finally { + await blocking.stop(); + } + }, 60_000); +}); + +/** + * The first IPv4 address of this machine in a private range, if it has one. + * GitHub's hosted runners do. + */ +function privateNetworkAddress(): string | undefined { + const isPrivate = (address: string): boolean => + /^10\./.test(address) || + /^192\.168\./.test(address) || + /^172\.(1[6-9]|2\d|3[01])\./.test(address); + + return Object.values(networkInterfaces()) + .flat() + .find(entry => entry?.family === 'IPv4' && !entry.internal && isPrivate(entry.address)) + ?.address; +} + +const privateAddress = privateNetworkAddress(); + +// Where a renderer usually meets its origin: a Docker service name, a Kubernetes +// service or an internal load balancer, all private addresses over plain http. +// Chromium applies its local network access checks there and not to loopback, +// which is why the fixture above cannot show this. +describe.skipIf(privateAddress === undefined)('an origin on a private network address', () => { + let privateOrigin: FixtureServer; + + beforeAll(async () => { + privateOrigin = await startFixtureServer(privateAddress); + }); + + afterAll(async () => { + await privateOrigin?.close(); + }); + + beforeEach(() => { + privateOrigin.reset(); + }); + + it('renders content from a script the page loads from its own origin', async () => { + const result = await renderer.render(`${privateOrigin.url}/external-script`); + + expect(result.html).toContain('content from an external script'); + expect(privateOrigin.requests).toContain('/app.js'); + }); + + it('reports a 302 without fetching the destination', async () => { + const result = await renderer.render(`${privateOrigin.url}/redirect`); + + expect(result.statusCode).toBe(302); + expect(result.headers.location).toBe('/ready'); + expect(privateOrigin.requests).toEqual(['/redirect']); + }); }); describe('status codes', () => { diff --git a/tests/unit/renderPage.test.ts b/tests/unit/renderPage.test.ts index e66aa1e..e5ec1a9 100644 --- a/tests/unit/renderPage.test.ts +++ b/tests/unit/renderPage.test.ts @@ -10,37 +10,15 @@ interface RouteHandlerEntry { handler: (route: FakeRoute) => unknown; } -class FakeApiResponse { - disposed = false; - private readonly statusCode: number; - private readonly responseHeaders: Record; - - constructor(statusCode: number, responseHeaders: Record = {}) { - this.statusCode = statusCode; - this.responseHeaders = responseHeaders; - } - - status = () => this.statusCode; - headers = () => this.responseHeaders; - dispose = vi.fn(async () => { - this.disposed = true; - }); -} - class FakeRoute { - fulfilled = false; aborted = false; continued = false; - fellBack = false; - fetchError: Error | undefined; private readonly requestUrl: string; private readonly resourceType: string; - private readonly fetchResponse: FakeApiResponse | undefined; - constructor(requestUrl: string, resourceType: string, fetchResponse?: FakeApiResponse) { + constructor(requestUrl: string, resourceType: string) { this.requestUrl = requestUrl; this.resourceType = resourceType; - this.fetchResponse = fetchResponse; } request = () => ({ @@ -48,20 +26,6 @@ class FakeRoute { url: () => this.requestUrl, }); - fetch = vi.fn(async (): Promise => { - if (this.fetchError) { - throw this.fetchError; - } - if (!this.fetchResponse) { - throw new Error('no fetch response configured'); - } - return this.fetchResponse; - }); - - fulfill = vi.fn(async () => { - this.fulfilled = true; - }); - abort = vi.fn(async () => { this.aborted = true; }); @@ -69,34 +33,125 @@ class FakeRoute { continue = vi.fn(async () => { this.continued = true; }); +} + +/** A response for the navigation to present at the response stage. */ +interface PausedResponse { + url: string; + status: number; + headers?: { name: string; value: string }[]; +} + +/** + * The CDP session redirect detection opens. `pause` plays Chromium's part: it + * emits `Fetch.requestPaused` and resolves once the listener has released the + * request, with whichever command released it. + */ +class FakeCDPSession { + readonly sent: { method: string; params: Record }[] = []; + detached = false; + /** Make every command after `Fetch.enable` fail, as when the page has closed. */ + releaseError: Error | undefined; + private listener: ((event: unknown) => void) | undefined; + private readonly releases = new Map void>(); + private nextRequestId = 1; + + on = vi.fn((event: string, listener: (event: unknown) => void) => { + if (event === 'Fetch.requestPaused') { + this.listener = listener; + } + return this; + }); - fallback = vi.fn(() => { - this.fellBack = true; + send = vi.fn(async (method: string, params: Record = {}) => { + this.sent.push({ method, params }); + const release = this.releases.get(String(params.requestId)); + if (method !== 'Fetch.enable' && this.releaseError) { + release?.('failed'); + throw this.releaseError; + } + release?.(method); + return {}; }); + + detach = vi.fn(async () => { + this.detached = true; + }); + + get enabled(): boolean { + return this.sent.some(command => command.method === 'Fetch.enable'); + } + + pause(response: PausedResponse): Promise { + const requestId = String(this.nextRequestId++); + const released = new Promise(resolve => this.releases.set(requestId, resolve)); + this.listener?.({ + requestId, + request: { url: response.url }, + responseStatusCode: response.status, + responseHeaders: response.headers ?? [], + }); + return released; + } + + sentMethods(): string[] { + return this.sent.map(command => command.method); + } } /** - * A page whose `goto` drives the registered route handler, the way a real + * A page whose `goto` presents its responses to the CDP session, the way a real * navigation would. That is what lets these tests exercise redirect detection * without a browser. */ class FakePage { readonly routes: RouteHandlerEntry[] = []; readonly viewports: { width: number; height: number }[] = []; + readonly session = new FakeCDPSession(); + cdpError: Error | undefined; gotoStatus = 200; gotoHeaders: Record = { 'content-type': 'text/html' }; gotoError: Error | undefined; gotoReturnsNull = false; contentError: Error | undefined; htmlContent = 'rendered'; - /** The route the navigation should present to the handler, if any. */ - navigationRoute: FakeRoute | undefined; + /** Document responses the navigation pauses on, in order, if interception is enabled. */ + pausedResponses: PausedResponse[] = []; url = vi.fn(() => 'https://example.test/'); setViewportSize = vi.fn(async (viewport: { width: number; height: number }) => { this.viewports.push(viewport); }); + newCDPSession = vi.fn(async () => { + if (this.cdpError) { + throw this.cdpError; + } + return this.session; + }); + + context = () => ({ newCDPSession: this.newCDPSession }); + + /** How the origin answers a probe from Node; unset makes the probe fail. */ + probeResponse: { status: number; headers: Record } | undefined; + probeDisposed = false; + + request = { + fetch: vi.fn(async () => { + const answer = this.probeResponse; + if (!answer) { + throw new Error('connect ECONNREFUSED'); + } + return { + status: () => answer.status, + headers: () => answer.headers, + dispose: async () => { + this.probeDisposed = true; + }, + }; + }), + }; + route = vi.fn( async (matcher: RouteHandlerEntry['matcher'], handler: RouteHandlerEntry['handler']) => { this.routes.push({ matcher, handler }); @@ -104,14 +159,12 @@ class FakePage { ); goto = vi.fn(async () => { - // Route handlers registered last run first, matching Playwright. - if (this.navigationRoute) { - for (const entry of [...this.routes].reverse()) { - await entry.handler(this.navigationRoute); - if (!this.navigationRoute.fellBack) { - break; + if (this.session.enabled) { + for (const response of this.pausedResponses) { + // A failed request is an aborted navigation, as Chromium reports it. + if ((await this.session.pause(response)) === 'Fetch.failRequest') { + throw new Error('net::ERR_ABORTED'); } - this.navigationRoute.fellBack = false; } } if (this.gotoError) { @@ -265,109 +318,196 @@ describe('renderPage', () => { describe('redirect detection', () => { it('returns the 3xx without loading the destination', async () => { const page = new FakePage(); - const apiResponse = new FakeApiResponse(302, { location: 'https://example.test/new' }); - page.navigationRoute = new FakeRoute('https://example.test/', 'document', apiResponse); - page.gotoError = new Error('net::ERR_ABORTED'); + page.pausedResponses = [ + { + url: 'https://example.test/', + status: 302, + headers: [{ name: 'Location', value: 'https://example.test/new' }], + }, + ]; const result = await renderPage(asPage(page), options()); expect(result).toMatchObject({ status: 302, isRedirect: true, html: '' }); expect(result.headers.location).toBe('https://example.test/new'); - expect(page.navigationRoute.aborted).toBe(true); - expect(page.navigationRoute.fulfilled).toBe(false); + expect(page.session.sentMethods()).toContain('Fetch.failRequest'); + expect(page.session.sentMethods()).not.toContain('Fetch.continueRequest'); + expect(page.request.fetch).not.toHaveBeenCalled(); }); - it('re-fetches the document with redirects disabled', async () => { + describe('a redirect Chromium made itself', () => { + const hstsUpgrade: PausedResponse = { + url: 'http://example.test/', + status: 307, + headers: [ + { name: 'Location', value: 'https://example.test/' }, + { name: 'Non-Authoritative-Reason', value: 'HSTS' }, + ], + }; + + it("reports the origin's own redirect instead", async () => { + const page = new FakePage(); + page.pausedResponses = [hstsUpgrade]; + page.probeResponse = { status: 301, headers: { location: 'https://example.test/' } }; + + const result = await renderPage(asPage(page), options({ url: 'http://example.test/' })); + + expect(result).toMatchObject({ status: 301, isRedirect: true, html: '' }); + expect(result.headers).toEqual({ location: 'https://example.test/' }); + expect(page.request.fetch).toHaveBeenCalledWith( + 'http://example.test/', + expect.objectContaining({ maxRedirects: 0 }), + ); + expect(page.probeDisposed).toBe(true); + }); + + it('is followed when the origin itself does not redirect', async () => { + const page = new FakePage(); + page.pausedResponses = [hstsUpgrade]; + page.probeResponse = { status: 200, headers: {} }; + + const result = await renderPage(asPage(page), options({ url: 'http://example.test/' })); + + expect(page.session.sentMethods()).toContain('Fetch.continueRequest'); + expect(result.isRedirect).toBe(false); + expect(page.probeDisposed).toBe(true); + }); + + it('is followed when the origin cannot be asked', async () => { + const page = new FakePage(); + page.pausedResponses = [hstsUpgrade]; + + const result = await renderPage(asPage(page), options({ url: 'http://example.test/' })); + + expect(page.session.sentMethods()).toContain('Fetch.continueRequest'); + expect(result.isRedirect).toBe(false); + }); + }); + + it('pauses document requests once their response headers arrive', async () => { const page = new FakePage(); - const apiResponse = new FakeApiResponse(301, { location: '/moved' }); - page.navigationRoute = new FakeRoute('https://example.test/', 'document', apiResponse); - page.gotoError = new Error('net::ERR_ABORTED'); await renderPage(asPage(page), options()); - expect(page.navigationRoute.fetch).toHaveBeenCalledWith({ maxRedirects: 0 }); + expect(page.session.sent[0]).toEqual({ + method: 'Fetch.enable', + params: { + patterns: [{ urlPattern: '*', resourceType: 'Document', requestStage: 'Response' }], + }, + }); }); - it('fulfills the navigation from its own fetch on a non-redirect', async () => { + // The browser has to receive the document from the network. A document + // fulfilled from outside it counts as public to Chromium, whose local network + // access checks then block the page's requests to a private-address origin. + it('lets a non-redirect response through instead of replacing it', async () => { const page = new FakePage(); - const apiResponse = new FakeApiResponse(200, { 'content-type': 'text/html' }); - page.navigationRoute = new FakeRoute('https://example.test/', 'document', apiResponse); + page.pausedResponses = [{ url: 'https://example.test/', status: 200 }]; const result = await renderPage(asPage(page), options()); - expect(page.navigationRoute.fulfilled).toBe(true); - expect(result.isRedirect).toBe(false); + expect(page.session.sentMethods()).toEqual(['Fetch.enable', 'Fetch.continueRequest']); + expect(page.route).not.toHaveBeenCalled(); + expect(result).toMatchObject({ status: 200, isRedirect: false }); expect(result.html).toBe('rendered'); }); - // Not disposing retains the response body for the life of the context, which - // leaks steadily under load. This was a real production fix. - it('always disposes the intercepted response', async () => { - const redirectResponse = new FakeApiResponse(302, {}); - const redirectPage = new FakePage(); - redirectPage.navigationRoute = new FakeRoute( - 'https://example.test/', - 'document', - redirectResponse, - ); - redirectPage.gotoError = new Error('net::ERR_ABORTED'); - await renderPage(asPage(redirectPage), options()); - expect(redirectResponse.disposed).toBe(true); - - const okResponse = new FakeApiResponse(200, {}); - const okPage = new FakePage(); - okPage.navigationRoute = new FakeRoute('https://example.test/', 'document', okResponse); - await renderPage(asPage(okPage), options()); - expect(okResponse.disposed).toBe(true); + it('reports header names lowercased, with repeated headers joined', async () => { + const page = new FakePage(); + page.pausedResponses = [ + { + url: 'https://example.test/', + status: 301, + headers: [ + { name: 'Location', value: '/moved' }, + { name: 'Link', value: '; rel=preload' }, + { name: 'link', value: '; rel=preload' }, + ], + }, + ]; + + const result = await renderPage(asPage(page), options()); + + expect(result.headers).toEqual({ + location: '/moved', + link: '; rel=preload, ; rel=preload', + }); }); - it('falls back to a normal navigation when the probe fetch fails', async () => { + it('releases a redirect on any other document, such as an iframe', async () => { const page = new FakePage(); - const route = new FakeRoute('https://example.test/', 'document'); - route.fetchError = new Error('connection reset'); - page.navigationRoute = route; + page.pausedResponses = [ + { url: 'https://example.test/', status: 200 }, + { url: 'https://ads.example.test/frame', status: 302 }, + ]; const result = await renderPage(asPage(page), options()); - expect(route.continued).toBe(true); + expect(page.session.sentMethods()).not.toContain('Fetch.failRequest'); expect(result.isRedirect).toBe(false); - expect(result.status).toBe(200); }); - it('leaves a non-document request at the same URL to the handler underneath', async () => { + it('matches the requested URL regardless of its fragment', async () => { + const page = new FakePage(); + page.pausedResponses = [{ url: 'https://example.test/', status: 302 }]; + + const result = await renderPage( + asPage(page), + options({ url: 'https://example.test/#section' }), + ); + + expect(result.isRedirect).toBe(true); + }); + + it('detaches the session once the render is done', async () => { const page = new FakePage(); - const route = new FakeRoute('https://example.test/', 'xhr'); - page.navigationRoute = route; await renderPage(asPage(page), options()); - expect(route.fetch).not.toHaveBeenCalled(); + expect(page.session.detached).toBe(true); }); - it('installs no interceptor at all when following redirects', async () => { + it('detaches the session when navigation fails too', async () => { const page = new FakePage(); - const route = new FakeRoute( - 'https://example.test/', - 'document', - new FakeApiResponse(302, {}), - ); - page.navigationRoute = route; + page.gotoError = new Error('net::ERR_CONNECTION_REFUSED'); + + await expect(renderPage(asPage(page), options())).rejects.toThrow(RenderError); + expect(page.session.detached).toBe(true); + }); + + it('navigates normally when interception is unavailable', async () => { + const page = new FakePage(); + page.cdpError = new Error('CDP session is only available in Chromium'); + + const result = await renderPage(asPage(page), options()); + + expect(result).toMatchObject({ status: 200, isRedirect: false }); + }); + + it('still finishes the render when a paused request cannot be released', async () => { + const page = new FakePage(); + page.pausedResponses = [{ url: 'https://example.test/', status: 200 }]; + page.session.releaseError = new Error('Target page, context or browser has been closed'); + + const result = await renderPage(asPage(page), options()); + + expect(result.isRedirect).toBe(false); + }); + + it('opens no session at all when following redirects', async () => { + const page = new FakePage(); + page.pausedResponses = [{ url: 'https://example.test/', status: 302 }]; const result = await renderPage(asPage(page), options({ followRedirects: true })); + expect(page.newCDPSession).not.toHaveBeenCalled(); expect(page.routes).toHaveLength(0); - expect(route.fetch).not.toHaveBeenCalled(); expect(result.isRedirect).toBe(false); }); it('propagates a genuine navigation failure rather than reporting a redirect', async () => { const page = new FakePage(); - const route = new FakeRoute( - 'https://example.test/', - 'document', - new FakeApiResponse(200, {}), - ); - page.navigationRoute = route; + page.pausedResponses = [{ url: 'https://example.test/', status: 200 }]; page.gotoError = new Error('net::ERR_CONNECTION_REFUSED'); await expect(renderPage(asPage(page), options())).rejects.toThrow(RenderError); @@ -383,16 +523,14 @@ describe('renderPage', () => { expect(page.routes).toHaveLength(0); }); - it('registers the blocklist beneath the redirect interceptor', async () => { + it('blocks through a single route, leaving redirect detection to the CDP session', async () => { const page = new FakePage(); await renderPage(asPage(page), options({ blockedResourceTypes: ['image'] })); - // Order matters: Playwright runs the last-registered handler first, so the - // document interceptor must be registered after the blocklist. - expect(page.routes).toHaveLength(2); + expect(page.routes).toHaveLength(1); expect(page.routes[0]?.matcher).toBe('**/*'); - expect(typeof page.routes[1]?.matcher).toBe('function'); + expect(page.session.enabled).toBe(true); }); it('aborts a blocked resource type and allows everything else', async () => { diff --git a/tests/unit/renderer.test.ts b/tests/unit/renderer.test.ts index 32678b5..72cb857 100644 --- a/tests/unit/renderer.test.ts +++ b/tests/unit/renderer.test.ts @@ -31,35 +31,45 @@ let lastViewport: { width: number; height: number } | undefined; function makeFakeBrowser() { const makePage = () => { - const handlers: ((route: unknown) => unknown)[] = []; + let onPaused: ((event: unknown) => void) | undefined; + let released: (() => void) | undefined; + + const session = { + on: vi.fn((_event: string, listener: (event: unknown) => void) => { + onPaused = listener; + }), + send: vi.fn(async (method: string) => { + if (method === 'Fetch.failRequest' || method === 'Fetch.continueRequest') { + released?.(); + } + return {}; + }), + detach: vi.fn(async () => {}), + }; return { setViewportSize: vi.fn(async (viewport: { width: number; height: number }) => { lastViewport = viewport; }), - route: vi.fn(async (_matcher: unknown, handler: (route: unknown) => unknown) => { - handlers.push(handler); - }), + route: vi.fn(async () => {}), + context: () => ({ newCDPSession: vi.fn(async () => session) }), goto: vi.fn(async () => { - // Drive the document interceptor the way a real navigation would, so the - // redirect path is genuinely exercised rather than simulated. - if (script.redirect) { - const response = { - status: () => script.redirect?.status ?? 302, - headers: () => script.redirect?.headers ?? {}, - dispose: vi.fn(async () => {}), - }; - const route = { - request: () => ({ resourceType: () => 'document', url: () => 'https://example.test/' }), - fetch: vi.fn(async () => response), - abort: vi.fn(async () => {}), - fulfill: vi.fn(async () => {}), - continue: vi.fn(async () => {}), - fallback: vi.fn(), - }; - for (const handler of [...handlers].reverse()) { - await handler(route); - } + // Pause the document on the session the way a real navigation would, so + // the redirect path is genuinely exercised rather than simulated. + if (script.redirect && onPaused) { + const release = new Promise(resolve => { + released = resolve; + }); + onPaused({ + requestId: '1', + request: { url: 'https://example.test/' }, + responseStatusCode: script.redirect.status, + responseHeaders: Object.entries(script.redirect.headers).map(([name, value]) => ({ + name, + value, + })), + }); + await release; throw new Error('net::ERR_ABORTED'); } if (script.gotoError) {