Bug 1299411 - separate serialization from sending r=aswan
authorRob Wu <rob@robwu.nl>
Sat, 24 Sep 2016 13:03:20 +0200
changeset 320062 a9c19ee017a44aeb9953858b663a100a85ccffab
parent 320061 0ce4a6653d1966333fa72a29f762fdde5614d02f
child 320063 57f7a5c7044db9fba2cd039773ac058b7269aac5
push id20749
push userryanvm@gmail.com
push dateSat, 29 Oct 2016 13:21:21 +0000
treeherderfx-team@1b170b39ed6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1299411
milestone52.0a1
Bug 1299411 - separate serialization from sending r=aswan 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
@@ -77,16 +77,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");