sd-bus: fix path of object-manager signals

Each signal of the ObjectManager interface carries the path of the object
in question as an argument. Therefore, a caller will deduce the object
this signal is generated for, by parsing the _argument_. A caller will
*not* use the object-path of the message itself (i.e., message->path).
This is done on purpose, so the caller can rely on message->path to be
the path of the actual object-manager that generated this signal, instead
of the path of the object that triggered this signal.

This commit fixes all InterfacesAdded/Removed signals to use the path of
the closest object-manager as message->path. 'closest' in this case means
closest parent with at least one object-manager registered.

This fix raises the question what happens if we stack object-managers in
a hierarchy. Two implementations are possible: First, we report each
object only on the nearest object-manager. Second, we report it on each
parent object-manager. This patch chooses the former. This is compatible
with other existing ObjectManager implementations, which are required to
call GetManagedObjects() recursively on each object they find, which
implements the ObjectManager interface.

2 files changed, 84 insertions, 16 deletions

diff --git a/src/libelogind/sd-bus/bus-objects.c b/src/libelogind/sd-bus/bus-objects.c
index cbdf6552f..7e3cd86f6 100644
--- a/src/libelogind/sd-bus/bus-objects.c
+++ b/src/libelogind/sd-bus/bus-objects.c
@@ -167,6 +167,7 @@ static int add_subtree_to_set(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
+                bool skip_subhierarchies,
                 Set *s,
                 sd_bus_error *error) {
 
@@ -198,11 +199,13 @@ static int add_subtree_to_set(
                 if (r < 0 && r != -EEXIST)
                         return r;
 
-                r = add_subtree_to_set(bus, prefix, i, s, error);
-                if (r < 0)
-                        return r;
-                if (bus->nodes_modified)
-                        return 0;
+                if (!skip_subhierarchies || !i->object_managers) {
+                        r = add_subtree_to_set(bus, prefix, i, skip_subhierarchies, s, error);
+                        if (r < 0)
+                                return r;
+                        if (bus->nodes_modified)
+                                return 0;
+                }
         }
 
         return 0;
@@ -212,6 +215,7 @@ static int get_child_nodes(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
+                bool skip_subhierarchies,
                 Set **_s,
                 sd_bus_error *error) {
 
@@ -227,7 +231,7 @@ static int get_child_nodes(
         if (!s)
                 return -ENOMEM;
 
-        r = add_subtree_to_set(bus, prefix, n, s, error);
+        r = add_subtree_to_set(bus, prefix, n, skip_subhierarchies, s, error);
         if (r < 0) {
                 set_free_free(s);
                 return r;
@@ -892,7 +896,7 @@ static int process_introspect(
         assert(n);
         assert(found_object);
 
-        r = get_child_nodes(bus, m->path, n, &s, &error);
+        r = get_child_nodes(bus, m->path, n, false, &s, &error);
         if (r < 0)
                 return bus_maybe_reply_error(m, r, &error);
         if (bus->nodes_modified)
@@ -1158,7 +1162,7 @@ static int process_get_managed_objects(
         if (require_fallback || !n->object_managers)
                 return 0;
 
-        r = get_child_nodes(bus, m->path, n, &s, &error);
+        r = get_child_nodes(bus, m->path, n, true, &s, &error);
         if (r < 0)
                 return r;
         if (bus->nodes_modified)
@@ -1467,6 +1471,32 @@ void bus_node_gc(sd_bus *b, struct node *n) {
         free(n);
 }
 
+static int bus_find_parent_object_manager(sd_bus *bus, struct node **out, const char *path) {
+        struct node *n;
+
+        assert(bus);
+        assert(path);
+
+        n = hashmap_get(bus->nodes, path);
+        if (!n) {
+                char *prefix;
+
+                prefix = alloca(strlen(path) + 1);
+                OBJECT_PATH_FOREACH_PREFIX(prefix, path) {
+                        n = hashmap_get(bus->nodes, prefix);
+                        if (n)
+                                break;
+                }
+        }
+
+        while (n && !n->object_managers)
+                n = n->parent;
+
+        if (out)
+                *out = n;
+        return !!n;
+}
+
 static int bus_add_object(
                 sd_bus *bus,
                 sd_bus_slot **slot,
@@ -2269,6 +2299,7 @@ _public_ int sd_bus_emit_object_added(sd_bus *bus, const char *path) {
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         int r;
 
         /*
@@ -2289,11 +2320,17 @@ _public_ int sd_bus_emit_object_added(sd_bus *bus, const char *path) {
         if (!BUS_IS_OPEN(bus->state))
                 return -ENOTCONN;
 
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
         do {
                 bus->nodes_modified = false;
                 m = sd_bus_message_unref(m);
 
-                r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
+                r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
                 if (r < 0)
                         return r;
 
@@ -2432,6 +2469,7 @@ _public_ int sd_bus_emit_object_removed(sd_bus *bus, const char *path) {
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         int r;
 
         /*
@@ -2452,11 +2490,17 @@ _public_ int sd_bus_emit_object_removed(sd_bus *bus, const char *path) {
         if (!BUS_IS_OPEN(bus->state))
                 return -ENOTCONN;
 
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
         do {
                 bus->nodes_modified = false;
                 m = sd_bus_message_unref(m);
 
-                r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
+                r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
                 if (r < 0)
                         return r;
 
@@ -2588,6 +2632,7 @@ _public_ int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, ch
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         char **i;
         int r;
 
@@ -2601,11 +2646,17 @@ _public_ int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, ch
         if (strv_isempty(interfaces))
                 return 0;
 
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
         do {
                 bus->nodes_modified = false;
                 m = sd_bus_message_unref(m);
 
-                r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
+                r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
                 if (r < 0)
                         return r;
 
@@ -2665,6 +2716,7 @@ _public_ int sd_bus_emit_interfaces_added(sd_bus *bus, const char *path, const c
 
 _public_ int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path, char **interfaces) {
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         int r;
 
         assert_return(bus, -EINVAL);
@@ -2677,7 +2729,13 @@ _public_ int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path,
         if (strv_isempty(interfaces))
                 return 0;
 
-        r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
+        r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
         if (r < 0)
                 return r;
 
diff --git a/src/libelogind/sd-bus/test-bus-objects.c b/src/libelogind/sd-bus/test-bus-objects.c
index 1db67ecfa..359984c7f 100644
--- a/src/libelogind/sd-bus/test-bus-objects.c
+++ b/src/libelogind/sd-bus/test-bus-objects.c
@@ -153,7 +153,7 @@ static int notify_test2(sd_bus_message *m, void *userdata, sd_bus_error *error)
 static int emit_interfaces_added(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_interfaces_added(sd_bus_message_get_bus(m), m->path, "org.freedesktop.systemd.test", NULL) >= 0);
+        assert_se(sd_bus_emit_interfaces_added(sd_bus_message_get_bus(m), "/value/a/x", "org.freedesktop.systemd.ValueTest", NULL) >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -164,7 +164,7 @@ static int emit_interfaces_added(sd_bus_message *m, void *userdata, sd_bus_error
 static int emit_interfaces_removed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_interfaces_removed(sd_bus_message_get_bus(m), m->path, "org.freedesktop.systemd.test", NULL) >= 0);
+        assert_se(sd_bus_emit_interfaces_removed(sd_bus_message_get_bus(m), "/value/a/x", "org.freedesktop.systemd.ValueTest", NULL) >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -175,7 +175,7 @@ static int emit_interfaces_removed(sd_bus_message *m, void *userdata, sd_bus_err
 static int emit_object_added(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_object_added(sd_bus_message_get_bus(m), m->path) >= 0);
+        assert_se(sd_bus_emit_object_added(sd_bus_message_get_bus(m), "/value/a/x") >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -186,7 +186,7 @@ static int emit_object_added(sd_bus_message *m, void *userdata, sd_bus_error *er
 static int emit_object_removed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_object_removed(sd_bus_message_get_bus(m), m->path) >= 0);
+        assert_se(sd_bus_emit_object_removed(sd_bus_message_get_bus(m), "/value/a/x") >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -228,6 +228,14 @@ static int enumerator_callback(sd_bus *bus, const char *path, void *userdata, ch
         return 1;
 }
 
+static int enumerator2_callback(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) {
+
+        if (object_path_startswith("/value/a", path))
+                assert_se(*nodes = strv_new("/value/a/x", "/value/a/y", "/value/a/z", NULL));
+
+        return 1;
+}
+
 static void *server(void *p) {
         struct context *c = p;
         sd_bus *bus = NULL;
@@ -246,7 +254,9 @@ static void *server(void *p) {
         assert_se(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.test2", vtable, c) >= 0);
         assert_se(sd_bus_add_fallback_vtable(bus, NULL, "/value", "org.freedesktop.systemd.ValueTest", vtable2, NULL, UINT_TO_PTR(20)) >= 0);
         assert_se(sd_bus_add_node_enumerator(bus, NULL, "/value", enumerator_callback, NULL) >= 0);
+        assert_se(sd_bus_add_node_enumerator(bus, NULL, "/value/a", enumerator2_callback, NULL) >= 0);
         assert_se(sd_bus_add_object_manager(bus, NULL, "/value") >= 0);
+        assert_se(sd_bus_add_object_manager(bus, NULL, "/value/a") >= 0);
 
         assert_se(sd_bus_start(bus) >= 0);