Implement MLSD & fix MLST

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

@@ -634,3 +634,199 @@ async fn rename(#[future] harness: Harness) {
 //     let size3 = size2.unwrap();
 //     assert_eq!(size3, fs::metadata(&file_in_root).unwrap().len() as usize, "Wrong size returned.");
 // }
+
+// MLSD and MLST tests using raw TCP since async_ftp doesn't support MLSD
+mod mlsd {
+    use std::path::Path;
+
+    use super::*;
+    use pretty_assertions::assert_eq;
+    use tokio::net::TcpStream;
+
+    async fn send_command(stream: &TcpStream, command: &str) {
+        loop {
+            stream.writable().await.unwrap();
+            match stream.try_write(format!("{}\r\n", command).as_bytes()) {
+                Ok(_) => break,
+                Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => continue,
+                Err(e) => panic!("Failed to send command: {}", e),
+            };
+        }
+    }
+
+    async fn connect(addr: &str) -> FtpStream {
+        let mut ftp_stream = FtpStream::connect(addr).await.unwrap();
+        ftp_stream.login("hoi", "jij").await.unwrap();
+        ftp_stream
+    }
+
+    fn parse_pasv_response(response: &str) -> u16 {
+        // Parse "Entering Passive Mode (127,0,0,1,p1,p2)"
+        // Port = p1 * 256 + p2
+        let start = response.find('(').unwrap() + 1;
+        let end = response.find(')').unwrap();
+        let coords = &response[start..end];
+        let parts: Vec<&str> = coords.split(',').collect();
+        let p1: u16 = parts[4].parse().unwrap();
+        let p2: u16 = parts[5].parse().unwrap();
+        p1 * 256 + p2
+    }
+
+    async fn connect_pasv(ftp: &mut FtpStream) -> TcpStream {
+        send_command(ftp.get_ref(), "PASV").await;
+        let pasv_response = ftp.read_response(227).await.unwrap();
+        let data_port = parse_pasv_response(&pasv_response.1);
+        TcpStream::connect(format!("127.0.0.1:{}", data_port)).await.unwrap()
+    }
+
+    async fn read_all(stream: &TcpStream, until_eof: bool) -> String {
+        let mut data = String::new();
+        let mut buffer = vec![0u8; 1024];
+        let mut blocks = 0;
+        loop {
+            match stream.try_read(&mut buffer) {
+                Ok(0) => break, // EOF
+                Ok(n) => {
+                    data.push_str(&String::from_utf8_lossy(&buffer[0..n]));
+                }
+                Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => {
+                    blocks += 1;
+                    if !until_eof && blocks >= 2 {
+                        break;
+                    }
+                    // Wait a bit for more data or connection close
+                    tokio::time::sleep(std::time::Duration::from_millis(10)).await;
+                    continue;
+                }
+                Err(_) => break, // Connection closed or error
+            }
+        }
+        data
+    }
+
+    fn setup_files(root: &Path) {
+        std::fs::write(&root.join("test_file.txt"), b"test content").unwrap();
+        std::fs::create_dir(root.join("test_dir")).unwrap();
+        std::fs::File::create(root.join("test_dir/file_in_subdir.txt")).unwrap();
+    }
+
+    fn check_facts(line: &str, expected: &[&str]) {
+        let parts: Vec<&str> = line.split(' ').collect();
+        assert_eq!(parts.len(), 2, "Line should have facts and filename separated by space");
+
+        // RFC 3659 section 7.2 requires facts to end with a semicolon
+        // Grammar: facts = 1*( fact ";" )
+        // This means every fact must be followed by a semicolon, including the last one
+        assert!(
+            parts[0].ends_with(';'),
+            "Facts string must end with a semicolon per RFC 3659 section 7.2, got: {}",
+            parts[0]
+        );
+
+        let facts = parts[0].split(';').collect::<Vec<_>>();
+        for fact in expected {
+            assert!(facts.contains(&fact), "Facts part should contain {}, got: {}", fact, parts[0]);
+        }
+    }
+
+    #[rstest]
+    #[awt]
+    #[tokio::test]
+    async fn basic_mlsd(#[future] harness: Harness) {
+        setup_files(&harness.root);
+
+        let mut ftp = connect(&harness.addr).await;
+        let data_stream = connect_pasv(&mut ftp).await;
+
+        // MLSD uses data channel. Example from RFC 3659 (truncated):
+        //
+        // C> MLSD tmp
+        // S> 150 BINARY connection open for MLSD tmp
+        // ...
+        // D> Type=file;Size=25730;Modify=19940728095854;Perm=; capmux.tar.z
+        // D> Type=file;Size=1830;Modify=19940916055648;Perm=r; hatch.c
+        // ...
+        // S> 226 MLSD completed
+        send_command(ftp.get_ref(), "MLSD").await;
+        ftp.read_response(150).await.unwrap();
+
+        let data = read_all(&data_stream, true).await;
+        let mut entries = data.split("\r\n").collect::<Vec<_>>();
+        assert!(entries.pop().unwrap().is_empty(), "Last line should be empty due to trailing CRLF");
+
+        assert_eq!(entries.len(), 2);
+        let file_line = *entries
+            .iter()
+            .find(|line| line.contains(" test_file.txt"))
+            .expect("Should return test_file.txt");
+        let dir_line = *entries.iter().find(|line| line.contains(" test_dir")).expect("Should return test_dir");
+
+        check_facts(file_line, &["type=file", "size=12"]);
+        check_facts(dir_line, &["type=dir"]);
+
+        ftp.read_response(226).await.unwrap();
+    }
+
+    #[rstest]
+    #[awt]
+    #[tokio::test]
+    async fn mlsd_with_path(#[future] harness: Harness) {
+        setup_files(&harness.root);
+
+        let mut ftp = connect(&harness.addr).await;
+        let data_stream = connect_pasv(&mut ftp).await;
+
+        send_command(ftp.get_ref(), "MLSD test_dir").await;
+        ftp.read_response(150).await.unwrap();
+
+        let data = read_all(&data_stream, true).await;
+        let mut entries = data.split("\r\n").collect::<Vec<_>>();
+        assert!(entries.pop().unwrap().is_empty(), "Last line should be empty due to trailing CRLF");
+        assert_eq!(entries.len(), 1);
+
+        let file_line = *entries
+            .iter()
+            .find(|line| line.contains(" file_in_subdir.txt"))
+            .expect("Should return file_in_subdir.txt");
+
+        check_facts(&file_line, &["type=file", "size=0"]);
+
+        ftp.read_response(226).await.unwrap();
+    }
+
+    #[rstest]
+    #[awt]
+    #[tokio::test]
+    async fn mlst(#[future] harness: Harness) {
+        setup_files(&harness.root);
+
+        let ftp = connect(&harness.addr).await;
+
+        // MLST uses control channel, not data channel. Format:
+        //
+        // 250- Listing
+        //  Type=file;Size=1234;... filename.txt
+        // 250 End
+        let stream = ftp.get_ref();
+        send_command(stream, "MLST test_file.txt").await;
+        let response = read_all(stream, false).await;
+        let line = response
+            .strip_prefix("250- Listing\r\n ")
+            .expect("Wrong prefix")
+            .strip_suffix("250 End\r\n")
+            .expect("Wrong suffix");
+
+        println!("line {line}");
+        check_facts(line, &["type=file", "size=12"]);
+
+        send_command(stream, "MLST test_dir").await;
+        let response = read_all(stream, false).await;
+
+        let line = response
+            .strip_prefix("250- Listing\r\n ")
+            .expect("Wrong prefix")
+            .strip_suffix("250 End\r\n")
+            .expect("Wrong suffix");
+        check_facts(line, &["type=dir"]);
+    }
+}

src/server/chancomms.rs

@@ -40,6 +40,10 @@ pub enum DataChanCmd {
         /// The path of the file/directory the clients wants to list.
         path: Option<String>,
     },
+    Mlsd {
+        /// The path of the directory the clients wants to list.
+        path: Option<String>,
+    },
 }
 
 impl DataChanCmd {
@@ -49,7 +53,8 @@ impl DataChanCmd {
             DataChanCmd::Retr { path, .. } => Some(path.clone()),
             DataChanCmd::Stor { path, .. } => Some(path.clone()),
             DataChanCmd::List { path, .. } => path.clone(),
-            DataChanCmd::Nlst { path, .. } => path.clone(),
+            DataChanCmd::Mlsd { path } => path.clone(),
+            DataChanCmd::Nlst { path } => path.clone(),
         }
     }
 }

src/server/controlchan/command.rs

@@ -67,6 +67,11 @@ pub enum Command {
         /// The path of the file/directory to get information about
         path: Option<String>,
     },
+    /// Machine List Directory (MLSD) command for getting machine-readable information about directory contents
+    Mlsd {
+        /// The path of the directory to list
+        path: Option<String>,
+    },
     Feat,
     Pwd,
     Cwd {

src/server/controlchan/commands/feat.rs

@@ -26,7 +26,7 @@ where
 {
     #[tracing_attributes::instrument]
     async fn handle(&self, args: CommandContext<Storage, User>) -> Result<Reply, ControlChanError> {
-        let mut feat_text = vec![" SIZE", " MDTM", " UTF8", " EPSV", " MLST"];
+        let mut feat_text = vec![" SIZE", " MDTM", " UTF8", " EPSV", " MLST", " MLSD"];
 
         // Add the features. According to the spec each feature line must be
         // indented by a space.

src/server/controlchan/commands/mlsd.rs

@@ -0,0 +1,61 @@
+//! The RFC 3659 Machine List Directory (`MLSD`) command
+//
+// This command causes a listing to be sent from the server to the passive DTP.
+// The server-DTP will send a list of the contents of the specified directory
+// over the data connection. Each file entry is formatted using the machine-readable
+// format defined in RFC 3659, making it much easier for FTP clients to parse
+// compared to the traditional LIST command output.
+
+use crate::server::chancomms::DataChanCmd;
+use crate::{
+    auth::UserDetail,
+    server::controlchan::{
+        Command, Reply, ReplyCode,
+        error::ControlChanError,
+        handler::{CommandContext, CommandHandler},
+    },
+    storage::{Metadata, StorageBackend},
+};
+use async_trait::async_trait;
+
+#[derive(Debug)]
+pub struct Mlsd;
+
+#[async_trait]
+impl<Storage, User> CommandHandler<Storage, User> for Mlsd
+where
+    User: UserDetail + 'static,
+    Storage: StorageBackend<User> + 'static,
+    Storage::Metadata: Metadata,
+{
+    #[tracing_attributes::instrument]
+    async fn handle(&self, args: CommandContext<Storage, User>) -> Result<Reply, ControlChanError> {
+        let mut session = args.session.lock().await;
+        let (cmd, path_opt): (DataChanCmd, Option<String>) = match args.parsed_command.clone() {
+            Command::Mlsd { path } => {
+                let path_clone = path.clone();
+                (DataChanCmd::Mlsd { path }, path_clone)
+            }
+            _ => panic!("Programmer error, expected command to be MLSD"),
+        };
+        let logger = args.logger;
+        match session.data_cmd_tx.take() {
+            Some(tx) => {
+                tokio::spawn(async move {
+                    if let Err(err) = tx.send(cmd).await {
+                        slog::warn!(logger, "MLSD: could not notify data channel to respond with MLSD. {}", err);
+                    }
+                });
+                Ok(Reply::new(ReplyCode::FileStatusOkay, "Sending directory list"))
+            }
+            None => {
+                if let Some(path) = path_opt {
+                    slog::warn!(logger, "MLSD: no data connection established for MLSDing {:?}", path);
+                } else {
+                    slog::warn!(logger, "MLSD: no data connection established for MLSD");
+                }
+                Ok(Reply::new(ReplyCode::CantOpenDataConnection, "No data connection established"))
+            }
+        }
+    }
+}

src/server/controlchan/commands/mlst.rs

@@ -54,30 +54,45 @@ where
             }
         };
 
-        let mut facts = Vec::new();
+        let facts_str = format_facts(&metadata);
+        let response = format!(" {} {}", facts_str, path.display());
+        Ok(Reply::new_multiline(ReplyCode::FileActionOkay, vec![" Listing", &response, "End"]))
+    }
+}
 
-        facts.push(if metadata.is_dir() { "type=dir" } else { "type=file" }.to_string());
+/// Format metadata into machine-readable facts string
+///
+/// Returns a semicolon-separated list of facts in the format defined by RFC 3659.
+/// Both MLST and MLSD use the same fact format, so this function provides
+/// consistent formatting for both commands.
+///
+/// # Arguments
+/// * `metadata` - The file/directory metadata to format
+///
+/// # Returns
+/// A string containing semicolon-separated facts like "type=file;size=1234;modify=20231201120000"
+pub fn format_facts<M: Metadata>(metadata: &M) -> String {
+    let mut facts = Vec::new();
 
-        facts.push(format!("size={}", metadata.len()));
+    facts.push(if metadata.is_dir() { "type=dir" } else { "type=file" }.to_string());
 
-        if let Ok(modified) = metadata.modified() {
-            let dt: DateTime<Utc> = modified.into();
-            facts.push(format!("modify={}", dt.format("%Y%m%d%H%M%S")));
-        }
+    facts.push(format!("size={}", metadata.len()));
 
-        // Choosing not to implement create, unique, perm, lang, media-type, charset or most of the
-        // UNIX.*, MACOS.* etc ones.
+    if let Ok(modified) = metadata.modified() {
+        let dt: DateTime<Utc> = modified.into();
+        facts.push(format!("modify={}", dt.format("%Y%m%d%H%M%S")));
+    }
 
-        if metadata.uid() > 0 {
-            facts.push(format!("unix.uid={}", metadata.uid()));
-        }
+    // Choosing not to implement create, unique, perm, lang, media-type, charset or most of the
+    // UNIX.*, MACOS.* etc ones.
 
-        if metadata.gid() > 0 {
-            facts.push(format!("unix.gid={}", metadata.uid()));
-        }
+    if metadata.uid() > 0 {
+        facts.push(format!("unix.uid={}", metadata.uid()));
+    }
 
-        let facts_str = facts.join(";");
-        let response = format!(" {} {}", facts_str, path.display());
-        Ok(Reply::new_multiline(ReplyCode::FileStatus, vec![response]))
+    if metadata.gid() > 0 {
+        facts.push(format!("unix.gid={}", metadata.gid()));
     }
+
+    format!("{};", facts.join(";"))
 }

src/server/controlchan/commands/mod.rs

@@ -19,7 +19,8 @@ mod list;
 mod md5;
 mod mdtm;
 mod mkd;
-mod mlst;
+mod mlsd;
+pub(crate) mod mlst;
 mod mode;
 mod nlst;
 mod noop;
@@ -60,6 +61,7 @@ pub use help::Help;
 pub use list::List;
 pub use mdtm::Mdtm;
 pub use mkd::Mkd;
+pub use mlsd::Mlsd;
 pub use mlst::Mlst;
 pub use mode::{Mode, ModeParam};
 pub use nlst::Nlst;

src/server/controlchan/control_loop.rs

@@ -471,6 +471,7 @@ where
             Command::Mdtm { file } => Box::new(commands::Mdtm::new(file)),
             Command::Md5 { file } => Box::new(commands::Md5::new(file)),
             Command::Mlst { path } => Box::new(commands::Mlst::new(path)),
+            Command::Mlsd { .. } => Box::new(commands::Mlsd),
             Command::Other { .. } => return Ok(Reply::new(ReplyCode::CommandSyntaxError, "Command not implemented")),
         };