search

bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
667 stars 78 forks source link

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

Open mvollmer opened 1 year ago

mvollmer commented 1 year ago

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 commented 1 year ago

(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.)

dvdhrm commented 1 year ago

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!

mvollmer commented 1 year ago

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!

smcv commented 2 months ago

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.)