From 4d669715dda43db1fe069e7a99f9358cf0efebb2 Mon Sep 17 00:00:00 2001 From: Soumya Ranjan Patnaik Date: Mon, 17 Jun 2024 00:41:11 +0530 Subject: [PATCH] portal-impl: fix config ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quoting portals.conf(5): > Each key in the group contains a semi-colon separated list of portal backend > implementation, to be searched for an implementation of the requested interface, > in the same order as specified in the configuration file. But this wasn't actually true. If the portals were set to z;y and z and y both implemented the same portal, y would be used. Fixing this requires reworking how portals are selected — going through the config file and selecting the first available configured portal, rather than going through each known portal and checking whether it's allowed in the config file. find_all_portal_implementations() is unchanged. The portal-first approach is fine for it, and it would be difficult for it to share as much code with find_portal_implementation() now that it's written to stop as soon as a configured portal is found. Fixes: https://github.com/flatpak/xdg-desktop-portal/issues/1111 Co-authored-by: Alyssa Ross --- src/portal-impl.c | 151 ++++++++++++++++++++++++++++++---------------- 1 file changed, 98 insertions(+), 53 deletions(-) diff --git a/src/portal-impl.c b/src/portal-impl.c index 8c1fd5353..7942192d4 100644 --- a/src/portal-impl.c +++ b/src/portal-impl.c @@ -535,63 +535,107 @@ portal_interface_prefers_none (const char *interface) } static gboolean -portal_impl_name_matches (const PortalImplementation *impl, - const PortalInterface *iface) +portal_impl_supports_iface (const PortalImplementation *impl, + const char *interface) { - /* Exact match */ - if (g_strv_contains ((const char * const *) iface->portals, impl->source)) - { - g_debug ("Found '%s' in configuration for %s", impl->source, iface->dbus_name); - return TRUE; - } + return g_strv_contains ((const char * const *) impl->interfaces, interface); +} - /* The "*" alias means "any" */ - if (g_strv_contains ((const char * const *) iface->portals, "*")) +static void +warn_please_use_portals_conf (void) +{ + g_warning_once ("The preferred method to match portal implementations " + "to desktop environments is to use the portals.conf(5) " + "configuration file"); +} + +static PortalImplementation * +find_any_portal_implementation (const char *interface) +{ + for (const GList *l = implementations; l != NULL; l = l->next) { - g_debug ("Found '*' in configuration for %s", iface->dbus_name); - return TRUE; + PortalImplementation *impl = l->data; + + if (!portal_impl_supports_iface (impl, interface)) + continue; + + g_debug ("Falling back to %s.portal for %s", impl->source, interface); + return impl; } - /* No portal */ - if (portal_interface_prefers_none (iface->dbus_name)) + return NULL; +} + +static PortalImplementation * +find_portal_implementation_by_name (const char *portal_name) +{ + if (portal_name == NULL) + return NULL; + + for (const GList *l = implementations; l != NULL; l = l->next) { - g_debug ("Found 'none' in configuration for %s", iface->dbus_name); - return FALSE; + PortalImplementation *impl = l->data; + + if (g_str_equal (impl->source, portal_name)) + return impl; } - return FALSE; + g_debug("Requested %s.portal is unrecognized", portal_name); + return NULL; } -static gboolean -portal_impl_matches_config (const PortalImplementation *impl, - const char *interface) +static PortalImplementation * +find_portal_implementation_iface (const PortalInterface *iface) { - if (config == NULL) - return FALSE; + if (iface == NULL) + return NULL; - /* Interfaces have precedence, followed by the "default" catch all, - * to allow for specific interfaces to override the default - */ - for (int i = 0; i < config->n_ifaces; i++) + for (size_t i = 0; iface->portals && iface->portals[i]; i++) { - const PortalInterface *iface = config->interfaces[i]; + const char *portal = iface->portals[i]; - if (g_strcmp0 (iface->dbus_name, interface) == 0) - return portal_impl_name_matches (impl, iface); - } + g_debug ("Found '%s' in configuration for %s", portal, iface->dbus_name); - if (config->default_portal) - return portal_impl_name_matches (impl, config->default_portal); + if (g_str_equal (portal, "*")) + return find_any_portal_implementation (iface->dbus_name); - return FALSE; + PortalImplementation *impl = find_portal_implementation_by_name (portal); + + if (!portal_impl_supports_iface (impl, iface->dbus_name)) + { + g_info ("Requested backend %s.portal does not support %s. Skipping...", + impl->source, iface->dbus_name); + continue; + } + + return impl; + } + + return NULL; } -static void -warn_please_use_portals_conf (void) +static PortalImplementation * +find_default_implementation_iface (const char *interface) { - g_warning_once ("The preferred method to match portal implementations " - "to desktop environments is to use the portals.conf(5) " - "configuration file"); + if (config == NULL || config->default_portal == NULL) + return NULL; + + PortalInterface *iface = config->default_portal; + for (size_t i = 0; iface->portals && iface->portals[i]; i++) + { + const char *portal = iface->portals[i]; + + g_debug ("Found '%s' in configuration for default", portal); + + if (g_str_equal (portal, "*")) + return find_any_portal_implementation (iface->dbus_name); + + PortalImplementation *impl = find_portal_implementation_by_name (portal); + + if (portal_impl_supports_iface (impl, interface)) + return impl; + } + return NULL; } PortalImplementation * @@ -604,14 +648,15 @@ find_portal_implementation (const char *interface) if (portal_interface_prefers_none (interface)) return NULL; - for (l = implementations; l != NULL; l = l->next) + if (config) { - PortalImplementation *impl = l->data; + PortalInterface *iface = find_matching_iface_config (interface); + PortalImplementation *impl = find_portal_implementation_iface (iface); - if (!g_strv_contains ((const char **)impl->interfaces, interface)) - continue; + if (!impl) + impl = find_default_implementation_iface(interface); - if (portal_impl_matches_config (impl, interface)) + if (impl != NULL) { g_debug ("Using %s.portal for %s (config)", impl->source, interface); return impl; @@ -627,7 +672,7 @@ find_portal_implementation (const char *interface) { PortalImplementation *impl = l->data; - if (!g_strv_contains ((const char **)impl->interfaces, interface)) + if (!portal_impl_supports_iface(impl, interface)) continue; if (impl->use_in != NULL && g_strv_case_contains ((const char **)impl->use_in, desktops[i])) @@ -654,7 +699,7 @@ find_portal_implementation (const char *interface) if (!g_str_equal (impl->dbus_name, "org.freedesktop.impl.portal.desktop.gtk")) continue; - if (!g_strv_contains ((const char **)impl->interfaces, interface)) + if (!portal_impl_supports_iface (impl, interface)) continue; g_warning ("Choosing %s.portal for %s as a last-resort fallback", @@ -670,26 +715,26 @@ GPtrArray * find_all_portal_implementations (const char *interface) { GPtrArray *impls; - GList *l; impls = g_ptr_array_new (); if (portal_interface_prefers_none (interface)) return impls; - for (l = implementations; l != NULL; l = l->next) - { - PortalImplementation *impl = l->data; + if (config) + { + PortalInterface *iface = find_matching_iface_config (interface); + PortalImplementation *impl = find_portal_implementation_iface (iface); - if (!g_strv_contains ((const char **)impl->interfaces, interface)) - continue; + if (!impl) + impl = find_default_implementation_iface(interface); - if (portal_impl_matches_config (impl, interface)) + if (impl != NULL) { g_debug ("Using %s.portal for %s (config)", impl->source, interface); g_ptr_array_add (impls, impl); } - } + } return impls; }