Hi Sonny,
On Sat, Sep 14, 2019 at 5:57 PM Sonny Sasaka <sonnysasaka@xxxxxxxxx> wrote:
>
> Currently at the start of client creation (g_dbus_client_new), the
> |owner| in |filter_data| is not set until the |name| is resolved. This
> creates a time window where the filter doesn't work properly, i.e. it
> filters in more than it should. To solve this issue, this patch does the
> following:
> 1. At the start of client creation, set the |owner| in |filter_data|
> based on the current resolved |name| if any, or set it explicitly to
> unknown (empty string) as opposed to NULL otherwise. The unknown |owner|
> lets the filter reject any message, unlike NULL |owner| which accepts
> any message.
> 2. Step 1 above reveals another bug: message_filter fails to accept
> messages which have |sender| set directly to D-Bus service name rather
> than D-Bus address. Therefore this patch relaxes the filter requirement
> in message_filter to accept a message if its |sender| is equal directly
> to our filter's |name|.
> 3. After the initial service name resolution (after GetNameOwner)
> returns, immediately update our name cache with the result, otherwise
> the filters' |owner| would be stuck to unknown (empty string) until
> "NameOwnerChanged" signal arrives.
>
> ---
> gdbus/watch.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/gdbus/watch.c b/gdbus/watch.c
> index 447e48671..2ae0fd5a7 100644
> --- a/gdbus/watch.c
> +++ b/gdbus/watch.c
> @@ -78,6 +78,8 @@ struct filter_data {
> gboolean registered;
> };
>
> +static const char *check_name_cache(const char *name);
> +
> static struct filter_data *filter_data_find_match(DBusConnection *connection,
> const char *name,
> const char *owner,
> @@ -265,7 +267,10 @@ proceed:
>
> data->connection = dbus_connection_ref(connection);
> data->name = g_strdup(name);
> - data->owner = g_strdup(owner);
> + if (name)
> + data->owner = g_strdup(check_name_cache(name) ? : "");
I follow this it would ignore the owner address and use the cache name
or set "" to be resolved shouldn't that use the owner instead? If the
called don't have it resolved then it should optionally set the owner
resolution.
> + else
> + data->owner = g_strdup(owner);
> data->path = g_strdup(path);
> data->interface = g_strdup(interface);
> data->member = g_strdup(member);
> @@ -534,8 +539,12 @@ static DBusHandlerResult
> message_filter(DBusConnection *connection,
> if (!sender && data->owner)
> continue;
>
> - if (data->owner && g_str_equal(sender, data->owner) == FALSE)
> + if (data->owner &&
> + g_str_equal(sender, data->owner) == FALSE &&
> + data->name &&
> + g_str_equal(sender, data->name) == FALSE) {
iirc messages never use the friendly name only the bus connection as
sender so I wonder if this really does make any difference, are there
any example of this not working? Perhaps it would be worth creating a
test case in unit/test-gdbus.c to capture this case.
> continue;
> + }
>
> if (data->path && g_str_equal(path, data->path) == FALSE)
> continue;
> @@ -627,6 +636,7 @@ static void service_reply(DBusPendingCall *call,
> void *user_data)
> DBUS_TYPE_INVALID) == FALSE)
> goto fail;
>
> + update_name_cache(data->name, data->owner);
> update_service(data);
>
> goto done;
> --
> 2.21.0
--
Luiz Augusto von Dentz
