Bug 1103120 - Part 8: Server: Use promises and results in allowConnection. r=past
authorJ. Ryan Stinnett <jryans@gmail.com>
Mon, 26 Jan 2015 12:47:13 -0600
changeset 225850 37e5dfcc7674dd44d1d9574f433ab4a07a11b064
parent 225849 6d131234e4d325969b57085d23cd24ce58b137c4
child 225851 63a17819ae9411808096062cec4770b23c671ce7
push id28176
push userryanvm@gmail.com
push dateMon, 26 Jan 2015 21:48:45 +0000
treeherdermozilla-central@38e4719e71af [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast
bugs1103120
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1103120 - Part 8: Server: Use promises and results in allowConnection. r=past
b2g/chrome/content/devtools/debugger.js
mobile/android/chrome/content/browser.js
testing/xpcshell/head.js
toolkit/devtools/security/auth.js
toolkit/devtools/security/prompt.js
toolkit/devtools/security/socket.js
toolkit/devtools/security/tests/unit/test_encryption.js
toolkit/devtools/server/main.js
toolkit/devtools/transport/tests/unit/head_dbg.js
toolkit/devtools/transport/tests/unit/test_dbgsocket.js
toolkit/devtools/transport/tests/unit/test_dbgsocket_connection_drop.js
--- a/b2g/chrome/content/devtools/debugger.js
+++ b/b2g/chrome/content/devtools/debugger.js
@@ -36,17 +36,20 @@ let RemoteDebugger = {
     shell.sendChromeEvent({
       "type": "remote-debugger-prompt"
     });
 
     while(!this._promptDone) {
       Services.tm.currentThread.processNextEvent(true);
     }
 
-    return this._promptAnswer;
+    if (this._promptAnswer) {
+      return DebuggerServer.AuthenticationResult.ALLOW;
+    }
+    return DebuggerServer.AuthenticationResult.DENY;
   },
 
   _listen: function() {
     if (this._listening) {
       return;
     }
 
     this.handleEvent = this.handleEvent.bind(this);
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -7297,17 +7297,18 @@ var RemoteDebugger = {
   _isEnabled: function rd_isEnabled() {
     return Services.prefs.getBoolPref("devtools.debugger.remote-enabled");
   },
 
   /**
    * Prompt the user to accept or decline the incoming connection.
    * This is passed to DebuggerService.init as a callback.
    *
-   * @return true if the connection should be permitted, false otherwise
+   * @return An AuthenticationResult value.
+   *         A promise that will be resolved to the above is also allowed.
    */
   _showConnectionPrompt: function rd_showConnectionPrompt() {
     let title = Strings.browser.GetStringFromName("remoteIncomingPromptTitle");
     let msg = Strings.browser.GetStringFromName("remoteIncomingPromptMessage");
     let disable = Strings.browser.GetStringFromName("remoteIncomingPromptDisable");
     let cancel = Strings.browser.GetStringFromName("remoteIncomingPromptCancel");
     let agree = Strings.browser.GetStringFromName("remoteIncomingPromptAccept");
 
@@ -7329,22 +7330,21 @@ var RemoteDebugger = {
     });
 
     // Spin this thread while we wait for a result.
     let thread = Services.tm.currentThread;
     while (result == null)
       thread.processNextEvent(true);
 
     if (result === 0)
-      return true;
+      return DebuggerServer.AuthenticationResult.ALLOW;
     if (result === 2) {
-      Services.prefs.setBoolPref("devtools.debugger.remote-enabled", false);
-      this._stop();
-    }
-    return false;
+      return DebuggerServer.AuthenticationResult.DISABLE_ALL;
+    }
+    return DebuggerServer.AuthenticationResult.DENY;
   },
 
   _restart: function rd_restart() {
     this._stop();
     this._start();
   },
 
   _start: function rd_start() {
--- a/testing/xpcshell/head.js
+++ b/testing/xpcshell/head.js
@@ -434,17 +434,19 @@ function _initDebugging(port) {
   do_print("")
   do_print("To connect the debugger, open a Firefox instance, select 'Connect'");
   do_print("from the Developer menu and specify the port as " + port);
   do_print("*******************************************************************");
   do_print("")
 
   let AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
   let authenticator = new AuthenticatorType.Server();
-  authenticator.allowConnection = () => true;
+  authenticator.allowConnection = () => {
+    return DebuggerServer.AuthenticationResult.ALLOW;
+  };
 
   let listener = DebuggerServer.createListener();
   listener.portOrPath = port;
   listener.authenticator = authenticator;
   listener.open();
 
   // spin an event loop until the debugger connects.
   let thr = Components.classes["@mozilla.org/thread-manager;1"]
--- a/toolkit/devtools/security/auth.js
+++ b/toolkit/devtools/security/auth.js
@@ -6,16 +6,60 @@
 
 "use strict";
 
 let Services = require("Services");
 loader.lazyRequireGetter(this, "prompt",
   "devtools/toolkit/security/prompt");
 
 /**
+ * A simple enum-like object with keys mirrored to values.
+ * This makes comparison to a specfic value simpler without having to repeat and
+ * mis-type the value.
+ */
+function createEnum(obj) {
+  for (let key in obj) {
+    obj[key] = key;
+  }
+  return obj;
+}
+
+/**
+ * |allowConnection| implementations can return various values as their |result|
+ * field to indicate what action to take.  By specifying these, we can
+ * centralize the common actions available, while still allowing embedders to
+ * present their UI in whatever way they choose.
+ */
+let AuthenticationResult = exports.AuthenticationResult = createEnum({
+
+  /**
+   * Close all listening sockets, and disable them from opening again.
+   */
+  DISABLE_ALL: null,
+
+  /**
+   * Deny the current connection.
+   */
+  DENY: null,
+
+  /**
+   * Allow the current connection.
+   */
+  ALLOW: null,
+
+  /**
+   * Allow the current connection, and persist this choice for future
+   * connections from the same client.  This requires a trustable mechanism to
+   * identify the client in the future, such as the cert used during OOB_CERT.
+   */
+  ALLOW_PERSIST: null
+
+});
+
+/**
  * An |Authenticator| implements an authentication mechanism via various hooks
  * in the client and server debugger socket connection path (see socket.js).
  *
  * |Authenticator|s are stateless objects.  Each hook method is passed the state
  * it needs by the client / server code in socket.js.
  *
  * Separate instances of the |Authenticator| are created for each use (client
  * connection, server listener) in case some methods are customized by the
@@ -78,21 +122,22 @@ Prompt.Server.prototype = {
    *            host,
    *            port
    *          },
    *          server: {
    *            host,
    *            port
    *          }
    *        }
-   * @return true if the connection should be permitted, false otherwise
+   * @return An AuthenticationResult value.
+   *         A promise that will be resolved to the above is also allowed.
    */
   authenticate(session) {
     if (!Services.prefs.getBoolPref("devtools.debugger.prompt-connection")) {
-      return true;
+      return AuthenticationResult.ALLOW;
     }
     session.authentication = this.mode;
     return this.allowConnection(session);
   },
 
   /**
    * Prompt the user to accept or decline the incoming connection. The default
    * implementation is used unless this is overridden on a particular
@@ -108,17 +153,18 @@ Prompt.Server.prototype = {
    *            host,
    *            port
    *          },
    *          server: {
    *            host,
    *            port
    *          }
    *        }
-   * @return true if the connection should be permitted, false otherwise
+   * @return An AuthenticationResult value.
+   *         A promise that will be resolved to the above is also allowed.
    */
   allowConnection: prompt.Server.defaultAllowConnection,
 
 };
 
 /**
  * The out-of-band (OOB) cert authenticator is based on self-signed X.509 certs
  * at both the client and server end.
@@ -202,17 +248,18 @@ OOBCert.Server.prototype = {
    *          server: {
    *            host,
    *            port,
    *            cert: {
    *              sha256
    *            }
    *          }
    *        }
-   * @return true if the connection should be permitted, false otherwise
+   * @return An AuthenticationResult value.
+   *         A promise that will be resolved to the above is also allowed.
    */
   authenticate(session) {
     session.authentication = this.mode;
     return this.allowConnection(session);
   },
 
   /**
    * Prompt the user to accept or decline the incoming connection. The default
@@ -235,17 +282,18 @@ OOBCert.Server.prototype = {
    *          server: {
    *            host,
    *            port,
    *            cert: {
    *              sha256
    *            }
    *          }
    *        }
-   * @return true if the connection should be permitted, false otherwise
+   * @return An AuthenticationResult value.
+   *         A promise that will be resolved to the above is also allowed.
    */
   allowConnection: prompt.Server.defaultAllowConnection,
 
 };
 
 exports.Authenticators = {
   get(mode) {
     if (!mode) {
--- a/toolkit/devtools/security/prompt.js
+++ b/toolkit/devtools/security/prompt.js
@@ -5,31 +5,34 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 let Services = require("Services");
 let DevToolsUtils = require("devtools/toolkit/DevToolsUtils");
 loader.lazyRequireGetter(this, "DebuggerSocket",
   "devtools/toolkit/security/socket", true);
+loader.lazyRequireGetter(this, "AuthenticationResult",
+  "devtools/toolkit/security/auth", true);
 
 DevToolsUtils.defineLazyGetter(this, "bundle", () => {
   const DBG_STRINGS_URI = "chrome://global/locale/devtools/debugger.properties";
   return Services.strings.createBundle(DBG_STRINGS_URI);
 });
 
 let Server = exports.Server = {};
 
 /**
  * Prompt the user to accept or decline the incoming connection. This is the
  * default implementation that products embedding the debugger server may
  * choose to override.  This can be overridden via |allowConnection| on the
  * socket's authenticator instance.
  *
- * @return true if the connection should be permitted, false otherwise
+ * @return An AuthenticationResult value.
+ *         A promise that will be resolved to the above is also allowed.
  */
 Server.defaultAllowConnection = ({ client, server }) => {
   let title = bundle.GetStringFromName("remoteIncomingPromptTitle");
   let header = bundle.GetStringFromName("remoteIncomingPromptHeader");
   let clientEndpoint = `${client.host}:${client.port}`;
   let clientMsg =
     bundle.formatStringFromName("remoteIncomingPromptClientEndpoint",
                                 [clientEndpoint], 1);
@@ -43,17 +46,15 @@ Server.defaultAllowConnection = ({ clien
   let prompt = Services.prompt;
   let flags = prompt.BUTTON_POS_0 * prompt.BUTTON_TITLE_OK +
               prompt.BUTTON_POS_1 * prompt.BUTTON_TITLE_CANCEL +
               prompt.BUTTON_POS_2 * prompt.BUTTON_TITLE_IS_STRING +
               prompt.BUTTON_POS_1_DEFAULT;
   let result = prompt.confirmEx(null, title, msg, flags, null, null,
                                 disableButton, null, { value: false });
   if (result === 0) {
-    return true;
+    return AuthenticationResult.ALLOW;
   }
   if (result === 2) {
-    // TODO: Will reimplement later in patch series
-    // DebuggerServer.closeAllListeners();
-    Services.prefs.setBoolPref("devtools.debugger.remote-enabled", false);
+    return AuthenticationResult.DISABLE_ALL;
   }
-  return false;
+  return AuthenticationResult.DENY;
 };
--- a/toolkit/devtools/security/socket.js
+++ b/toolkit/devtools/security/socket.js
@@ -20,16 +20,18 @@ loader.lazyRequireGetter(this, "Debugger
 loader.lazyRequireGetter(this, "DebuggerServer",
   "devtools/server/main", true);
 loader.lazyRequireGetter(this, "discovery",
   "devtools/toolkit/discovery/discovery");
 loader.lazyRequireGetter(this, "cert",
   "devtools/toolkit/security/cert");
 loader.lazyRequireGetter(this, "Authenticators",
   "devtools/toolkit/security/auth", true);
+loader.lazyRequireGetter(this, "AuthenticationResult",
+  "devtools/toolkit/security/auth", true);
 loader.lazyRequireGetter(this, "setTimeout", "Timer", true);
 loader.lazyRequireGetter(this, "clearTimeout", "Timer", true);
 
 DevToolsUtils.defineLazyGetter(this, "nsFile", () => {
   return CC("@mozilla.org/file/local;1", "nsIFile", "initWithPath");
 });
 
 DevToolsUtils.defineLazyGetter(this, "socketTransportService", () => {
@@ -523,26 +525,34 @@ ServerSocketConnection.prototype = {
     if (clientStatus.tlsVersionUsed != Ci.nsITLSClientStatus.TLS_VERSION_1_2) {
       this._handshakeDeferred.reject(Cr.NS_ERROR_CONNECTION_REFUSED);
       return;
     }
 
     this._handshakeDeferred.resolve();
   },
 
-  _authenticate() {
-    let result = this._listener.authenticator.authenticate({
+  _authenticate: Task.async(function*() {
+    let result = yield this._listener.authenticator.authenticate({
       client: this.client,
       server: this.server
     });
-    if (result) {
-      return promise.resolve();
+    switch (result) {
+      case AuthenticationResult.DISABLE_ALL:
+        DebuggerServer.closeAllListeners();
+        Services.prefs.setBoolPref("devtools.debugger.remote-enabled", false);
+        return promise.reject(Cr.NS_ERROR_CONNECTION_REFUSED);
+      case AuthenticationResult.DENY:
+        return promise.reject(Cr.NS_ERROR_CONNECTION_REFUSED);
+      case AuthenticationResult.ALLOW:
+        return promise.resolve();
+      default:
+        return promise.reject(Cr.NS_ERROR_CONNECTION_REFUSED);
     }
-    return promise.reject(Cr.NS_ERROR_CONNECTION_REFUSED);
-  },
+  }),
 
   deny(result) {
     let errorName = result;
     for (let name in Cr) {
       if (Cr[name] === result) {
         errorName = name;
         break;
       }
--- a/toolkit/devtools/security/tests/unit/test_encryption.js
+++ b/toolkit/devtools/security/tests/unit/test_encryption.js
@@ -25,17 +25,19 @@ add_task(function*() {
 });
 
 // Client w/ encryption connects successfully to server w/ encryption
 add_task(function*() {
   equal(DebuggerServer.listeningSockets, 0, "0 listening sockets");
 
   let AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
   let authenticator = new AuthenticatorType.Server();
-  authenticator.allowConnection = () => true;
+  authenticator.allowConnection = () => {
+    return DebuggerServer.AuthenticationResult.ALLOW;
+  };
 
   let listener = DebuggerServer.createListener();
   ok(listener, "Socket listener created");
   listener.portOrPath = -1 /* any available port */;
   listener.authenticator = authenticator;
   listener.encryption = true;
   yield listener.open();
   equal(DebuggerServer.listeningSockets, 1, "1 listening socket");
@@ -70,17 +72,19 @@ add_task(function*() {
 });
 
 // Client w/o encryption fails to connect to server w/ encryption
 add_task(function*() {
   equal(DebuggerServer.listeningSockets, 0, "0 listening sockets");
 
   let AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
   let authenticator = new AuthenticatorType.Server();
-  authenticator.allowConnection = () => true;
+  authenticator.allowConnection = () => {
+    return DebuggerServer.AuthenticationResult.ALLOW;
+  };
 
   let listener = DebuggerServer.createListener();
   ok(listener, "Socket listener created");
   listener.portOrPath = -1 /* any available port */;
   listener.authenticator = authenticator;
   listener.encryption = true;
   yield listener.open();
   equal(DebuggerServer.listeningSockets, 1, "1 listening socket");
--- a/toolkit/devtools/server/main.js
+++ b/toolkit/devtools/server/main.js
@@ -19,16 +19,19 @@ let DevToolsUtils = require("devtools/to
 let { dumpn, dumpv, dbg_assert } = DevToolsUtils;
 let EventEmitter = require("devtools/toolkit/event-emitter");
 let Debugger = require("Debugger");
 
 DevToolsUtils.defineLazyGetter(this, "DebuggerSocket", () => {
   let { DebuggerSocket } = require("devtools/toolkit/security/socket");
   return DebuggerSocket;
 });
+DevToolsUtils.defineLazyGetter(this, "Authentication", () => {
+  return require("devtools/toolkit/security/auth");
+});
 
 // On B2G, `this` != Global scope, so `Ci` won't be binded on `this`
 // (i.e. this.Ci is undefined) Then later, when using loadSubScript,
 // Ci,... won't be defined for sub scripts.
 this.Ci = Ci;
 this.Cc = Cc;
 this.CC = CC;
 this.Cu = Cu;
@@ -1097,16 +1100,24 @@ var DebuggerServer = {
         for (let connID of Object.getOwnPropertyNames(this._connections)) {
           this._connections[connID].rootActor.removeActorByName(name);
         }
       }
     }
   }
 };
 
+// Expose these to save callers the trouble of importing DebuggerSocket
+DevToolsUtils.defineLazyGetter(DebuggerServer, "Authenticators", () => {
+  return Authentication.Authenticators;
+});
+DevToolsUtils.defineLazyGetter(DebuggerServer, "AuthenticationResult", () => {
+  return Authentication.AuthenticationResult;
+});
+
 EventEmitter.decorate(DebuggerServer);
 
 if (this.exports) {
   exports.DebuggerServer = DebuggerServer;
 }
 // Needed on B2G (See header note)
 this.DebuggerServer = DebuggerServer;
 
--- a/toolkit/devtools/transport/tests/unit/head_dbg.js
+++ b/toolkit/devtools/transport/tests/unit/head_dbg.js
@@ -258,17 +258,19 @@ function writeTestTempFile(aFileName, aC
 }
 
 /*** Transport Factories ***/
 
 let socket_transport = Task.async(function*() {
   if (!DebuggerServer.listeningSockets) {
     let AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
     let authenticator = new AuthenticatorType.Server();
-    authenticator.allowConnection = () => true;
+    authenticator.allowConnection = () => {
+      return DebuggerServer.AuthenticationResult.ALLOW;
+    };
     let listener = DebuggerServer.createListener();
     listener.portOrPath = -1 /* any available port */;
     listener.authenticator = authenticator;
     yield listener.open();
   }
   let port = DebuggerServer._listeners[0].port;
   do_print("Debugger server port is " + port);
   return DebuggerClient.socketConnect({ host: "127.0.0.1", port });
--- a/toolkit/devtools/transport/tests/unit/test_dbgsocket.js
+++ b/toolkit/devtools/transport/tests/unit/test_dbgsocket.js
@@ -19,17 +19,19 @@ function run_test()
   run_next_test();
 }
 
 function* test_socket_conn()
 {
   do_check_eq(DebuggerServer.listeningSockets, 0);
   let AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
   let authenticator = new AuthenticatorType.Server();
-  authenticator.allowConnection = () => true;
+  authenticator.allowConnection = () => {
+    return DebuggerServer.AuthenticationResult.ALLOW;
+  };
   let listener = DebuggerServer.createListener();
   do_check_true(listener);
   listener.portOrPath = -1 /* any available port */;
   listener.authenticator = authenticator;
   listener.open();
   do_check_eq(DebuggerServer.listeningSockets, 1);
   gPort = DebuggerServer._listeners[0].port;
   do_print("Debugger server port is " + gPort);
--- a/toolkit/devtools/transport/tests/unit/test_dbgsocket_connection_drop.js
+++ b/toolkit/devtools/transport/tests/unit/test_dbgsocket_connection_drop.js
@@ -44,17 +44,19 @@ function test_socket_conn_drops_after_to
     rawPacket += rawPacket;
   }
   return test_helper(rawPacket + ':');
 }
 
 let test_helper = Task.async(function*(payload) {
   let AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
   let authenticator = new AuthenticatorType.Server();
-  authenticator.allowConnection = () => true;
+  authenticator.allowConnection = () => {
+    return DebuggerServer.AuthenticationResult.ALLOW;
+  };
 
   let listener = DebuggerServer.createListener();
   listener.portOrPath = -1;
   listener.authenticator = authenticator;
   listener.open();
 
   let transport = yield DebuggerClient.socketConnect({
     host: "127.0.0.1",