Change the format of LIST for symlinks

crates/unftp-sbe-fs/Cargo.toml

@@ -21,7 +21,7 @@ readme = "README.md"
 [dependencies]
 async-trait = "0.1.80"
 cfg-if = "1.0"
-cap-std = "2.0"
+cap-std = "3.1"
 futures = { version = "0.3.30", default-features = false, features = ["std"] }
 lazy_static = "1.4.0"
 libunftp = { version = "0.20.1", path = "../../" }
@@ -34,6 +34,7 @@ tracing-attributes = "0.1.27"
 [dev-dependencies]
 async_ftp = "6.0.0"
 async-trait = "0.1.80"
+chrono = "0.4.0"
 more-asserts = "0.3.1"
 nix = { version = "0.29.0", default-features = false, features = ["user"] }
 pretty_assertions = "1.4.0"

crates/unftp-sbe-fs/src/lib.rs

@@ -40,7 +40,7 @@ use std::{
 use tokio::io::AsyncSeekExt;
 
 #[cfg(unix)]
-use std::os::unix::fs::{MetadataExt, PermissionsExt};
+use cap_std::fs::{MetadataExt, PermissionsExt};
 
 /// The Filesystem struct is an implementation of the StorageBackend trait that keeps its files
 /// inside a specific root directory on local disk.
@@ -59,6 +59,7 @@ pub struct Filesystem {
 #[derive(Debug)]
 pub struct Meta {
     inner: cap_std::fs::Metadata,
+    target: Option<PathBuf>,
 }
 
 /// Strip the "/" prefix, if any, from a path.  Suitable for preprocessing the input pathnames
@@ -112,7 +113,19 @@ impl<User: UserDetail> StorageBackend<User> for Filesystem {
         let fs_meta = cap_fs::symlink_metadata(self.root_fd.clone(), &path)
             .await
             .map_err(|_| Error::from(ErrorKind::PermanentFileNotAvailable))?;
-        Ok(Meta { inner: fs_meta })
+        let target = if fs_meta.is_symlink() {
+            match self.root_fd.read_link_contents(path) {
+                Ok(p) => Some(p),
+                Err(_e) => {
+                    // XXX We should really log an error here.  But a logger object is not
+                    // available.
+                    None
+                }
+            }
+        } else {
+            None
+        };
+        Ok(Meta { inner: fs_meta, target })
     }
 
     #[allow(clippy::type_complexity)]
@@ -127,8 +140,21 @@ impl<User: UserDetail> StorageBackend<User> for Filesystem {
         let fis: Vec<Fileinfo<std::path::PathBuf, Self::Metadata>> = cap_fs::read_dir(self.root_fd.clone(), path)
             .and_then(|dirent| {
                 let entry_path: PathBuf = dirent.file_name().into();
-                cap_fs::symlink_metadata(self.root_fd.clone(), path.join(entry_path.clone())).map_ok(move |meta| {
-                    let metadata = Meta { inner: meta };
+                let fullpath = path.join(entry_path.clone());
+                cap_fs::symlink_metadata(self.root_fd.clone(), fullpath.clone()).map_ok(move |meta| {
+                    let target = if meta.is_symlink() {
+                        match self.root_fd.read_link_contents(&fullpath) {
+                            Ok(p) => Some(p),
+                            Err(_e) => {
+                                // XXX We should really log an error here.  But a logger object is
+                                // not available.
+                                None
+                            }
+                        }
+                    } else {
+                        None
+                    };
+                    let metadata = Meta { inner: meta, target };
                     Fileinfo { path: entry_path, metadata }
                 })
             })
@@ -286,6 +312,10 @@ impl Metadata for Meta {
             }
         }
     }
+
+    fn readlink(&self) -> Option<&Path> {
+        self.target.as_deref()
+    }
 }
 
 #[cfg(test)]

crates/unftp-sbe-fs/tests/main.rs

@@ -333,6 +333,103 @@ mod list {
         }
         assert!(found);
     }
+
+    /// test the exact format of the output for symlinks
+    #[cfg(unix)]
+    #[rstest]
+    #[case::relative(harness(), false)]
+    // Symlinks with absolute paths can be read, too
+    // https://github.com/bytecodealliance/cap-std/issues/353
+    #[case::absolute(harness(), true)]
+    #[awt]
+    #[tokio::test]
+    async fn symlink(
+        #[case]
+        #[future]
+        harness: Harness,
+        #[case] absolute: bool,
+    ) {
+        use regex::Regex;
+        use std::os::unix::fs::MetadataExt;
+
+        // Create a filename in the ftp root that we will look for in the `LIST` output
+        let path = harness.root.join("link");
+        let target = if absolute { "/target" } else { "target" };
+        std::os::unix::fs::symlink(target, &path).unwrap();
+        let md = std::fs::symlink_metadata(&path).unwrap();
+        let uid = md.uid();
+        let gid = md.gid();
+        let link_count = md.nlink();
+        let size = md.len();
+
+        let mut ftp_stream = FtpStream::connect(harness.addr).await.unwrap();
+
+        ensure_login_required(ftp_stream.list(None).await);
+
+        ftp_stream.login("hoi", "jij").await.unwrap();
+        let list = ftp_stream.list(None).await.unwrap();
+        let pat = format!("^l[rwx-]{{9}}\\s+{link_count}\\s+{uid}\\s+{gid}\\s+{size}.*link -> {target}");
+        let re = Regex::new(&pat).unwrap();
+        for entry in list {
+            if entry.contains("link") {
+                assert!(re.is_match(&entry), "\"{entry}\" did not match pattern {re:?}");
+                return;
+            }
+        }
+        panic!("Entry not found");
+    }
+}
+
+mod mdtm {
+    use super::*;
+    use pretty_assertions::assert_eq;
+
+    /// Get the modification time of a regular file
+    #[rstest]
+    #[awt]
+    #[tokio::test]
+    async fn regular(#[future] harness: Harness) {
+        // Create a filename in the ftp root that we will look for in the `LIST` output
+        let path = harness.root.join("test.txt");
+        let f = std::fs::File::create(path).unwrap();
+        let modified = f.metadata().unwrap().modified().unwrap();
+
+        let mut ftp_stream = FtpStream::connect(harness.addr).await.unwrap();
+
+        ensure_login_required(ftp_stream.list(None).await);
+
+        ftp_stream.login("hoi", "jij").await.unwrap();
+        let r = ftp_stream.mdtm("test.txt").await.unwrap().unwrap();
+        assert_eq!(r.to_rfc2822(), chrono::DateTime::<chrono::Utc>::from(modified).to_rfc2822());
+    }
+
+    /// Get the modification time of a symlink
+    #[rstest]
+    #[case::relative(harness(), false)]
+    #[case::absolute(harness(), true)]
+    #[awt]
+    #[tokio::test]
+    async fn symlink(
+        #[case]
+        #[future]
+        harness: Harness,
+        #[case] absolute: bool,
+    ) {
+        // Create a filename in the ftp root that we will look for in the `LIST` output
+        let path = harness.root.join("link");
+        let target = if absolute { "/target" } else { "target" };
+        std::os::unix::fs::symlink(target, &path).unwrap();
+        let md = std::fs::symlink_metadata(&path).unwrap();
+        let modified = md.modified().unwrap();
+
+        let mut ftp_stream = FtpStream::connect(harness.addr).await.unwrap();
+
+        ensure_login_required(ftp_stream.list(None).await);
+
+        ftp_stream.login("hoi", "jij").await.unwrap();
+        let r = ftp_stream.mdtm("link").await.unwrap().unwrap();
+        assert_eq!(r.to_rfc2822(), chrono::DateTime::<chrono::Utc>::from(modified).to_rfc2822());
+    }
 }
 
 #[rstest]

src/storage/storage_backend.rs

@@ -65,6 +65,11 @@ pub trait Metadata {
     fn permissions(&self) -> Permissions {
         Permissions(0o7755)
     }
+
+    /// If this is a symlink, return the path to its target
+    fn readlink(&self) -> Option<&Path> {
+        None
+    }
 }
 
 /// Represents the permissions of a _FTP File_
@@ -134,9 +139,20 @@ where
             }
         };
         let perms = format!("{}", self.metadata.permissions());
+        let link_target = if self.metadata.is_symlink() {
+            match self.metadata.readlink() {
+                Some(t) => format!(" -> {}", t.display()),
+                None => {
+                    // We ought to log an error here, but don't have access to the logger variable
+                    "".to_string()
+                }
+            }
+        } else {
+            "".to_string()
+        };
         write!(
             f,
-            "{filetype}{permissions} {links:>12} {owner:>12} {group:>12} {size:#14} {modified:>12} {path}",
+            "{filetype}{permissions} {links:>12} {owner:>12} {group:>12} {size:#14} {modified:>12} {path}{link_target}",
             filetype = if self.metadata.is_dir() {
                 "d"
             } else if self.metadata.is_symlink() {