From c339e1615b00d4fb0dfae4713f418ae30fd7f0df Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Fri, 6 Jan 2023 19:22:17 +0100
Subject: [PATCH] fix(fetch): handle invalid redirect URL (#19890)

Fixes https://github.com/microsoft/playwright/issues/19879.

This part is then similar to how node-fetch is doing it:
https://cs.github.com/node-fetch/node-fetch/blob/55a4870ae5f805d8ff9a890ea2c652c9977e048e/src/index.js?q=location#L152-L159

node-fetch also throws as of today with this URL. Before in Python it
was stalling, because the error was written to stdout and on Windows the
stdout wasn't working. On Node.js it ended up in an unhandled exception.
---
 packages/playwright-core/src/server/fetch.ts |  9 ++++++++-
 tests/library/browsercontext-fetch.spec.ts   | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts
index a58454005b..1bda7e2e44 100644
--- a/packages/playwright-core/src/server/fetch.ts
+++ b/packages/playwright-core/src/server/fetch.ts
@@ -319,7 +319,14 @@ export abstract class APIRequestContext extends SdkObject {
 
           // HTTP-redirect fetch step 4: If locationURL is null, then return response.
           if (response.headers.location) {
-            const locationURL = new URL(response.headers.location, url);
+            let locationURL;
+            try {
+              locationURL = new URL(response.headers.location, url);
+            } catch (error) {
+              reject(new Error(`uri requested responds with an invalid redirect URL: ${response.headers.location}`));
+              request.destroy();
+              return;
+            }
             notifyRequestFinished();
             fulfill(this._sendRequest(progress, locationURL, redirectOptions, postData));
             request.destroy();
diff --git a/tests/library/browsercontext-fetch.spec.ts b/tests/library/browsercontext-fetch.spec.ts
index 0c2bdb25c3..b797f0a934 100644
--- a/tests/library/browsercontext-fetch.spec.ts
+++ b/tests/library/browsercontext-fetch.spec.ts
@@ -751,6 +751,22 @@ it('should respect timeout after redirects', async function({ context, server })
   expect(error.message).toContain(`Request timed out after 100ms`);
 });
 
+it('should throw on a redirect with an invalid URL', async ({ context, server }) => {
+  server.setRedirect('/redirect', '/test');
+  server.setRoute('/test', (req, res) => {
+    // Node.js prevents us from responding with an invalid header, therefore we manually write the response.
+    const conn = res.connection;
+    conn.write('HTTP/1.1 302\r\n');
+    conn.write('Location: https://здравствуйте/\r\n');
+    conn.write('\r\n');
+    conn.uncork();
+    conn.end();
+  });
+  console.log(server.PREFIX + '/test');
+  const error = await context.request.get(server.PREFIX + '/redirect').catch(e => e);
+  expect(error.message).toContain('apiRequestContext.get: uri requested responds with an invalid redirect URL');
+});
+
 it('should not hang on a brotli encoded Range request', async ({ context, server }) => {
   it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/18190' });
   it.skip(+process.versions.node.split('.')[0] < 18);