Bugzilla – Bug 34870
Add path_prefix match rule
Last modified: 2011-04-07 11:30:01 UTC
This is needed for an optimized implementation of the client-side of the proposed ObjectManager interface, see bug 34869. See
See bug 34870 for the path_prefix bug. FWIW, I'd like to commit a version of the spec patch without path_prefix before resolving bug 34870... should be fine, exiting patch only mentions path_prefix in a non-normative way. Once we resolve bug 34870 we can change that section to use path_prefix again.
(In reply to comment #1)
> See bug 34870 for the path_prefix bug. FWIW, I'd like to commit a version of
> the spec patch without path_prefix before resolving bug 34870... should be
> fine, exiting patch only mentions path_prefix in a non-normative way. Once we
> resolve bug 34870 we can change that section to use path_prefix again.
(This was supposed to go to on bug 34869, not this bug. Sorry about that.)
Created attachment 44235 [details] [review]
Here's a patch that does this.
For the record, GDBusProxyManager (which will be part of GLib 2.30) now uses this match rule when available
Patch looks fine to me.
Review of attachment 44235 [details] [review]:
@@ +3724,3 @@
+ <entry>An object path optionally ending in a slash</entry>
It'd be good if the semantics here were the same as the semantics of arg0namespace in Bug #24317, either by changing this or by changing arg0namespace.
Should the pattern /foo/bar match /foo/bar/baz?
Should the pattern /foo/bar match /foo/bar?
Should the pattern /foo/bar match /foo/barrier?
Should the pattern /foo/bar/ match /foo/bar/baz?
Should the pattern /foo/bar/ match /foo/bar?
Should the pattern /foo/bar/ match /foo/barrier?
It appears your answers are: yes, yes, yes, yes, no, no.
The answers analogous to Will's proposed patch for Bug #24317 would be: yes, yes, no, yes, no, no.
However, we propose to simplify arg0namespace to "has the given namespace + "." as a prefix", which would change the answers to: yes, no, no, disallowed, disallowed, disallowed.
It's not clear to me whether your GDBus patch requires that path_prefix="/foo/bar" matches /foo/bar itself. Does it? I wouldn't particularly mind having that as a special case, i.e. "yes, yes, no, disallowed, disallowed, disallowed".
One thing Will has thought about in Bug #24317 is the effect on hypothetical kernel D-Bus:
> I made the semantics specific to bus name/interface namespaces; in effect, the
> rule has to match up to (and possibly including) a period in the message
> argument. I also narrowed the scope to matching the zeroeth argument. This
> makes its purpose and semantics clearer.
> (It also means that in a hypothetical Kernel D-Bus future where the bus daemon
> is replaced by multicast Unix sockets, with clients using socket filters to
> filter messages based on match rules, we only need to add a header containing
> each period-separated prefix of the zeroth argument (if it's a string), rather
> than every prefix of every string argument.)
Requiring implementations of match rules to support arbitrary prefixes, as opposed to prefixes at any "/" boundary, would require more kernel-level headers for this situation, I think?
@@ +3729,3 @@
+ Matches messages which are sent from or to an
+ object for which the object path is a prefix of
+ the given value. Examples of matches are
I think it's worth making this more explicit by saying which of the things above would be matched by /foo/bar (or /com/example/Application/ObjectManager or whatever).
I recommend using example paths in the example.com or example.net namespace, which would have the side-effect of reminding people that the syntax is not "everything starts with org" :-)
@@ +3737,3 @@
+ This match key was added in version 0.15 of the
+ D-Bus specification and implemented by the bus
+ daemon in dbus 1.4.7 and later.
I don't think this should be added to the 1.4.x stable branch; if we're adding things like this, there's no point in having a stable branch at all. Save it for 1.5.
Created attachment 45376 [details] [review]
path_prefix: anchor matches at path-component boundaries, and give examples
It seems wrong that path_prefix="/foo" matches /foobar, and it isn't
difficult or expensive to check.
Created attachment 45377 [details] [review]
specification: fix versioning
We've added things since 0.15, so this isn't still 0.15.
As discussed on IRC, I'm totally OK with this if we rename it to path_namespace. I still think the namespace rules are harder to grasp than simple prefix rules, but that's probably an ok trade-off for consistency... Thanks!
Created attachment 45382 [details] [review]
Rename path_prefix to path_namespace and disallow trailing '/'
Also disallow having both path and path_namespace in the same match rule
(it wouldn't make sense, path is more specific than path_namespace).
As per IRC discussion with davidz and wjt.
Created attachment 45384 [details] [review]
Check parsing (or otherwise) of path_namespace in match rules
Created attachment 45385 [details] [review]
Mention dbus-specification.xml's separate versioning in HACKING
(In reply to comment #10)
> Created an attachment (id=45382) [details]
> Rename path_prefix to path_namespace and disallow trailing '/'
> Also disallow having both path and path_namespace in the same match rule
> (it wouldn't make sense, path is more specific than path_namespace).
> As per IRC discussion with davidz and wjt.
Looks great to me. Thanks a bunch for doing this.
(In reply to comment #12)
> Created an attachment (id=45385) [details]
> Mention dbus-specification.xml's separate versioning in HACKING
Excellent idea. Looks good to me. Thanks
Fixed in git for 1.5.0
(In reply to comment #15)
> Fixed in git for 1.5.0
Cool, thanks! I can verify that this works in the ObjectManager implementation (and its test suite) destined for GLib 2.30. The change is here