fix: clean up request interceptor in recyclePage to prevent pool contamination
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 13m17s
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 13m17s
When renderUrlPdf() sets up request interception for SSRF DNS pinning,
the interceptor and event listener were never cleaned up in recyclePage().
This could cause subsequent HTML-to-PDF conversions on the same pooled
page to have external resources blocked by the stale interceptor.
- Export recyclePage for testability
- Add removeAllListeners('request') + setRequestInterception(false)
- Add browser-recycle.test.ts with TDD (red→green verified)
Tests: 443 passing (was 442)
This commit is contained in:
parent
b05bd44432
commit
024fa0084d
2 changed files with 62 additions and 1 deletions
58
src/__tests__/browser-recycle.test.ts
Normal file
58
src/__tests__/browser-recycle.test.ts
Normal file
|
|
@ -0,0 +1,58 @@
|
||||||
|
import { describe, it, expect, vi } from "vitest";
|
||||||
|
|
||||||
|
// Don't use the global mock — we test the real recyclePage
|
||||||
|
vi.unmock("../services/browser.js");
|
||||||
|
|
||||||
|
// Mock puppeteer so initBrowser doesn't launch real browsers
|
||||||
|
vi.mock("puppeteer", () => ({
|
||||||
|
default: {
|
||||||
|
launch: vi.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe("recyclePage", () => {
|
||||||
|
it("cleans up request interception listeners before navigating to about:blank", async () => {
|
||||||
|
// Dynamic import to get the real (unmocked) module
|
||||||
|
const { recyclePage } = await import("../services/browser.js");
|
||||||
|
|
||||||
|
const callOrder: string[] = [];
|
||||||
|
|
||||||
|
const mockPage = {
|
||||||
|
createCDPSession: vi.fn().mockResolvedValue({
|
||||||
|
send: vi.fn().mockResolvedValue(undefined),
|
||||||
|
detach: vi.fn().mockResolvedValue(undefined),
|
||||||
|
}),
|
||||||
|
removeAllListeners: vi.fn().mockImplementation((event: string) => {
|
||||||
|
callOrder.push(`removeAllListeners:${event}`);
|
||||||
|
return mockPage;
|
||||||
|
}),
|
||||||
|
setRequestInterception: vi.fn().mockImplementation((val: boolean) => {
|
||||||
|
callOrder.push(`setRequestInterception:${val}`);
|
||||||
|
return Promise.resolve();
|
||||||
|
}),
|
||||||
|
cookies: vi.fn().mockResolvedValue([]),
|
||||||
|
deleteCookie: vi.fn(),
|
||||||
|
goto: vi.fn().mockImplementation((url: string) => {
|
||||||
|
callOrder.push(`goto:${url}`);
|
||||||
|
return Promise.resolve();
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
|
||||||
|
await recyclePage(mockPage as any);
|
||||||
|
|
||||||
|
// Verify request interception cleanup happens
|
||||||
|
expect(mockPage.removeAllListeners).toHaveBeenCalledWith("request");
|
||||||
|
expect(mockPage.setRequestInterception).toHaveBeenCalledWith(false);
|
||||||
|
|
||||||
|
// Verify cleanup happens BEFORE navigation to about:blank
|
||||||
|
const removeIdx = callOrder.indexOf("removeAllListeners:request");
|
||||||
|
const interceptIdx = callOrder.indexOf("setRequestInterception:false");
|
||||||
|
const gotoIdx = callOrder.indexOf("goto:about:blank");
|
||||||
|
|
||||||
|
expect(removeIdx).toBeGreaterThanOrEqual(0);
|
||||||
|
expect(interceptIdx).toBeGreaterThanOrEqual(0);
|
||||||
|
expect(gotoIdx).toBeGreaterThanOrEqual(0);
|
||||||
|
expect(removeIdx).toBeLessThan(gotoIdx);
|
||||||
|
expect(interceptIdx).toBeLessThan(gotoIdx);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -40,11 +40,14 @@ export function getPoolStats() {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
async function recyclePage(page: Page): Promise<void> {
|
export async function recyclePage(page: Page): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const client = await page.createCDPSession();
|
const client = await page.createCDPSession();
|
||||||
await client.send("Network.clearBrowserCache").catch(() => {});
|
await client.send("Network.clearBrowserCache").catch(() => {});
|
||||||
await client.detach().catch(() => {});
|
await client.detach().catch(() => {});
|
||||||
|
// Clean up request interception (set by renderUrlPdf for SSRF protection)
|
||||||
|
page.removeAllListeners("request");
|
||||||
|
await page.setRequestInterception(false).catch(() => {});
|
||||||
const cookies = await page.cookies();
|
const cookies = await page.cookies();
|
||||||
if (cookies.length > 0) {
|
if (cookies.length > 0) {
|
||||||
await page.deleteCookie(...cookies);
|
await page.deleteCookie(...cookies);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue