From: Lennart Poettering Date: Tue, 16 Jun 2026 16:14:17 +0000 (+0200) Subject: sysupdate: split out component validation/enumeration into sysupdate-util.[ch] X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=180893f9d7eeb7b416f1dc0475151e0bdf074213;p=thirdparty%2Fsystemd.git sysupdate: split out component validation/enumeration into sysupdate-util.[ch] Just some refactoring. The code is slightly updated, for example it now uses string_is_safe(). But mostly this just splits out code from one large file to a smaller one. --- diff --git a/src/sysupdate/meson.build b/src/sysupdate/meson.build index da554ddcc3d..44d8d59c530 100644 --- a/src/sysupdate/meson.build +++ b/src/sysupdate/meson.build @@ -45,6 +45,11 @@ executables += [ 'objects' : ['systemd-sysupdate'], 'conditions' : ['ENABLE_SYSUPDATED'], }, + test_template + { + 'sources' : files('test-sysupdate-util.c'), + 'objects' : ['systemd-sysupdate'], + 'conditions' : ['ENABLE_SYSUPDATE'], + }, ] if conf.get('ENABLE_SYSUPDATED') == 1 diff --git a/src/sysupdate/sysupdate-util.c b/src/sysupdate/sysupdate-util.c index 1aba63cf782..b3ed72577e9 100644 --- a/src/sysupdate/sysupdate-util.c +++ b/src/sysupdate/sysupdate-util.c @@ -4,8 +4,14 @@ #include "bus-error.h" #include "bus-locator.h" +#include "conf-files.h" +#include "constants.h" #include "log.h" #include "login-util.h" +#include "path-util.h" +#include "set.h" +#include "string-util.h" +#include "strv.h" #include "sysupdate-util.h" int reboot_now(void) { @@ -24,3 +30,72 @@ int reboot_now(void) { return 0; } + +bool component_name_valid(const char *c) { + /* See if the specified string enclosed in the directory prefix+suffix would be a valid file name */ + + if (!string_is_safe(c, STRING_FILENAME_PART)) + return false; + + /* Stack allocation is safe, since STRING_FILENAME_PART includes a length check */ + const char *j = strjoina("sysupdate.", c, ".d"); + + return filename_is_valid(j); +} + +int get_component_list(const char *root, char ***ret) { + int r; + + assert(ret); + + ConfFile **directories = NULL; + size_t n_directories = 0; + CLEANUP_ARRAY(directories, n_directories, conf_file_free_array); + + r = conf_files_list_strv_full( + ".d", + root, + CONF_FILES_DIRECTORY|CONF_FILES_WARN, + (const char * const *) CONF_PATHS_STRV(""), + &directories, + &n_directories); + if (r < 0) + return r; + + _cleanup_set_free_ Set *names = NULL; + + FOREACH_ARRAY(i, directories, n_directories) { + ConfFile *e = *i; + + const char *s = startswith(e->filename, "sysupdate."); + if (!s) + continue; + + const char *a = endswith(s, ".d"); + if (!a) + continue; + + if (a == s) + continue; + + _cleanup_free_ char *n = strndup(s, a - s); + if (!n) + return log_oom(); + + if (!component_name_valid(n)) + continue; + + r = set_ensure_consume(&names, &string_hash_ops_free, TAKE_PTR(n)); + if (r < 0 && r != -EEXIST) + return r; + } + + _cleanup_strv_free_ char **z = set_to_strv(&names); + if (!z) + return -ENOMEM; + + strv_sort(z); + + *ret = TAKE_PTR(z); + return 0; +} diff --git a/src/sysupdate/sysupdate-util.h b/src/sysupdate/sysupdate-util.h index 8504fd24b7d..07d9def7087 100644 --- a/src/sysupdate/sysupdate-util.h +++ b/src/sysupdate/sysupdate-util.h @@ -1,8 +1,12 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ - #pragma once +#include + int reboot_now(void); -#define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) +#define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) #define SD_SYSUPDATE_FLAGS_ALL (SD_SYSUPDATE_OFFLINE) + +bool component_name_valid(const char *c); +int get_component_list(const char *root, char ***ret); diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index cd5850bd38d..a17fbdec83c 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -1711,34 +1711,12 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void return EXIT_SUCCESS; } -static int component_name_valid(const char *c) { - _cleanup_free_ char *j = NULL; - - /* See if the specified string enclosed in the directory prefix+suffix would be a valid file name */ - - if (isempty(c)) - return false; - - if (string_has_cc(c, NULL)) - return false; - - if (!utf8_is_valid(c)) - return false; - - j = strjoin("sysupdate.", c, ".d"); - if (!j) - return -ENOMEM; - - return filename_is_valid(j); -} - VERB_NOARG(verb_components, "components", "Show list of components"); static int verb_components(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(context_freep) Context* context = NULL; _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; - _cleanup_set_free_ Set *names = NULL; bool has_default_component = false; int r; @@ -1752,46 +1730,10 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda if (r < 0) return r; - ConfFile **directories = NULL; - size_t n_directories = 0; - - CLEANUP_ARRAY(directories, n_directories, conf_file_free_array); - - r = conf_files_list_strv_full(".d", arg_root, CONF_FILES_DIRECTORY|CONF_FILES_WARN, - (const char * const *) CONF_PATHS_STRV(""), &directories, &n_directories); + _cleanup_strv_free_ char **z = NULL; + r = get_component_list(arg_root, &z); if (r < 0) - return log_error_errno(r, "Failed to enumerate directories: %m"); - - FOREACH_ARRAY(i, directories, n_directories) { - ConfFile *e = *i; - - if (streq(e->filename, "sysupdate.d")) { - continue; - } - - const char *s = startswith(e->filename, "sysupdate."); - if (!s) - continue; - - const char *a = endswith(s, ".d"); - if (!a) - continue; - - _cleanup_free_ char *n = strndup(s, a - s); - if (!n) - return log_oom(); - - r = component_name_valid(n); - if (r < 0) - return log_error_errno(r, "Unable to validate component name '%s': %m", n); - if (r == 0) - continue; - - r = set_ensure_put(&names, &string_hash_ops_free, n); - if (r < 0 && r != -EEXIST) - return log_error_errno(r, "Failed to add component '%s' to set: %m", n); - TAKE_PTR(n); - } + return log_error_errno(r, "Failed to enumerate components: %m"); /* Does the system have at least one transfer file in /etc/sysupdate.d, which can be considered a * TARGET_HOST? See target_get_argument() in sysupdated.c */ @@ -1801,15 +1743,8 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda !arg_image && context->n_transfers > 0); - /* We use simple free() rather than strv_free() here, since set_free() will free the strings for us */ - _cleanup_free_ char **z = set_get_strv(names); - if (!z) - return log_oom(); - - strv_sort(z); - if (!sd_json_format_enabled(arg_json_format_flags)) { - if (!has_default_component && set_isempty(names)) { + if (!has_default_component && strv_isempty(z)) { log_info("No components defined."); return 0; } @@ -1912,10 +1847,7 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) { break; } - r = component_name_valid(opts.arg); - if (r < 0) - return log_error_errno(r, "Failed to determine if component name is valid: %m"); - if (r == 0) + if (!component_name_valid(opts.arg)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Component name invalid: %s", opts.arg); r = free_and_strdup_warn(&arg_component, opts.arg); diff --git a/src/sysupdate/test-sysupdate-util.c b/src/sysupdate/test-sysupdate-util.c new file mode 100644 index 00000000000..b9a5123e058 --- /dev/null +++ b/src/sysupdate/test-sysupdate-util.c @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "sysupdate-util.h" +#include "tests.h" + +TEST(component_name_valid) { + /* Valid component names: anything that turns "sysupdate..d" into a valid filename. */ + ASSERT_TRUE(component_name_valid("foo")); + ASSERT_TRUE(component_name_valid("foo-bar")); + ASSERT_TRUE(component_name_valid("foo.bar")); + ASSERT_TRUE(component_name_valid("foo_bar_baz")); + ASSERT_TRUE(component_name_valid("0815")); + ASSERT_TRUE(component_name_valid("über")); /* valid UTF-8 is fine */ + + /* Invalid: empty, slashes, control characters, invalid UTF-8. */ + ASSERT_FALSE(component_name_valid("")); + ASSERT_FALSE(component_name_valid("foo/bar")); + ASSERT_FALSE(component_name_valid("/foo")); + ASSERT_FALSE(component_name_valid("foo/")); + ASSERT_FALSE(component_name_valid("foo\tbar")); + ASSERT_FALSE(component_name_valid("foo\nbar")); + ASSERT_FALSE(component_name_valid("foo\x7f")); + ASSERT_FALSE(component_name_valid("\xff")); /* not valid UTF-8 */ +} + +DEFINE_TEST_MAIN(LOG_INFO);