Bug 1299411 - separate serialization from sending draft
authorRob Wu <rob@robwu.nl>
Sat, 24 Sep 2016 13:03:20 +0200
changeset 430571 e6f29fea8ecd0d24e8fcb046a80ae21f256b980a
parent 430570 8b3b2a7da7d6167a6ba86ecd8f63ef1c0f2a38c9
child 430572 6130a74a09b576560d6f8361ec4c865daf7abe64
push id33850
push userbmo:rob@robwu.nl
push dateThu, 27 Oct 2016 23:41:10 +0000
bugs1299411
milestone52.0a1
Bug 1299411 - separate serialization from sending Serialization of the message should happen in the same process as the extension context, whereas sending the message should be in the same process as the owner of the native messaging host. With webext-oop, the former is an addon process and the latter the main process. Therefore it is necessary to separate the two roles. MozReview-Commit-ID: 8BJZmn2QjLJ
toolkit/components/extensions/NativeMessaging.jsm
toolkit/components/extensions/ext-runtime.js
toolkit/components/extensions/test/xpcshell/test_native_messaging.js
--- a/toolkit/components/extensions/NativeMessaging.jsm
+++ b/toolkit/components/extensions/NativeMessaging.jsm
@@ -1,15 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["HostManifestManager", "NativeApp"];
+/* globals NativeApp */
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 const {EventEmitter} = Cu.import("resource://devtools/shared/event-emitter.js", {});
 
 XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
@@ -166,27 +167,22 @@ this.NativeApp = class extends EventEmit
     super();
 
     this.context = context;
     this.name = application;
 
     // We want a close() notification when the window is destroyed.
     this.context.callOnClose(this);
 
-    this.encoder = new TextEncoder();
     this.proc = null;
     this.readPromise = null;
     this.sendQueue = [];
     this.writePromise = null;
     this.sentDisconnect = false;
 
-    // Grab these once at startup
-    XPCOMUtils.defineLazyPreferenceGetter(this, "maxRead", PREF_MAX_READ, MAX_READ);
-    XPCOMUtils.defineLazyPreferenceGetter(this, "maxWrite", PREF_MAX_WRITE, MAX_WRITE);
-
     this.startupPromise = HostManifestManager.lookupApplication(application, context)
       .then(hostInfo => {
         if (!hostInfo) {
           throw new Error(`No such native application ${application}`);
         }
 
         if (!hostInfo.manifest.allowed_extensions.includes(extension.id)) {
           throw new Error(`This extension does not have permission to use native application ${application}`);
@@ -215,32 +211,46 @@ this.NativeApp = class extends EventEmit
         this._startStderrRead();
       }).catch(err => {
         this.startupPromise = null;
         Cu.reportError(err instanceof Error ? err : err.message);
         this._cleanup(err);
       });
   }
 
+  /**
+   * @param {BaseContext} context The scope from where `message` originates.
+   * @param {*} message A message from the extension, meant for a native app.
+   * @returns {ArrayBuffer} An ArrayBuffer that can be sent to the native app.
+   */
+  static encodeMessage(context, message) {
+    message = context.jsonStringify(message);
+    let buffer = new TextEncoder().encode(message).buffer;
+    if (buffer.byteLength > NativeApp.maxWrite) {
+      throw new context.cloneScope.Error("Write too big");
+    }
+    return buffer;
+  }
+
   // A port is definitely "alive" if this.proc is non-null.  But we have
   // to provide a live port object immediately when connecting so we also
   // need to consider a port alive if proc is null but the startupPromise
   // is still pending.
   get _isDisconnected() {
     return (!this.proc && !this.startupPromise);
   }
 
   _startRead() {
     if (this.readPromise) {
       throw new Error("Entered _startRead() while readPromise is non-null");
     }
     this.readPromise = this.proc.stdout.readUint32()
       .then(len => {
-        if (len > this.maxRead) {
-          throw new Error(`Native application tried to send a message of ${len} bytes, which exceeds the limit of ${this.maxRead} bytes.`);
+        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.`);
         }
         return this.proc.stdout.readJSON(len);
       }).then(msg => {
         this.emit("message", msg);
         this.readPromise = null;
         this._startRead();
       }).catch(err => {
         if (err.errorCode != Subprocess.ERROR_END_OF_FILE) {
@@ -299,26 +309,25 @@ this.NativeApp = class extends EventEmit
       }
     });
   }
 
   send(msg) {
     if (this._isDisconnected) {
       throw new this.context.cloneScope.Error("Attempt to postMessage on disconnected port");
     }
+    if (Cu.getClassName(msg, true) != "ArrayBuffer") {
+      // This error cannot be triggered by extensions; it indicates an error in
+      // our implementation.
+      throw new Error("The message to the native messaging host is not an ArrayBuffer");
+    }
 
-    let json;
-    try {
-      json = this.context.jsonStringify(msg);
-    } catch (err) {
-      throw new this.context.cloneScope.Error(err.message);
-    }
-    let buffer = this.encoder.encode(json).buffer;
+    let buffer = msg;
 
-    if (buffer.byteLength > this.maxWrite) {
+    if (buffer.byteLength > NativeApp.maxWrite) {
       throw new this.context.cloneScope.Error("Write too big");
     }
 
     this.sendQueue.push(buffer);
     if (!this.startupPromise && !this.writePromise) {
       this._startWrite();
     }
   }
@@ -383,16 +392,17 @@ this.NativeApp = class extends EventEmit
       disconnect: () => {
         if (this._isDisconnected) {
           throw new this.context.cloneScope.Error("Attempt to disconnect an already disconnected port");
         }
         this._cleanup();
       },
 
       postMessage: msg => {
+        msg = NativeApp.encodeMessage(this.context, msg);
         this.send(msg);
       },
 
       onDisconnect: new ExtensionUtils.SingletonEventManager(this.context, "native.onDisconnect", fire => {
         let listener = what => {
           this.context.runSafeWithoutClone(fire, port);
         };
         this.on("disconnect", listener);
@@ -437,8 +447,11 @@ this.NativeApp = class extends EventEmit
       responsePromise.catch(() => {});
 
       this._cleanup();
     });
 
     return result;
   }
 };
+
+XPCOMUtils.defineLazyPreferenceGetter(NativeApp, "maxRead", PREF_MAX_READ, MAX_READ);
+XPCOMUtils.defineLazyPreferenceGetter(NativeApp, "maxWrite", PREF_MAX_WRITE, MAX_WRITE);
--- a/toolkit/components/extensions/ext-runtime.js
+++ b/toolkit/components/extensions/ext-runtime.js
@@ -61,16 +61,17 @@ extensions.registerSchemaAPI("runtime", 
 
       connectNative(application) {
         let app = new NativeApp(extension, context, application);
         return app.portAPI();
       },
 
       sendNativeMessage(application, message) {
         let app = new NativeApp(extension, context, application);
+        message = NativeApp.encodeMessage(context, message);
         return app.sendMessage(message);
       },
 
       get lastError() {
         // TODO(robwu): Figure out how to make sure that errors in the parent
         // process are propagated to the child process.
         // lastError should not be accessed from the parent.
         return context.lastError;
--- a/toolkit/components/extensions/test/xpcshell/test_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/test_native_messaging.js
@@ -264,17 +264,18 @@ while True:
     let listener = (what, msg) => {
       equal(msg, MSG, "Received test message");
       app.off("message", listener);
       resolve();
     };
     app.on("message", listener);
   });
 
-  app.send(MSG);
+  let buffer = NativeApp.encodeMessage(context, MSG);
+  app.send(buffer);
   yield recvPromise;
 
   app._cleanup();
 
   do_print("waiting for async shutdown");
   Services.prefs.setBoolPref("toolkit.asyncshutdown.testing", true);
   AsyncShutdown.profileBeforeChange._trigger();
   Services.prefs.clearUserPref("toolkit.asyncshutdown.testing");