cache name owners and discriminate signals on owner

Create a name cache. Cache the name owner (the unique name) when the
user creates a proxy object client and keep the cache up to date when
the name owner changes on the NameOwnerChanged standard DBus signal.

Then when a signal comes in, make sure that the sender for the signal
actually owns the name based on the `sender` field in the message and
only emit the signal on the proxy interface if it matches.

This fixes a bug where if two external connections take different names
but export the same objects with the same interface names (common for
DBus interface standards), the signal would be emitted on all proxy
objects that emit for that path/interface combination regardless of
whether it was for the name they are listening on.

See the new test for a precise description of the error case.

Author: Tony Crisci

lib/bus.js

@@ -26,6 +26,23 @@ module.exports = function bus(conn, opts) {
   this.signals = new EventEmitter();
   this.exportedObjects = {};
 
+  this.nameOwners = {};
+
+  this._handleNameOwnerChanged = function(msg) {
+    let {sender, path, iface, member} = msg;
+    if (sender !== 'org.freedesktop.DBus' ||
+          path !== '/org/freedesktop/DBus' ||
+          iface !== 'org.freedesktop.DBus' ||
+          member !== 'NameOwnerChanged') {
+      return;
+    }
+    let [name, oldOwner, newOwner] = msg.body;
+    if (name.startsWith(':')) {
+      return;
+    }
+    this.nameOwners[name] = newOwner;
+  };
+
   this.invoke = function(msg, callback) {
     if (!msg.type) msg.type = constants.messageType.methodCall;
     msg.serial = self.serial++;
@@ -150,7 +167,8 @@ module.exports = function bus(conn, opts) {
         }
       }
     } else if (msg.type === constants.messageType.signal) {
-      self.signals.emit(self.mangle(msg), msg.body, msg.signature);
+      self._handleNameOwnerChanged(msg);
+      self.signals.emit(self.mangle(msg), msg);
     } else {
       // methodCall
       if (handleMethod(msg, self)) {
@@ -397,6 +415,19 @@ module.exports = function bus(conn, opts) {
     );
   };
 
+  this.cacheNameOwner = function(name) {
+    let that = this;
+    return new Promise((resolve, reject) => {
+      this.getNameOwner(name, function(err, owner) {
+        if (err) {
+          return reject(err);
+        }
+        that.nameOwners[name] = owner;
+        return resolve(owner);
+      });
+    });
+  }
+
   this.nameHasOwner = function(name, callback) {
     this.invokeDbus(
       { member: 'NameHasOwner', signature: 's', body: [name] },

lib/client/proxy-object.js

@@ -51,7 +51,11 @@ class ProxyObject {
               interface: iface.$name,
               member: signal.name
             });
-            this.bus.signals.on(event, (body, signature) => {
+            this.bus.signals.on(event, (msg) => {
+              let {body, signature, sender} = msg;
+              if (this.bus.nameOwners[this.name] !== sender) {
+                return;
+              }
               if (signature !== signal.signature) {
                 console.error(`warning: got signature ${signature} for signal ${iface.$name}.${signal.name} (expected ${signal.signature})`);
                 return;
@@ -72,7 +76,7 @@ class ProxyObject {
   }
 
   _init() {
-    return new Promise((resolve, reject) => {
+    let introspect = new Promise((resolve, reject) => {
       this.bus.invoke(
         {
           destination: this.name,
@@ -99,6 +103,16 @@ class ProxyObject {
         }
       );
     });
+
+    return new Promise((resolve, reject) => {
+      Promise.all([introspect, this.bus.cacheNameOwner(this.name)])
+        .then((result) => {
+          return resolve(result[0]);
+        })
+        .catch((err) => {
+          return reject(err);
+        });
+    });
   }
 
   getInterface(name) {

test/integration/signals.test.js

@@ -10,10 +10,12 @@ let {
 } = dbus.interface;
 
 const TEST_NAME = 'org.test.name';
+const TEST_NAME2 = 'org.test.name2';
 const TEST_PATH = '/org/test/path';
 const TEST_IFACE = 'org.test.iface';
 
 let bus = dbus.sessionBus();
+let bus2 = dbus.sessionBus();
 
 class SignalsInterface extends Interface {
   @signal({signature: 's'})
@@ -64,23 +66,29 @@ class SignalsInterface extends Interface {
 }
 
 let testIface = new SignalsInterface(TEST_IFACE);
+let testIface2 = new SignalsInterface(TEST_IFACE);
 
 beforeAll(async () => {
-  let name = await bus.requestName(TEST_NAME);
+  let [name, name2] = await Promise.all([
+    bus.requestName(TEST_NAME),
+    bus2.requestName(TEST_NAME2)
+  ]);
   name.export(TEST_PATH, testIface);
+  name2.export(TEST_PATH, testIface2);
 });
 
 afterAll(() => {
   bus.connection.stream.end();
+  bus2.connection.stream.end();
 });
 
 test('test that signals work correctly', async () => {
   let object = await bus.getProxyObject(TEST_NAME, TEST_PATH);
   let test = object.getInterface(TEST_IFACE);
 
-  let onHelloWorld = jest.fn(() => {});
-  let onSignalMultiple = jest.fn(() => {});
-  let onSignalComplicated = jest.fn(() => {});
+  let onHelloWorld = jest.fn();
+  let onSignalMultiple = jest.fn();
+  let onSignalComplicated = jest.fn();
 
   test.on('HelloWorld', onHelloWorld);
   test.on('SignalMultiple', onSignalMultiple);
@@ -92,3 +100,34 @@ test('test that signals work correctly', async () => {
   expect(onSignalMultiple).toHaveBeenCalledWith('hello', 'world');
   expect(onSignalComplicated).toHaveBeenCalledWith(testIface.complicated);
 });
+
+test('signals dont get mixed up between names that define objects on the same path and interface', async () => {
+  // Note that there is a really bad case where a single connection takes two
+  // names and exports the same interfaces and paths on them. Then there is no
+  // way to tell the signals apart from the names because the messages look
+  // identical to us. All we get is the unique name of the sender and not the
+  // well known name, and the well known name is what will be different. For
+  // this reason, I am going to recommend that people only use one name per bus
+  // connection until we can figure that out.
+  let object = await bus.getProxyObject(TEST_NAME, TEST_PATH);
+  let object2 = await bus.getProxyObject(TEST_NAME2, TEST_PATH);
+
+  let test = object.getInterface(TEST_IFACE);
+  let test2 = object2.getInterface(TEST_IFACE);
+
+  let cb = jest.fn();
+  let cb2 = jest.fn();
+
+  test.on('HelloWorld', cb);
+  test.on('SignalMultiple', cb);
+  test.on('SignalComplicated', cb);
+
+  test2.on('HelloWorld', cb2);
+  test2.on('SignalMultiple', cb2);
+  test2.on('SignalComplicated', cb2);
+
+  await test.EmitSignals();
+
+  expect(cb).toHaveBeenCalledTimes(3);
+  expect(cb2).toHaveBeenCalledTimes(0);
+});