Better error handling

repak/src/entry.rs

@@ -368,17 +368,12 @@ impl Entry {
             Some(Compression::Gzip) => decompress!(flate2::read::GzDecoder<&[u8]>),
             Some(Compression::Oodle) => {
                 #[cfg(not(target_os = "windows"))]
-                return Err(super::Error::Other(
-                    "Oodle compression only supported on Windows (or WINE)",
-                ));
+                return Err(super::Error::Oodle());
 
                 #[cfg(target_os = "windows")]
                 unsafe {
-                    let lib = libloading::Library::new("oo2core_9_win64.dll").map_err(|_| {
-                        super::Error::Other(
-                            "Could not find oo2core_9_win64.dll for Oodle compression",
-                        )
-                    })?;
+                    let lib = libloading::Library::new("oo2core_9_win64.dll")
+                        .map_err(|_| super::Error::OodleMissing())?;
 
                     /*
                     let set_printf: libloading::Symbol<
@@ -451,7 +446,7 @@ impl Entry {
                         3,
                     );
                     if out == 0 {
-                        return Err(super::Error::Other("decompression failed"));
+                        return Err(super::Error::DecompressionFailed(Compression::Oodle));
                     } else {
                         assert_eq!(
                             out as u64, self.uncompressed,

repak/src/error.rs

@@ -1,35 +1,79 @@
-#[derive(thiserror::Error, Debug)]
+use crate::Compression;
+
+#[derive(thiserror::Error)]
 pub enum Error {
     // dependency errors
     #[error("enum conversion: {0}")]
     Strum(#[from] strum::ParseError),
+
     #[error("key hash is an incorrect length")]
     Aes,
+
     #[error("malformed base64")]
     Base64,
+
     // std errors
     #[error("io error: {0}")]
     Io(#[from] std::io::Error),
+
     #[error("utf8 conversion: {0}")]
     Utf8(#[from] std::string::FromUtf8Error),
+
     #[error("utf16 conversion: {0}")]
     Utf16(#[from] std::string::FromUtf16Error),
+
     #[error("bufwriter dereference: {0}")]
     IntoInner(#[from] std::io::IntoInnerError<std::io::BufWriter<Vec<u8>>>),
+
     // crate errors
     #[error("got {0}, which is not a boolean")]
     Bool(u8),
+
     #[error("found magic of {0:#x} instead of {:#x}", super::MAGIC)]
     Magic(u32),
+
+    #[error("Oodle compression only supported on Windows (or WINE)")]
+    Oodle(),
+
+    #[error("Could not find oo2core_9_win64.dll for Oodle compression")]
+    OodleMissing(),
+
+    #[error("No entry found at {0}")]
+    MissingEntry(String),
+
+    #[error("Prefix \"{prefix}\" does not match path \"{path}\"")]
+    PrefixMismatch { prefix: String, path: String },
+
+    #[error("Attempted to write to \"{0}\" which outside of output directory")]
+    WriteOutsideOutput(String),
+
+    #[error("Output directory is not empty: \"{0}\"")]
+    OutputNotEmpty(String),
+
+    #[error("Input is not a directory: \"{0}\"")]
+    InputNotADirectory(String),
+
+    #[error("{0} decompression failed")]
+    DecompressionFailed(Compression),
+
     #[error("used version {used} but pak is version {version}")]
     Version {
         used: super::VersionMajor,
         version: super::VersionMajor,
     },
+
     #[error("pak is encrypted but no key was provided")]
     Encrypted,
+
     #[error("error with OsString")]
     OsString(std::ffi::OsString),
+
     #[error("{0}")]
-    Other(&'static str),
+    Other(String),
+}
+
+impl std::fmt::Debug for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Display::fmt(self, f)
+    }
 }

repak/src/pak.rs

@@ -91,7 +91,7 @@ impl PakReader {
                 _ => continue,
             }
         }
-        Err(super::Error::Other("version unsupported"))
+        Err(super::Error::Other("version unsupported".to_owned()))
     }
 
     pub fn version(&self) -> super::Version {
@@ -122,7 +122,7 @@ impl PakReader {
                 self.key.as_ref(),
                 writer,
             ),
-            None => Err(super::Error::Other("no file found at given path")),
+            None => Err(super::Error::MissingEntry(path.to_owned())),
         }
     }
 

repak_cli/src/main.rs

@@ -53,6 +53,10 @@ struct ActionUnpack {
     #[arg(short, long, default_value = "false")]
     verbose: bool,
 
+    /// Force overwrite existing files/directories.
+    #[arg(short, long, default_value = "false")]
+    force: bool,
+
     /// Files or directories to include. Can be specified multiple times. If not specified, everything is extracted.
     #[arg(action = clap::ArgAction::Append, short, long)]
     include: Vec<String>,
@@ -132,9 +136,6 @@ struct Args {
 fn main() -> Result<(), repak::Error> {
     let args = Args::parse();
 
-    //let aasdf = repak::Version::iter().map(|v| format!("{v}"));
-    //clap::builder::PossibleValuesParser::new(aasdf.map(|a| a.as_str()));
-
     match args.action {
         Action::Info(args) => info(args),
         Action::List(args) => list(args),
@@ -191,8 +192,10 @@ fn unpack(args: ActionUnpack) -> Result<(), repak::Error> {
         Err(ref e) if e.kind() == std::io::ErrorKind::AlreadyExists => Ok(()),
         Err(e) => Err(e),
     }?;
-    if output.read_dir()?.next().is_some() {
-        return Err(repak::Error::Other("output directory not empty"));
+    if !args.force && output.read_dir()?.next().is_some() {
+        return Err(repak::Error::OutputNotEmpty(
+            output.to_string_lossy().to_string(),
+        ));
     }
     let mount_point = PathBuf::from(pak.mount_point());
     let prefix = Path::new(&args.strip_prefix);
@@ -203,43 +206,62 @@ fn unpack(args: ActionUnpack) -> Result<(), repak::Error> {
         .map(|i| prefix.join(Path::new(i)))
         .collect::<Vec<_>>();
 
-    let counter = std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0));
+    struct UnpackEntry {
+        entry_path: String,
+        out_path: PathBuf,
+        out_dir: PathBuf,
+    }
 
-    pak.files().into_par_iter().try_for_each_init(
-        || (File::open(&args.input), includes.clone(), counter.clone()),
-        |(file, includes, counter), path| -> Result<(), repak::Error> {
-            let full_path = mount_point.join(&path);
-            if !includes.is_empty() && !includes.iter().any(|i| full_path.starts_with(i)) {
-                return Ok(());
-            }
+    let entries =
+        pak.files()
+            .into_iter()
+            .map(|entry_path| {
+                let full_path = mount_point.join(&entry_path);
+                if !includes.is_empty() && !includes.iter().any(|i| full_path.starts_with(i)) {
+                    return Ok(None);
+                }
+                let out_path = output
+                    .join(full_path.strip_prefix(prefix).map_err(|_| {
+                        repak::Error::PrefixMismatch {
+                            path: full_path.to_string_lossy().to_string(),
+                            prefix: prefix.to_string_lossy().to_string(),
+                        }
+                    })?)
+                    .clean();
+
+                if !out_path.starts_with(&output) {
+                    return Err(repak::Error::WriteOutsideOutput(
+                        out_path.to_string_lossy().to_string(),
+                    ));
+                }
+
+                let out_dir = out_path.parent().expect("will be a file").to_path_buf();
+
+                Ok(Some(UnpackEntry {
+                    entry_path,
+                    out_path,
+                    out_dir,
+                }))
+            })
+            .filter_map(|e| e.transpose())
+            .collect::<Result<Vec<_>, repak::Error>>()?;
+
+    entries.par_iter().try_for_each_init(
+        || File::open(&args.input),
+        |file, entry| -> Result<(), repak::Error> {
             if args.verbose {
-                println!("extracting {path}");
+                println!("extracting {}", entry.entry_path);
             }
-            let file_path = output.join(
-                full_path
-                    .strip_prefix(prefix)
-                    .map_err(|_| repak::Error::Other("prefix does not match"))?,
-            );
-            if !file_path.clean().starts_with(&output) {
-                return Err(repak::Error::Other(
-                    "tried to write file outside of output directory",
-                ));
-            }
-            fs::create_dir_all(file_path.parent().expect("will be a file"))?;
-            counter.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+            fs::create_dir_all(&entry.out_dir)?;
             pak.read_file(
-                &path,
+                &entry.entry_path,
                 &mut BufReader::new(file.as_ref().unwrap()), // TODO: avoid this unwrap
-                &mut fs::File::create(file_path)?,
+                &mut fs::File::create(&entry.out_path)?,
             )
         },
     )?;
 
-    println!(
-        "Unpacked {} files to {}",
-        counter.load(std::sync::atomic::Ordering::Relaxed),
-        output.display()
-    );
+    println!("Unpacked {} files to {}", entries.len(), output.display());
 
     Ok(())
 }
@@ -264,7 +286,9 @@ fn pack(args: ActionPack) -> Result<(), repak::Error> {
     }
     let input_path = Path::new(&args.input);
     if !input_path.is_dir() {
-        return Err(repak::Error::Other("input is not a directory"));
+        return Err(repak::Error::InputNotADirectory(
+            input_path.to_string_lossy().to_string(),
+        ));
     }
     let mut paths = vec![];
     collect_files(&mut paths, input_path)?;
@@ -309,7 +333,10 @@ fn get(args: ActionGet) -> Result<(), repak::Error> {
     let full_path = mount_point.join(args.file);
     let file = full_path
         .strip_prefix(prefix)
-        .map_err(|_| repak::Error::Other("prefix does not match"))?;
+        .map_err(|_| repak::Error::PrefixMismatch {
+            path: full_path.to_string_lossy().to_string(),
+            prefix: prefix.to_string_lossy().to_string(),
+        })?;
 
     use std::io::Write;
     std::io::stdout().write_all(&pak.get(&file.to_string_lossy(), &mut reader)?)?;