From 9000db5f6eec813f2d853d197de088c06a99ba42 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 28 Jun 2025 10:25:05 +0900 Subject: [PATCH] conf-files: fix an empty root handling in conf_files_list_strv() Before 50c81130b69d04288f50217bede709bac6ca2b1a, the function used chase(), hence if root is an empty string, each config directory made prefixed with the current working directory if it is relative. See implementation of chase(). With 50c81130b69d04288f50217bede709bac6ca2b1a, conf_files_list_strv() internally uses chaseat(), hence each config directory is not prefixed anymore even if it is relative. To restore the previous behavior, this makes - if root is an empty string, prefix each config directories with the current working directory if relative. - if root is relative, make it absolute to make the prefixed results also absolute, and debugging logs show absolute paths. - use chaseat_prefix_root() to prefix the results, for safety. Follow-ups for 50c81130b69d04288f50217bede709bac6ca2b1a. --- src/basic/conf-files.c | 85 ++++++++++++++++++++++++++++++-------- src/test/test-conf-files.c | 47 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 17 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 49fa1dce798..cabdbd0f4dd 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -196,9 +196,9 @@ static int copy_and_sort_files_from_hashmap(Hashmap *fh, const char *root, ConfF if (r < 0) return log_debug_errno(r, "Failed to extract filename from '%s': %m", *s); } else if (root) { - p = path_join(root, skip_leading_slash(*s)); - if (!p) - return log_oom_debug(); + r = chaseat_prefix_root(*s, root, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", *s, root); } if (p) @@ -325,6 +325,49 @@ static int conf_files_list_impl( return 0; } +static int prepare_dirs(const char *root, char * const *dirs, int *ret_rfd, char **ret_root, char ***ret_dirs) { + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; + int r; + + assert(ret_rfd); + assert(ret_root); + assert(ret_dirs); + + r = empty_or_root_harder_to_null(&root); + if (r < 0) + return log_debug_errno(r, "Failed to determine if '%s' points to the root directory: %m", strempty(root)); + + dirs_abs = strv_copy(dirs); + if (!dirs_abs) + return log_oom(); + + if (root) { + /* WHen a non-trivial root is specified, we will prefix the result later. Hence, it is not + * necessary to modify each config directories here. but needs to normalize the root directory. */ + r = path_make_absolute_cwd(root, &root_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make '%s' absolute: %m", root); + + path_simplify(root_abs); + } else { + /* When an empty root or "/" is specified, we will open "/" below, hence we need to make + * each config directory absolute if relative. */ + r = path_strv_make_absolute_cwd(dirs_abs); + if (r < 0) + return log_debug_errno(r, "Failed to make directories absolute: %m"); + } + + _cleanup_close_ int rfd = open(empty_to_root(root_abs), O_CLOEXEC|O_DIRECTORY|O_PATH); + if (rfd < 0) + return log_debug_errno(errno, "Failed to open '%s': %m", empty_to_root(root_abs)); + + *ret_rfd = TAKE_FD(rfd); + *ret_root = TAKE_PTR(root_abs); + *ret_dirs = TAKE_PTR(dirs_abs); + return 0; +} + int conf_files_list_strv( char ***ret, const char *suffix, @@ -333,19 +376,23 @@ int conf_files_list_strv( const char * const *dirs) { _cleanup_hashmap_free_ Hashmap *fh = NULL; + _cleanup_close_ int rfd = -EBADF; + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; int r; assert(ret); - _cleanup_close_ int rfd = open(empty_to_root(root), O_CLOEXEC|O_DIRECTORY|O_PATH); - if (rfd < 0) - return log_debug_errno(errno, "Failed to open '%s': %m", root); + r = prepare_dirs(root, (char**) dirs, &rfd, &root_abs, &dirs_abs); + if (r < 0) + return r; - r = conf_files_list_impl(suffix, rfd, root, flags, dirs, /* replacement = */ NULL, &fh, /* ret_inserted = */ NULL); + r = conf_files_list_impl(suffix, rfd, root_abs, flags, (const char * const *) dirs_abs, + /* replacement = */ NULL, &fh, /* ret_inserted = */ NULL); if (r < 0) return r; - return copy_and_sort_files_from_hashmap(fh, empty_to_root(root), flags, ret); + return copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret); } int conf_files_list_strv_at( @@ -414,29 +461,33 @@ int conf_files_list_with_replacement( _cleanup_hashmap_free_ Hashmap *fh = NULL; _cleanup_free_ char *inserted = NULL; ConfFilesFlags flags = CONF_FILES_REGULAR | CONF_FILES_FILTER_MASKED_BY_SYMLINK; + _cleanup_close_ int rfd = -EBADF; + _cleanup_free_ char *root_abs = NULL; + _cleanup_strv_free_ char **dirs_abs = NULL; int r; assert(ret_files); - root = empty_to_root(root); - _cleanup_close_ int rfd = open(root, O_CLOEXEC|O_DIRECTORY|O_PATH); - if (rfd < 0) - return log_debug_errno(errno, "Failed to open '%s': %m", root); + r = prepare_dirs(root, config_dirs, &rfd, &root_abs, &dirs_abs); + if (r < 0) + return r; - r = conf_files_list_impl(".conf", rfd, root, flags, (const char * const *) config_dirs, + r = conf_files_list_impl(".conf", rfd, root_abs, flags, (const char * const *) dirs_abs, replacement, &fh, ret_inserted ? &inserted : NULL); if (r < 0) return r; if (inserted) { - char *p = path_join(empty_to_root(root), skip_leading_slash(inserted)); - if (!p) - return log_oom_debug(); + char *p; + + r = chaseat_prefix_root(inserted, root_abs, &p); + if (r < 0) + return log_debug_errno(r, "Failed to prefix '%s' with root '%s': %m", inserted, empty_to_root(root_abs)); free_and_replace(inserted, p); } - r = copy_and_sort_files_from_hashmap(fh, empty_to_root(root), flags, ret_files); + r = copy_and_sort_files_from_hashmap(fh, empty_to_root(root_abs), flags, ret_files); if (r < 0) return r; diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index 44106ef7a81..ec60e24cffe 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -88,6 +88,18 @@ TEST(conf_files_list) { result = strv_free(result); + ASSERT_OK(conf_files_list(&result, NULL, "/", CONF_FILES_FILTER_MASKED, search1)); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, "///../../././//", CONF_FILES_FILTER_MASKED, search1)); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + + result = strv_free(result); + ASSERT_OK(conf_files_list(&result, NULL, t, CONF_FILES_FILTER_MASKED, "/dir1/")); strv_print(result); ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); @@ -106,6 +118,41 @@ TEST(conf_files_list) { result = strv_free(result); + /* search dir1 with relative path */ + ASSERT_OK_ERRNO(chdir("/tmp/")); + + ASSERT_OK(conf_files_list(&result, NULL, NULL, CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, "/", CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, "///../../././//", CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list(&result, NULL, t, CONF_FILES_FILTER_MASKED, "dir1")); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(search1_a, search1_b, search1_c))); + result = strv_free(result); + + ASSERT_OK(conf_files_list_at(&result, NULL, AT_FDCWD, CONF_FILES_FILTER_MASKED, path_startswith(search1, "/tmp/"))); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE(path_startswith(search1_a, "/tmp/"), + path_startswith(search1_b, "/tmp/"), + path_startswith(search1_c, "/tmp/")))); + result = strv_free(result); + + ASSERT_OK(conf_files_list_at(&result, NULL, tfd, CONF_FILES_FILTER_MASKED, "dir1")); + strv_print(result); + ASSERT_TRUE(strv_equal(result, STRV_MAKE("dir1/a.conf", "dir1/b.conf", "dir1/c.foo"))); + result = strv_free(result); + /* search dir1 with suffix */ ASSERT_OK(conf_files_list(&result, ".conf", NULL, CONF_FILES_FILTER_MASKED, search1)); strv_print(result); -- 2.47.3