From 057e466a65737fdadb6832348140bc601d7f1ad3 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 16 Jan 2020 16:58:02 -0800 Subject: [PATCH] fix(api): remove network events for data: urls (#512) --- src/chromium/crNetworkManager.ts | 4 +++- src/network.ts | 9 +-------- src/webkit/wkNetworkManager.ts | 6 +++++- test/interception.spec.js | 32 +++++++++++++++++--------------- test/navigation.spec.js | 11 +++++------ 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/chromium/crNetworkManager.ts b/src/chromium/crNetworkManager.ts index 74f36d5c5f..0806243158 100644 --- a/src/chromium/crNetworkManager.ts +++ b/src/chromium/crNetworkManager.ts @@ -154,7 +154,7 @@ export class CRNetworkManager { requestId: event.requestId }).catch(debugError); } - if (!event.networkId) + if (!event.networkId || event.request.url.startsWith('data:')) return; const requestId = event.networkId; @@ -169,6 +169,8 @@ export class CRNetworkManager { } _onRequest(event: Protocol.Network.requestWillBeSentPayload, interceptionId: string | null) { + if (event.request.url.startsWith('data:')) + return; let redirectChain: network.Request[] = []; if (event.redirectResponse) { const request = this._requestIdToRequest.get(event.requestId); diff --git a/src/network.ts b/src/network.ts index 6949be1128..16312e732d 100644 --- a/src/network.ts +++ b/src/network.ts @@ -112,6 +112,7 @@ export class Request { constructor(delegate: RequestDelegate | null, frame: frames.Frame | null, redirectChain: Request[], documentId: string | undefined, url: string, resourceType: string, method: string, postData: string | undefined, headers: Headers) { + assert(!url.startsWith('data:'), 'Data urls should not fire requests'); this._delegate = delegate; this._frame = frame; this._redirectChain = redirectChain; @@ -194,9 +195,6 @@ export class Request { } async abort(errorCode: string = 'failed') { - // Request interception is not supported for data: urls. - if (this.url().startsWith('data:')) - return; assert(this._delegate, 'Request Interception is not enabled!'); assert(!this._interceptionHandled, 'Request is already handled!'); this._interceptionHandled = true; @@ -204,8 +202,6 @@ export class Request { } async fulfill(response: { status: number; headers: Headers; contentType: string; body: (string | platform.BufferType); }) { // Mocking responses for dataURL requests is not currently supported. - if (this.url().startsWith('data:')) - return; assert(this._delegate, 'Request Interception is not enabled!'); assert(!this._interceptionHandled, 'Request is already handled!'); this._interceptionHandled = true; @@ -213,9 +209,6 @@ export class Request { } async continue(overrides: { headers?: { [key: string]: string } } = {}) { - // Request interception is not supported for data: urls. - if (this.url().startsWith('data:')) - return; assert(this._delegate, 'Request Interception is not enabled!'); assert(!this._interceptionHandled, 'Request is already handled!'); await this._delegate!.continue(overrides); diff --git a/src/webkit/wkNetworkManager.ts b/src/webkit/wkNetworkManager.ts index c1fb168d9d..94b9bb65ea 100644 --- a/src/webkit/wkNetworkManager.ts +++ b/src/webkit/wkNetworkManager.ts @@ -84,6 +84,8 @@ export class WKNetworkManager { } _onRequestWillBeSent(event: Protocol.Network.requestWillBeSentPayload) { + if (event.request.url.startsWith('data:')) + return; let redirectChain: network.Request[] = []; if (event.redirectResponse) { const request = this._requestIdToRequest.get(event.requestId); @@ -103,7 +105,9 @@ export class WKNetworkManager { } _onRequestIntercepted(event: Protocol.Network.requestInterceptedPayload) { - this._requestIdToRequest.get(event.requestId)!._interceptedCallback(); + const request = this._requestIdToRequest.get(event.requestId); + if (request) + request._interceptedCallback(); } _createResponse(request: InterceptableRequest, responsePayload: Protocol.Network.Response): network.Response { diff --git a/test/interception.spec.js b/test/interception.spec.js index defe57c260..3a3c6f2e91 100644 --- a/test/interception.spec.js +++ b/test/interception.spec.js @@ -288,38 +288,39 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p ])); expect(results).toEqual(['11', 'FAILED', '22']); }); - it.skip(FFOX)('should navigate to dataURL and fire dataURL requests', async({page, server}) => { + it('should navigate to dataURL and not fire dataURL requests', async({page, server}) => { await page.setRequestInterception(true); const requests = []; page.on('request', request => { - requests.push(request); + if (!utils.isFavicon(request)) + requests.push(request); request.continue(); }); const dataURL = 'data:text/html,
yo
'; const response = await page.goto(dataURL); - expect(response.status()).toBe(200); - expect(requests.length).toBe(1); - expect(requests[0].url()).toBe(dataURL); + expect(response).toBe(null); + expect(requests.length).toBe(0); }); - it.skip(FFOX)('should be able to fetch dataURL and fire dataURL requests', async({page, server}) => { + it('should be able to fetch dataURL and not fire dataURL requests', async({page, server}) => { await page.goto(server.EMPTY_PAGE); await page.setRequestInterception(true); const requests = []; page.on('request', request => { - requests.push(request); + if (!utils.isFavicon(request)) + requests.push(request); request.continue(); }); const dataURL = 'data:text/html,
yo
'; const text = await page.evaluate(url => fetch(url).then(r => r.text()), dataURL); expect(text).toBe('
yo
'); - expect(requests.length).toBe(1); - expect(requests[0].url()).toBe(dataURL); + expect(requests.length).toBe(0); }); it.skip(FFOX)('should navigate to URL with hash and and fire requests without hash', async({page, server}) => { await page.setRequestInterception(true); const requests = []; page.on('request', request => { - requests.push(request); + if (!utils.isFavicon(request)) + requests.push(request); request.continue(); }); const response = await page.goto(server.EMPTY_PAGE + '#hash'); @@ -343,19 +344,20 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p const response = await page.goto(server.PREFIX + '/malformed?rnd=%911'); expect(response.status()).toBe(200); }); - it.skip(FFOX)('should work with encoded server - 2', async({page, server}) => { + it('should work with encoded server - 2', async({page, server}) => { // The requestWillBeSent will report URL as-is, whereas interception will // report encoded URL for stylesheet. @see crbug.com/759388 await page.setRequestInterception(true); const requests = []; page.on('request', request => { request.continue(); - requests.push(request); + if (!utils.isFavicon(request)) + requests.push(request); }); const response = await page.goto(`data:text/html,`); - expect(response.status()).toBe(200); - expect(requests.length).toBe(2); - expect(requests[1].response().status()).toBe(404); + expect(response).toBe(null); + expect(requests.length).toBe(1); + expect(requests[0].response().status()).toBe(404); }); it('should not throw "Invalid Interception Id" if the request was cancelled', async({page, server}) => { await page.setContent(''); diff --git a/test/navigation.spec.js b/test/navigation.spec.js index 302b314b7e..69c07126e4 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -231,9 +231,9 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI const response = await page.goto(server.EMPTY_PAGE); expect(response.ok()).toBe(true); }); - it.skip(FFOX)('should work when navigating to data url', async({page, server}) => { + it('should work when navigating to data url', async({page, server}) => { const response = await page.goto('data:text/html,hello'); - expect(response.ok()).toBe(true); + expect(response).toBe(null); }); it('should work when navigating to 404', async({page, server}) => { const response = await page.goto(server.PREFIX + '/not-found'); @@ -278,14 +278,13 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI process.removeListener('warning', warningHandler); expect(warning).toBe(null); }); - it.skip(FFOX)('should navigate to dataURL and fire dataURL requests', async({page, server}) => { + it('should navigate to dataURL and not fire dataURL requests', async({page, server}) => { const requests = []; page.on('request', request => !utils.isFavicon(request) && requests.push(request)); const dataURL = 'data:text/html,
yo
'; const response = await page.goto(dataURL); - expect(response.status()).toBe(200); - expect(requests.length).toBe(1); - expect(requests[0].url()).toBe(dataURL); + expect(response).toBe(null); + expect(requests.length).toBe(0); }); it('should navigate to URL with hash and fire requests without hash', async({page, server}) => { const requests = [];