dbus-broker icon indicating copy to clipboard operation
dbus-broker copied to clipboard

AddMatch with pathname_space='/' doesn't match anything

Open mvollmer opened this issue 2 years ago • 4 comments

A rule that contains path_namespace='/' will never match anything, while I would expect this to be a wild-card, equivalent to omitting path_namespace altogether.

This probably happens because match_string_prefix ("/foo", "/", '/', false) returns false.

The workaround is of course trivial: never include path_namespace='/' in a AddMatch rule, and I don't think there is any code out there that would need path_namespace='/' to work. I am mostly filing this issue to document this edge case and be able to point to it in our code.

mvollmer avatar Jan 02 '23 12:01 mvollmer

(If you want to support this, setting keys->path_namespace to the empty string when parsing path_namespace='/' seems like a good solution. match_string_prefix ("/foo", "", '/', false) returns true.)

mvollmer avatar Jan 02 '23 12:01 mvollmer

Ugh! Thanks for reporting this. We intend to fully follow dbus-daemon in match-behavior, so we should mirror what dbus-daemon does. Did you check what the behavior there is? Otherwise, I might just look it up myself.

Thanks a lot! Currently on parental-leave, will see about this when back!

dvdhrm avatar Jan 04 '23 12:01 dvdhrm

We intend to fully follow dbus-daemon in match-behavior, so we should mirror what dbus-daemon does. Did you check what the behavior there is?

No, I did not.

Currently on parental-leave, will see about this when back!

Awesome! Parental leave is best leave!

mvollmer avatar Jan 04 '23 13:01 mvollmer

We intend to fully follow dbus-daemon in match-behavior, so we should mirror what dbus-daemon does. Did you check what the behavior there is?

dbus-daemon initially had the equivalent of this issue, but it was acknowledged to be a bug (in other words, we thought @mvollmer's interpretation of the spec was the correct one) and fixed in 2013: https://bugs.freedesktop.org/show_bug.cgi?id=70799

The workaround is of course trivial: never include path_namespace='/' in a AddMatch rule

This is less trivial if you are implementing some generic interface like ObjectManager where path_namespace='%s' is usually wanted, and you have to add a different code path for the uncommon case where the path is the root.

For example in GLib, working around the equivalent dbus-daemon bug (which also works around this issue) needed this change: https://gitlab.gnome.org/GNOME/glib/-/commit/3b28df1e008101341504f82c8e65f3109aca10cc

In practice this is most likely to happen with org.bluez, because having an ObjectManager at the root object path / is discouraged but org.bluez does it anyway.

A small reproducer: dbus-broker309.py.txt.

To reproduce: run it, with python3-gi or equivalent installed. If the session bus is dbus-daemon (tested with 1.14.10 on Debian 12), it receives two signals and exits. If the session bus is dbus-broker (tested with v36 on Arch Linux), it receives the first signal with path /, then waits forever for the second one with path /com/example/Beehive. If run with parameter --workaround, both dbus-daemon and dbus-broker work as expected.

(I ran into this with the Steam Runtime, which for historical reasons and ancient-distro compatibility includes a very old fallback copy of 32-bit GLib, which is too old to have the workaround for the equivalent dbus-daemon bug.)

smcv avatar May 03 '24 17:05 smcv