From 68fd7712932f94730b6186107a0509c233938084 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Thu, 30 May 2024 11:39:08 +0200 Subject: [PATCH] fix(webdriver): HTTPRequest redirect chain from first request (#12506) --- .../puppeteer-core/src/bidi/HTTPRequest.ts | 20 +++---- test/TestExpectations.json | 56 ------------------- .../requestinterception-experimental.spec.ts | 22 ++++++-- test/src/requestinterception.spec.ts | 20 +++++-- 4 files changed, 38 insertions(+), 80 deletions(-) diff --git a/packages/puppeteer-core/src/bidi/HTTPRequest.ts b/packages/puppeteer-core/src/bidi/HTTPRequest.ts index 923a82726e3..36f6496cd91 100644 --- a/packages/puppeteer-core/src/bidi/HTTPRequest.ts +++ b/packages/puppeteer-core/src/bidi/HTTPRequest.ts @@ -38,7 +38,8 @@ export class BidiHTTPRequest extends HTTPRequest { request.#initialize(); return request; } - #redirectBy: BidiHTTPRequest | undefined; + + #redirectChain: BidiHTTPRequest[]; #response: BidiHTTPResponse | null = null; override readonly id: string; readonly #frame: BidiFrame; @@ -47,7 +48,7 @@ export class BidiHTTPRequest extends HTTPRequest { private constructor( request: Request, frame: BidiFrame, - redirectBy?: BidiHTTPRequest + redirect?: BidiHTTPRequest ) { super(); requests.set(request, this); @@ -56,7 +57,7 @@ export class BidiHTTPRequest extends HTTPRequest { this.#request = request; this.#frame = frame; - this.#redirectBy = redirectBy; + this.#redirectChain = redirect ? redirect.#redirectChain : []; this.id = request.id; } @@ -67,6 +68,8 @@ export class BidiHTTPRequest extends HTTPRequest { #initialize() { this.#request.on('redirect', request => { const httpRequest = BidiHTTPRequest.from(request, this.#frame, this); + this.#redirectChain.push(this); + request.once('success', () => { this.#frame .page() @@ -170,16 +173,7 @@ export class BidiHTTPRequest extends HTTPRequest { } override redirectChain(): BidiHTTPRequest[] { - if (this.#redirectBy === undefined) { - return []; - } - const redirects = [this.#redirectBy]; - for (const redirect of redirects) { - if (redirect.#redirectBy !== undefined) { - redirects.push(redirect.#redirectBy); - } - } - return redirects; + return this.#redirectChain.slice(); } override frame(): BidiFrame { diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 2790b5bcb49..7b2664a0cb8 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -765,20 +765,6 @@ "expectations": ["FAIL"], "comment": "BiDi spec and WPT require expect the Hash" }, - { - "testIdPattern": "[requestinterception.spec] request interception Page.setRequestInterception should work with redirects", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"], - "comment": "`HTTPRequest.resourceType()` has no eqivalent in BiDi spec" - }, - { - "testIdPattern": "[requestinterception.spec] request interception Page.setRequestInterception should work with redirects for subresources", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"], - "comment": "`HTTPRequest.resourceType()` has no eqivalent in BiDi spec" - }, { "testIdPattern": "[requestinterception.spec] request interception Request.resourceType should work for document type", "platforms": ["darwin", "linux", "win32"], @@ -3343,20 +3329,6 @@ "expectations": ["SKIP"], "comment": "TODO: Needs support for file URIs in Firefox https://bugzilla.mozilla.org/show_bug.cgi?id=1826210" }, - { - "testIdPattern": "[requestinterception-experimental.spec] cooperative request interception Page.setRequestInterception should work with redirects", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "TODO: Needs support for BidiHTTPRequest.resourceType" - }, - { - "testIdPattern": "[requestinterception-experimental.spec] cooperative request interception Page.setRequestInterception should work with redirects for subresources", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "TODO: Needs support for BidiHTTPRequest.resourceType" - }, { "testIdPattern": "[requestinterception-experimental.spec] cooperative request interception Page.setRequestInterception should work with requests without networkId", "platforms": ["darwin", "linux", "win32"], @@ -3475,20 +3447,6 @@ "expectations": ["SKIP"], "comment": "TODO: Needs support for file URIs in Firefox https://bugzilla.mozilla.org/show_bug.cgi?id=1826210" }, - { - "testIdPattern": "[requestinterception.spec] request interception Page.setRequestInterception should work with redirects", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "TODO: Needs support for BidiHTTPRequest.resourceType" - }, - { - "testIdPattern": "[requestinterception.spec] request interception Page.setRequestInterception should work with redirects for subresources", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "TODO: Needs support for BidiHTTPRequest.resourceType" - }, { "testIdPattern": "[requestinterception.spec] request interception Page.setRequestInterception should work with requests without networkId", "platforms": ["darwin", "linux", "win32"], @@ -3893,20 +3851,6 @@ "expectations": ["FAIL"], "comment": "`request.postData()` has no eqivalent in BiDi spec" }, - { - "testIdPattern": "[requestinterception-experimental.spec] cooperative request interception Page.setRequestInterception should work with redirects", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "headless", "webDriverBiDi"], - "expectations": ["FAIL"], - "comment": "`HTTPRequest.resourceType()` has no eqivalent in BiDi spec" - }, - { - "testIdPattern": "[requestinterception-experimental.spec] cooperative request interception Page.setRequestInterception should work with redirects for subresources", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "headless", "webDriverBiDi"], - "expectations": ["FAIL"], - "comment": "`HTTPRequest.resourceType()` has no eqivalent in BiDi spec" - }, { "testIdPattern": "[requestinterception.spec] request interception Page.setRequestInterception should be abortable", "platforms": ["darwin", "linux", "win32"], diff --git a/test/src/requestinterception-experimental.spec.ts b/test/src/requestinterception-experimental.spec.ts index 48feba5f14f..daaae929168 100644 --- a/test/src/requestinterception-experimental.spec.ts +++ b/test/src/requestinterception-experimental.spec.ts @@ -196,10 +196,10 @@ describe('cooperative request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { + void request.continue({}, 0); if (!isFavicon(request)) { requests.push(request); } - void request.continue({}, 0); }); await page.goto(server.PREFIX + '/one-style.html'); expect(requests[1]!.url()).toContain('/one-style.css'); @@ -386,7 +386,9 @@ describe('cooperative request interception', function () { const requests: HTTPRequest[] = []; page.on('request', request => { void request.continue({}, 0); - requests.push(request); + if (!isFavicon(request)) { + requests.push(request); + } }); server.setRedirect( '/non-existing-page.html', @@ -526,7 +528,9 @@ describe('cooperative request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { - requests.push(request); + if (!isFavicon(request)) { + requests.push(request); + } void request.continue({}, 0); }); const dataURL = 'data:text/html,
yo
'; @@ -542,8 +546,10 @@ describe('cooperative request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { - !isFavicon(request) && requests.push(request); void request.continue({}, 0); + if (!isFavicon(request)) { + requests.push(request); + } }); const dataURL = 'data:text/html,
yo
'; const text = await page.evaluate((url: string) => { @@ -561,7 +567,9 @@ describe('cooperative request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { - requests.push(request); + if (!isFavicon(request)) { + requests.push(request); + } void request.continue({}, 0); }); const response = await page.goto(server.EMPTY_PAGE + '#hash'); @@ -606,7 +614,9 @@ describe('cooperative request interception', function () { const requests: HTTPRequest[] = []; page.on('request', request => { void request.continue({}, 0); - requests.push(request); + if (!isFavicon(request)) { + requests.push(request); + } }); const response = await page.goto( `data:text/html,` diff --git a/test/src/requestinterception.spec.ts b/test/src/requestinterception.spec.ts index 79314c5ffeb..cddc83b9582 100644 --- a/test/src/requestinterception.spec.ts +++ b/test/src/requestinterception.spec.ts @@ -303,7 +303,9 @@ describe('request interception', function () { const requests: HTTPRequest[] = []; page.on('request', request => { void request.continue(); - requests.push(request); + if (!isFavicon(request)) { + requests.push(request); + } }); server.setRedirect( '/non-existing-page.html', @@ -443,8 +445,10 @@ describe('request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { - requests.push(request); void request.continue(); + if (!isFavicon(request)) { + requests.push(request); + } }); const dataURL = 'data:text/html,
yo
'; const response = (await page.goto(dataURL))!; @@ -459,8 +463,10 @@ describe('request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { - !isFavicon(request) && requests.push(request); void request.continue(); + if (!isFavicon(request)) { + requests.push(request); + } }); const dataURL = 'data:text/html,
yo
'; const text = await page.evaluate((url: string) => { @@ -478,8 +484,10 @@ describe('request interception', function () { await page.setRequestInterception(true); const requests: HTTPRequest[] = []; page.on('request', request => { - requests.push(request); void request.continue(); + if (!isFavicon(request)) { + requests.push(request); + } }); const response = (await page.goto(server.EMPTY_PAGE + '#hash'))!; expect(response.status()).toBe(200); @@ -525,7 +533,9 @@ describe('request interception', function () { const requests: HTTPRequest[] = []; page.on('request', request => { void request.continue(); - requests.push(request); + if (!isFavicon(request)) { + requests.push(request); + } }); const response = (await page.goto( `data:text/html,`