glib-aux: fix handling ASCII control characters in nm_utils_buf_utf8safe_escape()

On architectures where "char" is signed, the check "ch < ' '" is
also TRUE for non-ASCII characters greater than 127. This is an
easy mistake to make. Fix it by using nm_ascii_is_control() which
gets this right.

It's a bug, but possibly not too bad because unnecesarily escaping
a UTF-8 characters is not a severe problem, because the user anyway must
be prepared to unescape the string.
This commit is contained in:
Thomas Haller 2021-07-16 07:57:23 +02:00
parent 5f54270d93
commit 83f888054b
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 17 additions and 33 deletions

View file

@ -9292,16 +9292,11 @@ _do_test_utils_str_utf8safe(const char * str,
g_assert(str);
g_assert(str_safe != str);
g_assert(str_safe == str_free_2);
if (nm_streq(str, "ab∞c")) {
/* Hack to pass test for broken behavior. */
g_assert(((char) -1) < 0);
} else {
g_assert(strchr(str, '\\') || !g_utf8_validate(str, -1, NULL)
|| (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
&& NM_STRCHAR_ANY(str, ch, (guchar) ch >= 127))
|| (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
&& NM_STRCHAR_ANY(str, ch, (guchar) ch < ' ')));
}
g_assert(strchr(str, '\\') || !g_utf8_validate(str, -1, NULL)
|| (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
&& NM_STRCHAR_ANY(str, ch, (guchar) ch >= 127))
|| (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
&& NM_STRCHAR_ANY(str, ch, (guchar) ch < ' ')));
g_assert(g_utf8_validate(str_safe, -1, NULL));
str_free_6 = g_strcompress(str_safe);
@ -9376,20 +9371,8 @@ test_utils_str_utf8safe(void)
do_test_utils_str_utf8safe_unescape("\n\\.", "\n.");
do_test_utils_str_utf8safe_unescape("\\n\\.3\\r", "\n.3\r");
if (((char) -1) < 0) {
/* Test buggy behavior on systems with signed "char". Will be fixed next. */
do_test_utils_str_utf8safe("ab∞c",
"ab\\342\\210\\236c",
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);
} else {
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∞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

View file

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