core: fix match spec behavior for a list of all "except:"

If the spec specifies only negative matches (and none of them matches),
then the result shall be positive.

Meaning:

    [connection*] match-device=except:dhcp-plugin:dhclient
    [connection*] match-device=except:interface-name:eth0
    [.config] enabled=except:nm-version:1.14

should be the same as:

    [connection*] match-device=*,except:dhcp-plugin:dhclient
    [connection*] match-device=*,except:interface-name:eth0
    [.config] enabled=*,except:nm-version:1.14

and match by default. Previously, such specs would never yield a
positive match, which seems wrong.

Note that "except:" already has a special meaning. It is not merely
"not:". That is because we don't support "and:" nor grouping, but all
matches are combined by an implicit "or:". With such a meaning, having
a "not:" would be unclear to define. Instead it is defined that any
"except:" match always wins and makes the entire condition to explicitly
not match. As such, it makes sense to treat a match that only consists
of "except:" matches special.

This is a change in behavior, but the alternative meaning makes
little sense.

3 files changed, 81 insertions, 32 deletions

diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml
index 65ebe013ad..e22aa20ad1 100644
--- a/man/NetworkManager.conf.xml
+++ b/man/NetworkManager.conf.xml
@@ -1217,6 +1217,8 @@ enable=env:TAG1
               be used to negate the match. Note that if one except-predicate
               matches, the entire configuration will be disabled.
               In other words, a except predicate always wins over other predicates.
+              If the setting only consists of "except:" matches and none of the
+              negative conditions are satisfied, the configuration is still enabled.
 <programlisting>
 # enable the configuration either when the environment variable
 # is present or the version is at least 1.2.0.
@@ -1411,7 +1413,11 @@ enable=nm-version-min:1.3,nm-version-min:1.2.6,nm-version-min:1.0.16
           <term>except:SPEC</term>
           <listitem><para>Negative match of a device. <literal>SPEC</literal> must be explicitly qualified with
              a prefix such as <literal>interface-name:</literal>. A negative match has higher priority then the positive
-             matches above.</para></listitem>
+             matches above.</para>
+             <para>If there is a list consisting only of negative matches, the behavior is the same as if there
+             is also match-all. That means, if none of all the negative matches is satisfied, the overall result is
+             still a positive match. That means, <literal>"except:interface-name:eth0"</literal> is the same as
+             <literal>"*,except:interface-name:eth0"</literal>.</para></listitem>
         </varlistentry>
         <varlistentry>
           <term>SPEC[,;]SPEC</term>
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 1c939495e6..6f168da3ae 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -1303,6 +1303,34 @@ match_device_hwaddr_eval (const char *spec_str,
 		_has; \
 	})
 
+static NMMatchSpecMatchType
+_match_result (gboolean has_except,
+               gboolean has_not_except,
+               gboolean has_match,
+               gboolean has_match_except)
+{
+	if (   has_except
+	    && !has_not_except) {
+		/* a match spec that only consists of a list of except matches is treated specially. */
+		nm_assert (!has_match);
+		if (has_match_except) {
+			/* one of the "except:" matches matched. The result is an explicit
+			 * negative match. */
+			return NM_MATCH_SPEC_NEG_MATCH;
+		} else {
+			/* none of the "except:" matches matched. The result is a positive match,
+			 * despite there being no positive match. */
+			return NM_MATCH_SPEC_MATCH;
+		}
+	}
+
+	if (has_match_except)
+		return NM_MATCH_SPEC_NEG_MATCH;
+	if (has_match)
+		return NM_MATCH_SPEC_MATCH;
+	return NM_MATCH_SPEC_NO_MATCH;
+}
+
 static const char *
 match_except (const char *spec_str, gboolean *out_except)
 {
@@ -1409,9 +1437,11 @@ nm_match_spec_device (const GSList *specs,
                       const char *dhcp_plugin)
 {
 	const GSList *iter;
-	NMMatchSpecMatchType match;
+	gboolean has_match = FALSE;
+	gboolean has_match_except = FALSE;
+	gboolean has_except = FALSE;
+	gboolean has_not_except = FALSE;
 	const char *spec_str;
-	gboolean except;
 	MatchDeviceData match_data = {
 	    .interface_name = interface_name,
 	    .device_type = nm_str_not_empty (device_type),
@@ -1431,19 +1461,9 @@ nm_match_spec_device (const GSList *specs,
 	if (!specs)
 		return NM_MATCH_SPEC_NO_MATCH;
 
-	match = NM_MATCH_SPEC_NO_MATCH;
-
-	/* pre-search for "*" */
 	for (iter = specs; iter; iter = iter->next) {
-		spec_str = iter->data;
-
-		if (spec_str && spec_str[0] == '*' && spec_str[1] == '\0') {
-			match = NM_MATCH_SPEC_MATCH;
-			break;
-		}
-	}
+		gboolean except;
 
-	for (iter = specs; iter; iter = iter->next) {
 		spec_str = iter->data;
 
 		if (!spec_str || !*spec_str)
@@ -1451,10 +1471,14 @@ nm_match_spec_device (const GSList *specs,
 
 		spec_str = match_except (spec_str, &except);
 
-		if (   !except
-		    && match == NM_MATCH_SPEC_MATCH) {
-			/* we have no "except-match" but already match. No need to evaluate
-			 * the match, we cannot match stronger. */
+		if (except)
+			has_except = TRUE;
+		else
+			has_not_except = TRUE;
+
+		if (   ( except && has_match_except)
+		    || (!except && has_match)) {
+			/* evaluating the match does not give new information. Skip it. */
 			continue;
 		}
 
@@ -1464,11 +1488,12 @@ nm_match_spec_device (const GSList *specs,
 			continue;
 
 		if (except)
-			return NM_MATCH_SPEC_NEG_MATCH;
-		match = NM_MATCH_SPEC_MATCH;
+			has_match_except = TRUE;
+		else
+			has_match = TRUE;
 	}
 
-	return match;
+	return _match_result (has_except, has_not_except, has_match, has_match_except);
 }
 
 static gboolean
@@ -1538,7 +1563,10 @@ NMMatchSpecMatchType
 nm_match_spec_config (const GSList *specs, guint cur_nm_version, const char *env)
 {
 	const GSList *iter;
-	NMMatchSpecMatchType match = NM_MATCH_SPEC_NO_MATCH;
+	gboolean has_match = FALSE;
+	gboolean has_match_except = FALSE;
+	gboolean has_except = FALSE;
+	gboolean has_not_except = FALSE;
 
 	if (!specs)
 		return NM_MATCH_SPEC_NO_MATCH;
@@ -1553,6 +1581,17 @@ nm_match_spec_config (const GSList *specs, guint cur_nm_version, const char *env
 
 		spec_str = match_except (spec_str, &except);
 
+		if (except)
+			has_except = TRUE;
+		else
+			has_not_except = TRUE;
+
+		if (   ( except && has_match_except)
+		    || (!except && has_match)) {
+			/* evaluating the match does not give new information. Skip it. */
+			continue;
+		}
+
 		if (_MATCH_CHECK (spec_str, MATCH_TAG_CONFIG_NM_VERSION))
 			v_match = match_config_eval (spec_str, MATCH_TAG_CONFIG_NM_VERSION, cur_nm_version);
 		else if (_MATCH_CHECK (spec_str, MATCH_TAG_CONFIG_NM_VERSION_MIN))
@@ -1562,15 +1601,18 @@ nm_match_spec_config (const GSList *specs, guint cur_nm_version, const char *env
 		else if (_MATCH_CHECK (spec_str, MATCH_TAG_CONFIG_ENV))
 			v_match = env && env[0] && !strcmp (spec_str, env);
 		else
+			v_match = FALSE;
+
+		if (!v_match)
 			continue;
 
-		if (v_match) {
-			if (except)
-				return NM_MATCH_SPEC_NEG_MATCH;
-			match = NM_MATCH_SPEC_MATCH;
-		}
+		if (except)
+			has_match_except = TRUE;
+		else
+			has_match = TRUE;
 	}
-	return match;
+
+	return _match_result (has_except, has_not_except, has_match, has_match_except);
 }
 
 #undef _MATCH_CHECK
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index c4e72dfb11..c9aad4b281 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -1236,8 +1236,8 @@ test_match_spec_device (void)
 	                            NULL,
 	                            NM_MAKE_STRV ("em*"));
 	_do_test_match_spec_device ("except:interface-name:em*",
-	                            NULL,
 	                            NM_MAKE_STRV ("", "eth", "eth1", "e1"),
+	                            NM_MAKE_STRV (NULL),
 	                            NM_MAKE_STRV ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3"));
 	_do_test_match_spec_device ("aa,bb,cc\\,dd,e,,",
 	                            NM_MAKE_STRV ("aa", "bb", "cc,dd", "e"),
@@ -1310,7 +1310,8 @@ _do_test_match_spec_config (const char *file, int line, const char *spec_str, gu
 	if (expected != match_result)
 		g_error ("%s:%d: failed comparing \"%s\" with %u.%u.%u. Expected %d, but got %d", file, line, spec_str, v_maj, v_min, v_mic, (int) expected, (int) match_result);
 
-	if (g_slist_length (specs) == 1 && match_result != NM_MATCH_SPEC_NEG_MATCH) {
+	if (   g_slist_length (specs) == 1
+	    && !g_str_has_prefix (specs->data, "except:")) {
 		/* there is only one spec in the list... test that we match except: */
 		char *sss = g_strdup_printf ("except:%s", (char *) specs->data);
 		GSList *specs2 = g_slist_append (NULL, sss);
@@ -1318,7 +1319,7 @@ _do_test_match_spec_config (const char *file, int line, const char *spec_str, gu
 
 		match_result2 = nm_match_spec_config (specs2, version, NULL);
 		if (match_result == NM_MATCH_SPEC_NO_MATCH)
-			g_assert_cmpint (match_result2, ==, NM_MATCH_SPEC_NO_MATCH);
+			g_assert_cmpint (match_result2, ==, NM_MATCH_SPEC_MATCH);
 		else
 			g_assert_cmpint (match_result2, ==, NM_MATCH_SPEC_NEG_MATCH);
 
@@ -1402,7 +1403,7 @@ test_match_spec_config (void)
 	do_test_match_spec_config ("nm-version-max:1", 1, 4, 30, NM_MATCH_SPEC_MATCH);
 	do_test_match_spec_config ("nm-version-max:1", 2, 4, 30, NM_MATCH_SPEC_NO_MATCH);
 
-	do_test_match_spec_config ("except:nm-version:1.4.8", 1, 6, 0, NM_MATCH_SPEC_NO_MATCH);
+	do_test_match_spec_config ("except:nm-version:1.4.8", 1, 6, 0, NM_MATCH_SPEC_MATCH);
 	do_test_match_spec_config ("nm-version-min:1.6,except:nm-version:1.4.8", 1, 6, 0, NM_MATCH_SPEC_MATCH);
 
 	do_test_match_spec_config ("nm-version-min:1.6,nm-version-min:1.4.6,nm-version-min:1.2.16,except:nm-version:1.4.8", 1, 2, 0, NM_MATCH_SPEC_NO_MATCH);