fix(network): disallow intercepting redirects (#2617)

WebKit and Firefox are only able to continue redirects.
Firefox is faking it on the backend, so you can't even stall it.

Instead, we just do not fire routes for redirects on all browsers,
to avoid surprises.

browsers.json

@@ -6,7 +6,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1112"
+      "revision": "1113"
     },
     {
       "name": "webkit",

src/chromium/crNetworkManager.ts

@@ -191,6 +191,13 @@ export class CRNetworkManager {
         this._client._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId });
       return;
     }
+    let allowInterception = this._userRequestInterceptionEnabled;
+    if (redirectedFrom) {
+      allowInterception = false;
+      // We do not support intercepting redirects.
+      if (requestPausedEvent)
+        this._client._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId });
+    }
     const isNavigationRequest = requestWillBeSentEvent.requestId === requestWillBeSentEvent.loaderId && requestWillBeSentEvent.type === 'Document';
     const documentId = isNavigationRequest ? requestWillBeSentEvent.loaderId : undefined;
     if (isNavigationRequest)
@@ -199,7 +206,7 @@ export class CRNetworkManager {
       client: this._client,
       frame,
       documentId,
-      allowInterception: this._userRequestInterceptionEnabled,
+      allowInterception,
       requestWillBeSentEvent,
       requestPausedEvent,
       redirectedFrom

src/network.ts

@@ -115,6 +115,7 @@ export class Request {
   constructor(routeDelegate: RouteDelegate | null, frame: frames.Frame, redirectedFrom: Request | null, documentId: string | undefined,
     url: string, resourceType: string, method: string, postData: string | null, headers: Headers) {
     assert(!url.startsWith('data:'), 'Data urls should not fire requests');
+    assert(!(routeDelegate && redirectedFrom), 'Should not be able to intercept redirects');
     this._routeDelegate = routeDelegate;
     this._frame = frame;
     this._redirectedFrom = redirectedFrom;

src/webkit/wkInterceptableRequest.ts

@@ -44,10 +44,12 @@ export class WKInterceptableRequest implements network.RouteDelegate {
   readonly _requestId: string;
   _interceptedCallback: () => void = () => {};
   private _interceptedPromise: Promise<unknown>;
+  readonly _allowInterception: boolean;
 
   constructor(session: WKSession, allowInterception: boolean, frame: frames.Frame, event: Protocol.Network.requestWillBeSentPayload, redirectedFrom: network.Request | null, documentId: string | undefined) {
     this._session = session;
     this._requestId = event.requestId;
+    this._allowInterception = allowInterception;
     const resourceType = event.type ? event.type.toLowerCase() : (redirectedFrom ? redirectedFrom.resourceType() : 'other');
     this.request = new network.Request(allowInterception ? this : null, frame, redirectedFrom, documentId, event.request.url,
         resourceType, event.request.method, event.request.postData || null, headersObject(event.request.headers));

src/webkit/wkPage.ts

@@ -841,7 +841,9 @@ export class WKPage implements PageDelegate {
     const documentId = isNavigationRequest ? event.loaderId : undefined;
     if (isNavigationRequest)
       this._page._frameManager.frameUpdatedDocumentIdForNavigation(event.frameId, documentId!);
-    const request = new WKInterceptableRequest(session, this._page._needsRequestInterception(), frame, event, redirectedFrom, documentId);
+    // We do not support intercepting redirects.
+    const allowInterception = this._page._needsRequestInterception() && !redirectedFrom;
+    const request = new WKInterceptableRequest(session, allowInterception, frame, event, redirectedFrom, documentId);
     this._requestIdToRequest.set(event.requestId, request);
     this._page._frameManager.requestStarted(request.request);
   }
@@ -856,9 +858,16 @@ export class WKPage implements PageDelegate {
 
   _onRequestIntercepted(event: Protocol.Network.requestInterceptedPayload) {
     const request = this._requestIdToRequest.get(event.requestId);
-    if (request)
+    if (!request)
+      return;
+    if (!request._allowInterception) {
+      // Intercepted, although we do not intend to allow interception.
+      // Just continue.
+      this._session.sendMayFail('Network.interceptWithRequest', { requestId: request._requestId });
+    } else {
       request._interceptedCallback();
     }
+  }
 
   _onResponseReceived(event: Protocol.Network.responseReceivedPayload) {
     const request = this._requestIdToRequest.get(event.requestId);

test/interception.spec.js

@@ -216,21 +216,26 @@ describe('Page.route', function() {
     else
       expect(error.message).toContain('net::ERR_FAILED');
   });
-  it('should work with redirects', async({page, server}) => {
-    const requests = [];
+  it('should not work with redirects', async({page, server}) => {
+    const intercepted = [];
     await page.route('**/*', route => {
       route.continue();
-      requests.push(route.request());
+      intercepted.push(route.request());
     });
     server.setRedirect('/non-existing-page.html', '/non-existing-page-2.html');
     server.setRedirect('/non-existing-page-2.html', '/non-existing-page-3.html');
     server.setRedirect('/non-existing-page-3.html', '/non-existing-page-4.html');
     server.setRedirect('/non-existing-page-4.html', '/empty.html');
+
     const response = await page.goto(server.PREFIX + '/non-existing-page.html');
     expect(response.status()).toBe(200);
     expect(response.url()).toContain('empty.html');
-    expect(requests.length).toBe(5);
-    expect(requests[2].resourceType()).toBe('document');
+
+    expect(intercepted.length).toBe(1);
+    expect(intercepted[0].resourceType()).toBe('document');
+    expect(intercepted[0].isNavigationRequest()).toBe(true);
+    expect(intercepted[0].url()).toContain('/non-existing-page.html');
+
     const chain = [];
     for (let r = response.request(); r; r = r.redirectedFrom()) {
       chain.push(r);
@@ -246,10 +251,10 @@ describe('Page.route', function() {
       expect(chain[i].redirectedTo()).toBe(i ? chain[i - 1] : null);
   });
   it('should work with redirects for subresources', async({page, server}) => {
-    const requests = [];
+    const intercepted = [];
     await page.route('**/*', route => {
       route.continue();
-      requests.push(route.request());
+      intercepted.push(route.request());
     });
     server.setRedirect('/one-style.css', '/two-style.css');
     server.setRedirect('/two-style.css', '/three-style.css');
@@ -259,14 +264,16 @@ describe('Page.route', function() {
     const response = await page.goto(server.PREFIX + '/one-style.html');
     expect(response.status()).toBe(200);
     expect(response.url()).toContain('one-style.html');
-    expect(requests.length).toBe(5);
-    expect(requests[0].resourceType()).toBe('document');
 
-    let r = requests.find(r => r.url().includes('/four-style.css'));
-    for (const url of ['/four-style.css', '/three-style.css', '/two-style.css', '/one-style.css']) {
+    expect(intercepted.length).toBe(2);
+    expect(intercepted[0].resourceType()).toBe('document');
+    expect(intercepted[0].url()).toContain('one-style.html');
+
+    let r = intercepted[1];
+    for (const url of ['/one-style.css', '/two-style.css', '/three-style.css', '/four-style.css']) {
       expect(r.resourceType()).toBe('stylesheet');
       expect(r.url()).toContain(url);
-      r = r.redirectedFrom();
+      r = r.redirectedTo();
     }
     expect(r).toBe(null);
   });
@@ -602,7 +609,6 @@ describe('Interception vs isNavigationRequest', () => {
     server.setRedirect('/rrredirect', '/frames/one-frame.html');
     await page.goto(server.PREFIX + '/rrredirect');
     expect(requests.get('rrredirect').isNavigationRequest()).toBe(true);
-    expect(requests.get('one-frame.html').isNavigationRequest()).toBe(true);
     expect(requests.get('frame.html').isNavigationRequest()).toBe(true);
     expect(requests.get('script.js').isNavigationRequest()).toBe(false);
     expect(requests.get('style.css').isNavigationRequest()).toBe(false);