Merge pull request #20 from veeso/issue-17-still-some-problems-with-symlinks

[BUG] Problems with symlinks

CHANGELOG.md

@@ -19,8 +19,14 @@
 
 Released on FIXME:
 
+- Enhancements:
+  - SCP file transfer:
+    - Added possibility to stat directories.
 - Bugfix:
   - [Issue 18](https://github.com/veeso/termscp/issues/18): Set file transfer type to `Binary` for FTP
+  - [Issue 17](https://github.com/veeso/termscp/issues/17)
+    - SCP: fixed symlink not properly detected
+    - FTP: added symlink support for Linux targets
   - [Issue 10](https://github.com/veeso/termscp/issues/10): Fixed port not being loaded from bookmarks into gui
   - [Issue 9](https://github.com/veeso/termscp/issues/9): Fixed issues related to paths on remote when using Windows
 - Dependencies:
@@ -39,6 +45,8 @@ Released on FIXME:
 
 Released on 27/03/2021
 
+> The UI refactoring update
+
 - **New explorer features**:
   - **Execute** a command pressing `X`. This feature is supported on both local and remote hosts (only SFTP/SCP protocols support this feature).
   - **Find**: search for files pressing `F` using wild matches.

src/filetransfer/ftp_transfer.rs

@@ -80,7 +80,7 @@ impl FtpFileTransfer {
     /// ### parse_list_line
     ///
     /// Parse a line of LIST command output and instantiates an FsEntry from it
-    fn parse_list_line(&self, path: &Path, line: &str) -> Result<FsEntry, ()> {
+    fn parse_list_line(&mut self, path: &Path, line: &str) -> Result<FsEntry, ()> {
         // Try to parse using UNIX syntax
         match self.parse_unix_list_line(path, line) {
             Ok(entry) => Ok(entry),
@@ -99,7 +99,7 @@ impl FtpFileTransfer {
     /// UNIX syntax has the following syntax:
     /// {FILE_TYPE}{UNIX_PEX} {HARD_LINKS} {USER} {GROUP} {SIZE} {DATE} {FILENAME}
     /// -rw-r--r--   1 cvisintin  staff   4968 27 Dic 10:46 CHANGELOG.md
-    fn parse_unix_list_line(&self, path: &Path, line: &str) -> Result<FsEntry, ()> {
+    fn parse_unix_list_line(&mut self, path: &Path, line: &str) -> Result<FsEntry, ()> {
         // Prepare list regex
         // NOTE: about this damn regex <https://stackoverflow.com/questions/32480890/is-there-a-regex-to-parse-the-values-from-an-ftp-directory-listing>
         lazy_static! {
@@ -116,7 +116,8 @@ impl FtpFileTransfer {
                 }
                 // Collect metadata
                 // Get if is directory and if is symlink
-                let (is_dir, _is_symlink): (bool, bool) = match metadata.get(1).unwrap().as_str() {
+                let (mut is_dir, is_symlink): (bool, bool) = match metadata.get(1).unwrap().as_str()
+                {
                     "-" => (false, false),
                     "l" => (false, true),
                     "d" => (true, false),
@@ -174,11 +175,60 @@ impl FtpFileTransfer {
                     .as_str()
                     .parse::<usize>()
                     .unwrap_or(0);
-                let file_name: String = String::from(metadata.get(8).unwrap().as_str());
+                // Split filename if required
+                let (file_name, symlink_path): (String, Option<PathBuf>) = match is_symlink {
+                    true => self.get_name_and_link(metadata.get(8).unwrap().as_str()),
+                    false => (String::from(metadata.get(8).unwrap().as_str()), None),
+                };
                 // Check if file_name is '.' or '..'
                 if file_name.as_str() == "." || file_name.as_str() == ".." {
                     return Err(());
                 }
+                // Get symlink
+                let symlink: Option<Box<FsEntry>> = match symlink_path {
+                    None => None,
+                    Some(p) => Some(Box::new(match p.to_string_lossy().ends_with('/') {
+                        true => {
+                            // NOTE: is_dir becomes true
+                            is_dir = true;
+                            FsEntry::Directory(FsDirectory {
+                                name: p
+                                    .file_name()
+                                    .unwrap_or(&std::ffi::OsStr::new(""))
+                                    .to_string_lossy()
+                                    .to_string(),
+                                abs_path: p.clone(),
+                                last_change_time: mtime,
+                                last_access_time: mtime,
+                                creation_time: mtime,
+                                readonly: false,
+                                symlink: None,
+                                user: uid,
+                                group: gid,
+                                unix_pex: Some(unix_pex),
+                            })
+                        }
+                        false => FsEntry::File(FsFile {
+                            name: p
+                                .file_name()
+                                .unwrap_or(&std::ffi::OsStr::new(""))
+                                .to_string_lossy()
+                                .to_string(),
+                            abs_path: p.clone(),
+                            last_change_time: mtime,
+                            last_access_time: mtime,
+                            creation_time: mtime,
+                            readonly: false,
+                            symlink: None,
+                            size: filesize,
+                            ftype: p.extension().map(|s| String::from(s.to_string_lossy())),
+                            user: uid,
+                            group: gid,
+                            unix_pex: Some(unix_pex),
+                        }),
+                    })),
+                };
+                eprintln!("{:?};{:?}", is_dir, symlink);
                 let mut abs_path: PathBuf = PathBuf::from(path);
                 abs_path.push(file_name.as_str());
                 let abs_path: PathBuf = Self::resolve(abs_path.as_path());
@@ -197,7 +247,7 @@ impl FtpFileTransfer {
                         last_access_time: mtime,
                         creation_time: mtime,
                         readonly: false,
-                        symlink: None,
+                        symlink,
                         user: uid,
                         group: gid,
                         unix_pex: Some(unix_pex),
@@ -211,7 +261,7 @@ impl FtpFileTransfer {
                         size: filesize,
                         ftype: extension,
                         readonly: false,
-                        symlink: None,
+                        symlink,
                         user: uid,
                         group: gid,
                         unix_pex: Some(unix_pex),
@@ -309,6 +359,16 @@ impl FtpFileTransfer {
             None => Err(()), // Invalid syntax
         }
     }
+
+    /// ### get_name_and_link
+    ///
+    /// Returns from a `ls -l` command output file name token, the name of the file and the symbolic link (if there is any)
+    fn get_name_and_link(&self, token: &str) -> (String, Option<PathBuf>) {
+        let tokens: Vec<&str> = token.split(" -> ").collect();
+        let filename: String = String::from(*tokens.get(0).unwrap());
+        let symlink: Option<PathBuf> = tokens.get(1).map(PathBuf::from);
+        (filename, symlink)
+    }
 }
 
 impl FileTransfer for FtpFileTransfer {
@@ -731,7 +791,7 @@ mod tests {
 
     #[test]
     fn test_filetransfer_ftp_parse_list_line_unix() {
-        let ftp: FtpFileTransfer = FtpFileTransfer::new(false);
+        let mut ftp: FtpFileTransfer = FtpFileTransfer::new(false);
         // Simple file
         let fs_entry: FsEntry = ftp
             .parse_list_line(
@@ -854,7 +914,7 @@ mod tests {
 
     #[test]
     fn test_filetransfer_ftp_parse_list_line_dos() {
-        let ftp: FtpFileTransfer = FtpFileTransfer::new(false);
+        let mut ftp: FtpFileTransfer = FtpFileTransfer::new(false);
         // Simple file
         let fs_entry: FsEntry = ftp
             .parse_list_line(

src/filetransfer/scp_transfer.rs

@@ -165,9 +165,9 @@ impl ScpFileTransfer {
                     true => self.get_name_and_link(metadata.get(8).unwrap().as_str()),
                     false => (String::from(metadata.get(8).unwrap().as_str()), None),
                 };
-                // Check if symlink points to a directory
+                // Check if file_name is '.' or '..'
-                if let Some(symlink_path) = symlink_path.as_ref() {
+                if file_name.as_str() == "." || file_name.as_str() == ".." {
-                    is_dir = symlink_path.is_dir();
+                    return Err(());
                 }
                 // Get symlink; PATH mustn't be equal to filename
                 let symlink: Option<Box<FsEntry>> = match symlink_path {
@@ -179,15 +179,18 @@ impl ScpFileTransfer {
                         true => None,
                         false => match self.stat(p.as_path()) {
                             // If path match filename
-                            Ok(e) => Some(Box::new(e)),
+                            Ok(e) => {
+                                // If e is a directory, set is_dir to true
+                                if e.is_dir() {
+                                    is_dir = true;
+                                }
+                                Some(Box::new(e))
+                            }
                             Err(_) => None, // Ignore errors
                         },
                     },
                 };
-                // Check if file_name is '.' or '..'
+                // Re-check if is directory
-                if file_name.as_str() == "." || file_name.as_str() == ".." {
-                    return Err(());
-                }
                 let mut abs_path: PathBuf = PathBuf::from(path);
                 abs_path.push(file_name.as_str());
                 let abs_path: PathBuf = Self::resolve(abs_path.as_path());
@@ -556,7 +559,7 @@ impl FileTransfer for ScpFileTransfer {
                 let p: PathBuf = self.wrkdir.clone();
                 match self.perform_shell_cmd_with_path(
                     p.as_path(),
-                    format!("unset LANG; ls -la \"{}\"", path.display()).as_str(),
+                    format!("unset LANG; ls -la \"{}/\"", path.display()).as_str(),
                 ) {
                     Ok(output) => {
                         // Split output by (\r)\n
@@ -703,12 +706,6 @@ impl FileTransfer for ScpFileTransfer {
     ///
     /// Stat file and return FsEntry
     fn stat(&mut self, path: &Path) -> Result<FsEntry, FileTransferError> {
-        if path.is_dir() {
-            return Err(FileTransferError::new_ex(
-                FileTransferErrorType::UnsupportedFeature,
-                String::from("stat is not supported for directories"),
-            ));
-        }
         let path: PathBuf = match path.is_absolute() {
             true => PathBuf::from(path),
             false => {
@@ -720,10 +717,12 @@ impl FileTransfer for ScpFileTransfer {
         match self.is_connected() {
             true => {
                 let p: PathBuf = self.wrkdir.clone();
-                match self.perform_shell_cmd_with_path(
+                // make command; Directories require `-d` option
-                    p.as_path(),
+                let cmd: String = match path.to_string_lossy().ends_with('/') {
-                    format!("ls -l \"{}\"", path.display()).as_str(),
+                    true => format!("ls -ld \"{}\"", path.display()),
-                ) {
+                    false => format!("ls -l \"{}\"", path.display()),
+                };
+                match self.perform_shell_cmd_with_path(p.as_path(), cmd.as_str()) {
                     Ok(line) => {
                         // Parse ls line
                         let parent: PathBuf = match path.as_path().parent() {