Merge pull request #18 from vrmiguel/nightly

Verify inputs when decompressing (canonicalize + checking if they're decompressible)

.gitignore

@@ -2,6 +2,8 @@
 # will have compiled files and executables
 target/
 
+makeshift_testing.py
+
 # Remove Cargo.lock from gitignore if creating an executable, leave it for libraries
 # More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html
 Cargo.lock

src/bytes.rs

@@ -17,7 +17,7 @@ impl Bytes {
 impl std::fmt::Display for Bytes {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let num = self.bytes;
-        debug_assert!(num >= 0.0);        
+        debug_assert!(num >= 0.0);
         if num < 1_f64 {
             return write!(f, "{} B", num);
         }
@@ -27,4 +27,4 @@ impl std::fmt::Display for Bytes {
         write!(f, "{:.2} ", num / delimiter.powi(exponent))?;
         write!(f, "{}", UNITS[exponent as usize])
     }
-}
+}

src/cli.rs

@@ -1,9 +1,12 @@
-use std::{env, ffi::OsString, io, path::PathBuf, vec::Vec};
+use std::{
+    env,
+    ffi::OsString,
+    path::{Path, PathBuf},
+    vec::Vec,
+};
 
 use oof::{arg_flag, flag};
 
-use crate::debug;
-
 pub const VERSION: &str = "0.1.5";
 
 #[derive(PartialEq, Eq, Debug)]
@@ -41,8 +44,28 @@ pub struct ParsedArgs {
     // pub program_called: OsString, // Useful?
 }
 
-fn canonicalize_files(files: Vec<PathBuf>) -> io::Result<Vec<PathBuf>> {
-    files.into_iter().map(|path| path.canonicalize()).collect()
+fn canonicalize<'a, P>(path: P) -> crate::Result<PathBuf>
+where
+    P: AsRef<Path> + 'a,
+{
+    match std::fs::canonicalize(&path.as_ref()) {
+        Ok(abs_path) => Ok(abs_path),
+        Err(io_err) => {
+            if !path.as_ref().exists() {
+                Err(crate::Error::FileNotFound(PathBuf::from(path.as_ref())))
+            } else {
+                eprintln!("{} {}", "[ERROR]", io_err);
+                Err(crate::Error::IoError)
+            }
+        }
+    }
+}
+
+fn canonicalize_files<'a, P>(files: Vec<P>) -> crate::Result<Vec<PathBuf>>
+where
+    P: AsRef<Path> + 'a,
+{
+    files.into_iter().map(canonicalize).collect()
 }
 
 pub fn parse_args_from(mut args: Vec<OsString>) -> crate::Result<ParsedArgs> {
@@ -87,18 +110,22 @@ pub fn parse_args_from(mut args: Vec<OsString>) -> crate::Result<ParsedArgs> {
         // Defaults to decompression when there is no subcommand
         None => {
             flags_info.push(arg_flag!('o', "output"));
-            debug!(&flags_info);
 
             // Parse flags
             let (args, mut flags) = oof::filter_flags(args, &flags_info)?;
-            debug!((&args, &flags));
 
-            let files: Vec<_> = args.into_iter().map(PathBuf::from).collect();
-            // TODO: This line doesn't seem to be working correctly
+            let files = args.into_iter().map(canonicalize);
+            for file in files.clone() {
+                if let Err(err) = file {
+                    return Err(err);
+                }
+            }
+            let files = files.map(Result::unwrap).collect();
+
             let output_folder = flags.take_arg("output").map(PathBuf::from);
 
-            // Is the output here fully correct?
-            // With the paths not canonicalized?
+            // TODO: ensure all files are decompressible
+
             let command = Command::Decompress {
                 files,
                 output_folder,
@@ -109,4 +136,4 @@ pub fn parse_args_from(mut args: Vec<OsString>) -> crate::Result<ParsedArgs> {
     };
 
     Ok(parsed_args)
-}
+}

src/compressors/bzip.rs

@@ -4,8 +4,8 @@ use colored::Colorize;
 
 use super::{Compressor, Entry};
 use crate::{
-    extension::CompressionFormat,
     bytes::Bytes,
+    extension::CompressionFormat,
     file::File,
     utils::{check_for_multiple_files, ensure_exists},
 };

src/compressors/gzip.rs

@@ -4,9 +4,9 @@ use colored::Colorize;
 
 use super::{Compressor, Entry};
 use crate::{
+    bytes::Bytes,
     extension::CompressionFormat,
     file::File,
-    bytes::Bytes,
     utils::{check_for_multiple_files, ensure_exists},
 };
 

src/compressors/lzma.rs

@@ -4,9 +4,9 @@ use colored::Colorize;
 
 use super::{Compressor, Entry};
 use crate::{
+    bytes::Bytes,
     extension::CompressionFormat,
     file::File,
-    bytes::Bytes,
     utils::{check_for_multiple_files, ensure_exists},
 };
 

src/compressors/zip.rs

@@ -68,7 +68,7 @@ impl ZipCompressor {
                 if entry_path.is_dir() {
                     continue;
                 }
-                
+
                 writer.start_file(entry_path.to_string_lossy(), options)?;
                 println!("Compressing {:?}", entry_path);
                 let file_bytes = std::fs::read(entry.path())?;

src/decompressors/tar.rs

@@ -8,13 +8,13 @@ use colored::Colorize;
 use tar::{self, Archive};
 
 use super::decompressor::{DecompressionResult, Decompressor};
-use crate::{dialogs::Confirmation, file::File, bytes::Bytes, utils};
+use crate::{bytes::Bytes, dialogs::Confirmation, file::File, utils};
 
 #[derive(Debug)]
 pub struct TarDecompressor {}
 
 impl TarDecompressor {
-    fn unpack_files(from: File, into: &Path, flags: &oof::Flags) -> crate::Result<Vec<PathBuf>> {        
+    fn unpack_files(from: File, into: &Path, flags: &oof::Flags) -> crate::Result<Vec<PathBuf>> {
         println!(
             "{}: attempting to decompress {:?}",
             "ouch".bright_blue(),

src/decompressors/to_memory.rs

@@ -6,8 +6,8 @@ use std::{
 use colored::Colorize;
 
 use super::decompressor::{DecompressionResult, Decompressor};
-use crate::utils;
 use crate::bytes::Bytes;
+use crate::utils;
 use crate::{extension::CompressionFormat, file::File};
 
 struct DecompressorToMemory {}

src/decompressors/zip.rs

@@ -8,7 +8,7 @@ use colored::Colorize;
 use zip::{self, read::ZipFile, ZipArchive};
 
 use super::decompressor::{DecompressionResult, Decompressor};
-use crate::{dialogs::Confirmation, file::File, bytes::Bytes, utils};
+use crate::{bytes::Bytes, dialogs::Confirmation, file::File, utils};
 
 #[cfg(unix)]
 fn __unix_set_permissions(file_path: &Path, file: &ZipFile) {

src/error.rs

@@ -5,7 +5,7 @@ use colored::Colorize;
 #[derive(PartialEq, Eq)]
 pub enum Error {
     UnknownExtensionError(String),
-    MissingExtensionError(String),
+    MissingExtensionError(PathBuf),
     // TODO: get rid of this error variant
     InvalidUnicode,
     InvalidInput,
@@ -35,7 +35,7 @@ impl fmt::Display for Error {
             Error::MissingExtensionError(filename) => {
                 write!(f, "{} ", "[ERROR]".red())?;
                 // TODO: show MIME type of the unsupported file
-                write!(f, "cannot compress to \'{}\', likely because it has an unsupported (or missing) extension.", filename)
+                write!(f, "cannot compress to {:?}, likely because it has an unsupported (or missing) extension.", filename)
             }
             Error::WalkdirError => {
                 // Already printed in the From block

src/evaluator.rs

@@ -8,20 +8,19 @@ use colored::Colorize;
 
 use crate::{
     bytes::Bytes,
-    cli::{VERSION, Command},
+    cli::{Command, VERSION},
     compressors::{
-        Entry, Compressor, BzipCompressor, GzipCompressor, LzmaCompressor, TarCompressor,
+        BzipCompressor, Compressor, Entry, GzipCompressor, LzmaCompressor, TarCompressor,
         ZipCompressor,
     },
     decompressors::{
         BzipDecompressor, DecompressionResult, Decompressor, GzipDecompressor, LzmaDecompressor,
         TarDecompressor, ZipDecompressor,
-    }, 
-    dialogs::Confirmation, 
-    extension::{CompressionFormat, Extension}, 
-    file::File, 
+    },
+    dialogs::Confirmation,
+    extension::{CompressionFormat, Extension},
+    file::File,
     utils,
-    debug
 };
 
 pub struct Evaluator {}
@@ -33,7 +32,6 @@ impl Evaluator {
     pub fn get_compressor(
         file: &File,
     ) -> crate::Result<(Option<BoxedCompressor>, BoxedCompressor)> {
-        
         let extension = match &file.extension {
             Some(extension) => extension.clone(),
             None => {
@@ -176,7 +174,6 @@ impl Evaluator {
         let confirm = Confirmation::new("Do you want to overwrite 'FILE'?", Some("FILE"));
         let (first_compressor, second_compressor) = Self::get_compressor(&output)?;
 
-        // TODO: use -y and -n here
         if output_path.exists()
             && !utils::permission_for_overwriting(&output_path, flags, &confirm)?
         {
@@ -214,8 +211,16 @@ impl Evaluator {
         file_path: &Path,
         output: Option<&Path>,
         flags: &oof::Flags,
-    ) -> crate::Result<()> {        
-        let file = debug!(File::from(file_path)?);
+    ) -> crate::Result<()> {
+        // The file to be decompressed
+        let file = File::from(file_path)?;
+        // The file must have a supported decompressible format
+        if file.extension == None {
+            return Err(crate::Error::MissingExtensionError(PathBuf::from(
+                file_path,
+            )));
+        }
+
         let output = match output {
             Some(inner) => Some(File::from(inner)?),
             None => None,
@@ -290,4 +295,4 @@ fn help_message() {
     println!("    ouch compress <input...> output-file");
     println!("DECOMPRESSION USAGE:");
     println!("    ouch <input> [-o/--output output-folder]");
-}
+}

src/extension.rs

@@ -57,7 +57,11 @@ impl Extension {
                 (os_str, snd) if os_str.is_empty() => (None, snd),
                 (fst, snd) => (Some(fst), snd),
             },
-            None => return Err(crate::Error::MissingExtensionError(to_utf(file_name))),
+            None => {
+                return Err(crate::Error::MissingExtensionError(PathBuf::from(
+                    file_name,
+                )))
+            }
         };
 
         let (first_ext, second_ext) = match (first_ext, second_ext) {
@@ -119,7 +123,7 @@ impl TryFrom<&PathBuf> for CompressionFormat {
         let ext = match ext.extension() {
             Some(ext) => ext,
             None => {
-                return Err(crate::Error::MissingExtensionError(String::new()));
+                return Err(crate::Error::MissingExtensionError(PathBuf::new()));
             }
         };
         extension_from_os_str(ext)
@@ -133,7 +137,7 @@ impl TryFrom<&str> for CompressionFormat {
         let file_name = Path::new(file_name);
         let ext = match file_name.extension() {
             Some(ext) => ext,
-            None => return Err(crate::Error::MissingExtensionError(String::new())),
+            None => return Err(crate::Error::MissingExtensionError(PathBuf::new())),
         };
 
         extension_from_os_str(ext)

src/file.rs

@@ -24,7 +24,7 @@ impl<'a> File<'a> {
         Ok(File {
             path,
             contents_in_memory: None,
-            extension
+            extension,
         })
     }
 }

src/main.rs

@@ -24,6 +24,5 @@ fn main() {
 
 fn run() -> crate::Result<()> {
     let ParsedArgs { command, flags } = cli::parse_args()?;
-    debug!(&command);
     Evaluator::evaluate(command, &flags)
 }

src/test.rs

@@ -27,7 +27,7 @@ where
 
 #[cfg(test)]
 mod argparsing {
-    use super::{make_dummy_files};
+    use super::make_dummy_files;
     use crate::cli;
     use crate::cli::Command;
     use std::{ffi::OsString, fs, path::PathBuf};
@@ -65,10 +65,13 @@ mod argparsing {
 
     #[test]
     fn test_arg_parsing_compress_subcommand() -> crate::Result<()> {
-        
         let files = vec!["a", "b", "c"];
         make_dummy_files(&*files)?;
-        let files= files.iter().map(fs::canonicalize).map(Result::unwrap).collect();
+        let files = files
+            .iter()
+            .map(fs::canonicalize)
+            .map(Result::unwrap)
+            .collect();
 
         let expected = Command::Compress {
             files,
@@ -83,23 +86,41 @@ mod argparsing {
     }
 
     #[test]
-    fn test_arg_parsing_decompress_subcommand() {
-        let files: Vec<_> = ["a", "b", "c"].iter().map(PathBuf::from).collect();
+    fn test_arg_parsing_decompress_subcommand() -> crate::Result<()> {
+        let files = vec!["d", "e", "f"];
+        make_dummy_files(&*files)?;
+        
+        let files: Vec<_> = files.iter().map(PathBuf::from).collect();
 
         let expected = Command::Decompress {
-            files: files.clone(),
+            files: files
+                .iter()
+                .map(fs::canonicalize)
+                .map(Result::unwrap)
+                .collect(),
             output_folder: None,
         };
-        assert_eq!(expected, parse!("a b c").command);
+        
+        assert_eq!(expected, parse!("d e f").command);
 
         let expected = Command::Decompress {
-            files,
+            files: files.iter().map(fs::canonicalize).map(Result::unwrap).collect(),
             output_folder: Some("folder".into()),
         };
-        assert_eq!(expected, parse!("a b c --output folder").command);
-        assert_eq!(expected, parse!("a b --output folder c").command);
-        assert_eq!(expected, parse!("a --output folder b c").command);
-        assert_eq!(expected, parse!("--output folder a b c").command);
+        assert_eq!(expected, parse!("d e f --output folder").command);
+        assert_eq!(expected, parse!("d e --output folder f").command);
+        assert_eq!(expected, parse!("d --output folder e f").command);
+        assert_eq!(expected, parse!("--output folder d e f").command);
+
+        assert_eq!(expected, parse!("d e f -o folder").command);
+        assert_eq!(expected, parse!("d e -o folder f").command);
+        assert_eq!(expected, parse!("d -o folder e f").command);
+        assert_eq!(expected, parse!("-o folder d e f").command);
+
+        fs::remove_file("d")?;
+        fs::remove_file("e")?;
+        fs::remove_file("f")?;
+        Ok(())
     }
 }
 
@@ -107,70 +128,37 @@ mod argparsing {
 mod byte_pretty_printing {
     use crate::bytes::Bytes;
     #[test]
-    fn bytes () {
-        assert_eq!(
-            &format!("{}", Bytes::new(234)),
-            "234.00 B"
-        );
+    fn bytes() {
+        assert_eq!(&format!("{}", Bytes::new(234)), "234.00 B");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(999)),
-            "999.00 B"
-        );
+        assert_eq!(&format!("{}", Bytes::new(999)), "999.00 B");
     }
 
     #[test]
-    fn kilobytes () {
-        assert_eq!(
-            &format!("{}", Bytes::new(2234)),
-            "2.23 kB"
-        );
+    fn kilobytes() {
+        assert_eq!(&format!("{}", Bytes::new(2234)), "2.23 kB");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(62500)),
-            "62.50 kB"
-        );
+        assert_eq!(&format!("{}", Bytes::new(62500)), "62.50 kB");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(329990)),
-            "329.99 kB"
-        );
+        assert_eq!(&format!("{}", Bytes::new(329990)), "329.99 kB");
     }
 
     #[test]
-    fn megabytes () {
-        assert_eq!(
-            &format!("{}", Bytes::new(2750000)),
-            "2.75 MB"
-        );
+    fn megabytes() {
+        assert_eq!(&format!("{}", Bytes::new(2750000)), "2.75 MB");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(55000000)),
-            "55.00 MB"
-        );
+        assert_eq!(&format!("{}", Bytes::new(55000000)), "55.00 MB");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(987654321)),
-            "987.65 MB"
-        );
+        assert_eq!(&format!("{}", Bytes::new(987654321)), "987.65 MB");
     }
 
     #[test]
-    fn gigabytes () {
-        assert_eq!(
-            &format!("{}", Bytes::new(5280000000)),
-            "5.28 GB"
-        );
+    fn gigabytes() {
+        assert_eq!(&format!("{}", Bytes::new(5280000000)), "5.28 GB");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(95200000000)),
-            "95.20 GB"
-        );
+        assert_eq!(&format!("{}", Bytes::new(95200000000)), "95.20 GB");
 
-        assert_eq!(
-            &format!("{}", Bytes::new(302000000000)),
-            "302.00 GB"
-        );
+        assert_eq!(&format!("{}", Bytes::new(302000000000)), "302.00 GB");
     }
 }
 

src/utils.rs

@@ -12,16 +12,19 @@ use crate::{dialogs::Confirmation, extension::CompressionFormat, file::File};
 #[macro_export]
 #[cfg(debug_assertions)]
 macro_rules! debug {
-    ($x:expr) => { dbg!($x) }
+    ($x:expr) => {
+        dbg!($x)
+    };
 }
 
 #[macro_export]
 #[cfg(not(debug_assertions))]
 macro_rules! debug {
-    ($x:expr) => { std::convert::identity($x) }
+    ($x:expr) => {
+        std::convert::identity($x)
+    };
 }
 
-
 pub(crate) fn ensure_exists<'a, P>(path: P) -> crate::Result<()>
 where
     P: AsRef<Path> + 'a,
@@ -82,7 +85,9 @@ pub(crate) fn change_dir_and_return_parent(filename: &Path) -> crate::Result<Pat
         return Err(crate::Error::CompressingRootFolder);
     };
 
-    env::set_current_dir(parent).ok().ok_or(crate::Error::CompressingRootFolder)?;
+    env::set_current_dir(parent)
+        .ok()
+        .ok_or(crate::Error::CompressingRootFolder)?;
 
     Ok(previous_location)
 }