Bug 1286712 - Validate runtime.sendMessage parameters r=billm

The main motive for this patch is to remove the use of the GlobalManager
global (which was used to see if an extension ID is valid, which was
specifically added in order to create thebrowser_ext_lastError.js test).
To preserve test coverage I implemented a full validation of the
runtime.sendMessage method.

Now the error for a non-existent extension is identical in both the
content script and background pages. Note that this also fixes a
minor privacy leak: Previously extensions could see whether another
extension is installed by sending a message to the specified extension
and using the different responses to see whether another extension is
installed.

MozReview-Commit-ID: 82R97Ei25Xr

--HG--
extra : rebase_source : 900a65e695afd6db83d5102f515dc29d46d001f1

browser/components/extensions/test/browser/browser_ext_lastError.js

@@ -2,11 +2,11 @@
 
 function* sendMessage(options) {
   function background(options) {
-    browser.runtime.sendMessage("invalid-extension-id", {}, {}, result => {
+    browser.runtime.sendMessage(result => {
       browser.test.assertEq(undefined, result, "Argument value");
       if (options.checkLastError) {
         let lastError = browser[options.checkLastError].lastError;
-        browser.test.assertEq("Invalid extension ID",
+        browser.test.assertEq("runtime.sendMessage's message argument is missing",
                               lastError && lastError.message,
                               "lastError value");
       }
@@ -34,7 +34,7 @@ add_task(function* testLastError() {
   // checked.
   for (let api of ["extension", "runtime"]) {
     let waitForConsole = new Promise(resolve => {
-      SimpleTest.monitorConsole(resolve, [{message: /Invalid extension ID/, forbid: true}]);
+      SimpleTest.monitorConsole(resolve, [{message: /message argument is missing/, forbid: true}]);
     });
 
     yield sendMessage({checkLastError: api});
@@ -45,7 +45,7 @@ add_task(function* testLastError() {
 
   // Check that we do have a console message when lastError is not checked.
   let waitForConsole = new Promise(resolve => {
-    SimpleTest.monitorConsole(resolve, [{message: /Unchecked lastError value: Error: Invalid extension ID/}]);
+    SimpleTest.monitorConsole(resolve, [{message: /Unchecked lastError value: Error: runtime.sendMessage's message argument is missing/}]);
   });
 
   yield sendMessage({});

toolkit/components/extensions/MessageChannel.jsm

@@ -517,7 +517,11 @@ this.MessageChannel = {
     };
     deferred.promise.then(cleanup, cleanup);
 
-    target.sendAsyncMessage(MESSAGE_MESSAGE, message);
+    try {
+      target.sendAsyncMessage(MESSAGE_MESSAGE, message);
+    } catch (e) {
+      deferred.reject(e);
+    }
     return deferred.promise;
   },
 

toolkit/components/extensions/ext-runtime.js

@@ -74,20 +74,38 @@ extensions.registerSchemaAPI("runtime", context => {
       sendMessage: function(...args) {
         let options; // eslint-disable-line no-unused-vars
         let extensionId, message, responseCallback;
-        if (args.length == 1) {
+        if (typeof args[args.length - 1] == "function") {
+          responseCallback = args.pop();
+        }
+        if (!args.length) {
+          return Promise.reject({message: "runtime.sendMessage's message argument is missing"});
+        } else if (args.length == 1) {
           message = args[0];
         } else if (args.length == 2) {
-          [message, responseCallback] = args;
+          if (typeof args[0] == "string" && args[0]) {
+            [extensionId, message] = args;
+          } else {
+            [message, options] = args;
+          }
+        } else if (args.length == 3) {
+          [extensionId, message, options] = args;
+        } else if (args.length == 4 && !responseCallback) {
+          return Promise.reject({message: "runtime.sendMessage's last argument is not a function"});
         } else {
-          [extensionId, message, options, responseCallback] = args;
+          return Promise.reject({message: "runtime.sendMessage received too many arguments"});
         }
+
+        if (extensionId != null && typeof extensionId != "string") {
+          return Promise.reject({message: "runtime.sendMessage's extensionId argument is invalid"});
+        }
+        if (options != null && typeof options != "object") {
+          return Promise.reject({message: "runtime.sendMessage's options argument is invalid"});
+        }
+        // TODO(robwu): Validate option keys and values when we support it.
+
         extensionId = extensionId || extension.id;
         let recipient = {extensionId};
 
-        if (!GlobalManager.extensionMap.has(recipient.extensionId)) {
-          return context.wrapPromise(Promise.reject({message: "Invalid extension ID"}),
-                                     responseCallback);
-        }
         return context.messenger.sendMessage(Services.cpmm, message, recipient, responseCallback);
       },
 

toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js

@@ -0,0 +1,64 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(function* test_sendMessage_error() {
+  function background() {
+    let circ = {};
+    circ.circ = circ;
+    let testCases = [
+      // [arguments, expected error string],
+      [[], "runtime.sendMessage's message argument is missing"],
+      [[null, null, null, null], "runtime.sendMessage's last argument is not a function"],
+      [[null, null, 1], "runtime.sendMessage's options argument is invalid"],
+      [[1, null, null], "runtime.sendMessage's extensionId argument is invalid"],
+      [[null, null, null, null, null], "runtime.sendMessage received too many arguments"],
+
+      // Even when the parameters are accepted, we still expect an error
+      // because there is no onMessage listener.
+      [[null, null, null], "Could not establish connection. Receiving end does not exist."],
+
+      // Structural cloning doesn't work with DOM but we fall back
+      // JSON serialization, so we don't expect another error.
+      [[null, location, null], "Could not establish connection. Receiving end does not exist."],
+
+      // Structured cloning supports cyclic self-references.
+      [[null, [circ, location], null], "cyclic object value"],
+      // JSON serialization does not support cyclic references.
+      [[null, circ, null], "Could not establish connection. Receiving end does not exist."],
+      // (the last two tests shows whether sendMessage is implemented as structured cloning).
+    ];
+
+    // Repeat all tests with the undefined value instead of null.
+    for (let [args, expectedError] of testCases.slice()) {
+      args = args.map(arg => arg === null ? undefined : arg);
+      testCases.push([args, expectedError]);
+    }
+
+    function next() {
+      if (!testCases.length) {
+        browser.test.notifyPass("sendMessage parameter validation");
+        return;
+      }
+      let [args, expectedError] = testCases.shift();
+      let description = `runtime.sendMessage(${args.map(String).join(", ")})`;
+      return browser.runtime.sendMessage(...args)
+        .then(() => {
+          browser.test.fail(`Unexpectedly got no error for ${description}`);
+        }, err => {
+          browser.test.assertEq(expectedError, err.message, `expected error message for ${description}`);
+        }).then(next);
+    }
+    next();
+  }
+  let extensionData = {
+    background,
+  };
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  yield extension.startup();
+
+  yield extension.awaitFinish("sendMessage parameter validation");
+
+  yield extension.unload();
+});

toolkit/components/extensions/test/xpcshell/xpcshell.ini

@@ -39,6 +39,7 @@ skip-if = release_build
 [test_ext_runtime_connect_no_receiver.js]
 [test_ext_runtime_getPlatformInfo.js]
 [test_ext_runtime_sendMessage.js]
+[test_ext_runtime_sendMessage_errors.js]
 [test_ext_runtime_sendMessage_no_receiver.js]
 [test_ext_schemas.js]
 [test_ext_schemas_api_injection.js]