Bug 1299411 - Error messages for native messaging r=aswan

- Combine the errors for a non-existing app and lacking permissions to
  avoid information leakage.
- Do not treat normal application exit as an error.
- Create errors in the right context.
- Add tests that check the error messages.

MozReview-Commit-ID: HxBpeCSyyGN

--HG--
extra : rebase_source : f9406cec6bddd4ab740f5042dd7cc64d4a63e447

toolkit/components/extensions/NativeMessaging.jsm

@@ -163,6 +163,10 @@ this.HostManifestManager = {
 };
 
 this.NativeApp = class extends EventEmitter {
+  /**
+   * @param {BaseContext} context The context that initiated the native app.
+   * @param {string} application The identifier of the native app.
+   */
   constructor(context, application) {
     super();
 
@@ -180,12 +184,10 @@ this.NativeApp = class extends EventEmitter {
 
     this.startupPromise = HostManifestManager.lookupApplication(application, context)
       .then(hostInfo => {
-        if (!hostInfo) {
-          throw new Error(`No such native application ${application}`);
-        }
-
-        if (!hostInfo.manifest.allowed_extensions.includes(context.extension.id)) {
-          throw new Error(`This extension does not have permission to use native application ${application}`);
+        // Put the two errors together to not leak information about whether a native
+        // application is installed to addons that do not have the right permission.
+        if (!hostInfo || !hostInfo.manifest.allowed_extensions.includes(context.extension.id)) {
+          throw new context.cloneScope.Error(`This extension does not have permission to use native application ${application} (or the application is not installed)`);
         }
 
         let command = hostInfo.manifest.path;
@@ -230,7 +232,7 @@ this.NativeApp = class extends EventEmitter {
   static onConnectNative(context, messageManager, portId, sender, application) {
     let app = new NativeApp(context, application);
     let port = new ExtensionUtils.Port(context, messageManager, [messageManager], "", portId, sender, sender);
-    app.once("disconnect", (what, err) => port.disconnect(err && err.message));
+    app.once("disconnect", (what, err) => port.disconnect(err));
 
     /* eslint-disable mozilla/balanced-listeners */
     app.on("message", (what, msg) => port.postMessage(msg));
@@ -269,7 +271,7 @@ this.NativeApp = class extends EventEmitter {
     this.readPromise = this.proc.stdout.readUint32()
       .then(len => {
         if (len > NativeApp.maxRead) {
-          throw new Error(`Native application tried to send a message of ${len} bytes, which exceeds the limit of ${NativeApp.maxRead} bytes.`);
+          throw new this.context.cloneScope.Error(`Native application tried to send a message of ${len} bytes, which exceeds the limit of ${NativeApp.maxRead} bytes.`);
         }
         return this.proc.stdout.readJSON(len);
       }).then(msg => {
@@ -400,6 +402,9 @@ this.NativeApp = class extends EventEmitter {
 
     if (!this.sentDisconnect) {
       this.sentDisconnect = true;
+      if (err && err.errorCode == Subprocess.ERROR_END_OF_FILE) {
+        err = null;
+      }
       this.emit("disconnect", err);
     }
   }

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

@@ -329,7 +329,9 @@ add_task(function* test_read_limit() {
   function background() {
     const PAYLOAD = "0123456789A";
     let port = browser.runtime.connectNative("echo");
-    port.onDisconnect.addListener(() => {
+    port.onDisconnect.addListener(msgPort => {
+      browser.test.assertEq(port, msgPort, "onDisconnect handler should receive the port as the first argument");
+      browser.test.assertEq("Native application tried to send a message of 13 bytes, which exceeds the limit of 10 bytes.", port.error && port.error.message);
       browser.test.sendMessage("result", "disconnected");
     });
     port.onMessage.addListener(msg => {
@@ -383,7 +385,9 @@ add_task(function* test_ext_permission() {
 add_task(function* test_app_permission() {
   function background() {
     let port = browser.runtime.connectNative("echo");
-    port.onDisconnect.addListener(() => {
+    port.onDisconnect.addListener(msgPort => {
+      browser.test.assertEq(port, msgPort, "onDisconnect handler should receive the port as the first argument");
+      browser.test.assertEq("This extension does not have permission to use native application echo (or the application is not installed)", port.error && port.error.message);
       browser.test.sendMessage("result", "disconnected");
     });
     port.onMessage.addListener(msg => {
@@ -444,7 +448,9 @@ add_task(function* test_child_process() {
 add_task(function* test_stderr() {
   function background() {
     let port = browser.runtime.connectNative("stderr");
-    port.onDisconnect.addListener(() => {
+    port.onDisconnect.addListener(msgPort => {
+      browser.test.assertEq(port, msgPort, "onDisconnect handler should receive the port as the first argument");
+      browser.test.assertEq(null, port.error, "Normal application exit is not an error");
       browser.test.sendMessage("finished");
     });
   }