From 189ced5d89ac1f2a03e900b01467547a08c32c90 Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Mon, 13 May 2024 15:46:14 -0600
Subject: [PATCH] unftp-sbe-fs: fix the format of the LIST command

* Actually implement Metadata::permissions
* Correct some cfg expressions, fixing uid, gid, and nlinks
---
 crates/unftp-sbe-fs/Cargo.toml    |  2 ++
 crates/unftp-sbe-fs/src/lib.rs    | 22 +++++++++++++-----
 crates/unftp-sbe-fs/tests/main.rs | 38 +++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/crates/unftp-sbe-fs/Cargo.toml b/crates/unftp-sbe-fs/Cargo.toml
index 407f24fc..80023390 100644
--- a/crates/unftp-sbe-fs/Cargo.toml
+++ b/crates/unftp-sbe-fs/Cargo.toml
@@ -35,8 +35,10 @@ tracing-attributes = "0.1.27"
 async_ftp = "6.0.0"
 async-trait = "0.1.77"
 more-asserts = "0.3.1"
+nix = { version = "0.26.4", default-features = false, features = ["user"] }
 pretty_assertions = "1.4.0"
 pretty_env_logger = "0.5.0"
+regex = "1.10.3"
 rstest = "0.18.2"
 serde = { version = "1.0.196", features = ["derive"] }
 serde_json = "1.0.113"
diff --git a/crates/unftp-sbe-fs/src/lib.rs b/crates/unftp-sbe-fs/src/lib.rs
index 041db946..6b7f6c67 100644
--- a/crates/unftp-sbe-fs/src/lib.rs
+++ b/crates/unftp-sbe-fs/src/lib.rs
@@ -30,7 +30,7 @@ use cfg_if::cfg_if;
 use futures::{future::TryFutureExt, stream::TryStreamExt};
 use lazy_static::lazy_static;
 use libunftp::auth::UserDetail;
-use libunftp::storage::{Error, ErrorKind, Fileinfo, Metadata, Result, StorageBackend};
+use libunftp::storage::{Error, ErrorKind, Fileinfo, Metadata, Permissions, Result, StorageBackend};
 use std::{
     fmt::Debug,
     io,
@@ -40,8 +40,8 @@ use std::{
 };
 use tokio::io::AsyncSeekExt;
 
-#[cfg(target_os = "unix")]
-use std::os::unix::fs::MetadataExt;
+#[cfg(unix)]
+use std::os::unix::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.
@@ -250,7 +250,7 @@ impl Metadata for Meta {
 
     fn gid(&self) -> u32 {
         cfg_if! {
-            if #[cfg(target_os = "unix")] {
+            if #[cfg(unix)] {
                 self.inner.gid()
             } else {
                 0
@@ -260,7 +260,7 @@ impl Metadata for Meta {
 
     fn uid(&self) -> u32 {
         cfg_if! {
-            if #[cfg(target_os = "unix")] {
+            if #[cfg(unix)] {
                 self.inner.uid()
             } else {
                 0
@@ -270,13 +270,23 @@ impl Metadata for Meta {
 
     fn links(&self) -> u64 {
         cfg_if! {
-            if #[cfg(target_os = "unix")] {
+            if #[cfg(unix)] {
                 self.inner.nlink()
             } else {
                 1
             }
         }
     }
+
+    fn permissions(&self) -> Permissions {
+        cfg_if! {
+            if #[cfg(unix)] {
+                Permissions(self.inner.permissions().mode())
+            } else {
+                Permissions(0o7755)
+            }
+        }
+    }
 }
 
 #[cfg(test)]
diff --git a/crates/unftp-sbe-fs/tests/main.rs b/crates/unftp-sbe-fs/tests/main.rs
index 3c8d9e75..323a6f37 100644
--- a/crates/unftp-sbe-fs/tests/main.rs
+++ b/crates/unftp-sbe-fs/tests/main.rs
@@ -243,6 +243,44 @@ async fn put(#[future] harness: Harness) {
 mod list {
     use super::*;
 
+    /// test the exact format of the output
+    #[cfg(unix)]
+    #[rstest]
+    #[awt]
+    #[tokio::test]
+    async fn format(#[future] harness: Harness) {
+        use regex::Regex;
+        use std::os::unix::fs::{fchown, MetadataExt, OpenOptionsExt};
+
+        // 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::OpenOptions::new().read(true).write(true).create(true).mode(0o754).open(path).unwrap();
+        // Because most OSes set the file's gid to its parent directory's, and the parent
+        // directory's is often root, deliberately set it to something more interesting.
+        fchown(&f, None, Some(nix::unistd::Gid::effective().as_raw())).unwrap();
+        let md = f.metadata().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!("^-rwxr-xr--\\s+{link_count}\\s+{uid}\\s+{gid}\\s+{size}.*test.txt");
+        let re = Regex::new(&pat).unwrap();
+        for entry in list {
+            if entry.contains("test.txt") {
+                assert!(re.is_match(&entry), "\"{entry}\" did not match pattern {re:?}");
+                return;
+            }
+        }
+        panic!("Entry not found");
+    }
+
     #[rstest]
     #[awt]
     #[tokio::test]