Make interception work with redirects (#218)

This patch:
- changes interception API so that it better aligns with what we'd like to see
  in #121
- fixes the issue with redirect interception

Fixes #217.

docs/api.md

@@ -113,7 +113,7 @@
     + [response.url](#responseurl)
   * [class: InterceptedRequest](#class-interceptedrequest)
     + [interceptedRequest.abort()](#interceptedrequestabort)
-    + [interceptedRequest.continue()](#interceptedrequestcontinue)
+    + [interceptedRequest.continue([overrides])](#interceptedrequestcontinueoverrides)
     + [interceptedRequest.headers](#interceptedrequestheaders)
     + [interceptedRequest.method](#interceptedrequestmethod)
     + [interceptedRequest.postData](#interceptedrequestpostdata)
@@ -1112,40 +1112,38 @@ Contains the URL of the response.
 
 ### class: InterceptedRequest
 
-[InterceptedRequest] represents an intercepted request, which can be mutated and either continued or aborted. [InterceptedRequest] which is not continued or aborted will be in a 'hanging' state.
+[InterceptedRequest] represents an intercepted request, which can be either continued or aborted. [InterceptedRequest] which is not continued or aborted will be in a 'hanging' state.
 
 #### interceptedRequest.abort()
 
 Aborts request.
 
-#### interceptedRequest.continue()
+#### interceptedRequest.continue([overrides])
+- `overrides` <[Object]> Optional request overwrites, which could be one of the following:
+  - `url` <[string]> If set, the request url will be changed
+  - `method` <[string]> If set changes the request method (e.g. `GET` or `POST`)
+  - `postData` <[string]> If set changes the post data of request
+  - `headers` <[Map]> If set changes the request HTTP headers
 
-Continues request.
+Continues request with optional request overrides.
 
 #### interceptedRequest.headers
 - <[Map]> A map of HTTP headers associated with the request.
 
-Headers could be mutated. Must not be changed in response to an authChallenge.
-
 #### interceptedRequest.method
 - <[string]>
 
 Contains the request's method (GET, POST, etc.)
 
-If set this allows the request method to be overridden. Must not be changed in response to an authChallenge.
 
 #### interceptedRequest.postData
 - <[string]>
 
 Contains `POST` data for `POST` requests.
 
-`request.postData` is mutable and could be written to. Must not be changed in response to an authChallenge.
-
 #### interceptedRequest.url
 - <[string]>
 
-If changed, the request url will be modified in a way that's not observable by page. Must not be changed in response to an authChallenge.
-
 
 [Array]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array "Array"
 [boolean]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type "Boolean"

lib/NetworkManager.js

@@ -247,17 +247,23 @@ class InterceptedRequest {
     });
   }
 
-  continue() {
+  /**
+   * @param {!Object} overrides
+   */
+  continue(overrides = {}) {
     console.assert(!this._handled, 'This request is already handled!');
     this._handled = true;
-    let headers = {};
-    for (let entry of this.headers.entries())
-      headers[entry[0]] = entry[1];
+    let headers = undefined;
+    if (overrides.headers) {
+      headers = {};
+      for (let entry of overrides.headers.entries())
+        headers[entry[0]] = entry[1];
+    }
     this._client.send('Network.continueInterceptedRequest', {
       interceptionId: this._interceptionId,
-      url: this.url,
-      method: this.method,
-      postData: this.postData,
+      url: overrides.url,
+      method: overrides.method,
+      postData: overrides.postData,
       headers: headers
     });
   }

phantom_shim/WebPage.js

@@ -236,12 +236,16 @@ class WebPage {
        */
     function resourceInterceptor(request) {
       let requestData = new RequestData(request);
-      let phantomRequest = new PhantomRequest(request);
+      let phantomRequest = new PhantomRequest();
       callback(requestData, phantomRequest);
-      if (phantomRequest._aborted)
+      if (phantomRequest._aborted) {
         request.abort();
-      else
-        request.continue();
+      } else {
+        request.continue({
+          url: phantomRequest._url,
+          headers: phantomRequest._headers,
+        });
+      }
     }
   }
 
@@ -608,11 +612,9 @@ class WebPageSettings {
 }
 
 class PhantomRequest {
-  /**
-   * @param {!InterceptedRequest} request
-   */
-  constructor(request) {
-    this._request = request;
+  constructor() {
+    this._url = undefined;
+    this._headers = undefined;
   }
 
   /**
@@ -620,7 +622,9 @@ class PhantomRequest {
    * @param {string} value
    */
   setHeader(key, value) {
-    this._request.headers.set(key, value);
+    if (!this._headers)
+      this._headers = new Map();
+    this._headers.set(key, value);
   }
 
   abort() {
@@ -631,7 +635,7 @@ class PhantomRequest {
    * @param {string} url
    */
   changeUrl(newUrl) {
-    this._request.url = newUrl;
+    this._url = newUrl;
   }
 }
 

test/test.js

@@ -724,7 +724,7 @@ describe('Page', function() {
 
   describe('Page.setRequestInterceptor', function() {
     it('should intercept', SX(async function() {
-      page.setRequestInterceptor(request => {
+      await page.setRequestInterceptor(request => {
         expect(request.url).toContain('empty.html');
         expect(request.headers.has('User-Agent')).toBeTruthy();
         expect(request.method).toBe('GET');
@@ -738,7 +738,7 @@ describe('Page', function() {
       await page.setExtraHTTPHeaders(new Map(Object.entries({
         foo: 'bar'
       })));
-      page.setRequestInterceptor(request => {
+      await page.setRequestInterceptor(request => {
         expect(request.headers.get('foo')).toBe('bar');
         request.continue();
       });
@@ -746,7 +746,7 @@ describe('Page', function() {
       expect(response.ok).toBe(true);
     }));
     it('should be abortable', SX(async function() {
-      page.setRequestInterceptor(request => {
+      await page.setRequestInterceptor(request => {
         if (request.url.endsWith('.css'))
           request.abort();
         else
@@ -759,11 +759,12 @@ describe('Page', function() {
       expect(failedRequests).toBe(1);
     }));
     it('should amend HTTP headers', SX(async function() {
-      await page.navigate(EMPTY_PAGE);
-      page.setRequestInterceptor(request => {
-        request.headers.set('foo', 'bar');
-        request.continue();
+      await page.setRequestInterceptor(request => {
+        let headers = new Map(request.headers);
+        headers.set('foo', 'bar');
+        request.continue({headers});
       });
+      await page.navigate(EMPTY_PAGE);
       const [request] = await Promise.all([
         server.waitForRequest('/sleep.zzz'),
         page.evaluate(() => fetch('/sleep.zzz'))
@@ -771,7 +772,7 @@ describe('Page', function() {
       expect(request.headers['foo']).toBe('bar');
     }));
     it('should fail navigation when aborting main resource', SX(async function() {
-      page.setRequestInterceptor(request => request.abort());
+      await page.setRequestInterceptor(request => request.abort());
       let error = null;
       try {
         await page.navigate(EMPTY_PAGE);
@@ -781,6 +782,12 @@ describe('Page', function() {
       expect(error).toBeTruthy();
       expect(error.message).toContain('Failed to navigate');
     }));
+    it('should work with redirects', SX(async function() {
+      server.setRedirect('/non-existing-page.html', '/empty.html');
+      await page.setRequestInterceptor(request => request.continue());
+      let response = await page.navigate(PREFIX + '/non-existing-page.html');
+      expect(response.status).toBe(200);
+    }));
   });
 
   describe('Page.Events.Dialog', function() {