config: only handle 'option+' and 'option-' keys for known settings

It is wrong to blindly merge keys that have an 'option+' or 'option-'.
Merging options is only possibly when we understand what the option
means and how to merge it.

No longer handle every setting but only those that are explicitly known
to be string-lists (or device-specs).

(cherry picked from commit a1ea678f7801188f3a1850a319dcc249a4c3bd37)

6 files changed, 134 insertions, 31 deletions

diff --git a/src/nm-config.c b/src/nm-config.c
index c5be85a85f..c318eea790 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -529,6 +529,27 @@ _sort_groups_cmp (const char **pa, const char **pb, gpointer dummy)
 }
 
 static gboolean
+_setting_is_device_spec (const char *group, const char *key)
+{
+#define _IS(group_v, key_v) (strcmp (group, (""group_v)) == 0 && strcmp (key, (""key_v)) == 0)
+	return    _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "no-auto-default")
+	       || _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "ignore-carrier")
+	       || _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "assume-ipv6ll-only")
+	       || _IS (NM_CONFIG_KEYFILE_GROUP_KEYFILE, "unmanaged-devices")
+	       || (g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION) && !strcmp (key, "match-device"));
+}
+
+static gboolean
+_setting_is_string_list (const char *group, const char *key)
+{
+	return    _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "plugins")
+	       || _IS (NM_CONFIG_KEYFILE_GROUP_MAIN, "debug")
+	       || _IS (NM_CONFIG_KEYFILE_GROUP_LOGGING, "domains")
+	       || g_str_has_prefix (group, NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST);
+#undef _IS
+}
+
+static gboolean
 read_config (GKeyFile *keyfile, const char *path, GError **error)
 {
 	GKeyFile *kf;
@@ -593,29 +614,72 @@ read_config (GKeyFile *keyfile, const char *path, GError **error)
 			if (   key_len > 1
 			    && (last_char == '+' || last_char == '-')) {
 				gs_free char *base_key = g_strndup (key, key_len - 1);
-				gs_strfreev  char **old_val = g_key_file_get_string_list (keyfile, group, base_key, NULL, NULL);
-				gs_free char **new_val = g_key_file_get_string_list (kf, group, key, NULL, NULL);
-				gs_unref_ptrarray GPtrArray *new = g_ptr_array_new_with_free_func (g_free);
-				char **iter_val;
-
-				for (iter_val = old_val; iter_val && *iter_val; iter_val++) {
-					if (   last_char != '-'
-					    || _nm_utils_strv_find_first (new_val, -1, *iter_val) < 0)
-						g_ptr_array_add (new, g_strdup (*iter_val));
+				gboolean is_string_list;
+
+				is_string_list = _setting_is_string_list (group, base_key);
+
+				if (   is_string_list
+				    || _setting_is_device_spec (group, base_key)) {
+					gs_unref_ptrarray GPtrArray *new = g_ptr_array_new_with_free_func (g_free);
+					char **iter_val;
+					gs_strfreev  char **old_val = NULL;
+					gs_free char **new_val = NULL;
+
+					if (is_string_list) {
+						old_val = g_key_file_get_string_list (keyfile, group, base_key, NULL, NULL);
+						new_val = g_key_file_get_string_list (kf, group, key, NULL, NULL);
+					} else {
+						gs_free char *old_sval = nm_config_keyfile_get_value (keyfile, group, base_key, NM_CONFIG_GET_VALUE_TYPE_SPEC);
+						gs_free char *new_sval = nm_config_keyfile_get_value (kf, group, key, NM_CONFIG_GET_VALUE_TYPE_SPEC);
+						gs_free_slist GSList *old_specs = nm_match_spec_split (old_sval);
+						gs_free_slist GSList *new_specs = nm_match_spec_split (new_sval);
+
+						/* the key is a device spec. This is a special kind of string-list, that
+						 * we must split differently. */
+						old_val = _nm_utils_slist_to_strv (old_specs, FALSE);
+						new_val = _nm_utils_slist_to_strv (new_specs, FALSE);
+					}
+
+					/* merge the string lists, by omiting duplicates. */
+
+					for (iter_val = old_val; iter_val && *iter_val; iter_val++) {
+						if (   last_char != '-'
+						    || _nm_utils_strv_find_first (new_val, -1, *iter_val) < 0)
+							g_ptr_array_add (new, g_strdup (*iter_val));
+					}
+					for (iter_val = new_val; iter_val && *iter_val; iter_val++) {
+						/* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */
+						if (   last_char == '+'
+						    && _nm_utils_strv_find_first (old_val, -1, *iter_val) < 0)
+							g_ptr_array_add (new, *iter_val);
+						else
+							g_free (*iter_val);
+					}
+
+					if (new->len > 0) {
+						if (is_string_list)
+							nm_config_keyfile_set_string_list (keyfile, group, base_key, (const char *const*) new->pdata, new->len);
+						else {
+							gs_free_slist GSList *specs = NULL;
+							gs_free char *specs_joined = NULL;
+
+							g_ptr_array_add (new, NULL);
+							specs = _nm_utils_strv_to_slist ((char **) new->pdata, FALSE);
+
+							specs_joined = nm_match_spec_join (specs);
+
+							g_key_file_set_value (keyfile, group, base_key, specs_joined);
+						}
+					} else {
+						if (is_string_list)
+							g_key_file_remove_key (keyfile, group, base_key, NULL);
+						else
+							g_key_file_set_value (keyfile, group, base_key, "");
+					}
+				} else {
+					/* For any other settings we don't support extending the option with +/-.
+					 * Just drop the key. */
 				}
-				for (iter_val = new_val; iter_val && *iter_val; iter_val++) {
-					/* don't add duplicates. That means an "option=a,b"; "option+=a,c" results in "option=a,b,c" */
-					if (   last_char == '+'
-					    && _nm_utils_strv_find_first (old_val, -1, *iter_val) < 0)
-						g_ptr_array_add (new, *iter_val);
-					else
-						g_free (*iter_val);
-				}
-
-				if (new->len > 0)
-					nm_config_keyfile_set_string_list (keyfile, group, base_key, (const char *const*) new->pdata, new->len);
-				else
-					g_key_file_remove_key (keyfile, group, base_key, NULL);
 				continue;
 			}
 
diff --git a/src/nm-config.h b/src/nm-config.h
index 20dbab388f..5b096f165d 100644
--- a/src/nm-config.h
+++ b/src/nm-config.h
@@ -49,6 +49,7 @@ G_BEGIN_DECLS
 #define NM_CONFIG_KEYFILE_LIST_SEPARATOR ','
 
 #define NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION            "connection"
+#define NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST ".test-append-stringlist"
 
 #define NM_CONFIG_KEYFILE_GROUP_MAIN                        "main"
 #define NM_CONFIG_KEYFILE_GROUP_LOGGING                     "logging"
diff --git a/src/tests/config/conf.d/00-overrides.conf b/src/tests/config/conf.d/00-overrides.conf
index cb0116edd8..f26ed93b9a 100644
--- a/src/tests/config/conf.d/00-overrides.conf
+++ b/src/tests/config/conf.d/00-overrides.conf
@@ -1,9 +1,16 @@
 [main]
 dhcp=dhcpcd
 
+no-auto-default=spec1,spec2
+ignore-carrier=\s space1 \s
+
 [logging]
 domains=PLATFORM,DNS,WIFI
 
+[appendable-test]
+non-appendable-key1+=i-will-be-dropped
+non-appendable-key2-=i-will-be-dropped
+
 [order]
 a=0
 b=0
@@ -32,7 +39,7 @@ ord.key08=B-1.3.08
 ord.key09=B-1.3.09
 
 
-[append]
+[.test-append-stringlist.1]
 val1=a,b
 
 val2-=VAL2
diff --git a/src/tests/config/conf.d/10-more.conf b/src/tests/config/conf.d/10-more.conf
index a1959c1948..eadb7f96f6 100644
--- a/src/tests/config/conf.d/10-more.conf
+++ b/src/tests/config/conf.d/10-more.conf
@@ -1,5 +1,12 @@
 [main]
 extra=hello
+
+no-auto-default-=spec1
+no-auto-default+=spec3
+
+ignore-carrier+=\sspace2\t	
+
+[.test-append-stringlist.0]
 new+=something
 
 [connectivity]
@@ -29,5 +36,5 @@ ord.key09=C-2.3.09
 ord.ovw01=C-0.1.ovw01
 
 
-[append]
+[.test-append-stringlist.1]
 val1-=b
diff --git a/src/tests/config/conf.d/90-last.conf b/src/tests/config/conf.d/90-last.conf
index c75dcc4710..7d078788de 100644
--- a/src/tests/config/conf.d/90-last.conf
+++ b/src/tests/config/conf.d/90-last.conf
@@ -4,5 +4,5 @@ plugins+=one,two
 [order]
 a=90
 
-[append]
+[.test-append-stringlist.1]
 val1+=c,a
diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c
index 24592071ef..8b8028e63a 100644
--- a/src/tests/config/test-config.c
+++ b/src/tests/config/test-config.c
@@ -283,6 +283,7 @@ test_config_confdir (void)
 	NMConfig *config;
 	const char **plugins;
 	char *value;
+	GSList *specs;
 
 	config = setup_config (NULL, SRCDIR "/NetworkManager.conf", SRCDIR "/conf.d", NULL);
 
@@ -305,7 +306,23 @@ test_config_confdir (void)
 	g_assert_cmpstr (value, ==, "hello");
 	g_free (value);
 
-	value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "new", NM_CONFIG_GET_VALUE_NONE);
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "no-auto-default", NM_CONFIG_GET_VALUE_TYPE_SPEC);
+	specs = nm_match_spec_split (value);
+	g_free (value);
+	g_assert_cmpint (g_slist_length (specs), ==, 2);
+	g_assert_cmpstr (g_slist_nth_data (specs, 0), ==, "spec2");
+	g_assert_cmpstr (g_slist_nth_data (specs, 1), ==, "spec3");
+	g_slist_free_full (specs, g_free);
+
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), "main", "ignore-carrier", NM_CONFIG_GET_VALUE_TYPE_SPEC);
+	specs = nm_match_spec_split (value);
+	g_free (value);
+	g_assert_cmpint (g_slist_length (specs), ==, 2);
+	g_assert_cmpstr (g_slist_nth_data (specs, 0), ==, "  space1  ");
+	g_assert_cmpstr (g_slist_nth_data (specs, 1), ==, " space2\t");
+	g_slist_free_full (specs, g_free);
+
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".0", "new", NM_CONFIG_GET_VALUE_NONE);
 	g_assert_cmpstr (value, ==, "something"); /* not ",something" */
 	g_free (value);
 
@@ -319,6 +336,13 @@ test_config_confdir (void)
 	g_assert_cmpstr (value, ==, "0");
 	g_free (value);
 
+	g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key1", NM_CONFIG_GET_VALUE_RAW));
+	g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key1+", NM_CONFIG_GET_VALUE_RAW));
+	g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key1-", NM_CONFIG_GET_VALUE_RAW));
+	g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key2", NM_CONFIG_GET_VALUE_RAW));
+	g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key2+", NM_CONFIG_GET_VALUE_RAW));
+	g_assert (!nm_config_data_has_value (nm_config_get_data_orig (config), "appendable-test", "non-appendable-key2-", NM_CONFIG_GET_VALUE_RAW));
+
 #define ASSERT_GET_CONN_DEFAULT(xconfig, xname, xvalue) \
 	G_STMT_START { \
 		gs_free char *_value = nm_config_data_get_connection_default (nm_config_get_data_orig (xconfig), (xname), NULL); \
@@ -336,23 +360,23 @@ test_config_confdir (void)
 	ASSERT_GET_CONN_DEFAULT (config, "ord.key09", "C-2.1.09");
 	ASSERT_GET_CONN_DEFAULT (config, "ord.ovw01", "C-0.1.ovw01");
 
-	value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val1", NM_CONFIG_GET_VALUE_NONE);
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val1", NM_CONFIG_GET_VALUE_NONE);
 	g_assert_cmpstr (value, ==, "a,c");
 	g_free (value);
 
-	value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val2", NM_CONFIG_GET_VALUE_NONE);
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val2", NM_CONFIG_GET_VALUE_NONE);
 	g_assert_cmpstr (value, ==, "VAL2");
 	g_free (value);
 
-	value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val3", NM_CONFIG_GET_VALUE_NONE);
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val3", NM_CONFIG_GET_VALUE_NONE);
 	g_assert_cmpstr (value, ==, NULL);
 	g_free (value);
 
-	value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val4", NM_CONFIG_GET_VALUE_NONE);
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val4", NM_CONFIG_GET_VALUE_NONE);
 	g_assert_cmpstr (value, ==, "vb,vb");
 	g_free (value);
 
-	value = nm_config_data_get_value (nm_config_get_data_orig (config), "append", "val5", NM_CONFIG_GET_VALUE_NONE);
+	value = nm_config_data_get_value (nm_config_get_data_orig (config), NM_CONFIG_KEYFILE_GROUPPREFIX_TEST_APPEND_STRINGLIST".1", "val5", NM_CONFIG_GET_VALUE_NONE);
 	g_assert_cmpstr (value, ==, "VAL5");
 	g_free (value);