Repository: jubako/arx (language: python)

Issue:
Arx crash if given path doesn't exist. 
Hi,

I'd like to report a crash in `arx`. Apologies if this is a duplicate issue, I’m reporting it quickly.

### Steps to Reproduce:
1. Tried running the following command:
   ```bash
   arx create --outfile ExampleArchive ExampleArchive.arx
   ```
2. `arx` crashed with the following error message:
   ```bash
   Well, this is embarrassing.
   arx had a problem and crashed...
   ```

3. Retried with:
   ```bash
   arx create --outfile ExampleArchive/ ExampleArchive.arx
   ```
   It crashed again with a similar error.

### Environment:
- OS: Arch Linux [64-bit]
- arx version: 0.2.1
- Crate version: 0.2.1

### Error Details:
- The crash occurred in the file `/libarx-0.2.1/src/create/fs_adder.rs` at line 143.
- Cause: `called Result::unwrap() on an Err value: Error { depth: 0, inner: Io { path: Some("ExampleArchive.arx"), err: Os { code: 2, kind: NotFound, message: "No such file or directory" } } }`.

### Crash Report (as extracted):
```toml
"name" = "arx"
"operating_system" = "Arch Linux [64-bit]"
"crate_version" = "0.2.1"
"explanation" = """
Panic occurred in file '/home/temo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/libarx-0.2.1/src/create/fs_adder.rs' at line 143
"""
"cause" = 'called `Result::unwrap()` on an `Err` value: Error { depth: 0, inner: Io { path: Some("ExampleArchive.arx"), err: Os { code: 2, kind: NotFound, message: "No such file or directory" } } }'
"method" = "Panic"
"backtrace" = """

   0: 0x5e85a33ebce6 - core::result::unwrap_failed::h635713c629d81684
   1: 0x5e85a33ff8d7 - libarx::create::fs_adder::FsAdder::add_from_path_with_filter::h3eaec6ef31c11959
   2: 0x5e85a34071cc - arx::create::create::hb3e18840522cc531
   3: 0x5e85a3423601 - arx::main::h417fd83e1d4276d2
   4: 0x5e85a3416b03 - std::sys_common::backtrace::__rust_begin_short_backtrace::h95ea882ff6bfd673
   5: 0x5e85a3426a39 - std::rt::lang_start::{{closure}}::ha16fb5f386b2120b
   6: 0x5e85a377ab40 - std::rt::lang_start_internal::h5add80e16e4e0045
   7: 0x5e85a3426a25 - main
   8: 0x73b12df7de08 - <unresolved>
   9: 0x73b12df7decc - __libc_start_main
  10: 0x5e85a33ebf05 - _start
  11:        0x0 - <unresolved>"""
```

Thanks for looking into this! Let me know if you need further details.



Discussion / hints:
This is because the value after the option `--outfile` must be the name of the archive to create. Here you pass the directory to archive as outfile. And arx try to archive the archive itself (which doesn't exist yet)
The syntax is `arx create --outfile archive_path.arx directory/to/archive`

But anyway, arx should properly exit with an error message and not crash as it is now. Thanks for the report, I will fix it. 

Fix patch:
diff --git a/arx/src/create.rs b/arx/src/create.rs
index b77d666..9ca385d 100644
--- a/arx/src/create.rs
+++ b/arx/src/create.rs
@@ -3,7 +3,7 @@ use log::{debug, info};
 use std::cell::Cell;
 use std::fs::File;
 use std::io::{BufRead, BufReader};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 use std::sync::Arc;
 
@@ -20,7 +20,7 @@ pub struct Options {
         short,
         long,
         value_parser,
-        required_unless_present_any(["list_compressions","outfile_old"]),
+        required_unless_present("list_compressions"),
         value_hint=ValueHint::FilePath
     )]
     outfile: Option<PathBuf>,
@@ -92,37 +92,64 @@ pub struct Options {
     #[arg(long, default_value_t = false, action)]
     progress: bool,
 
+    #[arg(short, long, required = false, default_value_t = false, action)]
+    force: bool,
+
     #[arg(from_global)]
     verbose: u8,
-
-    #[arg(
-        short = 'f',
-        long = "file",
-        hide = true,
-        conflicts_with("outfile"),
-        required_unless_present_any(["list_compressions", "outfile"])
-    )]
-    outfile_old: Option<PathBuf>,
 }
 
 fn get_files_to_add(options: &Options) -> Result<Vec<PathBuf>> {
     let file_list = if let Some(file_list) = &options.file_list {
         let file = File::open(file_list)
             .with_context(|| format!("Cannot open {}", file_list.display()))?;
-        let mut files = Vec::new();
-        for line in BufReader::new(file).lines() {
-            files.push(line?.into());
-        }
-        files
+        BufReader::new(file)
+            .lines()
+            .map(|l| -> Result<PathBuf> { Ok(l?.into()) })
+            .collect::<Result<Vec<_>>>()?
     } else {
-        options.infiles.clone()
+        options
+            .infiles
+            .iter()
+            .map(|f| -> Result<PathBuf> {
+                if f.is_absolute() {
+                    Err(anyhow!("Input file ({}) must be relative.", f.display()))
+                } else {
+                    Ok(f.clone())
+                }
+            })
+            .collect::<Result<Vec<_>>>()?
     };
+    Ok(file_list)
+}
+
+fn check_input_paths_exist(file_list: &[PathBuf]) -> Result<()> {
+    // Check that input files actually exists
     for file in file_list.iter() {
-        if file.is_absolute() {
-            return Err(anyhow!("Input file ({}) must be relative.", file.display()));
+        if !file.exists() {
+            return Err(anyhow!(
+                "Input {} path doesn't exist or cannot be accessed",
+                file.display()
+            ));
         }
     }
-    Ok(file_list)
+    Ok(())
+}
+
+fn check_output_path_writable(out_file: &Path, force: bool) -> Result<()> {
+    if !out_file.parent().unwrap().is_dir() {
+        Err(anyhow!(
+            "Directory {} doesn't exist",
+            out_file.parent().unwrap().display()
+        ))
+    } else if out_file.exists() && !force {
+        Err(anyhow!(
+            "File {} already exists. Use option --force to overwrite it.",
+            out_file.display()
+        ))
+    } else {
+        Ok(())
+    }
 }
 
 struct ProgressBar {
@@ -200,13 +227,16 @@ pub fn create(options: Options) -> Result<()> {
         None => arx::PathBuf::new(),
     };
 
-    let out_file = if let Some(ref outfile) = options.outfile_old {
-        outfile
-    } else {
-        options.outfile.as_ref().unwrap()
-    };
+    let out_file = options.outfile.as_ref().expect(
+        "Clap unsure it is Some, except if we have list_compressions, and so we return early",
+    );
     let out_file = std::env::current_dir()?.join(out_file);
+    check_output_path_writable(&out_file, options.force)?;
     let files_to_add = get_files_to_add(&options)?;
+    if let Some(base_dir) = &options.base_dir {
+        std::env::set_current_dir(base_dir)?;
+    };
+    check_input_paths_exist(&files_to_add)?;
 
     let jbk_progress: Arc<dyn jbk::creator::Progress> = if options.progress {
         Arc::new(ProgressBar::new())
@@ -225,10 +255,6 @@ pub fn create(options: Options) -> Result<()> {
         options.compression,
     )?;
 
-    if let Some(base_dir) = &options.base_dir {
-        std::env::set_current_dir(base_dir)?;
-    };
-
     let mut fs_adder = arx::create::FsAdder::new(&mut creator, strip_prefix);
     for infile in files_to_add {
         fs_adder.add_from_path(&infile, options.recurse)?;


Test patch:
diff --git a/arx/tests/create.rs b/arx/tests/create.rs
index d7cc658..f108783 100644
--- a/arx/tests/create.rs
+++ b/arx/tests/create.rs
@@ -1,3 +1,5 @@
+use std::io::Read;
+
 #[cfg(all(unix, not(feature = "in_ci")))]
 mod inner {
     pub use std::path::Path;
@@ -34,6 +36,140 @@ macro_rules! cmd {
     }};
 }
 
+#[test]
+fn test_crate_non_existant_input() {
+    use std::path::Path;
+
+    let arx_tmp_dir = tempfile::tempdir_in(Path::new(env!("CARGO_TARGET_TMPDIR")))
+        .expect("Creating tempdir should work");
+    let arx_file = arx_tmp_dir.path().join("test.arx");
+    let output = cmd!("arx", "create", "--outfile", &arx_file, "non_existant_dir");
+    let stdout = String::from_utf8(output.stdout).unwrap();
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    println!("Out : {}", stdout);
+    println!("Err : {}", stderr);
+    assert_eq!("", stdout);
+    assert_eq!(
+        "[ERROR arx] Error : Input non_existant_dir path doesn't exist or cannot be accessed\n",
+        stderr
+    );
+    assert!(!output.status.success());
+    assert!(!arx_file.exists());
+}
+
+#[cfg(all(unix, not(feature = "in_ci")))]
+#[test]
+fn test_crate_non_existant_output_directory() {
+    use inner::*;
+    use std::path::Path;
+
+    let (_source_mount_handle, source_mount_point) = spawn_mount().unwrap();
+    let source_mount_point = source_mount_point.path();
+    let arx_tmp_dir = tempfile::tempdir_in(Path::new(env!("CARGO_TARGET_TMPDIR")))
+        .expect("Creating tempdir should work");
+    let arx_file = arx_tmp_dir
+        .path()
+        .join("non_existant_directory")
+        .join("test.arx");
+    let output = cmd!(
+        "arx",
+        "create",
+        "--outfile",
+        &arx_file,
+        "-C",
+        source_mount_point.parent().unwrap(),
+        "--strip-prefix",
+        source_mount_point.file_name().unwrap(),
+        source_mount_point.file_name().unwrap()
+    );
+    let stdout = String::from_utf8(output.stdout).unwrap();
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    println!("Out : {}", stdout);
+    println!("Err : {}", stderr);
+    assert_eq!("", stdout);
+    assert_eq!(
+        format!(
+            "[ERROR arx] Error : Directory {} doesn't exist\n",
+            arx_file.parent().unwrap().display()
+        ),
+        stderr
+    );
+    assert!(!output.status.success());
+    assert!(!arx_file.exists());
+}
+
+#[cfg(all(unix, not(feature = "in_ci")))]
+#[test]
+fn test_crate_existant_output() {
+    use inner::*;
+    use std::path::Path;
+
+    let (_source_mount_handle, source_mount_point) = spawn_mount().unwrap();
+    let source_mount_point = source_mount_point.path();
+    let arx_tmp_dir = tempfile::tempdir_in(Path::new(env!("CARGO_TARGET_TMPDIR")))
+        .expect("Creating tempdir should work");
+    let arx_file = arx_tmp_dir.path().join("test.arx");
+    {
+        use std::io::Write;
+        let mut f = std::fs::File::create(&arx_file).unwrap();
+        f.write_all(b"Some dummy content").unwrap();
+    }
+
+    // Try to write without --force
+    let output = cmd!(
+        "arx",
+        "create",
+        "--outfile",
+        &arx_file,
+        "-C",
+        source_mount_point.parent().unwrap(),
+        "--strip-prefix",
+        source_mount_point.file_name().unwrap(),
+        source_mount_point.file_name().unwrap()
+    );
+    let stdout = String::from_utf8(output.stdout).unwrap();
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    println!("Out : {}", stdout);
+    println!("Err : {}", stderr);
+    assert_eq!("", stdout);
+    assert_eq!(
+        format!(
+            "[ERROR arx] Error : File {} already exists. Use option --force to overwrite it.\n",
+            arx_file.display()
+        ),
+        stderr
+    );
+    assert!(!output.status.success());
+    assert_eq!(std::fs::read(&arx_file).unwrap(), b"Some dummy content");
+
+    // Try to write without --force
+    let output = cmd!(
+        "arx",
+        "create",
+        "--outfile",
+        &arx_file,
+        "-C",
+        source_mount_point.parent().unwrap(),
+        "--strip-prefix",
+        source_mount_point.file_name().unwrap(),
+        source_mount_point.file_name().unwrap(),
+        "--force"
+    );
+    let stdout = String::from_utf8(output.stdout).unwrap();
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    println!("Out : {}", stdout);
+    println!("Err : {}", stderr);
+    assert_eq!("", stdout);
+    assert_eq!("", stderr);
+    assert!(output.status.success());
+    {
+        let mut f = std::fs::File::open(&arx_file).unwrap();
+        let mut buf = [0; 10];
+        f.read_exact(&mut buf).unwrap();
+        assert_eq!(&buf, b"jbkC\x00\x00\x00\x00\x00\x02");
+    }
+}
+
 #[cfg(all(unix, not(feature = "in_ci")))]
 #[test]
 fn test_create_and_mount() {