all: merge branch 'th/ascii-control-chars'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/932

8 files changed, 153 insertions, 53 deletions

diff --git a/src/core/settings/plugins/ifcfg-rh/shvar.c b/src/core/settings/plugins/ifcfg-rh/shvar.c
index 5f9cd2f57b..d82efb3cb9 100644
--- a/src/core/settings/plugins/ifcfg-rh/shvar.c
+++ b/src/core/settings/plugins/ifcfg-rh/shvar.c
@@ -172,7 +172,7 @@ _escape_ansic(const char *source)
             n_alloc += 2;
             break;
         default:
-            if ((*p < ' ') || (*p >= 0177))
+            if (!nm_ascii_is_regular(*p))
                 n_alloc += 4;
             else
                 n_alloc += 1;
@@ -221,7 +221,7 @@ _escape_ansic(const char *source)
             *q++ = *p;
             break;
         default:
-            if ((*p < ' ') || (*p >= 0177)) {
+            if (!nm_ascii_is_regular(*p)) {
                 *q++ = '\\';
                 *q++ = '0' + (((*p) >> 6) & 07);
                 *q++ = '0' + (((*p) >> 3) & 07);
@@ -262,13 +262,14 @@ svEscape(const char *s, char **to_free)
             mangle++;
         else if (_char_req_quotes(s[slen]))
             requires_quotes = TRUE;
-        else if (((guchar) s[slen]) < ' ') {
-            /* if the string contains newline we can only express it using ANSI C quotation
-             * (as we don't support line continuation).
-             * Additionally, ANSI control characters look odd with regular quotation, so handle
-             * them too. */
-            return (*to_free = _escape_ansic(s));
-        } else if (((guchar) s[slen]) >= 0177) {
+        else if (!nm_ascii_is_regular(s[slen])) {
+            if (nm_ascii_is_ctrl_or_del(s[slen])) {
+                /* if the string contains newline we can only express it using ANSI C quotation
+                 * (as we don't support line continuation).
+                 * Additionally, ANSI control characters look odd with regular quotation, so handle
+                 * them too. */
+                return (*to_free = _escape_ansic(s));
+            }
             all_ascii       = FALSE;
             requires_quotes = TRUE;
         }
diff --git a/src/libnm-client-impl/nm-libnm-utils.c b/src/libnm-client-impl/nm-libnm-utils.c
index 3cc88ef4d8..671466cf93 100644
--- a/src/libnm-client-impl/nm-libnm-utils.c
+++ b/src/libnm-client-impl/nm-libnm-utils.c
@@ -160,14 +160,17 @@ _fixup_string(const char *       desc,
         p  = q + 1;
     }
 
-    /* replace '_', ',', ASCII control characters and parentheses, with space. */
+    /* replace '_', ',', ASCII control characters and everything inside parentheses, with space. */
     for (p = desc_full; p[0]; p++) {
         if (*p == '(')
             in_paren = TRUE;
-        if (NM_IN_SET(*p, '_', ',') || *p < ' ' || in_paren)
-            *p = ' ';
-        if (*p == ')')
+        else if (*p == ')')
             in_paren = FALSE;
+        else if (NM_IN_SET(*p, '_', ',') || nm_ascii_is_ctrl_or_del(*p) || in_paren) {
+            /* pass */
+        } else
+            continue;
+        *p = ' ';
     }
 
     /* Attempt to shorten ID by ignoring certain phrases */
diff --git a/src/libnm-client-impl/tests/test-libnm.c b/src/libnm-client-impl/tests/test-libnm.c
index c260aebed6..18bb09da9d 100644
--- a/src/libnm-client-impl/tests/test-libnm.c
+++ b/src/libnm-client-impl/tests/test-libnm.c
@@ -219,7 +219,7 @@ test_fixup_vendor_string(void)
         T_DATA("Memorex", "Memorex"),
         T_DATA("Micrel-Kendin", "Micrel-Kendin"),
         T_DATA("Microchip Technology, Inc.", "Microchip"),
-        T_DATA("Microcomputer Systems (M) Son", "Microcomputer"),
+        T_DATA("Microcomputer Systems (M) Son", "Microcomputer Son"),
         T_DATA("Microsoft Corp.", "Microsoft"),
         T_DATA("Microsoft Corporation", "Microsoft"),
         T_DATA("Micro-Star International Co., Ltd. [MSI]", "MSI"),
@@ -594,11 +594,11 @@ test_fixup_product_string(void)
         T_DATA("82599 Ethernet Controller Virtual Function", "82599 Virtual Function"),
         T_DATA("82599 Virtual Function", "82599 Virtual Function"),
         T_DATA("82801BA/BAM/CA/CAM Ethernet Controller", "82801BA/BAM/CA/CAM"),
-        T_DATA("82801CAM (ICH3) PRO/100 VE Ethernet Controller", "82801CAM"),
-        T_DATA("82801CAM (ICH3) PRO/100 VE (LOM) Ethernet Controller", "82801CAM"),
-        T_DATA("82801CAM (ICH3) PRO/100 VM Ethernet Controller", "82801CAM"),
-        T_DATA("82801CAM (ICH3) PRO/100 VM (KM) Ethernet Controller", "82801CAM"),
-        T_DATA("82801CAM (ICH3) PRO/100 VM (LOM) Ethernet Controller", "82801CAM"),
+        T_DATA("82801CAM (ICH3) PRO/100 VE Ethernet Controller", "82801CAM PRO/100 VE"),
+        T_DATA("82801CAM (ICH3) PRO/100 VE (LOM) Ethernet Controller", "82801CAM PRO/100 VE"),
+        T_DATA("82801CAM (ICH3) PRO/100 VM Ethernet Controller", "82801CAM PRO/100 VM"),
+        T_DATA("82801CAM (ICH3) PRO/100 VM (KM) Ethernet Controller", "82801CAM PRO/100 VM"),
+        T_DATA("82801CAM (ICH3) PRO/100 VM (LOM) Ethernet Controller", "82801CAM PRO/100 VM"),
         T_DATA("82801DB PRO/100 VE (CNR) Ethernet Controller", "82801DB PRO/100 VE"),
         T_DATA("82801DB PRO/100 VE (LOM) Ethernet Controller", "82801DB PRO/100 VE"),
         T_DATA("82801DB PRO/100 VE (MOB) Ethernet Controller", "82801DB PRO/100 VE"),
@@ -1009,25 +1009,25 @@ test_fixup_product_string(void)
         T_DATA("Ethernet Adapter", NULL),
         T_DATA("Ethernet adapter [U2L 100P-Y1]", "U2L 100P-Y1"),
         T_DATA("Ethernet Adaptive Virtual Function", "Adaptive Virtual Function"),
-        T_DATA("Ethernet Connection (2) I218-LM", NULL),
-        T_DATA("Ethernet Connection (2) I218-V", NULL),
-        T_DATA("Ethernet Connection (2) I219-LM", NULL),
-        T_DATA("Ethernet Connection (2) I219-V", NULL),
-        T_DATA("Ethernet Connection (3) I218-LM", NULL),
-        T_DATA("Ethernet Connection (3) I218-V", NULL),
-        T_DATA("Ethernet Connection (3) I219-LM", NULL),
-        T_DATA("Ethernet Connection (4) I219-LM", NULL),
-        T_DATA("Ethernet Connection (4) I219-V", NULL),
-        T_DATA("Ethernet Connection (5) I219-LM", NULL),
-        T_DATA("Ethernet Connection (5) I219-V", NULL),
-        T_DATA("Ethernet Connection (6) I219-LM", NULL),
-        T_DATA("Ethernet Connection (6) I219-V", NULL),
-        T_DATA("Ethernet Connection (7) I219-LM", NULL),
-        T_DATA("Ethernet Connection (7) I219-V", NULL),
-        T_DATA("Ethernet Connection (8) I219-LM", NULL),
-        T_DATA("Ethernet Connection (8) I219-V", NULL),
-        T_DATA("Ethernet Connection (9) I219-LM", NULL),
-        T_DATA("Ethernet Connection (9) I219-V", NULL),
+        T_DATA("Ethernet Connection (2) I218-LM", "I218-LM"),
+        T_DATA("Ethernet Connection (2 I218-V", NULL),
+        T_DATA("Ethernet Connection (2 I219-LM", NULL),
+        T_DATA("Ethernet Connection (2 I219-V", NULL),
+        T_DATA("Ethernet Connection (3) I218-LM", "I218-LM"),
+        T_DATA("Ethernet Connection (3) I218-V", "I218-V"),
+        T_DATA("Ethernet Connection (3 I219-LM", NULL),
+        T_DATA("Ethernet Connection (4 I219-LM", NULL),
+        T_DATA("Ethernet Connection (4 I219-V", NULL),
+        T_DATA("Ethernet Connection (5 I219-LM", NULL),
+        T_DATA("Ethernet Connection (5 I219-V", NULL),
+        T_DATA("Ethernet Connection (6 I219-LM", NULL),
+        T_DATA("Ethernet Connection (6 I219-V", NULL),
+        T_DATA("Ethernet Connection (7 I219-LM", NULL),
+        T_DATA("Ethernet Connection (7 I219-V", NULL),
+        T_DATA("Ethernet Connection (8 I219-LM", NULL),
+        T_DATA("Ethernet Connection (8 I219-V", NULL),
+        T_DATA("Ethernet Connection (9 I219-LM", NULL),
+        T_DATA("Ethernet Connection (9 I219-V", NULL),
         T_DATA("Ethernet Connection I217-LM", "I217-LM"),
         T_DATA("Ethernet Connection I217-V", "I217-V"),
         T_DATA("Ethernet Connection I218-LM", "I218-LM"),
@@ -2160,12 +2160,12 @@ test_fixup_product_string(void)
                "WLM-20U2/GN-1080"),
         T_DATA("WLP-UC-AG300 Wireless LAN Adapter", "WLP-UC-AG300"),
         T_DATA("WM168g 802.11bg Wireless Adapter [Intersil ISL3886]", "WM168g"),
-        T_DATA("WN111(v2) RangeMax Next Wireless [Atheros AR9170+AR9101]", "WN111"),
+        T_DATA("WN111(v2) RangeMax Next Wireless [Atheros AR9170+AR9101]", "WN111 RangeMax Next"),
         T_DATA("WNA1000M 802.11bgn [Realtek RTL8188CUS]", "WNA1000M"),
         T_DATA("WNA1000Mv2 802.11bgn [Realtek RTL8188CUS?]", "WNA1000Mv2"),
         T_DATA("WNA1000 Wireless-N 150 [Atheros AR9170+AR9101]", "WNA1000 150"),
         T_DATA("WNA1100 Wireless-N 150 [Atheros AR9271]", "WNA1100 150"),
-        T_DATA("WNA3100M(v1) Wireless-N 300 [Realtek RTL8192CU]", "WNA3100M"),
+        T_DATA("WNA3100M(v1) Wireless-N 300 [Realtek RTL8192CU]", "WNA3100M 300"),
         T_DATA("WNDA3100v1 802.11abgn [Atheros AR9170+AR9104]", "WNDA3100v1"),
         T_DATA("WNDA3200 802.11abgn Wireless Adapter [Atheros AR7010+AR9280]", "WNDA3200"),
         T_DATA("WNDA4100 802.11abgn 3x3:3 [Ralink RT3573]", "WNDA4100"),
diff --git a/src/libnm-core-impl/nm-keyfile-utils.c b/src/libnm-core-impl/nm-keyfile-utils.c
index 88f9c4f928..f8c2f387b0 100644
--- a/src/libnm-core-impl/nm-keyfile-utils.c
+++ b/src/libnm-core-impl/nm-keyfile-utils.c
@@ -531,7 +531,7 @@ _keyfile_key_encode(const char *name, char **out_to_free)
             if (ch == '\0')
                 return name;
 
-            if (ch < 0x20 || ch >= 127 || NM_IN_SET(ch, '=', '[', ']')
+            if (!nm_ascii_is_regular(ch) || NM_IN_SET(ch, '=', '[', ']')
                 || (ch == '\\' && g_ascii_isxdigit(name[i + 1]) && g_ascii_isxdigit(name[i + 2]))
                 || (ch == ' ' && name[i + 1] == '\0'))
                 break;
@@ -557,7 +557,7 @@ _keyfile_key_encode(const char *name, char **out_to_free)
         if (ch == '\0')
             break;
 
-        if (ch < 0x20 || ch >= 127 || NM_IN_SET(ch, '=', '[', ']')
+        if (!nm_ascii_is_regular(ch) || NM_IN_SET(ch, '=', '[', ']')
             || (ch == '\\' && g_ascii_isxdigit(name[i + 1]) && g_ascii_isxdigit(name[i + 2]))
             || (ch == ' ' && name[i + 1] == '\0')) {
             nm_str_buf_append_c(&str, '\\');
diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c
index 503613160c..824ac5a206 100644
--- a/src/libnm-core-impl/tests/test-general.c
+++ b/src/libnm-core-impl/tests/test-general.c
@@ -9228,6 +9228,12 @@ _do_test_utils_str_utf8safe(const char *            str,
     ((nmtst_get_rand_bool()) ? NM_UTILS_STR_UTF8_SAFE_FLAG_NONE \
                              : NM_UTILS_STR_UTF8_SAFE_FLAG_SECRET)
 
+    if (expected && strlen(expected) == str_len && memcmp(str, expected, str_len) == 0) {
+        g_error("Test error: pass expected as NULL (instead of \"%s\", if the escaping will "
+                "produce no difference.",
+                expected);
+    }
+
     buf_safe = nm_utils_buf_utf8safe_escape(str, str_len, flags | RND_FLAG, &str_free_1);
 
     str_safe = nm_utils_str_utf8safe_escape(str, flags | RND_FLAG, &str_free_2);
@@ -9364,6 +9370,13 @@ test_utils_str_utf8safe(void)
     do_test_utils_str_utf8safe_unescape("\n\\012", "\n\012");
     do_test_utils_str_utf8safe_unescape("\n\\.", "\n.");
     do_test_utils_str_utf8safe_unescape("\\n\\.3\\r", "\n.3\r");
+
+    do_test_utils_str_utf8safe("ab∞c", NULL, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL);
+    do_test_utils_str_utf8safe("ab\ab∞c", "ab\\007b∞c", NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL);
+    do_test_utils_str_utf8safe("ab\ab∞c",
+                               "ab\\007b\\342\\210\\236c",
+                               NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL
+                                   | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII);
 }
 
 /*****************************************************************************/
diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c
index 1b4c3cbc4f..106cc70fad 100644
--- a/src/libnm-glib-aux/nm-shared-utils.c
+++ b/src/libnm-glib-aux/nm-shared-utils.c
@@ -2991,13 +2991,13 @@ nm_utils_buf_utf8safe_escape(gconstpointer           buf,
     if (g_utf8_validate(str, buflen, &p) && nul_terminated) {
         /* note that g_utf8_validate() does not allow NUL character inside @str. Good.
          * We can treat @str like a NUL terminated string. */
-        if (!NM_STRCHAR_ANY(
-                str,
-                ch,
-                (ch == '\\'
-                 || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) && ch < ' ')
-                 || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
-                     && ((guchar) ch) >= 127))))
+        if (!NM_STRCHAR_ANY(str,
+                            ch,
+                            (ch == '\\'
+                             || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
+                                 && nm_ascii_is_ctrl_or_del(ch))
+                             || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
+                                 && nm_ascii_is_non_ascii(ch)))))
             return str;
     }
 
@@ -3014,9 +3014,10 @@ nm_utils_buf_utf8safe_escape(gconstpointer           buf,
             nm_assert(ch);
             if (ch == '\\')
                 nm_str_buf_append_c(&strbuf, '\\', '\\');
-            else if ((NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL) && ch < ' ')
+            else if ((NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
+                      && nm_ascii_is_ctrl_or_del(ch))
                      || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
-                         && ((guchar) ch) >= 127))
+                         && nm_ascii_is_non_ascii(ch)))
                 _str_buf_append_c_escape_octal(&strbuf, ch);
             else
                 nm_str_buf_append_c(&strbuf, ch);
diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h
index aa6fdc70c7..a18463f4c5 100644
--- a/src/libnm-glib-aux/nm-shared-utils.h
+++ b/src/libnm-glib-aux/nm-shared-utils.h
@@ -1462,10 +1462,14 @@ GType nm_g_type_find_implementing_class_for_property(GType gtype, const char *pn
 typedef enum {
     NM_UTILS_STR_UTF8_SAFE_FLAG_NONE = 0,
 
-    /* This flag only has an effect during escaping. */
+    /* This flag only has an effect during escaping.
+     *
+     * It will backslash escape ascii characters according to nm_ascii_is_ctrl_or_del(). */
     NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL = 0x0001,
 
-    /* This flag only has an effect during escaping. */
+    /* This flag only has an effect during escaping.
+     *
+     * It will backslash escape ascii characters according to nm_ascii_is_non_ascii(). */
     NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII = 0x0002,
 
     /* This flag only has an effect during escaping to ensure we
@@ -2481,6 +2485,40 @@ nm_hexchar(int x, gboolean upper_case)
     return upper_case ? _nm_hexchar_table_upper[x & 15] : _nm_hexchar_table_lower[x & 15];
 }
 
+static inline gboolean
+nm_ascii_is_ctrl(char ch)
+{
+    /* 0 to ' '-1 is the C0 range.
+     *
+     * Other ranges may also be considered control characters, but NOT
+     * CONSIDERED by this function. For example:
+     *   - DEL (127) is also a control character.
+     *   - SP (' ', 0x20) is also considered a control character.
+     *   - DEL+1 (0x80) to 0x9F is C1 range.
+     *   - NBSP (0xA0) and SHY (0xAD) are ISO 8859 special characters
+     */
+    return ((guchar) ch) < ' ';
+}
+
+static inline gboolean
+nm_ascii_is_ctrl_or_del(char ch)
+{
+    return ((guchar) ch) < ' ' || ch == 127;
+}
+
+static inline gboolean
+nm_ascii_is_non_ascii(char ch)
+{
+    return ((guchar) ch) > 127;
+}
+
+static inline gboolean
+nm_ascii_is_regular(char ch)
+{
+    /* same as(!nm_ascii_is_ctrl_or_del(ch) && !nm_ascii_is_non_ascii(ch)) */
+    return ch >= ' ' && ch < 127;
+}
+
 char *nm_utils_bin2hexstr_full(gconstpointer addr,
                                gsize         length,
                                char          delimiter,
diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c
index 593b8c42a1..98b98669bf 100644
--- a/src/libnm-glib-aux/tests/test-shared-general.c
+++ b/src/libnm-glib-aux/tests/test-shared-general.c
@@ -1370,6 +1370,49 @@ test_nm_g_source_sentinel(void)
 
 /*****************************************************************************/
 
+static void
+test_nm_ascii(void)
+{
+    int i;
+
+    for (i = 0; i < 256; i++) {
+        const char ch = i;
+        gboolean   is_space;
+
+        if (ch == 127) {
+            g_assert(nm_ascii_is_ctrl_or_del(ch));
+            g_assert(!nm_ascii_is_ctrl(ch));
+        } else
+            g_assert(nm_ascii_is_ctrl_or_del(ch) == nm_ascii_is_ctrl(ch));
+        g_assert(nm_ascii_is_ctrl_or_del(ch) == g_ascii_iscntrl(ch));
+
+        g_assert(nm_ascii_is_non_ascii(ch) == (i >= 128));
+
+        g_assert(!nm_ascii_is_ctrl_or_del(ch) || !nm_ascii_is_non_ascii(ch));
+
+        g_assert((nm_ascii_is_ctrl_or_del(ch) || nm_ascii_is_regular(ch))
+                 != nm_ascii_is_non_ascii(ch));
+
+        g_assert(nm_ascii_is_regular(ch)
+                 == (!nm_ascii_is_ctrl_or_del(ch) && !nm_ascii_is_non_ascii(ch)));
+
+        is_space = g_ascii_isspace(ch);
+        if (NM_IN_SET(ch, '\t', '\n', '\f', '\r')) {
+            /* hack is-space, so that the check below works to check for regular ASCII characters. */
+            g_assert(!nm_ascii_is_regular(ch));
+            g_assert(is_space);
+            is_space = FALSE;
+        }
+        g_assert(nm_ascii_is_regular(ch)
+                 == (g_ascii_isalnum(ch) || g_ascii_isalpha(ch) || g_ascii_isdigit(ch)
+                     || g_ascii_isgraph(ch) || g_ascii_islower(ch) || g_ascii_isprint(ch)
+                     || g_ascii_ispunct(ch) || is_space || g_ascii_isupper(ch)
+                     || g_ascii_isxdigit(ch)));
+    }
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE();
 
 int
@@ -1402,6 +1445,7 @@ main(int argc, char **argv)
     g_test_add_func("/general/test_strv_dup_packed", test_strv_dup_packed);
     g_test_add_func("/general/test_utils_hashtable_cmp", test_utils_hashtable_cmp);
     g_test_add_func("/general/test_nm_g_source_sentinel", test_nm_g_source_sentinel);
+    g_test_add_func("/general/test_nm_ascii", test_nm_ascii);
 
     return g_test_run();
 }