colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libnm: use nm_utils_escaped_tokens_*() for parsing NMIPRoutingRule

Browse Source 
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).
This commit is contained in:
Thomas Haller
2019-04-12 12:38:11 +02:00
parent ba956bd499
commit b6d0be2d3b
2 changed files with 14 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
libnm-core/nm-setting-ip-config.c
View File

@@ -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) {

6
libnm-core/tests/test-setting.c
View File

@@ -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);