fix: Choose a random free tcp port to be used for the Firefox Desktop remote debugging server

src/extension-runners/chromium.js

@@ -83,7 +83,7 @@ export class ChromiumExtensionRunner {
   async setupInstance(): Promise<void> {
     // Start a websocket server on a free localhost TCP port.
     this.wss = new WebSocket.Server({
-      port: 0,
+      port: 6005,
       host: 'localhost',
     });
 

src/extension-runners/firefox-android.js

@@ -5,7 +5,6 @@
  * in a Firefox for Android instance.
  */
 
-import net from 'net';
 import path from 'path';
 import readline from 'readline';
 
@@ -22,6 +21,7 @@ import {
 import * as defaultFirefoxApp from '../firefox';
 import {
   connectWithMaxRetries as defaultFirefoxConnector,
+  findFreeTcpPort,
 } from '../firefox/remote';
 import {createLogger} from '../util/logger';
 import {isTTY, setRawMode} from '../util/stdin';
@@ -567,7 +567,7 @@ export class FirefoxAndroidExtensionRunner {
 
     log.debug(`RDP Socket File selected: ${this.selectedRDPSocketFile}`);
 
-    const tcpPort = await this.chooseLocalTcpPort();
+    const tcpPort = await findFreeTcpPort();
 
     // Log the choosen tcp port at info level (useful to the user to be able
     // to connect the Firefox DevTools to the Firefox for Android instance).
@@ -586,17 +586,6 @@ export class FirefoxAndroidExtensionRunner {
     this.selectedTCPPort = tcpPort;
   }
 
-  chooseLocalTcpPort(): Promise<number> {
-    return new Promise((resolve) => {
-      const srv = net.createServer();
-      // $FLOW_FIXME: flow has his own opinions on this method signature.
-      srv.listen(0, () => {
-        const freeTcpPort = srv.address().port;
-        srv.close(() => resolve(freeTcpPort));
-      });
-    });
-  }
-
   async rdpInstallExtensions() {
     const {
       selectedTCPPort,

src/firefox/index.js

@@ -13,10 +13,9 @@ import isDirectory from '../util/is-directory';
 import {isErrorWithCode, UsageError, WebExtError} from '../errors';
 import {getPrefs as defaultPrefGetter} from './preferences';
 import {getManifestId} from '../util/manifest';
+import {findFreeTcpPort as defaultRemotePortFinder} from './remote';
 import {createLogger} from '../util/logger';
-import {connect as defaultFirefoxConnector, REMOTE_PORT} from './remote';
 // Import flow types
-import type {FirefoxConnectorFn} from './remote';
 import type {
   PreferencesAppName,
   PreferencesGetterFn,
@@ -36,48 +35,9 @@ export const defaultFirefoxEnv = {
 
 // defaultRemotePortFinder types and implementation.
 
-export type RemotePortFinderParams = {|
-  portToTry?: number,
-  retriesLeft?: number,
-  connectToFirefox?: FirefoxConnectorFn,
-|};
 
 export type RemotePortFinderFn =
-  (params?: RemotePortFinderParams) => Promise<number>;
-
-export async function defaultRemotePortFinder(
-  {
-    portToTry = REMOTE_PORT,
-    retriesLeft = 10,
-    connectToFirefox = defaultFirefoxConnector,
-  }: RemotePortFinderParams = {}
-): Promise<number> {
-  log.debug(`Checking if remote Firefox port ${portToTry} is available`);
-
-  let client;
-
-  while (retriesLeft >= 0) {
-    try {
-      client = await connectToFirefox(portToTry);
-      log.debug(`Remote Firefox port ${portToTry} is in use ` +
-                `(retries remaining: ${retriesLeft})`);
-    } catch (error) {
-      if (isErrorWithCode('ECONNREFUSED', error)) {
-        // The connection was refused so this port is good to use.
-        return portToTry;
-      }
-
-      throw error;
-    }
-
-    client.disconnect();
-    portToTry++;
-    retriesLeft--;
-  }
-
-  throw new WebExtError('Too many retries on port search');
-}
-
+  () => Promise<number>;
 
 // Declare the needed 'fx-runner' module flow types.
 

src/firefox/remote.js

@@ -1,4 +1,6 @@
 /* @flow */
+import net from 'net';
+
 import defaultFirefoxConnector from '@cliqz-oss/node-firefox-connect';
 // RemoteFirefox types and implementation
 import type FirefoxClient from '@cliqz-oss/firefox-client';
@@ -13,10 +15,6 @@ import {
 
 const log = createLogger(__filename);
 
-// The default port that Firefox's remote debugger will listen on and the
-// client will connect to.
-export const REMOTE_PORT = 6005;
-
 export type FirefoxConnectorFn =
   (port?: number) => Promise<FirefoxClient>;
 
@@ -192,7 +190,7 @@ export type ConnectOptions = {|
 |};
 
 export async function connect(
-  port: number = REMOTE_PORT,
+  port: number,
   {connectToFirefox = defaultFirefoxConnector}: ConnectOptions = {}
 ): Promise<RemoteFirefox> {
   log.debug(`Connecting to Firefox on port ${port}`);
@@ -249,3 +247,14 @@ export async function connectWithMaxRetries(
   log.debug('Connecting to the remote Firefox debugger');
   return establishConnection();
 }
+
+export function findFreeTcpPort(): Promise<number> {
+  return new Promise((resolve) => {
+    const srv = net.createServer();
+    // $FLOW_FIXME: flow has his own opinions on this method signature.
+    srv.listen(0, () => {
+      const freeTcpPort = srv.address().port;
+      srv.close(() => resolve(freeTcpPort));
+    });
+  });
+}

tests/functional/fake-firefox-binary.js

@@ -22,7 +22,19 @@ function toRDP(msg) {
   return [data.length, ':', data].join('');
 }
 
-// Start a TCP Server
+// Get the debugger server port from the cli arguments
+function getPortFromArgs() {
+  const index = process.argv.indexOf('-start-debugger-server');
+  if (index === -1) {
+    throw new Error('The -start-debugger-server parameter is not present.');
+  }
+  const port = process.argv[index + 1];
+  if (isNaN(port)) {
+    throw new Error(`Value of port must be a number. ${port} is not a number.`);
+  }
+
+  return parseInt(port, 10);
+}
 net.createServer(function(socket) {
   socket.on('data', function(data) {
     if (String(data) === toRDP(REQUEST_LIST_TABS)) {
@@ -43,4 +55,4 @@ net.createServer(function(socket) {
   });
 
   socket.write(toRDP(REPLY_INITIAL));
-}).listen(6005);
+}).listen(getPortFromArgs());

tests/unit/test-firefox/test.firefox.js

@@ -14,14 +14,10 @@ import {withTempDir} from '../../../src/util/temp-dir';
 import {
   basicManifest,
   fixturePath,
-  fake,
   makeSureItFails,
   manifestWithoutApps,
-  TCPConnectError,
   ErrorWithCode,
 } from '../helpers';
-import {RemoteFirefox} from '../../../src/firefox/remote';
-import type {RemotePortFinderParams} from '../../../src/firefox/index';
 
 const {defaultFirefoxEnv} = firefox;
 
@@ -975,76 +971,4 @@ describe('firefox', () => {
 
   });
 
-  describe('defaultRemotePortFinder', () => {
-
-    function findRemotePort({...args}: RemotePortFinderParams = {}) {
-      return firefox.defaultRemotePortFinder({...args});
-    }
-
-    it('resolves to an open port', () => {
-      const connectToFirefox = sinon.spy(
-        () => Promise.reject(new TCPConnectError()));
-      return findRemotePort({connectToFirefox})
-        .then((port) => {
-          assert.isNumber(port);
-        });
-    });
-
-    it('returns a port on first try', () => {
-      const connectToFirefox = sinon.spy(() => new Promise(
-        (resolve, reject) => {
-          reject(
-            new TCPConnectError('first call connection fails - port is free')
-          );
-        }));
-      return findRemotePort({connectToFirefox, retriesLeft: 2})
-        .then((port) => {
-          sinon.assert.calledOnce(connectToFirefox);
-          assert.isNumber(port);
-        });
-    });
-
-    it('cancels search after too many fails', () => {
-      const client = fake(RemoteFirefox.prototype);
-      const connectToFirefox = sinon.spy(() => new Promise(
-        (resolve) => resolve(client)));
-      return findRemotePort({connectToFirefox, retriesLeft: 2})
-        .catch((err) => {
-          assert.equal(err, 'WebExtError: Too many retries on port search');
-          sinon.assert.calledThrice(connectToFirefox);
-        });
-    });
-
-    it('retries port discovery after first failure', () => {
-      const client = fake(RemoteFirefox.prototype);
-      let callCount = 0;
-      const connectToFirefox = sinon.spy(() => {
-        callCount++;
-        return new Promise((resolve, reject) => {
-          if (callCount === 2) {
-            reject(new TCPConnectError('port is free'));
-          } else {
-            resolve(client);
-          }
-        });
-      });
-      return findRemotePort({connectToFirefox, retriesLeft: 2})
-        .then((port) => {
-          assert.isNumber(port);
-          sinon.assert.calledTwice(connectToFirefox);
-        });
-    });
-
-    it('throws on unexpected errors', async () => {
-      const connectToFirefox = sinon.spy(async () => {
-        throw new Error('Unexpected connect error');
-      });
-
-      await assert.isRejected(findRemotePort({connectToFirefox}),
-                              /Unexpected connect error/);
-
-      sinon.assert.calledOnce(connectToFirefox);
-    });
-
-  });
 });

tests/unit/test-firefox/test.remote.js

@@ -13,6 +13,7 @@ import {
   connect as defaultConnector,
   connectWithMaxRetries,
   RemoteFirefox,
+  findFreeTcpPort,
 } from '../../../src/firefox/remote';
 import {
   fakeFirefoxClient,
@@ -25,7 +26,7 @@ describe('firefox.remote', () => {
 
   describe('connect', () => {
 
-    function prepareConnection(port = undefined, options = {}) {
+    function prepareConnection(port = 6005, options = {}) {
       options = {
         connectToFirefox:
           sinon.spy(() => Promise.resolve(fakeFirefoxClient())),
@@ -375,4 +376,38 @@ describe('firefox.remote', () => {
 
   });
 
+  describe('findFreeTcpPort', async () => {
+
+    const port = await findFreeTcpPort();
+
+    function prepareConnection(portNumber = port, options = {}) {
+      options = {
+        connectToFirefox:
+          sinon.spy(() => Promise.resolve(fakeFirefoxClient())),
+        ...options,
+      };
+      const connect = defaultConnector(portNumber, options);
+      return {options, connect};
+    }
+
+    it('resolves to an open port', () => {
+
+      assert.isNumber(port);
+      assert.isAtLeast(port, 1024);
+    });
+
+    it('creates a connection with the resolved port', async () => {
+      const {connect, options} = prepareConnection(port);
+      await connect;
+      assert.equal(options.connectToFirefox.args[0], port);
+    });
+
+    it('resolves to a new port each time', async () => {
+      const newPort = await findFreeTcpPort();
+
+      assert.notStrictEqual(port, newPort);
+    });
+
+  });
+
 });