libnm/vpn: merge branch 'th/vpn-load-plugin-bgo765225'

Allow loading VPN plugins without absolute path.

Omit nm_vpn_editor_plugin_load() function from backporting
as that is 1.4 API.

https://bugzilla.gnome.org/show_bug.cgi?id=765225

libnm-core/Makefile.am

@@ -11,6 +11,7 @@ AM_CPPFLAGS = \
 	-DLOCALEDIR=\"$(datadir)/locale\" \
 	-DNMCONFDIR=\"$(nmconfdir)\" \
 	-DNMLIBDIR=\"$(nmlibdir)\" \
+	-DNMPLUGINDIR=\"$(pkglibdir)\" \
 	-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_LIB \
 	$(GLIB_CFLAGS) \
 	$(CODE_COVERAGE_CFLAGS)

libnm-core/nm-core-internal.h

@@ -72,6 +72,7 @@
 #include "nm-utils.h"
 #include "nm-vpn-dbus-interface.h"
 #include "nm-core-types-internal.h"
+#include "nm-vpn-editor-plugin.h"
 
 /* NM_SETTING_COMPARE_FLAG_INFERRABLE: check whether a device-generated
  * connection can be replaced by a already-defined connection. This flag only

libnm-core/nm-vpn-editor-plugin.c

@@ -71,69 +71,59 @@ nm_vpn_editor_plugin_default_init (NMVpnEditorPluginInterface *iface)
 
 /*********************************************************************/
 
-/**
- * nm_vpn_editor_plugin_load_from_file:
- * @plugin_filename: The path to the share library to load.
- *  Apply some common heuristics to find the library, such as
- *  appending "so" file ending.
- *  If the path is not an absolute path or no matching module
- *  can be found, lookup inside a directory defined at compile time.
- *  Due to this, @check_file might be called for two different paths.
- * @check_service: if not-null, check that the loaded plugin advertises
- *  the given service.
- * @check_owner: if non-negative, check whether the file is owned
- *  by UID @check_owner or by root. In this case also check that
- *  the file is not writable by anybody else.
- * @check_file: (scope call): optional callback to validate the file prior to
- *   loading the shared library.
- * @user_data: user data for @check_file
- * @error: on failure the error reason.
- *
- * Load the shared libary @plugin_filename and create a new
- * #NMVpnEditorPlugin instace via the #NMVpnEditorPluginFactory
- * function.
- *
- * Returns: (transfer full): a new plugin instance or %NULL on error.
- *
- * Since: 1.2
- */
-NMVpnEditorPlugin *
-nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
-                                      const char *check_service,
-                                      int check_owner,
-                                      NMUtilsCheckFilePredicate check_file,
-                                      gpointer user_data,
-                                      GError **error)
+static NMVpnEditorPlugin *
+_nm_vpn_editor_plugin_load (const char *plugin_name,
+                            gboolean do_file_checks,
+                            const char *check_service,
+                            int check_owner,
+                            NMUtilsCheckFilePredicate check_file,
+                            gpointer user_data,
+                            GError **error)
 {
 	GModule *module = NULL;
-	gs_free_error GError *local = NULL;
 	NMVpnEditorPluginFactory factory = NULL;
 	NMVpnEditorPlugin *editor_plugin = NULL;
+	gs_free char *plugin_filename_free = NULL;
+	const char *plugin_filename;
 
-	g_return_val_if_fail (plugin_filename && *plugin_filename, NULL);
+	g_return_val_if_fail (plugin_name && *plugin_name, NULL);
 
-	/* _nm_utils_check_module_file() fails with ENOENT if the plugin file
-	 * does not exist. That is relevant, because nm-applet checks for that. */
-	if (_nm_utils_check_module_file (plugin_filename,
-		                             check_owner,
-		                             check_file,
-		                             user_data,
-		                             &local))
-		module = g_module_open (plugin_filename, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
-
-	if (!module) {
-		if (local) {
-			g_propagate_error (error, local);
-			local = NULL;
-		} else {
-			g_set_error (error,
-			             NM_VPN_PLUGIN_ERROR,
-			             NM_VPN_PLUGIN_ERROR_FAILED,
-			             _("cannot load plugin %s"), plugin_filename);
+	/* if @do_file_checks is FALSE, we pass plugin_name directly to
+	 * g_module_open().
+	 *
+	 * Otherwise, we allow for library names without path component.
+	 * In which case, we prepend the plugin directory and form an
+	 * absolute path. In that case, we perform checks on the file.
+	 *
+	 * One exception is that we don't allow for the "la" suffix. The
+	 * reason is that g_module_open() interprets files with this extension
+	 * special and we don't want that. */
+	plugin_filename = plugin_name;
+	if (do_file_checks) {
+		if (   !strchr (plugin_name, '/')
+		    && !g_str_has_suffix (plugin_name, ".la")) {
+			plugin_filename_free = g_module_build_path (NMPLUGINDIR, plugin_name);
+			plugin_filename = plugin_filename_free;
 		}
+
+		/* _nm_utils_check_module_file() fails with ENOENT if the plugin file
+		 * does not exist. That is relevant, because nm-applet checks for that. */
+		if (!_nm_utils_check_module_file (plugin_filename,
+		                                  check_owner,
+		                                  check_file,
+		                                  user_data,
+		                                  error))
+			return NULL;
+	}
+
+	module = g_module_open (plugin_filename, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
+	if (!module) {
+		g_set_error (error,
+		             NM_VPN_PLUGIN_ERROR,
+		             NM_VPN_PLUGIN_ERROR_FAILED,
+		             _("cannot load plugin %s"), plugin_name);
 		return NULL;
 	}
-	g_clear_error (&local);
 
 	if (g_module_symbol (module, "nm_vpn_editor_plugin_factory", (gpointer) &factory)) {
 		gs_free_error GError *factory_error = NULL;
@@ -180,7 +170,7 @@ nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
 				g_set_error (error,
 				             NM_VPN_PLUGIN_ERROR,
 				             NM_VPN_PLUGIN_ERROR_FAILED,
-				             _("unknown error initializing plugin %s"), plugin_filename);
+				             _("unknown error initializing plugin %s"), plugin_name);
 			}
 		}
 
@@ -201,6 +191,52 @@ nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
 	return editor_plugin;
 }
 
+/**
+ * nm_vpn_editor_plugin_load_from_file:
+ * @plugin_name: The path or name of the shared library to load.
+ *  The path must either be an absolute filename to an existing file.
+ *  Alternatively, it can be the name (without path) of a library in the
+ *  plugin directory of NetworkManager.
+ * @check_service: if not-null, check that the loaded plugin advertises
+ *  the given service.
+ * @check_owner: if non-negative, check whether the file is owned
+ *  by UID @check_owner or by root. In this case also check that
+ *  the file is not writable by anybody else.
+ * @check_file: (scope call): optional callback to validate the file prior to
+ *   loading the shared library.
+ * @user_data: user data for @check_file
+ * @error: on failure the error reason.
+ *
+ * Load the shared libary @plugin_name and create a new
+ * #NMVpnEditorPlugin instace via the #NMVpnEditorPluginFactory
+ * function.
+ *
+ * If @plugin_name is not an absolute path name, it assumes the file
+ * is in the plugin directory of NetworkManager. In any case, the call
+ * will do certain checks on the file before passing it to dlopen.
+ * A consequence for that is, that you cannot omit the ".so" suffix.
+ *
+ * Returns: (transfer full): a new plugin instance or %NULL on error.
+ *
+ * Since: 1.2
+ */
+NMVpnEditorPlugin *
+nm_vpn_editor_plugin_load_from_file  (const char *plugin_name,
+                                      const char *check_service,
+                                      int check_owner,
+                                      NMUtilsCheckFilePredicate check_file,
+                                      gpointer user_data,
+                                      GError **error)
+{
+	return _nm_vpn_editor_plugin_load (plugin_name,
+	                                   TRUE,
+	                                   check_service,
+	                                   check_owner,
+	                                   check_file,
+	                                   user_data,
+	                                   error);
+}
+
 /*********************************************************************/
 
 /**

libnm-core/nm-vpn-editor-plugin.h

@@ -140,7 +140,7 @@ char         *nm_vpn_editor_plugin_get_suggested_filename (NMVpnEditorPlugin *pl
                                                            NMConnection *connection);
 
 NM_AVAILABLE_IN_1_2
-NMVpnEditorPlugin *nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
+NMVpnEditorPlugin *nm_vpn_editor_plugin_load_from_file  (const char *plugin_name,
                                                          const char *check_service,
                                                          int check_owner,
                                                          NMUtilsCheckFilePredicate check_file,

libnm-core/nm-vpn-plugin-info.c

@@ -620,12 +620,12 @@ nm_vpn_plugin_info_get_program (NMVpnPluginInfo *self)
 gboolean
 nm_vpn_plugin_info_supports_multiple (NMVpnPluginInfo *self)
 {
+	const char *s;
+
 	g_return_val_if_fail (NM_IS_VPN_PLUGIN_INFO (self), FALSE);
 
-	return g_key_file_get_boolean (NM_VPN_PLUGIN_INFO_GET_PRIVATE (self)->keyfile,
-	                               NM_VPN_PLUGIN_INFO_KF_GROUP_CONNECTION,
-	                               "supports-multiple-connections",
-	                               NULL);
+	s = nm_vpn_plugin_info_lookup_property (self, NM_VPN_PLUGIN_INFO_KF_GROUP_CONNECTION, "supports-multiple-connections");
+	return _nm_utils_ascii_str_to_bool (s, FALSE);
 }
 
 
@@ -877,15 +877,16 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error)
 		for (j = 0; keys && keys[j]; j++) {
 			char *s;
 
-			/* Lookup the value via get_string(). We want that behavior.
-			 * You could still lookup the original values via g_key_file_get_value()
-			 * based on priv->keyfile. */
+			/* Lookup the value via get_string(). We want that behavior for all our
+			 * values. */
 			s = g_key_file_get_string (priv->keyfile, groups[i], keys[j], NULL);
 			if (s)
 				g_hash_table_insert (priv->keys, _nm_utils_strstrdictkey_create (groups[i], keys[j]), s);
 		}
 	}
 
+	g_clear_pointer (&priv->keyfile, g_key_file_unref);
+
 	return TRUE;
 }
 
@@ -948,9 +949,10 @@ finalize (GObject *object)
 	g_free (priv->service);
 	g_strfreev (priv->aliases);
 	g_free (priv->filename);
-	g_key_file_unref (priv->keyfile);
 	g_hash_table_unref (priv->keys);
 
+	g_clear_pointer (&priv->keyfile, g_key_file_unref);
+
 	G_OBJECT_CLASS (nm_vpn_plugin_info_parent_class)->finalize (object);
 }
 

shared/nm-shared-utils.c

@@ -102,6 +102,40 @@ _nm_utils_ascii_str_to_int64 (const char *str, guint base, gint64 min, gint64 ma
 
 /*****************************************************************************/
 
+gint
+_nm_utils_ascii_str_to_bool (const char *str,
+                             gint default_value)
+{
+	gsize len;
+	char *s = NULL;
+
+	if (!str)
+		return default_value;
+
+	while (str[0] && g_ascii_isspace (str[0]))
+		str++;
+
+	if (!str[0])
+		return default_value;
+
+	len = strlen (str);
+	if (g_ascii_isspace (str[len - 1])) {
+		s = g_strdup (str);
+		g_strchomp (s);
+		str = s;
+	}
+
+	if (!g_ascii_strcasecmp (str, "true") || !g_ascii_strcasecmp (str, "yes") || !g_ascii_strcasecmp (str, "on") || !g_ascii_strcasecmp (str, "1"))
+		default_value = TRUE;
+	else if (!g_ascii_strcasecmp (str, "false") || !g_ascii_strcasecmp (str, "no") || !g_ascii_strcasecmp (str, "off") || !g_ascii_strcasecmp (str, "0"))
+		default_value = FALSE;
+	if (s)
+		g_free (s);
+	return default_value;
+}
+
+/*****************************************************************************/
+
 G_DEFINE_QUARK (nm-utils-error-quark, nm_utils_error)
 
 void

shared/nm-shared-utils.h

@@ -26,6 +26,9 @@
 
 gint64 _nm_utils_ascii_str_to_int64 (const char *str, guint base, gint64 min, gint64 max, gint64 fallback);
 
+gint _nm_utils_ascii_str_to_bool (const char *str,
+                                  gint default_value);
+
 /******************************************************************************/
 
 /**

src/nm-config.c

@@ -125,7 +125,7 @@ gint
 nm_config_parse_boolean (const char *str,
                          gint default_value)
 {
-	return nm_utils_ascii_str_to_bool (str, default_value);
+	return _nm_utils_ascii_str_to_bool (str, default_value);
 }
 
 gint

src/nm-core-utils.c

@@ -154,40 +154,6 @@ _nm_singleton_instance_register_destruction (GObject *instance)
 
 /*****************************************************************************/
 
-gint
-nm_utils_ascii_str_to_bool (const char *str,
-                            gint default_value)
-{
-	gsize len;
-	char *s = NULL;
-
-	if (!str)
-		return default_value;
-
-	while (str[0] && g_ascii_isspace (str[0]))
-		str++;
-
-	if (!str[0])
-		return default_value;
-
-	len = strlen (str);
-	if (g_ascii_isspace (str[len - 1])) {
-		s = g_strdup (str);
-		g_strchomp (s);
-		str = s;
-	}
-
-	if (!g_ascii_strcasecmp (str, "true") || !g_ascii_strcasecmp (str, "yes") || !g_ascii_strcasecmp (str, "on") || !g_ascii_strcasecmp (str, "1"))
-		default_value = TRUE;
-	else if (!g_ascii_strcasecmp (str, "false") || !g_ascii_strcasecmp (str, "no") || !g_ascii_strcasecmp (str, "off") || !g_ascii_strcasecmp (str, "0"))
-		default_value = FALSE;
-	if (s)
-		g_free (s);
-	return default_value;
-}
-
-/*****************************************************************************/
-
 /*
  * nm_ethernet_address_is_valid:
  * @addr: pointer to a binary or ASCII Ethernet address

src/nm-core-utils.h

@@ -91,11 +91,6 @@ GETTER (void) \
 
 /*****************************************************************************/
 
-gint nm_utils_ascii_str_to_bool (const char *str,
-                                 gint default_value);
-
-/*****************************************************************************/
-
 gboolean nm_ethernet_address_is_valid (gconstpointer addr, gssize len);
 
 in_addr_t nm_utils_ip4_address_clear_host_address (in_addr_t addr, guint8 plen);