Revert "fix: get rid of url.parse in network code" (#36654)

packages/playwright-core/src/server/utils/network.ts

@@ -39,8 +39,9 @@ export type HTTPRequestParams = {
 export const NET_DEFAULT_TIMEOUT = 30_000;
 
 export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.IncomingMessage) => void, onError: (error: Error) => void): { cancel(error: Error | undefined): void } {
-  const parsedUrl = new URL(params.url);
-  const options: https.RequestOptions = {
+  const parsedUrl = url.parse(params.url);
+  let options: https.RequestOptions = {
+    ...parsedUrl,
     agent: parsedUrl.protocol === 'https:' ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent,
     method: params.method || 'GET',
     headers: params.headers,
@@ -50,15 +51,19 @@ export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.Inco
 
   const proxyURL = getProxyForUrl(params.url);
   if (proxyURL) {
-    const parsedProxyURL = new URL(proxyURL);
+    const parsedProxyURL = url.parse(proxyURL);
     if (params.url.startsWith('http:')) {
-      parsedUrl.pathname = parsedUrl.href;
-      parsedUrl.host = parsedProxyURL.host;
+      options = {
+        path: parsedUrl.href,
+        host: parsedProxyURL.hostname,
+        port: parsedProxyURL.port,
+        headers: options.headers,
+        method: options.method
+      };
     } else {
-      options.agent = new HttpsProxyAgent({
-        ...convertURLtoLegacyUrl(parsedProxyURL),
-        secureProxy: parsedProxyURL.protocol === 'https:',
-      });
+      (parsedProxyURL as any).secureProxy = parsedProxyURL.protocol === 'https:';
+
+      options.agent = new HttpsProxyAgent(parsedProxyURL);
       options.rejectUnauthorized = false;
     }
   }
@@ -76,8 +81,8 @@ export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.Inco
     }
   };
   const request = options.protocol === 'https:' ?
-    https.request(parsedUrl, options, requestCallback) :
-    http.request(parsedUrl, options, requestCallback);
+    https.request(options, requestCallback) :
+    http.request(options, requestCallback);
   request.on('error', onError);
   if (params.socketTimeout !== undefined) {
     request.setTimeout(params.socketTimeout, () =>  {
@@ -132,27 +137,23 @@ export function createProxyAgent(proxy?: ProxySettings, forUrl?: URL) {
   if (!/^\w+:\/\//.test(proxyServer))
     proxyServer = 'http://' + proxyServer;
 
-  const proxyOpts = new URL(proxyServer);
+  const proxyOpts = url.parse(proxyServer);
   if (proxyOpts.protocol?.startsWith('socks')) {
     return new SocksProxyAgent({
       host: proxyOpts.hostname,
       port: proxyOpts.port || undefined,
     });
   }
-  if (proxy.username) {
-    proxyOpts.username = proxy.username;
-    proxyOpts.password = proxy.password || '';
-  }
+  if (proxy.username)
+    proxyOpts.auth = `${proxy.username}:${proxy.password || ''}`;
 
   if (forUrl && ['ws:', 'wss:'].includes(forUrl.protocol)) {
     // Force CONNECT method for WebSockets.
-    // TODO: switch to URL instance instead of legacy object once https-proxy-agent supports it.
-    return new HttpsProxyAgent(convertURLtoLegacyUrl(proxyOpts));
+    return new HttpsProxyAgent(proxyOpts);
   }
 
   // TODO: We should use HttpProxyAgent conditional on proxyOpts.protocol instead of always using CONNECT method.
-  // TODO: switch to URL instance instead of legacy object once https-proxy-agent supports it.
-  return new HttpsProxyAgent(convertURLtoLegacyUrl(proxyOpts));
+  return new HttpsProxyAgent(proxyOpts);
 }
 
 export function createHttpServer(requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): http.Server;
@@ -225,20 +226,3 @@ function decorateServer(server: net.Server) {
     return close.call(server, callback);
   };
 }
-
-function convertURLtoLegacyUrl(url: URL): url.Url {
-  return {
-    auth: url.username ? url.username + ':' + url.password : null,
-    hash: url.hash || null,
-    host: url.hostname ? url.hostname + ':' + url.port : null,
-    hostname: url.hostname || null,
-    href: url.href,
-    path: url.pathname + url.search,
-    pathname: url.pathname,
-    protocol: url.protocol,
-    search: url.search || null,
-    slashes: true,
-    port: url.port || null,
-    query: url.search.slice(1) || null,
-  };
-}

tests/installation/playwright-cli-install-should-work.spec.ts

@@ -16,6 +16,7 @@
 import { test, expect } from './npmTest';
 import { chromium } from '@playwright/test';
 import path from 'path';
+import { TestProxy } from '../config/proxy';
 
 test.use({ isolateBrowsers: true });
 
@@ -60,6 +61,22 @@ test('install command should work', async ({ exec, checkInstalledSoftwareOnDisk
   }
 });
 
+test('install command should work with proxy', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/36650' } }, async ({ exec, checkInstalledSoftwareOnDisk }) => {
+  await exec('npm i playwright');
+  const proxy = await TestProxy.create(8947 + test.info().workerIndex * 4);
+  proxy.forwardTo(443, { preserveHostname: true });
+  await test.step('playwright install chromium', async () => {
+    const result = await exec('npx playwright install chromium', {
+      env: {
+        HTTPS_PROXY: proxy.URL,
+      }
+    });
+    expect(result).toHaveLoggedSoftwareDownload(['chromium', 'chromium-headless-shell', 'ffmpeg', ...extraInstalledSoftware]);
+    await checkInstalledSoftwareOnDisk(['chromium', 'chromium-headless-shell', 'ffmpeg', ...extraInstalledSoftware]);
+  });
+  await proxy.stop();
+});
+
 test('should be able to remove browsers', async ({ exec, checkInstalledSoftwareOnDisk }) => {
   await exec('npm i playwright');
   await exec('npx playwright install chromium');