diff --git a/src/basic/path-util.c b/src/basic/path-util.c index f3398418c4..50ba44492e 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -819,11 +819,21 @@ const char *last_path_component(const char *path) { int path_extract_filename(const char *p, char **ret) { _cleanup_free_ char *a = NULL; const char *c; + size_t n; /* Extracts the filename part (i.e. right-most component) from a path, i.e. string that passes - * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing slashes. Returns - * -EADDRNOTAVAIL if specified parameter includes no filename (i.e. is "/" or so). Returns -EINVAL if - * not a valid path in the first place. */ + * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing + * slashes. Returns: + * + * -EINVAL → if the passed in path is not a valid path + * -EADDRNOTAVAIL → if only a directory was specified, but no filename, i.e. the root dir itself is specified + * -ENOMEM → no memory + * + * Returns >= 0 on success. If the input path has a trailing slash, returns O_DIRECTORY, to indicate + * the referenced file must be a directory. + * + * This function guarantees to return a fully valid filename, i.e. one that passes + * filename_is_valid() – this means "." and ".." are not accepted. */ if (!path_is_valid(p)) return -EINVAL; @@ -834,14 +844,57 @@ int path_extract_filename(const char *p, char **ret) { return -EADDRNOTAVAIL; c = last_path_component(p); + n = strcspn(c, "/"); - a = strndup(c, strcspn(c, "/")); + a = strndup(c, n); if (!a) return -ENOMEM; if (!filename_is_valid(a)) return -EINVAL; + *ret = TAKE_PTR(a); + return c[n] == '/' ? O_DIRECTORY : 0; +} + +int path_extract_directory(const char *p, char **ret) { + _cleanup_free_ char *a = NULL; + const char *c; + + /* The inverse of path_extract_filename(), i.e. returns the directory path prefix. Returns: + * + * -EINVAL → if the passed in path is not a valid path + * -EDESTADDRREQ → if no directory was specified in the passed in path, i.e. only a filename was passed + * -EADDRNOTAVAIL → if the passed in parameter had no filename but did have a directory, i.e. the root dir itself was specified + * -ENOMEM → no memory (surprise!) + * + * This function guarantees to return a fully valid path, i.e. one that passes path_is_valid(). + */ + + if (!path_is_valid(p)) + return -EINVAL; + + /* Special case the root dir, because otherwise for an input of "///" last_path_component() returns + * the pointer to the last slash only, which might be seen as a valid path below. */ + if (path_equal(p, "/")) + return -EADDRNOTAVAIL; + + c = last_path_component(p); + + /* Delete trailing slashes, but keep one */ + while (c > p+1 && c[-1] == '/') + c--; + + if (p == c) /* No path whatsoever? Then return a recognizable error */ + return -EDESTADDRREQ; + + a = strndup(p, c - p); + if (!a) + return -ENOMEM; + + if (!path_is_valid(a)) + return -EINVAL; + *ret = TAKE_PTR(a); return 0; } diff --git a/src/basic/path-util.h b/src/basic/path-util.h index ba12b03dbe..74ee6362ea 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -147,6 +147,7 @@ int fsck_exists(const char *fstype); char* dirname_malloc(const char *path); const char *last_path_component(const char *path); int path_extract_filename(const char *p, char **ret); +int path_extract_directory(const char *p, char **ret); bool filename_is_valid(const char *p) _pure_; bool path_is_valid(const char *p) _pure_; diff --git a/src/basic/tmpfile-util.c b/src/basic/tmpfile-util.c index bac5eb6b26..5ee71d0158 100644 --- a/src/basic/tmpfile-util.c +++ b/src/basic/tmpfile-util.c @@ -94,16 +94,11 @@ int fmkostemp_safe(char *pattern, const char *mode, FILE **ret_f) { } int tempfn_xxxxxx(const char *p, const char *extra, char **ret) { - const char *fn; - char *t; + _cleanup_free_ char *d = NULL, *fn = NULL, *nf = NULL; + int r; assert(ret); - if (isempty(p)) - return -EINVAL; - if (path_equal(p, "/")) - return -EINVAL; - /* * Turns this: * /foo/bar/waldo @@ -112,34 +107,41 @@ int tempfn_xxxxxx(const char *p, const char *extra, char **ret) { * /foo/bar/.#waldoXXXXXX */ - fn = basename(p); - if (!filename_is_valid(fn)) - return -EINVAL; + r = path_extract_directory(p, &d); + if (r < 0 && r != -EDESTADDRREQ) /* EDESTADDRREQ → No directory specified, just a filename */ + return r; - extra = strempty(extra); + r = path_extract_filename(p, &fn); + if (r < 0) + return r; - t = new(char, strlen(p) + 2 + strlen(extra) + 6 + 1); - if (!t) + nf = strjoin(".#", strempty(extra), fn, "XXXXXX"); + if (!nf) return -ENOMEM; - strcpy(stpcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), ".#"), extra), fn), "XXXXXX"); + if (!filename_is_valid(nf)) /* New name is not valid? (Maybe because too long?) Refuse. */ + return -EINVAL; + + if (d) { + char *j; + + j = path_join(d, nf); + if (!j) + return -ENOMEM; + + *ret = path_simplify(j, false); + } else + *ret = TAKE_PTR(nf); - *ret = path_simplify(t, false); return 0; } int tempfn_random(const char *p, const char *extra, char **ret) { - const char *fn; - char *t, *x; - uint64_t u; + _cleanup_free_ char *d = NULL, *fn = NULL, *nf = NULL; + int r; assert(ret); - if (isempty(p)) - return -EINVAL; - if (path_equal(p, "/")) - return -EINVAL; - /* * Turns this: * /foo/bar/waldo @@ -148,27 +150,34 @@ int tempfn_random(const char *p, const char *extra, char **ret) { * /foo/bar/.#waldobaa2a261115984a9 */ - fn = basename(p); - if (!filename_is_valid(fn)) - return -EINVAL; + r = path_extract_directory(p, &d); + if (r < 0 && r != -EDESTADDRREQ) /* EDESTADDRREQ → No directory specified, just a filename */ + return r; - extra = strempty(extra); + r = path_extract_filename(p, &fn); + if (r < 0) + return r; - t = new(char, strlen(p) + 2 + strlen(extra) + 16 + 1); - if (!t) + if (asprintf(&nf, ".#%s%s%016" PRIx64, + strempty(extra), + fn, + random_u64()) < 0) return -ENOMEM; - x = stpcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), ".#"), extra), fn); + if (!filename_is_valid(nf)) /* Not valid? (maybe because too long now?) — refuse early */ + return -EINVAL; - u = random_u64(); - for (unsigned i = 0; i < 16; i++) { - *(x++) = hexchar(u & 0xF); - u >>= 4; - } + if (d) { + char *j; - *x = 0; + j = path_join(d, nf); + if (!j) + return -ENOMEM; + + *ret = path_simplify(j, false); + } else + *ret = TAKE_PTR(nf); - *ret = path_simplify(t, false); return 0; } diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 1ae10e5880..e5befe50e1 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -1872,6 +1872,9 @@ static int import_tar(int argc, char *argv[], void *userdata) { r = path_extract_filename(path, &fn); if (r < 0) return log_error_errno(r, "Cannot extract container name from filename: %m"); + if (r == O_DIRECTORY) + return log_error_errno(SYNTHETIC_ERRNO(EISDIR), + "Path '%s' refers to directory, but we need a regular file: %m", path); local = fn; } @@ -1932,6 +1935,9 @@ static int import_raw(int argc, char *argv[], void *userdata) { r = path_extract_filename(path, &fn); if (r < 0) return log_error_errno(r, "Cannot extract container name from filename: %m"); + if (r == O_DIRECTORY) + return log_error_errno(SYNTHETIC_ERRNO(EISDIR), + "Path '%s' refers to directory, but we need a regular file: %m", path); local = fn; } diff --git a/src/test/meson.build b/src/test/meson.build index c567a4cfcb..62e31fd830 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -379,6 +379,8 @@ tests += [ [['src/test/test-clock.c']], + [['src/test/test-tmpfile-util.c']], + [['src/test/test-architecture.c']], [['src/test/test-log.c']], diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 59308473c1..b92a77b9f4 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -570,7 +570,10 @@ static void test_path_extract_filename_one(const char *input, const char *output int r; r = path_extract_filename(input, &k); - log_info("%s → %s/%s [expected: %s/%s]", strnull(input), strnull(k), strerror_safe(r), strnull(output), strerror_safe(ret)); + log_info_errno(r, "%s → %s/%m [expected: %s/%s]", + strnull(input), + strnull(k), /* strerror(r) is printed via %m, to avoid that the two strerror()'s overwrite each other's buffers */ + strnull(output), ret < 0 ? strerror_safe(ret) : "-"); assert_se(streq_ptr(k, output)); assert_se(r == ret); } @@ -580,7 +583,7 @@ static void test_path_extract_filename(void) { test_path_extract_filename_one(NULL, NULL, -EINVAL); test_path_extract_filename_one("a/b/c", "c", 0); - test_path_extract_filename_one("a/b/c/", "c", 0); + test_path_extract_filename_one("a/b/c/", "c", O_DIRECTORY); test_path_extract_filename_one("/", NULL, -EADDRNOTAVAIL); test_path_extract_filename_one("//", NULL, -EADDRNOTAVAIL); test_path_extract_filename_one("///", NULL, -EADDRNOTAVAIL); @@ -589,13 +592,13 @@ static void test_path_extract_filename(void) { test_path_extract_filename_one("././", NULL, -EINVAL); test_path_extract_filename_one("././/", NULL, -EINVAL); test_path_extract_filename_one("/foo/a", "a", 0); - test_path_extract_filename_one("/foo/a/", "a", 0); + test_path_extract_filename_one("/foo/a/", "a", O_DIRECTORY); test_path_extract_filename_one("", NULL, -EINVAL); test_path_extract_filename_one("a", "a", 0); - test_path_extract_filename_one("a/", "a", 0); + test_path_extract_filename_one("a/", "a", O_DIRECTORY); test_path_extract_filename_one("/a", "a", 0); - test_path_extract_filename_one("/a/", "a", 0); - test_path_extract_filename_one("/////////////a/////////////", "a", 0); + test_path_extract_filename_one("/a/", "a", O_DIRECTORY); + test_path_extract_filename_one("/////////////a/////////////", "a", O_DIRECTORY); test_path_extract_filename_one("xx/.", NULL, -EINVAL); test_path_extract_filename_one("xx/..", NULL, -EINVAL); test_path_extract_filename_one("..", NULL, -EINVAL); @@ -606,6 +609,64 @@ static void test_path_extract_filename(void) { test_path_extract_filename_one("./", NULL, -EINVAL); } +static void test_path_extract_directory_one(const char *input, const char *output, int ret) { + _cleanup_free_ char *k = NULL; + int r; + + r = path_extract_directory(input, &k); + log_info_errno(r, "%s → %s/%m [expected: %s/%s]", + strnull(input), + strnull(k), /* we output strerror_safe(r) via %m here, since otherwise the error buffer might be overwritten twice */ + strnull(output), strerror_safe(ret)); + assert_se(streq_ptr(k, output)); + assert_se(r == ret); + + /* Extra safety check: let's make sure that if we split out the filename too (and it works) the + * joined parts are identical to the original again */ + if (r >= 0) { + _cleanup_free_ char *f = NULL; + + r = path_extract_filename(input, &f); + if (r >= 0) { + _cleanup_free_ char *j = NULL; + + assert_se(j = path_join(k, f)); + assert_se(path_equal(input, j)); + } + } +} + +static void test_path_extract_directory(void) { + log_info("/* %s */", __func__); + + test_path_extract_directory_one(NULL, NULL, -EINVAL); + test_path_extract_directory_one("a/b/c", "a/b", 0); + test_path_extract_directory_one("a/b/c/", "a/b", 0); + test_path_extract_directory_one("/", NULL, -EADDRNOTAVAIL); + test_path_extract_directory_one("//", NULL, -EADDRNOTAVAIL); + test_path_extract_directory_one("///", NULL, -EADDRNOTAVAIL); + test_path_extract_directory_one(".", NULL, -EDESTADDRREQ); + test_path_extract_directory_one("./.", ".", 0); + test_path_extract_directory_one("././", ".", 0); + test_path_extract_directory_one("././/", ".", 0); + test_path_extract_directory_one("/foo/a", "/foo", 0); + test_path_extract_directory_one("/foo/a/", "/foo", 0); + test_path_extract_directory_one("", NULL, -EINVAL); + test_path_extract_directory_one("a", NULL, -EDESTADDRREQ); + test_path_extract_directory_one("a/", NULL, -EDESTADDRREQ); + test_path_extract_directory_one("/a", "/", 0); + test_path_extract_directory_one("/a/", "/", 0); + test_path_extract_directory_one("/////////////a/////////////", "/", 0); + test_path_extract_directory_one("xx/.", "xx", 0); + test_path_extract_directory_one("xx/..", "xx", 0); + test_path_extract_directory_one("..", NULL, -EDESTADDRREQ); + test_path_extract_directory_one("/..", "/", 0); + test_path_extract_directory_one("../", NULL, -EDESTADDRREQ); + test_path_extract_directory_one(".", NULL, -EDESTADDRREQ); + test_path_extract_directory_one("/.", "/", 0); + test_path_extract_directory_one("./", NULL, -EDESTADDRREQ); +} + static void test_filename_is_valid(void) { char foo[NAME_MAX+2]; @@ -793,6 +854,7 @@ int main(int argc, char **argv) { test_file_in_same_dir(); test_last_path_component(); test_path_extract_filename(); + test_path_extract_directory(); test_filename_is_valid(); test_path_is_valid(); test_hidden_or_backup_file(); diff --git a/src/test/test-tmpfile-util.c b/src/test/test-tmpfile-util.c new file mode 100644 index 0000000000..83bac15d00 --- /dev/null +++ b/src/test/test-tmpfile-util.c @@ -0,0 +1,102 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "alloc-util.h" +#include "errno-util.h" +#include "log.h" +#include "string-util.h" +#include "tests.h" +#include "tmpfile-util.h" + +static void test_tempfn_random_one(const char *p, const char *extra, const char *expect, int ret) { + _cleanup_free_ char *s = NULL; + int r; + + r = tempfn_random(p, extra, &s); + log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret)); + + assert(!s == !expect); + if (s) { + const char *suffix; + + assert_se(suffix = startswith(s, expect)); + assert_se(in_charset(suffix, HEXDIGITS)); + assert_se(strlen(suffix) == 16); + } + assert(ret == r); +} + +static void test_tempfn_random(void) { + test_tempfn_random_one("", NULL, NULL, -EINVAL); + test_tempfn_random_one(".", NULL, NULL, -EINVAL); + test_tempfn_random_one("..", NULL, NULL, -EINVAL); + test_tempfn_random_one("/", NULL, NULL, -EADDRNOTAVAIL); + + test_tempfn_random_one("foo", NULL, ".#foo", 0); + test_tempfn_random_one("foo", "bar", ".#barfoo", 0); + test_tempfn_random_one("/tmp/foo", NULL, "/tmp/.#foo", 0); + test_tempfn_random_one("/tmp/foo", "bar", "/tmp/.#barfoo", 0); + test_tempfn_random_one("./foo", NULL, "./.#foo", 0); + test_tempfn_random_one("./foo", "bar", "./.#barfoo", 0); + test_tempfn_random_one("../foo", NULL, "../.#foo", 0); + test_tempfn_random_one("../foo", "bar", "../.#barfoo", 0); + + test_tempfn_random_one("foo/", NULL, ".#foo", 0); + test_tempfn_random_one("foo/", "bar", ".#barfoo", 0); + test_tempfn_random_one("/tmp/foo/", NULL, "/tmp/.#foo", 0); + test_tempfn_random_one("/tmp/foo/", "bar", "/tmp/.#barfoo", 0); + test_tempfn_random_one("./foo/", NULL, "./.#foo", 0); + test_tempfn_random_one("./foo/", "bar", "./.#barfoo", 0); + test_tempfn_random_one("../foo/", NULL, "../.#foo", 0); + test_tempfn_random_one("../foo/", "bar", "../.#barfoo", 0); +} + +static void test_tempfn_xxxxxx_one(const char *p, const char *extra, const char *expect, int ret) { + _cleanup_free_ char *s = NULL; + int r; + + r = tempfn_xxxxxx(p, extra, &s); + log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret)); + + assert(!s == !expect); + if (s) { + const char *suffix; + + assert_se(suffix = startswith(s, expect)); + assert_se(streq(suffix, "XXXXXX")); + } + assert(ret == r); +} + +static void test_tempfn_xxxxxx(void) { + test_tempfn_xxxxxx_one("", NULL, NULL, -EINVAL); + test_tempfn_xxxxxx_one(".", NULL, NULL, -EINVAL); + test_tempfn_xxxxxx_one("..", NULL, NULL, -EINVAL); + test_tempfn_xxxxxx_one("/", NULL, NULL, -EADDRNOTAVAIL); + + test_tempfn_xxxxxx_one("foo", NULL, ".#foo", 0); + test_tempfn_xxxxxx_one("foo", "bar", ".#barfoo", 0); + test_tempfn_xxxxxx_one("/tmp/foo", NULL, "/tmp/.#foo", 0); + test_tempfn_xxxxxx_one("/tmp/foo", "bar", "/tmp/.#barfoo", 0); + test_tempfn_xxxxxx_one("./foo", NULL, "./.#foo", 0); + test_tempfn_xxxxxx_one("./foo", "bar", "./.#barfoo", 0); + test_tempfn_xxxxxx_one("../foo", NULL, "../.#foo", 0); + test_tempfn_xxxxxx_one("../foo", "bar", "../.#barfoo", 0); + + test_tempfn_xxxxxx_one("foo/", NULL, ".#foo", 0); + test_tempfn_xxxxxx_one("foo/", "bar", ".#barfoo", 0); + test_tempfn_xxxxxx_one("/tmp/foo/", NULL, "/tmp/.#foo", 0); + test_tempfn_xxxxxx_one("/tmp/foo/", "bar", "/tmp/.#barfoo", 0); + test_tempfn_xxxxxx_one("./foo/", NULL, "./.#foo", 0); + test_tempfn_xxxxxx_one("./foo/", "bar", "./.#barfoo", 0); + test_tempfn_xxxxxx_one("../foo/", NULL, "../.#foo", 0); + test_tempfn_xxxxxx_one("../foo/", "bar", "../.#barfoo", 0); +} + +int main(int argc, char **argv) { + test_setup_logging(LOG_DEBUG); + + test_tempfn_random(); + test_tempfn_xxxxxx(); + + return 0; +}