fix: support IP:PORT short notation to specify proxy server (#3568)

Short notation implies `http://` scheme. Fixes #3233
microsoft · Aug 28, 2020 · 45e178f · 45e178f
1 parent 97e4561
commit 45e178f
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 6 deletions.
diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts
@@ -279,13 +279,20 @@ export function verifyGeolocation(geolocation?: types.Geolocation) {
     throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`);
 }
 
-export function verifyProxySettings(proxy: types.ProxySettings): types.ProxySettings {
+export function normalizeProxySettings(proxy: types.ProxySettings): types.ProxySettings {
   let { server, bypass } = proxy;
-  let url = new URL(server);
-  if (!['http:', 'https:', 'socks5:'].includes(url.protocol)) {
+  let url;
+  try {
+    // new URL('127.0.0.1:8080') throws
+    // new URL('localhost:8080') fails to parse host or protocol
+    // In both of these cases, we need to try re-parse URL with `http://` prefix.
+    url = new URL(server);
+    if (!url.host || !url.protocol)
+      url = new URL('http://' + server);
+  } catch (e) {
     url = new URL('http://' + server);
-    server = `${url.protocol}//${url.host}`;
   }
+  server = url.protocol + '//' + url.host;
   if (bypass)
     bypass = bypass.split(',').map(t => t.trim()).join(',');
   return { ...proxy, server, bypass };

diff --git a/src/server/browserType.ts b/src/server/browserType.ts
@@ -18,7 +18,7 @@ import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
 import * as util from 'util';
-import { BrowserContext, verifyProxySettings, validateBrowserContextOptions } from './browserContext';
+import { BrowserContext, normalizeProxySettings, validateBrowserContextOptions } from './browserContext';
 import * as browserPaths from '../utils/browserPaths';
 import { ConnectionTransport, WebSocketTransport } from './transport';
 import { BrowserOptions, Browser, BrowserProcess } from './browser';
@@ -88,7 +88,7 @@ export abstract class BrowserTypeBase implements BrowserType {
   }
 
   async _innerLaunch(progress: Progress, options: types.LaunchOptions, persistent: types.BrowserContextOptions | undefined, userDataDir?: string): Promise<Browser> {
-    options.proxy = options.proxy ? verifyProxySettings(options.proxy) : undefined;
+    options.proxy = options.proxy ? normalizeProxySettings(options.proxy) : undefined;
     const { browserProcess, downloadsPath, transport } = await this._launchProcess(progress, options, !!persistent, userDataDir);
     if ((options as any).__testHookBeforeCreateBrowser)
       await (options as any).__testHookBeforeCreateBrowser();

diff --git a/test/proxy.spec.ts b/test/proxy.spec.ts
@@ -41,6 +41,20 @@ it('should use proxy', async ({browserType, defaultBrowserOptions, server}) => {
   await browser.close();
 });
 
+it('should work with IP:PORT notion', async ({browserType, defaultBrowserOptions, server}) => {
+  server.setRoute('/target.html', async (req, res) => {
+    res.end('<html><title>Served by the proxy</title></html>');
+  });
+  const browser = await browserType.launch({
+    ...defaultBrowserOptions,
+    proxy: { server: `127.0.0.1:${server.PORT}` }
+  });
+  const page = await browser.newPage();
+  await page.goto('http://non-existent.com/target.html');
+  expect(await page.title()).toBe('Served by the proxy');
+  await browser.close();
+});
+
 it('should authenticate', async ({browserType, defaultBrowserOptions, server}) => {
   server.setRoute('/target.html', async (req, res) => {
     const auth = req.headers['proxy-authorization'];