From df1cd7312845772fe3ad7ce5ec751a1dee017db2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Jun 2015 12:06:14 +0200 Subject: [PATCH] config: fix usage of g_key_file_get_value() vs. g_key_file_get_string() g_key_file_get_value() returns the raw value as stored in the file. When accessing a string value, in most cases it is correct to use g_key_file_get_string() instead. When working with internals, such as comparing two keyfiles for equality, g_key_file_get_value() is correct. When parsing booleans, we parse it based on the raw value. Fix the usages. This is a change in behavior if the config file contained unusual strings. (cherry picked from commit 0abb502ff389704e5979c6b16f88129ec5609be6) --- src/nm-config-data.c | 39 ++++++++++++++------------- src/nm-config.c | 23 +++++++++------- src/nm-config.h | 2 +- src/settings/plugins/keyfile/plugin.c | 2 +- 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 97cc36217b..e78d547201 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -110,10 +110,12 @@ nm_config_data_get_config_description (const NMConfigData *self) char * nm_config_data_get_value (const NMConfigData *self, const char *group, const char *key) { - g_return_val_if_fail (self, NULL); + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), NULL); g_return_val_if_fail (group && *group, NULL); g_return_val_if_fail (key && *key, NULL); + /* nm_config_data_get_value() translates to g_key_file_get_string(), because we want + * to use the string representation, not the (raw) GKeyFile value. */ return g_key_file_get_string (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); } @@ -123,7 +125,12 @@ nm_config_data_get_value_boolean (const NMConfigData *self, const char *group, c char *str; gint value = default_value; - str = nm_config_data_get_value (self, group, key); + g_return_val_if_fail (NM_IS_CONFIG_DATA (self), default_value); + g_return_val_if_fail (group && *group, default_value); + g_return_val_if_fail (key && *key, default_value); + + /* when parsing the boolean, base it on the raw value from g_key_file_get_value(). */ + str = g_key_file_get_value (NM_CONFIG_DATA_GET_PRIVATE (self)->keyfile, group, key, NULL); if (str) { value = nm_config_parse_boolean (str, default_value); g_free (str); @@ -313,7 +320,7 @@ nm_config_data_get_connection_default (const NMConfigData *self, char *value; gboolean match; - value = g_key_file_get_value (priv->keyfile, connection_info->group_name, property, NULL); + value = g_key_file_get_string (priv->keyfile, connection_info->group_name, property, NULL); if (!value && !connection_info->stop_match) continue; @@ -331,17 +338,13 @@ nm_config_data_get_connection_default (const NMConfigData *self, static void _get_connection_info_init (ConnectionInfo *connection_info, GKeyFile *keyfile, char *group) { - char *value; - /* pass ownership of @group on... */ connection_info->group_name = group; - value = g_key_file_get_value (keyfile, group, "match-device", NULL); - if (value) { - connection_info->match_device.has = TRUE; - connection_info->match_device.spec = nm_match_spec_split (value); - g_free (value); - } + connection_info->match_device.spec = nm_config_get_device_match_spec (keyfile, + group, + "match-device", + &connection_info->match_device.has); connection_info->stop_match = nm_config_keyfile_get_boolean (keyfile, group, "stop-match", FALSE); } @@ -568,23 +571,23 @@ constructed (GObject *object) priv->connection_infos = _get_connection_infos (priv->keyfile); - priv->connectivity.uri = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL); - priv->connectivity.response = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); + priv->connectivity.uri = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL); + priv->connectivity.response = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); /* On missing config value, fallback to 300. On invalid value, disable connectivity checking by setting * the interval to zero. */ - interval = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); + interval = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); priv->connectivity.interval = interval ? _nm_utils_ascii_str_to_int64 (interval, 10, 0, G_MAXUINT, 0) : NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL; g_free (interval); - priv->dns_mode = g_key_file_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL); + priv->dns_mode = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL); - priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier"); - priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only"); + priv->ignore_carrier = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier", NULL); + priv->assume_ipv6ll_only = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only", NULL); - priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default"); + priv->no_auto_default.specs_config = nm_config_get_device_match_spec (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default", NULL); G_OBJECT_CLASS (nm_config_data_parent_class)->constructed (object); } diff --git a/src/nm-config.c b/src/nm-config.c index b82a817b17..6cd5328ab7 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -780,14 +780,14 @@ read_entire_config (const NMConfigCmdLineOptions *cli, g_strfreev (plugins_tmp); if (cli && cli->configure_and_quit) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", "true"); + g_key_file_set_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", TRUE); if (cli && cli->connectivity_uri && cli->connectivity_uri[0]) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", cli->connectivity_uri); + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", cli->connectivity_uri); if (cli && cli->connectivity_interval >= 0) g_key_file_set_integer (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", cli->connectivity_interval); if (cli && cli->connectivity_response && cli->connectivity_response[0]) - g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response); + g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", cli->connectivity_response); *out_config_main_file = o_config_main_file; *out_config_description = o_config_description; @@ -795,11 +795,16 @@ read_entire_config (const NMConfigCmdLineOptions *cli, } GSList * -nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key) +nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key, gboolean *out_has_key) { gs_free char *value = NULL; - value = g_key_file_get_string ((GKeyFile *) keyfile, group, key, NULL); + /* nm_match_spec_split() already supports full escaping and is basically + * a modified version of g_key_file_parse_value_as_string(). So we first read + * the raw value (g_key_file_get_value()), and do the parsing ourselves. */ + value = g_key_file_get_value ((GKeyFile *) keyfile, group, key, NULL); + if (out_has_key) + *out_has_key = !!value; return nm_match_spec_split (value); } @@ -1011,12 +1016,12 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->auth_polkit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "auth-polkit", NM_CONFIG_DEFAULT_AUTH_POLKIT); - priv->dhcp_client = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL); + priv->dhcp_client = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dhcp", NULL); - priv->log_level = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL); - priv->log_domains = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL); + priv->log_level = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "level", NULL); + priv->log_domains = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains", NULL); - priv->debug = g_key_file_get_value (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "debug", NULL); + priv->debug = g_key_file_get_string (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "debug", NULL); priv->configure_and_quit = nm_config_keyfile_get_boolean (keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "configure-and-quit", FALSE); diff --git a/src/nm-config.h b/src/nm-config.h index 67d375c141..448e2f4e2d 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -121,7 +121,7 @@ void nm_config_keyfile_set_string_list (GKeyFile *keyfile, const char *key, const char *const* strv, gssize len); -GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key); +GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key, gboolean *out_has_key); G_END_DECLS diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index cbfb9f500f..7f21e71e67 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -591,7 +591,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) key_file = nm_config_create_keyfile (); if (parse_key_file_allow_none (priv, key_file, &error)) - specs = nm_config_get_device_match_spec (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices"); + specs = nm_config_get_device_match_spec (key_file, NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices", NULL); if (error) { nm_log_warn (LOGD_SETTINGS, "keyfile: error getting unmanaged specs: %s", error->message);