util-lib: use trailing slash in chase_symlinks, fd_is_mount_point, path_is_mount_point

The kernel will reply with -ENOTDIR when we try to access a non-directory under
a name which ends with a slash. But our functions would strip the trailing slash
under various circumstances. Keep the trailing slash, so that

path_is_mount_point("/path/to/file/") return -ENOTDIR when /path/to/file/ is a file.

Tests are added for this change in behaviour.

Also, when called with a trailing slash, path_is_mount_point() would get
"" from basename(), and call name_to_handle_at(3, "", ...), and always
return -ENOENT. Now it'll return -ENOTDIR if the mount point is a file, and
true if it is a directory and a mount point.

v2:
- use strip_trailing_chars()

v3:
- instead of stripping trailing chars(), do the opposite — preserve them.

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index b5fd95ae8e03837242d894cadf2618c3a8f1de42..e74e75a41ca3fd7c23e26031fd889a78e9929056 100644 (file)
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -661,9 +661,18 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
 
                 todo += m;
 
+                /* Empty? Then we reached the end. */
+                if (isempty(first))
+                        break;
+
                 /* Just a single slash? Then we reached the end. */
-                if (isempty(first) || path_equal(first, "/"))
+                if (path_equal(first, "/")) {
+                        /* Preserve the trailing slash */
+                        if (!strextend(&done, "/", NULL))
+                                return -ENOMEM;
+
                         break;
+                }
 
                 /* Just a dot? Then let's eat this up. */
                 if (path_equal(first, "/."))

diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c
index e32502308a72be30f9b161efe41791d8e16e9707..51c84491be9e6fe75b24e93f223f4b514579416a 100644 (file)
--- a/src/basic/mount-util.c
+++ b/src/basic/mount-util.c
@@ -277,6 +277,7 @@ int path_is_mount_point(const char *t, const char *root, int flags) {
         int r;
 
         assert(t);
+        assert((flags & ~AT_SYMLINK_FOLLOW) == 0);
 
         if (path_equal(t, "/"))
                 return 1;
@@ -301,7 +302,7 @@ int path_is_mount_point(const char *t, const char *root, int flags) {
         if (fd < 0)
                 return -errno;
 
-        return fd_is_mount_point(fd, basename(t), flags);
+        return fd_is_mount_point(fd, last_path_component(t), flags);
 }
 
 int path_get_mnt_id(const char *path, int *ret) {

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 883856abc22f93b233b299647e0444bbebed3974..059e4fcf6a51230a2a37614322b75fb3dac6499e 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -703,6 +703,34 @@ char* dirname_malloc(const char *path) {
         return dir2;
 }
 
+const char *last_path_component(const char *path) {
+        /* Finds the last component of the path, preserving the
+         * optional trailing slash that signifies a directory.
+         *    a/b/c → c
+         *    a/b/c/ → c/
+         *    / → /
+         *    // → /
+         *    /foo/a → a
+         *    /foo/a/ → a/
+         * This is different than basename, which returns "" when
+         * a trailing slash is present.
+         */
+
+        unsigned l, k;
+
+        l = k = strlen(path);
+        while (k > 0 && path[k-1] == '/')
+                k--;
+
+        if (k == 0) /* the root directory */
+                return path + l - 1;
+
+        while (k > 0 && path[k-1] != '/')
+                k--;
+
+        return path + k;
+}
+
 bool filename_is_valid(const char *p) {
         const char *e;
 

diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 9bd783eaf45ab9bbd9f700b67da5e1c74890e26b..f79cdf928ecf7d21374872fd0e2caba20a9b74a9 100644 (file)
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -130,6 +130,7 @@ char *prefix_root(const char *root, const char *path);
 int parse_path_argument_and_warn(const char *path, bool suppress_root, char **arg);
 
 char* dirname_malloc(const char *path);
+const char *last_path_component(const char *path);
 
 bool filename_is_valid(const char *p) _pure_;
 bool path_is_normalized(const char *p) _pure_;

diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index 3c958917bb6ec007306f76dea63f49c6f7c8a943..83ddc398b8834239418245c7db6774753974b309 100644 (file)
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -35,7 +35,7 @@
 static void test_chase_symlinks(void) {
         _cleanup_free_ char *result = NULL;
         char temp[] = "/tmp/test-chase.XXXXXX";
-        const char *top, *p, *q;
+        const char *top, *p, *pslash, *q, *qslash;
         int r;
 
         assert_se(mkdtemp(temp));
@@ -66,93 +66,114 @@ static void test_chase_symlinks(void) {
         r = chase_symlinks(p, NULL, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, "/usr"));
+        result = mfree(result);
 
+        pslash = strjoina(p, "/");
+        r = chase_symlinks(pslash, NULL, 0, &result);
+        assert_se(r > 0);
+        assert_se(path_equal(result, "/usr/"));
         result = mfree(result);
+
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r == -ENOENT);
 
+        r = chase_symlinks(pslash, temp, 0, &result);
+        assert_se(r == -ENOENT);
+
         q = strjoina(temp, "/usr");
 
         r = chase_symlinks(p, temp, CHASE_NONEXISTENT, &result);
         assert_se(r == 0);
         assert_se(path_equal(result, q));
+        result = mfree(result);
 
-        assert_se(mkdir(q, 0700) >= 0);
+        qslash = strjoina(q, "/");
 
+        r = chase_symlinks(pslash, temp, CHASE_NONEXISTENT, &result);
+        assert_se(r == 0);
+        assert_se(path_equal(result, qslash));
         result = mfree(result);
+
+        assert_se(mkdir(q, 0700) >= 0);
+
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, q));
+        result = mfree(result);
+
+        r = chase_symlinks(pslash, temp, 0, &result);
+        assert_se(r > 0);
+        assert_se(path_equal(result, qslash));
+        result = mfree(result);
 
         p = strjoina(temp, "/slash");
         assert_se(symlink("/", p) >= 0);
 
-        result = mfree(result);
         r = chase_symlinks(p, NULL, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, "/"));
-
         result = mfree(result);
+
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, temp));
+        result = mfree(result);
 
         /* Paths that would "escape" outside of the "root" */
 
         p = strjoina(temp, "/6dots");
         assert_se(symlink("../../..", p) >= 0);
 
-        result = mfree(result);
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r > 0 && path_equal(result, temp));
+        result = mfree(result);
 
         p = strjoina(temp, "/6dotsusr");
         assert_se(symlink("../../../usr", p) >= 0);
 
-        result = mfree(result);
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r > 0 && path_equal(result, q));
+        result = mfree(result);
 
         p = strjoina(temp, "/top/8dotsusr");
         assert_se(symlink("../../../../usr", p) >= 0);
 
-        result = mfree(result);
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r > 0 && path_equal(result, q));
+        result = mfree(result);
 
         /* Paths that contain repeated slashes */
 
         p = strjoina(temp, "/slashslash");
         assert_se(symlink("///usr///", p) >= 0);
 
-        result = mfree(result);
         r = chase_symlinks(p, NULL, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, "/usr"));
-
         result = mfree(result);
+
         r = chase_symlinks(p, temp, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, q));
+        result = mfree(result);
 
         /* Paths using . */
 
-        result = mfree(result);
         r = chase_symlinks("/etc/./.././", NULL, 0, &result);
         assert_se(r > 0);
         assert_se(path_equal(result, "/"));
-
         result = mfree(result);
+
         r = chase_symlinks("/etc/./.././", "/etc", 0, &result);
         assert_se(r > 0 && path_equal(result, "/etc"));
-
         result = mfree(result);
+
         r = chase_symlinks("/etc/machine-id/foo", NULL, 0, &result);
         assert_se(r == -ENOTDIR);
+        result = mfree(result);
 
         /* Path that loops back to self */
 
-        result = mfree(result);
         p = strjoina(temp, "/recursive-symlink");
         assert_se(symlink("recursive-symlink", p) >= 0);
         r = chase_symlinks(p, NULL, 0, &result);

diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c
index b3434bd12cbb568160fc811652455227ebb5ed11..09a624842c81dcd6870ad41e9b2b0e1ada2e734b 100644 (file)
--- a/src/test/test-mount-util.c
+++ b/src/test/test-mount-util.c
@@ -111,15 +111,23 @@ static void test_path_is_mount_point(void) {
 
         assert_se(path_is_mount_point("/", NULL, AT_SYMLINK_FOLLOW) > 0);
         assert_se(path_is_mount_point("/", NULL, 0) > 0);
+        assert_se(path_is_mount_point("//", NULL, AT_SYMLINK_FOLLOW) > 0);
+        assert_se(path_is_mount_point("//", NULL, 0) > 0);
 
         assert_se(path_is_mount_point("/proc", NULL, AT_SYMLINK_FOLLOW) > 0);
         assert_se(path_is_mount_point("/proc", NULL, 0) > 0);
+        assert_se(path_is_mount_point("/proc/", NULL, AT_SYMLINK_FOLLOW) > 0);
+        assert_se(path_is_mount_point("/proc/", NULL, 0) > 0);
 
         assert_se(path_is_mount_point("/proc/1", NULL, AT_SYMLINK_FOLLOW) == 0);
         assert_se(path_is_mount_point("/proc/1", NULL, 0) == 0);
+        assert_se(path_is_mount_point("/proc/1/", NULL, AT_SYMLINK_FOLLOW) == 0);
+        assert_se(path_is_mount_point("/proc/1/", NULL, 0) == 0);
 
         assert_se(path_is_mount_point("/sys", NULL, AT_SYMLINK_FOLLOW) > 0);
         assert_se(path_is_mount_point("/sys", NULL, 0) > 0);
+        assert_se(path_is_mount_point("/sys/", NULL, AT_SYMLINK_FOLLOW) > 0);
+        assert_se(path_is_mount_point("/sys/", NULL, 0) > 0);
 
         /* we'll create a hierarchy of different kinds of dir/file/link
          * layouts:
@@ -191,12 +199,21 @@ static void test_path_is_mount_point(void) {
 
         /* these tests will only work as root */
         if (mount(file1, file2, NULL, MS_BIND, NULL) >= 0) {
-                int rt, rf, rlt, rlf, rl1t, rl1f;
+                int rf, rt, rdf, rdt, rlf, rlt, rl1f, rl1t;
+                const char *file2d;
 
                 /* files */
                 /* capture results in vars, to avoid dangling mounts on failure */
+                log_info("%s: %s", __func__, file2);
                 rf = path_is_mount_point(file2, NULL, 0);
                 rt = path_is_mount_point(file2, NULL, AT_SYMLINK_FOLLOW);
+
+                file2d = strjoina(file2, "/");
+                log_info("%s: %s", __func__, file2d);
+                rdf = path_is_mount_point(file2d, NULL, 0);
+                rdt = path_is_mount_point(file2d, NULL, AT_SYMLINK_FOLLOW);
+
+                log_info("%s: %s", __func__, link2);
                 rlf = path_is_mount_point(link2, NULL, 0);
                 rlt = path_is_mount_point(link2, NULL, AT_SYMLINK_FOLLOW);
 
@@ -204,6 +221,8 @@ static void test_path_is_mount_point(void) {
 
                 assert_se(rf == 1);
                 assert_se(rt == 1);
+                assert_se(rdf == -ENOTDIR);
+                assert_se(rdt == -ENOTDIR);
                 assert_se(rlf == 0);
                 assert_se(rlt == 1);
 
@@ -216,10 +235,13 @@ static void test_path_is_mount_point(void) {
 
                 assert_se(mount(dir2, dir1, NULL, MS_BIND, NULL) >= 0);
 
+                log_info("%s: %s", __func__, dir1);
                 rf = path_is_mount_point(dir1, NULL, 0);
                 rt = path_is_mount_point(dir1, NULL, AT_SYMLINK_FOLLOW);
+                log_info("%s: %s", __func__, dirlink1);
                 rlf = path_is_mount_point(dirlink1, NULL, 0);
                 rlt = path_is_mount_point(dirlink1, NULL, AT_SYMLINK_FOLLOW);
+                log_info("%s: %s", __func__, dirlink1file);
                 /* its parent is a mount point, but not /file itself */
                 rl1f = path_is_mount_point(dirlink1file, NULL, 0);
                 rl1t = path_is_mount_point(dirlink1file, NULL, AT_SYMLINK_FOLLOW);

diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index fed829f1f7200f5e4cc36e4a9a58f47b2ba08d53..21d52f5d6e6736db3386f1ee610c50cc9d88ddc8 100644 (file)
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -399,6 +399,20 @@ static void test_file_in_same_dir(void) {
         free(t);
 }
 
+static void test_last_path_component(void) {
+        assert_se(streq(last_path_component("a/b/c"), "c"));
+        assert_se(streq(last_path_component("a/b/c/"), "c/"));
+        assert_se(streq(last_path_component("/"), "/"));
+        assert_se(streq(last_path_component("//"), "/"));
+        assert_se(streq(last_path_component("///"), "/"));
+        assert_se(streq(last_path_component("."), "."));
+        assert_se(streq(last_path_component("./."), "."));
+        assert_se(streq(last_path_component("././"), "./"));
+        assert_se(streq(last_path_component("././/"), ".//"));
+        assert_se(streq(last_path_component("/foo/a"), "a"));
+        assert_se(streq(last_path_component("/foo/a/"), "a/"));
+}
+
 static void test_filename_is_valid(void) {
         char foo[FILENAME_MAX+2];
         int i;
@@ -484,6 +498,7 @@ int main(int argc, char **argv) {
         test_path_startswith();
         test_prefix_root();
         test_file_in_same_dir();
+        test_last_path_component();
         test_filename_is_valid();
         test_hidden_or_backup_file();
         test_skip_dev_prefix();