From c6809df4cdf2f909ffcd98e842447fca523f1c0b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Jun 2020 21:19:42 +0200 Subject: [PATCH 1/5] shared: make NM_STR_BUF_INIT() an inline function In the previous form, NM_STR_BUF_INIT() was a macro. That makes sense, however it's not really possible to make that a macro without evaluating the reservation length multiple times. That means, NMStrBuf strbuf = NM_STR_BUF_INIT (nmtst_get_rand_uint32 () % 100, FALSE); leads to a crash. That is unfortunate, so instead make it an inline function that returns a NMStrBut struct. Usually, we avoid functions that returns structs, but here we do it. --- shared/nm-glib-aux/nm-str-buf.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/shared/nm-glib-aux/nm-str-buf.h b/shared/nm-glib-aux/nm-str-buf.h index 7121ba609a..b582e2c8a6 100644 --- a/shared/nm-glib-aux/nm-str-buf.h +++ b/shared/nm-glib-aux/nm-str-buf.h @@ -39,13 +39,18 @@ _nm_str_buf_assert (NMStrBuf *strbuf) nm_assert (strbuf->_priv_len <= strbuf->_priv_allocated); } -#define NM_STR_BUF_INIT(len, do_bzero_mem) \ - ((NMStrBuf) { \ - ._priv_str = (len) ? g_malloc (len) : NULL, \ - ._priv_allocated = (len), \ - ._priv_len = 0, \ - ._priv_do_bzero_mem = (do_bzero_mem), \ - }) +static inline NMStrBuf +NM_STR_BUF_INIT (gsize allocated, gboolean do_bzero_mem) +{ + NMStrBuf strbuf = { + ._priv_str = allocated ? g_malloc (allocated) : NULL, + ._priv_allocated = allocated, + ._priv_len = 0, + ._priv_do_bzero_mem = do_bzero_mem, + }; + + return strbuf; +} static inline void nm_str_buf_init (NMStrBuf *strbuf, From a2142e884b9090f5b5f42b55381a1f848c2426bb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Jun 2020 21:25:17 +0200 Subject: [PATCH 2/5] shared: add nm_str_buf_append_c_repeated() helper --- shared/nm-glib-aux/nm-str-buf.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/shared/nm-glib-aux/nm-str-buf.h b/shared/nm-glib-aux/nm-str-buf.h index b582e2c8a6..13af81c01d 100644 --- a/shared/nm-glib-aux/nm-str-buf.h +++ b/shared/nm-glib-aux/nm-str-buf.h @@ -166,6 +166,19 @@ nm_str_buf_erase (NMStrBuf *strbuf, /*****************************************************************************/ +static inline void +nm_str_buf_append_c_repeated (NMStrBuf *strbuf, + char ch, + guint len) +{ + if (len > 0) { + nm_str_buf_maybe_expand (strbuf, len + 1, FALSE); + do { + strbuf->_priv_str[strbuf->_priv_len++] = ch; + } while (--len > 0); + } +} + static inline void nm_str_buf_append_c (NMStrBuf *strbuf, char ch) From 2a6ecf21285d2a7c0c806b6add360437493ff3ba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Jun 2020 19:21:17 +0200 Subject: [PATCH 3/5] tests: add nmtst_extract_first_word_all() for testing We usually don't want to use internal API of systemd for our own purposes. Here, we will use it to check our implementation against systemd's. Add an accessor to extract_first_word() for testing. --- shared/systemd/nm-sd-utils-shared.c | 40 +++++++++++++++++++++++++++++ shared/systemd/nm-sd-utils-shared.h | 4 +++ 2 files changed, 44 insertions(+) diff --git a/shared/systemd/nm-sd-utils-shared.c b/shared/systemd/nm-sd-utils-shared.c index 4444e6c7f6..4fe82ca053 100644 --- a/shared/systemd/nm-sd-utils-shared.c +++ b/shared/systemd/nm-sd-utils-shared.c @@ -137,3 +137,43 @@ nm_sd_http_url_is_valid_https (const char *url) nm_assert (_http_url_is_valid (url, FALSE) == http_url_is_valid (url)); return _http_url_is_valid (url, TRUE); } + +/*****************************************************************************/ + +int +nmtst_systemd_extract_first_word_all (const char *str, char ***out_strv) +{ + gs_unref_ptrarray GPtrArray *arr = NULL; + + /* we implement a str split function to parse `/proc/cmdline`. This + * code should behave like systemd, which uses extract_first_word() + * for that. + * + * As we want to unit-test our implementation to match systemd, + * expose this function for testing. */ + + g_assert (out_strv); + g_assert (!*out_strv); + + if (!str) + return 0; + + arr = g_ptr_array_new_with_free_func (g_free); + + for (;;) { + gs_free char *word = NULL; + int r; + + r = extract_first_word (&str, &word, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX); + if (r < 0) + return r; + if (r == 0) + break; + g_ptr_array_add (arr, g_steal_pointer (&word)); + } + + g_ptr_array_add (arr, NULL); + + *out_strv = (char **) g_ptr_array_free (g_steal_pointer (&arr), FALSE); + return 1; +} diff --git a/shared/systemd/nm-sd-utils-shared.h b/shared/systemd/nm-sd-utils-shared.h index a3ca1edc03..75e38b8422 100644 --- a/shared/systemd/nm-sd-utils-shared.h +++ b/shared/systemd/nm-sd-utils-shared.h @@ -38,4 +38,8 @@ gboolean nm_sd_hostname_is_valid(const char *s, bool allow_trailing_dot); gboolean nm_sd_http_url_is_valid_https (const char *url); +/*****************************************************************************/ + +int nmtst_systemd_extract_first_word_all (const char *str, char ***out_strv); + #endif /* __NM_SD_UTILS_SHARED_H__ */ From 10779d545a6fe0af8f29e065d251882ff25411fc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Jun 2020 19:43:35 +0200 Subject: [PATCH 4/5] shared: add nm_utils_strsplit_quoted() We want to parse "/proc/cmdline". That is space separated with support for quoting and escaping. Our implementation becomes part of stable behavior, and we should interpret the kernel command line the same way as the system does. That means, our implementation should match systemd's. --- libnm-core/tests/test-general.c | 229 +++++++++++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.c | 94 +++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 4 + 3 files changed, 327 insertions(+) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 714dcd4236..f70a7d5a65 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -13,6 +13,8 @@ #include "nm-std-aux/c-list-util.h" #include "nm-glib-aux/nm-enum-utils.h" +#include "nm-glib-aux/nm-str-buf.h" +#include "systemd/nm-sd-utils-shared.h" #include "nm-utils.h" #include "nm-setting-private.h" @@ -8869,7 +8871,232 @@ test_connection_ovs_ifname (gconstpointer test_data) } } +/*****************************************************************************/ +static gboolean +_strsplit_quoted_char_needs_escaping (char ch) +{ + return NM_IN_SET (ch, '\'', '\"', '\\') + || strchr (NM_ASCII_WHITESPACES, ch); +} + +static char * +_strsplit_quoted_create_str_rand (gssize len) +{ + NMStrBuf strbuf = NM_STR_BUF_INIT (nmtst_get_rand_uint32 () % 200, nmtst_get_rand_bool ()); + + g_assert (len >= -1); + + if (len == -1) + len = nmtst_get_rand_word_length (NULL); + + while (len-- > 0) { + char ch; + + ch = nmtst_rand_select ('a', ' ', '\\', '"', '\'', nmtst_get_rand_uint32 () % 255 + 1); + g_assert (ch); + nm_str_buf_append_c (&strbuf, ch); + } + + if (!strbuf.allocated) + nm_str_buf_maybe_expand (&strbuf, 1, nmtst_get_rand_bool ()); + return nm_str_buf_finalize (&strbuf, NULL); +} + +static char ** +_strsplit_quoted_create_strv_rand (void) +{ + guint len = nmtst_get_rand_word_length (NULL); + char **ptr; + guint i; + + ptr = g_new (char *, len + 1); + for (i = 0; i < len; i++) + ptr[i] = _strsplit_quoted_create_str_rand (-1); + ptr[i] = NULL; + return ptr; +} + +static char * +_strsplit_quoted_join_strv_rand (const char *const*strv) +{ + NMStrBuf strbuf = NM_STR_BUF_INIT (nmtst_get_rand_uint32 () % 200, nmtst_get_rand_bool ()); + char *result; + gsize l; + gsize l2; + gsize *p_l2 = nmtst_get_rand_bool () ? &l2 : NULL; + gsize i; + + g_assert (strv); + + nm_str_buf_append_c_repeated (&strbuf, ' ', nmtst_get_rand_word_length (NULL) / 4); + for (i = 0; strv[i]; i++) { + const char *s = strv[i]; + gsize j; + char quote; + + nm_str_buf_append_c_repeated (&strbuf, ' ', 1 + nmtst_get_rand_word_length (NULL) / 4); + + j = 0; + quote = '\0'; + while (TRUE) { + char ch = s[j++]; + + /* extract_first_word*/ + if (quote != '\0') { + if (ch == '\0') { + nm_str_buf_append_c (&strbuf, quote); + break; + } + if ( ch == quote + || ch == '\\' + || nmtst_get_rand_uint32 () % 5 == 0) + nm_str_buf_append_c (&strbuf, '\\'); + nm_str_buf_append_c (&strbuf, ch); + if (nmtst_get_rand_uint32 () % 3 == 0) { + nm_str_buf_append_c (&strbuf, quote); + quote = '\0'; + goto next_maybe_quote; + } + continue; + } + + if (ch == '\0') { + if (s == strv[i]) { + quote = nmtst_rand_select ('\'', '"'); + nm_str_buf_append_c_repeated (&strbuf, quote, 2); + } + break; + } + + if ( _strsplit_quoted_char_needs_escaping (ch) + || nmtst_get_rand_uint32 () % 5 == 0) + nm_str_buf_append_c (&strbuf, '\\'); + + nm_str_buf_append_c (&strbuf, ch); + +next_maybe_quote: + if (nmtst_get_rand_uint32 () % 5 == 0) { + quote = nmtst_rand_select ('\'', '\"'); + nm_str_buf_append_c (&strbuf, quote); + if (nmtst_get_rand_uint32 () % 5 == 0) { + nm_str_buf_append_c (&strbuf, quote); + quote = '\0'; + } + } + } + } + nm_str_buf_append_c_repeated (&strbuf, ' ', nmtst_get_rand_word_length (NULL) / 4); + + nm_str_buf_maybe_expand (&strbuf, 1, nmtst_get_rand_bool ()); + + l = strbuf.len; + result = nm_str_buf_finalize (&strbuf, p_l2); + g_assert (!p_l2 || l == *p_l2); + g_assert (strlen (result) == l); + return result; +} + +static void +_strsplit_quoted_assert_strv (const char *topic, + const char *str, + const char *const*strv1, + const char *const*strv2) +{ + nm_auto_str_buf NMStrBuf s1 = { }; + nm_auto_str_buf NMStrBuf s2 = { }; + gs_free char *str_escaped = NULL; + int i; + + g_assert (str); + g_assert (strv1); + g_assert (strv2); + + if (_nm_utils_strv_equal ((char **) strv1, (char **) strv2)) + return; + + for (i = 0; strv1[i]; i++) { + gs_free char *s = g_strescape (strv1[i], NULL); + + g_print (">>> [%s] strv1[%d] = \"%s\"\n", topic, i, s); + if (i > 0) + nm_str_buf_append_c (&s1, ' '); + nm_str_buf_append_printf (&s1, "\"%s\"", s); + } + + for (i = 0; strv2[i]; i++) { + gs_free char *s = g_strescape (strv2[i], NULL); + + g_print (">>> [%s] strv2[%d] = \"%s\"\n", topic, i, s); + if (i > 0) + nm_str_buf_append_c (&s2, ' '); + nm_str_buf_append_printf (&s2, "\"%s\"", s); + } + + nm_str_buf_maybe_expand (&s1, 1, FALSE); + nm_str_buf_maybe_expand (&s2, 1, FALSE); + + str_escaped = g_strescape (str, NULL); + g_error ("compared words differs: [%s] str=\"%s\"; strv1=%s; strv2=%s", topic, str_escaped, nm_str_buf_get_str (&s1), nm_str_buf_get_str (&s2)); +} + +static void +_strsplit_quoted_test (const char *str, + const char *const*strv_expected) +{ + gs_strfreev char **strv_systemd = NULL; + gs_strfreev char **strv_nm = NULL; + int r; + + g_assert (str); + + r = nmtst_systemd_extract_first_word_all (str, &strv_systemd); + g_assert_cmpint (r, ==, 1); + g_assert (strv_systemd); + + if (!strv_expected) + strv_expected = (const char *const*) strv_systemd; + + _strsplit_quoted_assert_strv ("systemd", str, strv_expected, (const char *const*) strv_systemd); + + strv_nm = nm_utils_strsplit_quoted (str); + g_assert (strv_nm); + _strsplit_quoted_assert_strv ("nm", str, strv_expected, (const char *const*) strv_nm); +} + +static void +test_strsplit_quoted (void) +{ + int i_run; + + _strsplit_quoted_test ("", NM_MAKE_STRV ()); + _strsplit_quoted_test (" ", NM_MAKE_STRV ()); + _strsplit_quoted_test (" ", NM_MAKE_STRV ()); + _strsplit_quoted_test (" \t", NM_MAKE_STRV ()); + _strsplit_quoted_test ("a b", NM_MAKE_STRV ("a", "b")); + _strsplit_quoted_test ("a\\ b", NM_MAKE_STRV ("a b")); + _strsplit_quoted_test (" a\\ \"b\"", NM_MAKE_STRV ("a b")); + _strsplit_quoted_test (" a\\ \"b\" c \n", NM_MAKE_STRV ("a b", "c")); + + for (i_run = 0; i_run < 1000; i_run++) { + gs_strfreev char **strv = NULL; + gs_free char *str = NULL; + + /* create random strv array and join them carefully so that splitting + * them will yield the original value. */ + strv = _strsplit_quoted_create_strv_rand (); + str = _strsplit_quoted_join_strv_rand ((const char *const*) strv); + _strsplit_quoted_test (str, (const char *const*) strv); + } + + /* Create random words and assert that systemd and our implementation can + * both split them (and in the exact same way). */ + for (i_run = 0; i_run < 1000; i_run++) { + gs_free char *s = _strsplit_quoted_create_str_rand (nmtst_get_rand_uint32 () % 150); + + _strsplit_quoted_test (s, NULL); + } +} /*****************************************************************************/ @@ -9047,5 +9274,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/test_nm_ip_addr_zero", test_nm_ip_addr_zero); + g_test_add_func ("/core/general/test_strsplit_quoted", test_strsplit_quoted); + return g_test_run (); } diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 39dfe6f473..a2e90273c5 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -2022,6 +2022,100 @@ nm_utils_escaped_tokens_options_split (char *str, /*****************************************************************************/ +/** + * nm_utils_strsplit_quoted: + * @str: the string to split (e.g. from /proc/cmdline). + * + * This basically does that systemd's extract_first_word() does + * with the flags "EXTRACT_UNQUOTE | EXTRACT_RELAX". This is what + * systemd uses to parse /proc/cmdline, and we do too. + * + * Splits the string. We have nm_utils_strsplit_set() which + * supports a variety of flags. However, extending that already + * complex code to also support quotation and escaping is hard. + * Instead, add a naive implementation. + * + * Returns: (transfer full): the split string. + */ +char ** +nm_utils_strsplit_quoted (const char *str) +{ + gs_unref_ptrarray GPtrArray *arr = NULL; + gs_free char *str_out = NULL; + guint8 ch_lookup[256]; + + nm_assert (str); + + _char_lookup_table_init (ch_lookup, NM_ASCII_WHITESPACES); + + for (;;) { + char quote; + gsize j; + + while (_char_lookup_has (ch_lookup, str[0])) + str++; + + if (str[0] == '\0') + break; + + if (!str_out) + str_out = g_new (char, strlen (str) + 1); + + quote = '\0'; + j = 0; + for (;;) { + if (str[0] == '\\') { + str++; + if (str[0] == '\0') + break; + str_out[j++] = str[0]; + str++; + continue; + } + if (quote) { + if (str[0] == '\0') + break; + if (str[0] == quote) { + quote = '\0'; + str++; + continue; + } + str_out[j++] = str[0]; + str++; + continue; + } + if (str[0] == '\0') + break; + if (NM_IN_SET (str[0], '\'', '"')) { + quote = str[0]; + str++; + continue; + } + if (_char_lookup_has (ch_lookup, str[0])) { + str++; + break; + } + str_out[j++] = str[0]; + str++; + } + + if (!arr) + arr = g_ptr_array_new (); + g_ptr_array_add (arr, g_strndup (str_out, j)); + } + + if (!arr) + return g_new0 (char *, 1); + + g_ptr_array_add (arr, NULL); + + /* We want to return an optimally sized strv array, with no excess + * memory allocated. Hence, clone once more. */ + return nm_memdup (arr->pdata, sizeof (char *) * arr->len); +} + +/*****************************************************************************/ + /** * nm_utils_strv_find_first: * @list: the strv list to search diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 0831f11c3d..893ec11c0d 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -681,6 +681,10 @@ nm_utils_escaped_tokens_escape_gstr (const char *str, /*****************************************************************************/ +char **nm_utils_strsplit_quoted (const char *str); + +/*****************************************************************************/ + static inline const char ** nm_utils_escaped_tokens_options_split_list (const char *str) { From 27041e9f05da0473d56866e54218414c480bbfb4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 23 Jun 2020 00:12:45 +0200 Subject: [PATCH 5/5] core: use nm_utils_strsplit_quoted() for splitting the kernel command line The kernel command line supports escaping and quoting (at least, according to systemd's parser, which is our example to follow). Use nm_utils_strsplit_quoted() which supports that. --- src/nm-core-utils.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index bc80dabf35..dc76d1fd9d 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2778,19 +2778,13 @@ nm_utils_proc_cmdline_split (void) again: proc_cmdline = g_atomic_pointer_get (&proc_cmdline_cached); if (G_UNLIKELY (!proc_cmdline)) { - gs_free const char **split = NULL; + gs_strfreev char **split = NULL; - /* FIXME(release-blocker): support quotation, like systemd's proc_cmdline_extract_first(). - * For that, add a new NMUtilsStrsplitSetFlags flag. */ - split = nm_utils_strsplit_set_full (nm_utils_proc_cmdline (), - NM_ASCII_WHITESPACES, - NM_UTILS_STRSPLIT_SET_FLAGS_NONE); - proc_cmdline = split - ?: NM_PTRARRAY_EMPTY (const char *); - if (!g_atomic_pointer_compare_and_exchange (&proc_cmdline_cached, NULL, proc_cmdline)) + split = nm_utils_strsplit_quoted (nm_utils_proc_cmdline ()); + if (!g_atomic_pointer_compare_and_exchange (&proc_cmdline_cached, NULL, (gpointer) split)) goto again; - g_steal_pointer (&split); + proc_cmdline = (const char *const*) g_steal_pointer (&split); } return proc_cmdline;