libnm: use nm_utils_escaped_tokens_*() for parsing NMIPRoutingRule

Replace nm_utils_str_simpletokens_extract_next() by
nm_utils_escaped_tokens_split().

nm_utils_escaped_tokens_split() should become our first choice for
parsing and tokenizing.

Note that both nm_utils_str_simpletokens_extract_next() and
nm_utils_escaped_tokens_split() need to strdup the string once,
and tokenizing takes O(n). So, they are roughtly the same performance
wise. The only difference is, that as we iterate through the tokens,
we might abort early on error with nm_utils_str_simpletokens_extract_next()
and not parse the entire string. But that is a small benefit, since we
anyway always strdup() the string (being O(n) already).

Note that to-string will no longer escape ',' and ';'. This is a change
in behavior, of unreleased API. Also note, that escaping these is no
longer necessary, because nmcli soon will also use nm_utils_escaped_tokens_*().

Another change in behavior is that nm_utils_str_simpletokens_extract_next()
treated invalid escape sequences (backslashes followed by an arbitrary
character), buy stripping the backslash. nm_utils_escaped_tokens_*()
leaves such backslashes as is, and only honors them if they are followed
by a whitespace (the delimiter) or another backslash. The disadvantage
of the new approach is that backslashes are treated differently
depending on the following character. The benefit is, that most
backslashes can now be written verbatim, not requiring them to escape
them with a double-backslash.

Yes, there is a problem with these nested escape schemes:

  - the caller may already need to escape backslash in shell.

  - then nmcli will use backslash escaping to split the rules at ','.

  - then nm_ip_routing_rule_from_string() will honor backslash escaping
    for spaces.

  - then iifname and oifname use backslash escaping for nm_utils_buf_utf8safe_escape()
    to express non-UTF-8 characters (because interface names are not
    necessarily UTF-8).

This is only redeamed because escaping is really only necessary for very
unusual cases, if you want to embed a backslash, a space, a comma, or a
non-UTF-8 character. But if you have to, now you will be able to express
that.

The other upside of these layers of escaping is that they become all
indendent from each other:

  - shell can accept quoted/escaped arguments and will unescape them.

  - nmcli can do the tokenizing for ',' (and escape the content
    unconditionally when converting to string).

  - nm_ip_routing_rule_from_string() can do its tokenizing without
    special consideration of utf8safe escaping.

  - NMIPRoutingRule takes iifname/oifname as-is and is not concerned
    about nm_utils_buf_utf8safe_escape(). However, before configuring
    the rule in kernel, this utf8safe escape will be unescaped to get
    the interface name (which is non-UTF8 binary).

2 files changed, 14 insertions, 30 deletions

diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c
index 48815c3484..d4a38161dd 100644
--- a/libnm-core/nm-setting-ip-config.c
+++ b/libnm-core/nm-setting-ip-config.c
@@ -2954,9 +2954,8 @@ nm_ip_routing_rule_from_string (const char *str,
                                 GError **error)
 {
 	nm_auto_unref_ip_routing_rule NMIPRoutingRule *self = NULL;
-	gs_free char *str_clone = NULL;
-	char *str_remainder;
-	char *str_word;
+	gs_free const char **tokens = NULL;
+	gsize i_token;
 	gboolean any_words = FALSE;
 	char *word0 = NULL;
 	char *word1 = NULL;
@@ -3022,10 +3021,9 @@ nm_ip_routing_rule_from_string (const char *str,
 
 	addr_family = _rr_string_addr_family_from_flags (to_string_flags);
 
-	str_clone = g_strdup (str);
-	str_remainder = str_clone;
-
-	while ((str_word = nm_utils_str_simpletokens_extract_next (&str_remainder))) {
+	tokens = nm_utils_escaped_tokens_split (str, NM_ASCII_SPACES);
+	for (i_token = 0; tokens && tokens[i_token]; i_token++) {
+		char *str_word = (char *) tokens[i_token];
 
 		any_words = TRUE;
 		if (!word0)
@@ -3325,24 +3323,6 @@ _rr_string_append_inet_addr (GString *str,
 	}
 }
 
-static void
-_rr_string_append_escaped (GString *str,
-                           const char *s)
-{
-	for (; s[0]; s++) {
-		/* We need to escape spaces and '\\', because that
-		 * is what nm_utils_str_simpletokens_extract_next() uses to split
-		 * words.
-		 * We also escape ',' because nmcli uses that to concatenate values.
-		 * We also escape ';', in case somebody wants to use ';' instead of ','.
-		 */
-		if (   NM_IN_SET (s[0], '\\', ',', ';')
-		    || g_ascii_isspace (s[0]))
-			g_string_append_c (str, '\\');
-		g_string_append_c (str, s[0]);
-	}
-}
-
 /**
  * nm_ip_routing_rule_to_string:
  * @self: the #NMIPRoutingRule instance to convert to string.
@@ -3486,13 +3466,17 @@ nm_ip_routing_rule_to_string (const NMIPRoutingRule *self,
 	if (self->iifname) {
 		g_string_append (nm_gstring_add_space_delimiter (str),
 		                 "iif ");
-		_rr_string_append_escaped (str, self->iifname);
+		nm_utils_escaped_tokens_escape_gstr (self->iifname,
+		                                     NM_ASCII_SPACES,
+		                                     str);
 	}
 
 	if (self->oifname) {
 		g_string_append (nm_gstring_add_space_delimiter (str),
 		                 "oif ");
-		_rr_string_append_escaped (str, self->oifname);
+		nm_utils_escaped_tokens_escape_gstr (self->oifname,
+		                                     NM_ASCII_SPACES,
+		                                     str);
 	}
 
 	if (self->table != 0) {
diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c
index 70c0d6a4c6..57af014021 100644
--- a/libnm-core/tests/test-setting.c
+++ b/libnm-core/tests/test-setting.c
@@ -2886,7 +2886,7 @@ test_routing_rule (gconstpointer test_data)
 	char ifname_buf[16];
 
 	_rr_from_str ("priority 5 from 0.0.0.0 table 1",
-	              "  from 0.0.0\\.0  \\priority  5 lookup 1 ");
+	              "  from 0.0.0.0  priority  5 lookup 1 ");
 	_rr_from_str ("priority 5 from 0.0.0.0/0 table 4");
 	_rr_from_str ("priority 5 to 0.0.0.0 table 6");
 	_rr_from_str ("priority 5 to 0.0.0.0 table 254",
@@ -2905,7 +2905,7 @@ test_routing_rule (gconstpointer test_data)
 	              "priority 5 from :: to ::/0 table 0x19 fwmark 0x00/4294967295");
 	_rr_from_str ("priority 5 from :: iif aab table 25");
 	_rr_from_str ("priority 5 from :: iif aab oif er table 25",
-	              "priority 5 from :: table 0x19 dev \\a\\a\\b oif er");
+	              "priority 5 from :: table 0x19 dev aab oif er");
 	_rr_from_str ("priority 5 from :: iif a\\\\303b table 25");
 	_rr_from_str ("priority 5 to 0.0.0.0 sport 10 table 6",
 	              "priority 5 to 0.0.0.0 sport 10-10 table 6");
@@ -2952,7 +2952,7 @@ test_routing_rule (gconstpointer test_data)
 	g_assert_cmpint (0x10, ==, nm_ip_routing_rule_get_tos (rr1));
 	nm_clear_pointer (&rr1, nm_ip_routing_rule_unref);
 
-	rr1 = _rr_from_str_get ("priority 5 from :: iif a\\\\303\\\\261\\,x\\;b table 254",
+	rr1 = _rr_from_str_get ("priority 5 from :: iif a\\\\303\\\\261,x;b table 254",
 	                        "priority 5 from :: iif a\\\\303\\\\261,x;b table 254");
 	g_assert_cmpstr (nm_ip_routing_rule_get_iifname (rr1), ==, "a\\303\\261,x;b");
 	success = nm_ip_routing_rule_get_xifname_bin (rr1, FALSE, ifname_buf);