Properties.Get fallibility

[Simon McVittie]

On Fri, 28 Jul 2023 at 09:32:02 +0200, David Rheinsberg wrote:
> On Thu, Jul 27, 2023, at 5:04 PM, Illia Volchanetskyi6439 wrote:
> > Is replying with an error to an org.freedesktop.DBus.Properties.Get 
> > call okay? I assumed that it was and now I am running into some 
> > difficulties.
> 
> It is definitely ok to reply with an error. I think the bigger question
> is what exactly the cascading effect will be.

As a matter of "what is in the widely interoperable subset of D-Bus?",
I would recommend that D-Bus interface designers should avoid properties
that are not always readable: replying with an error is allowed, but
many client libraries will cope poorly with that.

In particular, if a client library represents D-Bus Properties as some
runtime system's properties (GObject, Qt or similar), it will probably
be difficult or even impossible to represent a failing Get() in the
client library's representation, because something like g_object_get()
has no way to report failure. Clients that use those client libraries
will probably have to bypass the client library's normal handling of
D-Bus Properties and call Get()/GetAll() themselves.

Write-only properties (access="write") are something else that is allowed,
but best avoided when aiming for wide interoperability.

> The sd-bus interface, as an example, allows marking fields to:
>  - be of constant value
>  - never trigger `changed` notifications
>  - send `changed` notifications, but not include the value (called 'invalidation')

Those are definitely fine, the D-Bus Specification has specific support
for those.

>  - be omitted from batch-requests (called 'explicit')

I would advise using this cautiously or not at all in code that is meant
to be widely interoperable: for round-trip reduction (which is the purpose
of GetAll()), I would expect it to be very common for client libraries to
assume that GetAll() does indeed return all of the properties as its name
would suggest.

> > The library that I am using, 
> > which implements GetAll for me, replies with the error to GetAll calls 
> > in this case, which makes GetAll less useful as the "client" would have 
> > to fall back to reading the properties one by one with Get.
> 
> This is a valid decision for a library to make.

Actually, the D-Bus Specification says implementations shouldn't do this:

    If org.freedesktop.DBus.Properties.GetAll [...] is called with a
    valid interface name for which some properties are not accessible
    to the caller (for example, due to per-property access control
    implemented in the service), those properties should be silently
    omitted from the result array. If org.freedesktop.DBus.Properties.Get
    is called for any such properties, an appropriate access control
    error should be returned.

> Similarly, a library
> is free to decide to drop fields on error, but still continue serving the
> `GetAll` request.

That's what the spec recommends. For example if your interface looks
like this:

    property Public: s, readable       # always available
    property Private: s, readable      # only available to privileged clients

then GetAll() and Get() should return one of these:

    # for a client that is allowed to read Private
    GetAll(your interface) -> { "Public": <"hello">, "Private": <"p4ssw0rd"> }
    Get(your interface, "Public") -> <"hello">
    Get(your interface, "Private") -> <"p4ssw0rd">

    # for a client that is not allowed to read Private
    GetAll(your interface) -> { "Public": <"hello"> }
    Get(your interface, "Public") -> <"hello">
    Get(your interface, "Private") -> error

Similarly, if your interface has methods and/or signals but no properties,
the spec recommends that GetAll() should return an empty dictionary
(technically an empty array of dict-entries) without error.

> > The last issue is InterfacesAdded not being emitted, which I am relying 
> > upon. The signal must include the properties and their values. 
> > Similarly to GetAll, the expected behaviour in case of an error is not 
> > clear to me. Not sending the signal at all does not look good to me.
> 
> This is part of the ObjectManager interface, rather than the Properties
> interface, so this requires separate setup, right?

It's reasonable for a library to reuse generic properties-handling code
for its implementation of ObjectManager.

The spec says the signal should report the same properties as GetAll().
Because it's a broadcast signal, if some properties are considered to be
secret and are available to some but not all clients, then the
implementation would have to emit InterfacesAdded with the subset of
properties that are available to every client (so, for my example above,
Public would be included but Private would not).

    smcv