Fix the issue when path is suffixed by "/"

src/libos/src/fs/file_ops/link.rs

@@ -15,6 +15,9 @@ pub fn do_linkat(old_fs_path: &FsPath, new_fs_path: &FsPath, flags: LinkFlags) -
 
     let newpath = new_fs_path.to_abs_path()?;
     let (new_dir_path, new_file_name) = split_path(&newpath);
+    if new_file_name.ends_with("/") {
+        return_errno!(EISDIR, "new path is dir");
+    }
     let (inode, new_dir_inode) = {
         let oldpath = old_fs_path.to_abs_path()?;
         let current = current!();

src/libos/src/fs/file_ops/mkdir.rs

@@ -4,7 +4,7 @@ pub fn do_mkdirat(fs_path: &FsPath, mode: FileMode) -> Result<()> {
     debug!("mkdirat: fs_path: {:?}, mode: {:#o}", fs_path, mode.bits());
 
     let path = fs_path.to_abs_path()?;
-    let (dir_path, file_name) = split_path(&path);
+    let (dir_path, file_name) = split_path(&path.trim_end_matches('/'));
     let current = current!();
     let inode = {
         let fs = current.fs().read().unwrap();

src/libos/src/fs/file_ops/rename.rs

@@ -19,19 +19,23 @@ pub fn do_renameat(old_fs_path: &FsPath, new_fs_path: &FsPath) -> Result<()> {
     let current = current!();
     let fs = current.fs().read().unwrap();
 
-    let (old_dir_path, old_file_name) = split_path(&oldpath);
+    // The source and target to be renamed could be dirs
-    let (new_dir_path, new_file_name) = split_path(&newpath);
+    let (old_dir_path, old_file_name) = split_path(&oldpath.trim_end_matches('/'));
+    let (new_dir_path, new_file_name) = split_path(&newpath.trim_end_matches('/'));
     let old_dir_inode = fs.lookup_inode(old_dir_path)?;
     let new_dir_inode = fs.lookup_inode(new_dir_path)?;
     let old_file_mode = {
         let old_file_inode = old_dir_inode.find(old_file_name)?;
         let metadata = old_file_inode.metadata()?;
+        // oldpath is directory, the old_file_inode should be directory
+        if oldpath.ends_with("/") && metadata.type_ != FileType::Dir {
+            return_errno!(ENOTDIR, "old path is not a directory");
+        }
         FileMode::from_bits_truncate(metadata.mode)
     };
     if old_file_mode.has_sticky_bit() {
         warn!("ignoring the sticky bit");
     }
-    // TODO: support to modify file's absolute path
     old_dir_inode.move_(old_file_name, &new_dir_inode, new_file_name)?;
     Ok(())
 }

src/libos/src/fs/file_ops/rmdir.rs

@@ -3,7 +3,7 @@ use super::*;
 pub fn do_rmdir(path: &str) -> Result<()> {
     debug!("rmdir: path: {:?}", path);
 
-    let (dir_path, file_name) = split_path(&path);
+    let (dir_path, file_name) = split_path(path.trim_end_matches('/'));
     let dir_inode = {
         let current = current!();
         let fs = current.fs().read().unwrap();

src/libos/src/fs/file_ops/symlink.rs

@@ -36,6 +36,9 @@ pub fn do_symlinkat(target: &str, link_path: &FsPath) -> Result<usize> {
 
     let link_path = link_path.to_abs_path()?;
     let (dir_path, link_name) = split_path(&link_path);
+    if link_name.ends_with('/') {
+        return_errno!(EISDIR, "link path is dir");
+    }
     let dir_inode = {
         let current = current!();
         let fs = current.fs().read().unwrap();

src/libos/src/fs/file_ops/unlink.rs

@@ -8,6 +8,9 @@ bitflags! {
 
 fn do_unlink(path: &str) -> Result<()> {
     let (dir_path, file_name) = split_path(&path);
+    if file_name.ends_with("/") {
+        return_errno!(EISDIR, "unlink on directory");
+    }
     let dir_inode = {
         let current = current!();
         let fs = current.fs().read().unwrap();

src/libos/src/fs/fs_view.rs

@@ -68,6 +68,12 @@ impl FsView {
                     inode
                 }
                 Err(e) if e.errno() == ENOENT && creation_flags.can_create() => {
+                    if creation_flags.must_be_directory() {
+                        return_errno!(ENOTDIR, "cannot create directory");
+                    }
+                    if path.ends_with("/") {
+                        return_errno!(EISDIR, "path is a directory");
+                    }
                     let (dir_path, file_name) = split_path(&path);
                     let dir_inode = self.lookup_inode(dir_path)?;
                     if !dir_inode.allow_write()? {
@@ -94,8 +100,17 @@ impl FsView {
                     inode
                 }
                 Err(e) if e.errno() == ENOENT && creation_flags.can_create() => {
+                    if creation_flags.must_be_directory() {
+                        return_errno!(ENOTDIR, "cannot create directory");
+                    }
+                    if path.ends_with("/") {
+                        return_errno!(EISDIR, "path is a directory");
+                    }
                     let real_path = self.lookup_real_path(&path)?;
                     let (dir_path, file_name) = split_path(&real_path);
+                    if file_name.ends_with("/") {
+                        return_errno!(EISDIR, "path refers to a directory");
+                    }
                     let dir_inode = self.lookup_inode(dir_path)?;
                     if !dir_inode.allow_write()? {
                         return_errno!(EPERM, "file cannot be created");
@@ -111,31 +126,29 @@ impl FsView {
 
     /// Recursively lookup the real path of giving path, dereference symlinks
     pub fn lookup_real_path(&self, path: &str) -> Result<String> {
-        let (dir_path, file_name) = split_path(&path);
+        let (dir_path, file_name) = split_path(path);
         let dir_inode = self.lookup_inode(dir_path)?;
-        match dir_inode.find(file_name) {
+        match dir_inode.find(file_name.trim_end_matches('/')) {
             // Handle symlink
             Ok(inode) if inode.metadata()?.type_ == FileType::SymLink => {
                 let new_path = {
                     let mut content = vec![0u8; PATH_MAX];
                     let len = inode.read_at(0, &mut content)?;
-                    let path = std::str::from_utf8(&content[..len])
+                    let link_path = std::str::from_utf8(&content[..len])
                         .map_err(|_| errno!(ENOENT, "invalid symlink content"))?;
-                    let path = String::from(path);
+                    let link_path = String::from(link_path);
-                    match path.chars().next() {
+
-                        None => unreachable!(),
+                    let mut new_path = match link_path.chars().next() {
+                        None => return_errno!(ENOENT, "empty symlink"),
                         // absolute path
-                        Some('/') => path,
+                        Some('/') => link_path,
                         // relative path
-                        _ => {
+                        _ => String::from(dir_path) + "/" + &link_path,
-                            let dir_path = if dir_path.ends_with("/") {
+                    };
-                                String::from(dir_path)
+                    if path.ends_with("/") && !new_path.ends_with("/") {
-                            } else {
+                        new_path += "/";
-                                String::from(dir_path) + "/"
-                            };
-                            dir_path + &path
-                        }
                     }
+                    new_path
                 };
                 self.lookup_real_path(&new_path)
             }
@@ -147,7 +160,8 @@ impl FsView {
         }
     }
 
-    /// Lookup INode from the cwd of the process. If path is a symlink, do not dereference it
+    /// Lookup INode from the cwd of the process.
+    /// If last component is a symlink, do not dereference it
     pub fn lookup_inode_no_follow(&self, path: &str) -> Result<Arc<dyn INode>> {
         debug!(
             "lookup_inode_no_follow: cwd: {:?}, path: {:?}",
@@ -155,8 +169,13 @@ impl FsView {
             path
         );
         let (dir_path, file_name) = split_path(&path);
-        let dir_inode = self.lookup_inode(dir_path)?;
+        let inode = if file_name.ends_with("/") {
-        Ok(dir_inode.lookup(file_name)?)
+            self.lookup_inode(path)?
+        } else {
+            let dir_inode = self.lookup_inode(dir_path)?;
+            dir_inode.lookup(file_name)?
+        };
+        Ok(inode)
     }
 
     /// Lookup INode from the cwd of the process, dereference symlink

src/libos/src/fs/mod.rs

@@ -59,14 +59,33 @@ mod stdio;
 mod syscalls;
 mod timer_file;
 
-/// Split a `path` str to `(base_path, file_name)`
+/// Split a `path` to (`dir_path`, `file_name`).
+///
+/// The `dir_path` must be a directory.
+///
+/// The `file_name` is the last component. It can be suffixed by "/".
+///
+/// Example:
+///
+/// The path "/dir/file/" will be split to ("/dir", "file/").
 fn split_path(path: &str) -> (&str, &str) {
+    let file_name = path
+        .split_inclusive('/')
+        .filter(|&x| x != "/")
+        .last()
+        .unwrap_or(".");
+
     let mut split = path.trim_end_matches('/').rsplitn(2, '/');
-    let file_name = split.next().unwrap();
+    let dir_path = if split.next().unwrap().is_empty() {
-    let mut dir_path = split.next().unwrap_or(".");
+        "/"
-    if dir_path == "" {
+    } else {
-        dir_path = "/";
+        let mut dir = split.next().unwrap_or(".").trim_end_matches('/');
-    }
+        if dir.is_empty() {
+            dir = "/";
+        }
+        dir
+    };
+
     (dir_path, file_name)
 }
 

src/libos/src/fs/rootfs.rs

@@ -80,8 +80,12 @@ pub fn umount_nonroot_fs(
         root.lookup_follow(abs_path, MAX_SYMLINKS)?
     } else {
         let (dir_path, file_name) = split_path(abs_path);
-        root.lookup_follow(dir_path, MAX_SYMLINKS)?
+        if file_name.ends_with("/") {
-            .lookup(file_name)?
+            root.lookup_follow(abs_path, MAX_SYMLINKS)?
+        } else {
+            root.lookup_follow(dir_path, MAX_SYMLINKS)?
+                .lookup(file_name)?
+        }
     };
 
     mount_dir.downcast_ref::<MNode>().unwrap().umount()?;
@@ -173,9 +177,13 @@ pub fn mount_fs_at(
         parent_inode.lookup_follow(path, MAX_SYMLINKS)?
     } else {
         let (dir_path, file_name) = split_path(path);
-        parent_inode
+        if file_name.ends_with("/") {
-            .lookup_follow(dir_path, MAX_SYMLINKS)?
+            parent_inode.lookup_follow(path, MAX_SYMLINKS)?
-            .lookup(file_name)?
+        } else {
+            parent_inode
+                .lookup_follow(dir_path, MAX_SYMLINKS)?
+                .lookup(file_name)?
+        }
     };
     mount_dir.downcast_ref::<MNode>().unwrap().mount(fs)?;
     Ok(())

test/hostfs/main.c

@@ -211,7 +211,7 @@ static int test_truncate() {
 }
 
 static int test_mkdir_then_rmdir() {
-    const char *dir_path = "/host/hostfs_dir";
+    const char *dir_path = "/host/hostfs_dir/";
     struct stat stat_buf;
     int dir_fd;
 

test/mkdir/main.c

@@ -48,6 +48,24 @@ static int __test_mkdir(const char *dir_path) {
     return 0;
 }
 
+static int __test_mkdir_with_suffix(const char *dir_path) {
+    struct stat stat_buf;
+    char new_dir_path[PATH_MAX] = { 0 };
+    snprintf(new_dir_path, sizeof(new_dir_path), "%s/", dir_path);
+    mode_t mode = 00775;
+
+    if (mkdir(new_dir_path, mode) < 0) {
+        THROW_ERROR("failed to mkdir");
+    }
+    if (stat(new_dir_path, &stat_buf) < 0) {
+        THROW_ERROR("failed to stat dir");
+    }
+    if (!S_ISDIR(stat_buf.st_mode)) {
+        THROW_ERROR("failed to check if it is dir");
+    }
+    return 0;
+}
+
 static int __test_mkdirat(const char *dir_path) {
     struct stat stat_buf;
     mode_t mode = 00775;
@@ -95,6 +113,10 @@ static int test_mkdir() {
     return test_mkdir_framework(__test_mkdir);
 }
 
+static int test_mkdir_with_suffix() {
+    return test_mkdir_framework(__test_mkdir_with_suffix);
+}
+
 static int test_mkdirat() {
     return test_mkdir_framework(__test_mkdirat);
 }
@@ -189,6 +211,7 @@ static int test_rmdir_via_unlinkat() {
 
 static test_case_t test_cases[] = {
     TEST_CASE(test_mkdir),
+    TEST_CASE(test_mkdir_with_suffix),
     TEST_CASE(test_mkdirat),
     TEST_CASE(test_chdir),
     TEST_CASE(test_rmdir_via_unlinkat),

test/open/main.c

@@ -27,17 +27,33 @@ static int __test_open(const char *file_path, int flags, int mode) {
         THROW_ERROR("failed to open a file");
     }
     close(fd);
+
+    return 0;
+}
+
+static int __test_open_file_as_dir(const char *file_path, int flags, int mode) {
+    char dir_path[PATH_MAX] = { 0 };
+    snprintf(dir_path, sizeof(dir_path), "%s/", file_path);
+
+    int fd = open(dir_path, flags, mode);
+    if (!(fd < 0 && errno == EISDIR)) {
+        THROW_ERROR("failed check open a file as dir");
+    }
+
+    if (__test_open(file_path, flags, mode) < 0) {
+        THROW_ERROR("failed to create file");
+    }
     return 0;
 }
 
 static int __test_open_file_with_dir_flags(const char *file_path, int flags, int mode) {
-    flags = O_DIRECTORY | O_RDWR | O_CREAT;
     int fd = open(file_path, flags, mode);
     if (fd < 0) {
-        THROW_ERROR("failed to check creating file with O_DIRECTORY");
+        THROW_ERROR("failed to open a file");
     }
     close(fd);
 
+    flags = O_DIRECTORY | O_RDWR;
     fd = open(file_path, flags, mode);
     if (!(fd < 0 && errno == ENOTDIR)) {
         THROW_ERROR("open file with O_DIRECTORY should return ENOTDIR");
@@ -132,6 +148,10 @@ static int test_open() {
     return test_open_framework(__test_open);
 }
 
+static int test_open_file_as_dir() {
+    return test_open_framework(__test_open_file_as_dir);
+}
+
 static int test_open_file_with_dir_flags() {
     return test_open_framework(__test_open_file_with_dir_flags);
 }
@@ -158,6 +178,7 @@ static int test_creat() {
 
 static test_case_t test_cases[] = {
     TEST_CASE(test_open),
+    TEST_CASE(test_open_file_as_dir),
     TEST_CASE(test_open_file_with_dir_flags),
     TEST_CASE(test_open_dir_with_write_flags),
     TEST_CASE(test_openat_with_abs_path),

test/rename/main.c

@@ -132,7 +132,7 @@ static int test_renameat() {
 }
 
 static int test_rename_dir() {
-    const char *old_dir = "/root/test_old_dir";
+    const char *old_dir = "/root/test_old_dir/";
     const char *new_dir = "/root/test_new_dir";
     const char *file_name = "test_file.txt";
     char file_buf[128] = { 0 };
@@ -203,6 +203,26 @@ static int test_rename_dir_to_subdir() {
     return 0;
 }
 
+static int test_rename_file_as_dir() {
+    const char *old_file = "/root/test_old_file";
+    const char *old_dir = "/root/test_old_file/";
+    const char *new_dir = "/root/test_new_dir";
+
+    if (create_file_with_content(old_file, WRITE_MSG) < 0) {
+        THROW_ERROR("failed to create old file with content");
+    }
+
+    int ret = rename(old_dir, new_dir);
+    if (ret == 0 || errno != ENOTDIR) {
+        THROW_ERROR("failed to check rename file as dir");
+    }
+
+    if (unlink(old_file) < 0) {
+        THROW_ERROR("failed to remove file");
+    }
+    return 0;
+}
+
 // ============================================================================
 // Test suite main
 // ============================================================================
@@ -214,6 +234,7 @@ static test_case_t test_cases[] = {
     TEST_CASE(test_renameat),
     TEST_CASE(test_rename_dir),
     TEST_CASE(test_rename_dir_to_subdir),
+    TEST_CASE(test_rename_file_as_dir),
 };
 
 int main(int argc, const char *argv[]) {

test/stat/main.c

@@ -47,6 +47,19 @@ static int __test_stat(const char *file_path) {
     return 0;
 }
 
+static int __test_stat_file_as_dir(const char *file_path) {
+    struct stat stat_buf;
+    char dir_path[PATH_MAX] = { 0 };
+    snprintf(dir_path, sizeof(dir_path), "%s/", file_path);
+
+    int ret = stat(dir_path, &stat_buf);
+    if (ret == 0 || errno != ENOTDIR) {
+        THROW_ERROR("failed to check stat file as dir");
+    }
+
+    return 0;
+}
+
 static int __test_fstat(const char *file_path) {
     struct stat stat_buf;
     int fd, ret;
@@ -155,6 +168,10 @@ static int test_stat() {
     return test_stat_framework(__test_stat);
 }
 
+static int test_stat_file_as_dir() {
+    return test_stat_framework(__test_stat_file_as_dir);
+}
+
 static int test_fstat() {
     return test_stat_framework(__test_fstat);
 }
@@ -181,6 +198,7 @@ static int test_fstatat_with_dirfd() {
 
 static test_case_t test_cases[] = {
     TEST_CASE(test_stat),
+    TEST_CASE(test_stat_file_as_dir),
     TEST_CASE(test_fstat),
     TEST_CASE(test_lstat),
     TEST_CASE(test_fstatat_with_abs_path),