From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 25 Jan 2021 18:50:47 +0000 (+0100)
Subject: path-util: tighten path_extract_filename()
X-Git-Tag: v248-rc1~222
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3cdcbdd32f8deaf4fbfeaaa908e77917229c661a;p=thirdparty%2Fsystemd.git

path-util: tighten path_extract_filename()

Let's tighten the logic behind path_extract_filename() a bit: first of
all, refuse all cases of invalid paths with -EINVAL. More importantly
though return a recognizable error when a valid path is specified that
does not contain any filename. Specifically, "/" will now result in
-EADDRNOTAVAIL.

This changes API, but none of the existing callers care about the return
value, hence the change should be fine.
---

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 5bcbc7a7947..3dff09b1519 100644
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -823,6 +823,8 @@ const char *last_path_component(const char *path) {
          *    Also, the empty string is mapped to itself.
          *
          * This is different than basename(), which returns "" when a trailing slash is present.
+         *
+         * This always succeeds (except if you pass NULL in which case it returns NULL, too).
          */
 
         unsigned l, k;
@@ -848,24 +850,24 @@ 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, *e = NULL, *q;
+        const char *c;
 
         /* 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. */
+         * 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. */
 
-        if (!p)
+        if (!path_is_valid(p))
                 return -EINVAL;
 
-        c = last_path_component(p);
-
-        for (q = c; *q != 0; q++)
-                if (*q != '/')
-                        e = q + 1;
+        /* Special case the root dir, because in that case we simply have no filename, but
+         * last_path_component() won't complain */
+        if (path_equal(p, "/"))
+                return -EADDRNOTAVAIL;
 
-        if (!e) /* no valid character? */
-                return -EINVAL;
+        c = last_path_component(p);
 
-        a = strndup(c, e - c);
+        a = strndup(c, strcspn(c, "/"));
         if (!a)
                 return -ENOMEM;
 
@@ -873,7 +875,6 @@ int path_extract_filename(const char *p, char **ret) {
                 return -EINVAL;
 
         *ret = TAKE_PTR(a);
-
         return 0;
 }
 
diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index fcaffa45391..206f5fd436d 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -578,9 +578,9 @@ 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("/", NULL, -EINVAL);
-        test_path_extract_filename_one("//", NULL, -EINVAL);
-        test_path_extract_filename_one("///", NULL, -EINVAL);
+        test_path_extract_filename_one("/", NULL, -EADDRNOTAVAIL);
+        test_path_extract_filename_one("//", NULL, -EADDRNOTAVAIL);
+        test_path_extract_filename_one("///", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one(".", NULL, -EINVAL);
         test_path_extract_filename_one("./.", NULL, -EINVAL);
         test_path_extract_filename_one("././", NULL, -EINVAL);