Bug 1299411 - Error messages for native messaging
- 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
--- a/toolkit/components/extensions/NativeMessaging.jsm
+++ b/toolkit/components/extensions/NativeMessaging.jsm
@@ -158,16 +158,20 @@ this.HostManifestManager = {
if (!VALID_APPLICATION.test(application)) {
throw new Error(`Invalid application "${application}"`);
}
return this.init().then(() => this._lookup(application, context));
},
};
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();
this.context = context;
this.name = application;
// We want a close() notification when the window is destroyed.
this.context.callOnClose(this);
@@ -175,22 +179,20 @@ this.NativeApp = class extends EventEmit
this.proc = null;
this.readPromise = null;
this.sendQueue = [];
this.writePromise = null;
this.sentDisconnect = false;
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;
if (AppConstants.platform == "win") {
// OS.Path.join() ignores anything before the last absolute path
// it sees, so if command is already absolute, it remains unchanged
// here. If it is relative, we get the proper absolute path here.
command = OS.Path.join(OS.Path.dirname(hostInfo.path), command);
@@ -225,17 +227,17 @@ this.NativeApp = class extends EventEmit
* @param {string} portId A unique internal ID that identifies the port.
* @param {object} sender The object describing the creator of the connection
* request.
* @param {string} application The name of the native messaging host.
*/
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));
/* eslint-enable mozilla/balanced-listeners */
port.registerOnMessage(msg => app.send(msg));
port.registerOnDisconnect(msg => app.close());
}
@@ -264,17 +266,17 @@ this.NativeApp = class extends EventEmit
_startRead() {
if (this.readPromise) {
throw new Error("Entered _startRead() while readPromise is non-null");
}
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 => {
this.emit("message", msg);
this.readPromise = null;
this._startRead();
}).catch(err => {
if (err.errorCode != Subprocess.ERROR_END_OF_FILE) {
@@ -395,16 +397,19 @@ this.NativeApp = class extends EventEmit
if (this.proc) {
doCleanup();
} else if (this.startupPromise) {
this.startupPromise.then(doCleanup);
}
if (!this.sentDisconnect) {
this.sentDisconnect = true;
+ if (err && err.errorCode == Subprocess.ERROR_END_OF_FILE) {
+ err = null;
+ }
this.emit("disconnect", err);
}
}
// Called from Context when the extension is shut down.
close() {
this._cleanup();
}
--- a/toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_native_messaging.js
@@ -324,17 +324,19 @@ add_task(function* test_read_limit() {
function clearPref() {
Services.prefs.clearUserPref(PREF_MAX_READ);
}
do_register_cleanup(clearPref);
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 => {
browser.test.sendMessage("result", "message");
});
port.postMessage(PAYLOAD);
}
@@ -378,17 +380,19 @@ add_task(function* test_ext_permission()
yield extension.unload();
});
// Test that an extension that is not listed in allowed_extensions for
// a native application cannot use that application.
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 => {
browser.test.sendMessage("result", "message");
});
port.postMessage({test: "test"});
}
@@ -439,17 +443,19 @@ add_task(function* test_child_process()
let exitPromise = waitForSubprocessExit();
yield extension.unload();
yield exitPromise;
});
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");
});
}
let {messages} = yield promiseConsoleOutput(function* () {
let extension = ExtensionTestUtils.loadExtension({
background,
manifest: {