From a877c24f05a7264adee43bebf268c1a2706af998 Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Tue, 17 Nov 2020 16:56:04 -0800
Subject: [PATCH] fix(route): throw on attempt to fulfill with redirect in
 WebKit (#4449)

---
 src/server/webkit/wkInterceptableRequest.ts |  3 ++
 test/page-route.spec.ts                     | 44 +++++++++++++++++++--
 test/request-continue.spec.ts               | 11 ++++--
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/src/server/webkit/wkInterceptableRequest.ts b/src/server/webkit/wkInterceptableRequest.ts
index 87fc749754..c66aef8218 100644
--- a/src/server/webkit/wkInterceptableRequest.ts
+++ b/src/server/webkit/wkInterceptableRequest.ts
@@ -74,6 +74,9 @@ export class WKInterceptableRequest implements network.RouteDelegate {
   }
 
   async fulfill(response: types.NormalizedFulfillResponse) {
+    if (300 <= response.status && response.status < 400)
+      throw new Error('Cannot fulfill with redirect status: ' + response.status);
+
     await this._interceptedPromise;
 
     // In certain cases, protocol will return error if the request was already canceled
diff --git a/test/page-route.spec.ts b/test/page-route.spec.ts
index e55bdc47f0..4af508ec6d 100644
--- a/test/page-route.spec.ts
+++ b/test/page-route.spec.ts
@@ -394,15 +394,17 @@ it('should intercept main resource during cross-process navigation', async ({pag
   expect(intercepted).toBe(true);
 });
 
-it('should create a redirect', async ({page, server}) => {
-  await page.goto(server.PREFIX + '/empty.html');
+it('should fulfill with redirect status', (test, { browserName, headful}) => {
+  test.fixme(browserName === 'webkit', 'in WebKit the redirects are handled by the network stack and we intercept before');
+}, async ({page, server}) => {
+  await page.goto(server.PREFIX + '/title.html');
   await page.route('**/*', async (route, request) => {
     if (request.url() !== server.PREFIX + '/redirect_this')
       return route.continue();
     await route.fulfill({
       status: 301,
       headers: {
-        'location': '/empty.html',
+        'location': '/title.html',
       }
     });
   });
@@ -411,7 +413,41 @@ it('should create a redirect', async ({page, server}) => {
     const data = await fetch(url);
     return data.text();
   }, server.PREFIX + '/redirect_this');
-  expect(text).toBe('');
+  expect(text).toBe('<title>Woof-Woof</title>\n');
+});
+
+it('should not fulfill with redirect status', (test, { browserName, headful}) => {
+  test.skip(browserName !== 'webkit', 'we should support fulfill with redirect in webkit and delete this test');
+}, async ({page, server}) => {
+  await page.goto(server.PREFIX + '/empty.html');
+
+  let status;
+  let fulfill;
+  let reject;
+  await page.route('**/*', async (route, request) => {
+    if (request.url() !== server.PREFIX + '/redirect_this')
+      return route.continue();
+    try {
+      await route.fulfill({
+        status,
+        headers: {
+          'location': '/empty.html',
+        }
+      });
+      reject('fullfill didn\'t throw');
+    } catch (e) {
+      fulfill(e);
+    }
+  });
+
+  for (status = 300; status < 310; status++) {
+    const exception = await Promise.race([
+      page.evaluate(url => location.href = url, server.PREFIX + '/redirect_this'),
+      new Promise((f, r) => {fulfill = f; reject = r;})
+    ]) as any;
+    expect(exception).toBeTruthy();
+    expect(exception.message.includes('Cannot fulfill with redirect status')).toBe(true);
+  }
 });
 
 it('should support cors with GET', async ({page, server}) => {
diff --git a/test/request-continue.spec.ts b/test/request-continue.spec.ts
index 14baf010fe..343539424b 100644
--- a/test/request-continue.spec.ts
+++ b/test/request-continue.spec.ts
@@ -50,11 +50,16 @@ it('should amend method', async ({page, server}) => {
 });
 
 it('should override request url', async ({page, server}) => {
-  const request = server.waitForRequest('/empty.html');
+  const request = server.waitForRequest('/global-var.html');
   await page.route('**/foo', route => {
-    route.continue({ url: server.EMPTY_PAGE });
+    route.continue({ url: server.PREFIX + '/global-var.html' });
   });
-  await page.goto(server.PREFIX + '/foo');
+  const [response] = await Promise.all([
+    page.waitForEvent('response'),
+    page.goto(server.PREFIX + '/foo'),
+  ]);
+  expect(response.url()).toBe(server.PREFIX + '/foo');
+  expect(await page.evaluate(() => window['globalVar'])).toBe(123);
   expect((await request).method).toBe('GET');
 });