Created attachment 39415 [details] [review] [PATCH]: fix property handling when one introspection XML contains multiple interfaces I've found that when introspection XML contains multiple interfaces then when properties of 2nd (and next) interfaces are queried, the error message is spit: Error org.freedesktop.DBus.Error.AccessDenied: Interface "<interface name>" isn't exported (or may not exist), can't access property "<property name>" It looks like a regression caused by commit: 510bdcd63ae4e588a5cb72727696d5ad7fd5298b "Respect property access flags for writing, allow disabling for reads" The error can be seen during the following steps: 1) Establish a VPN connection with NetworkManager 2) Run these commands: dbus-send --system --print-reply --dest=org.freedesktop.NetworkManager /org/freedesktop/NetworkManager/ActiveConnection/2 org.freedesktop.DBus.Properties.Get string:org.freedesktop.NetworkManager.VPN.Connection string:Banner dbus-send --system --print-reply --dest=org.freedesktop.NetworkManager /org/freedesktop/NetworkManager/ActiveConnection/2 org.freedesktop.DBus.Properties.Get string:org.freedesktop.NetworkManager.VPN.Connection string:VpnState (of course change "2" to your actual active VPN connection) The patch in attachment fixes the issue.
I did a patch too which was more ambitious and also has some testcases along with it. Attaching for posterity.
Created attachment 42473 [details] [review] fix handling of properties from multiple interfaces
Created attachment 42478 [details] [review] Another take at the problem (Updated with clearer meaning for the restrictions in lookup_property_name)
Comment on attachment 42473 [details] [review] fix handling of properties from multiple interfaces >lookup_object_info_by_iface_cb() incorrectly checked only the first >property when looking up the object info for a given interface, causing >secondary interfaces to fail lookup, and thus Get/Set/GetAll requests >for the secondary interfaces to fail. Ouch. >If legacy property access was disabled, lookup_property_name() should >not have returned properties that did not match the requested interface. > >Third, in _dbus_gutils_wincaps_to_uscore(), a '-' should be replaced >with a '_' so that requests for "foobar-baz" get converted into >"foobar_baz" to match GObject property names used internally. This is a big change actually...I seem to recall making a similar change before and backing it out. >+ else > { >- lookup_data->info = info; >- lookup_data->iface_type = gtype; >+ const char *propsig; >+ >+ /* Iterate over properties until we find one with the requested interface */ It's a layering violation to do this inside lookup_object_info_by_iface_cb; it's currently not property-specific. We may need to add an enum { LOOKUP_MODE_NONE, LOOKUP_MODE_PROPERTY } LookupMode; ? >+ propsig = property_iterate (propsig, object_info->format_version, &piface, &pname, &puscore, &access_type); >+ if (g_strcmp0 (piface, wincaps_propiface) == 0 && >+ (g_strcmp0 (pname, requested_propname) == 0 || >+ g_strcmp0 (puscore, requested_propname) == 0 || >+ g_strcmp0 (pname, uscore_name) == 0 || >+ g_strcmp0 (puscore, uscore_name) == 0)) >+ { >+ found = TRUE; >+ break; >+ } >+ } This one might be a candidate for a helper function?
So for NetworkManager we fixed our code to not have multiple interfaces in the same XML; so feel free to close this bug if you like. If we don't plan to fix this (since it's pretty clearly broken and has been forever) then perhaps the binding tool should error out if there are multiple interfaces in the same XML.
Just going to close this since we don't care much about it anymore, and I'm sure there are higher priority things to fix.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.