From 96309a207c4250a9db64112a5cfab09b7f65fc60 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Tue, 15 Aug 2017 13:55:48 -0700 Subject: [PATCH] Fix request interception corner cases (#261) This patch: - teaches request interception to ignore data URLs. Currently protocol doesn't send interceptions for data URLs. - teaches request interception to properly process URLs with hashes. Currently `Network.requestIntercepted` sends url with a hash, whereas `Network.requestWillBeSent` doesn't report hashes in its urls. @see crbug.com/755456 - skips one more header that I spotted during debugging interception on the realworld websites. Fixes #258, #259. --- lib/NetworkManager.js | 24 ++++++++++++++++++++-- test/test.js | 46 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/lib/NetworkManager.js b/lib/NetworkManager.js index 65cf4707..2b308f5f 100644 --- a/lib/NetworkManager.js +++ b/lib/NetworkManager.js @@ -16,6 +16,7 @@ const EventEmitter = require('events'); const helper = require('./helper'); const Multimap = require('./Multimap'); +const {URL} = require('url'); class NetworkManager extends EventEmitter { /** @@ -84,6 +85,9 @@ class NetworkManager extends EventEmitter { * @param {!Object} event */ _onRequestIntercepted(event) { + // Strip out url hash to be consistent with requestWillBeSent. @see crbug.com/755456 + event.request.url = removeURLHash(event.request.url); + if (event.redirectStatusCode) { let request = this._interceptionIdToRequest.get(event.interceptionId); console.assert(request, 'INTERNAL ERROR: failed to find request for interception redirect.'); @@ -127,7 +131,7 @@ class NetworkManager extends EventEmitter { * @param {!Object} event */ _onRequestWillBeSent(event) { - if (this._requestInterceptionEnabled) { + if (this._requestInterceptionEnabled && !event.request.url.startsWith('data:')) { // All redirects are handled in requestIntercepted. if (event.redirectResponse) return; @@ -236,6 +240,9 @@ class Request { * @param {!Object=} overrides */ continue(overrides = {}) { + // DataURL's are not interceptable. In this case, do nothing. + if (this.url.startsWith('data:')) + return; console.assert(this._interceptionId, 'Request Interception is not enabled!'); console.assert(!this._interceptionHandled, 'Request is already handled!'); this._interceptionHandled = true; @@ -255,6 +262,9 @@ class Request { } abort() { + // DataURL's are not interceptable. In this case, do nothing. + if (this.url.startsWith('data:')) + return; console.assert(this._interceptionId, 'Request Interception is not enabled!'); console.assert(!this._interceptionHandled, 'Request is already handled!'); this._interceptionHandled = true; @@ -338,13 +348,23 @@ function generateRequestHash(request) { let headers = Object.keys(request.headers); headers.sort(); for (let header of headers) { - if (header === 'Accept' || header === 'Referer') + if (header === 'Accept' || header === 'Referer' || header === 'X-DevTools-Emulate-Network-Conditions-Client-Id') continue; hash.headers[header] = request.headers[header]; } return JSON.stringify(hash); } +/** + * @param {string} url + * @return {string} + */ +function removeURLHash(url) { + let urlObject = new URL(url); + urlObject.hash = ''; + return urlObject.toString(); +} + NetworkManager.Events = { Request: 'request', Response: 'response', diff --git a/test/test.js b/test/test.js index 54ca66d5..2af7bdd9 100644 --- a/test/test.js +++ b/test/test.js @@ -518,7 +518,7 @@ describe('Page', function() { })); }); - describe('Page.navigate', function() { + describe('Page.goto', function() { it('should navigate to about:blank', SX(async function() { let response = await page.goto('about:blank'); expect(response).toBe(null); @@ -690,6 +690,24 @@ describe('Page', function() { process.removeListener('warning', warningHandler); expect(warning).toBe(null); })); + it('should navigate to dataURL and fire dataURL requests', SX(async function() { + let requests = []; + page.on('request', request => requests.push(request)); + let dataURL = 'data:text/html,
yo
'; + let response = await page.goto(dataURL); + expect(response.status).toBe(200); + expect(requests.length).toBe(1); + expect(requests[0].url).toBe(dataURL); + })); + it('should navigate to URL with hash and fire requests without hash', SX(async function() { + let requests = []; + page.on('request', request => requests.push(request)); + let response = await page.goto(EMPTY_PAGE + '#hash'); + expect(response.status).toBe(200); + expect(response.url).toBe(EMPTY_PAGE); + expect(requests.length).toBe(1); + expect(requests[0].url).toBe(EMPTY_PAGE); + })); }); describe('Page.waitForNavigation', function() { @@ -871,6 +889,32 @@ describe('Page', function() { ])); expect(results).toEqual(['11', 'FAILED', '22']); })); + it('should navigate to dataURL and fire dataURL requests', SX(async function() { + await page.setRequestInterceptionEnabled(true); + let requests = []; + page.on('request', request => { + requests.push(request); + request.continue(); + }); + let dataURL = 'data:text/html,
yo
'; + let response = await page.goto(dataURL); + expect(response.status).toBe(200); + expect(requests.length).toBe(1); + expect(requests[0].url).toBe(dataURL); + })); + it('should navigate to URL with hash and and fire requests without hash', SX(async function() { + await page.setRequestInterceptionEnabled(true); + let requests = []; + page.on('request', request => { + requests.push(request); + request.continue(); + }); + let response = await page.goto(EMPTY_PAGE + '#hash'); + expect(response.status).toBe(200); + expect(response.url).toBe(EMPTY_PAGE); + expect(requests.length).toBe(1); + expect(requests[0].url).toBe(EMPTY_PAGE); + })); }); describe('Page.Events.Dialog', function() {