From 2dad11d0ba20a96430822e72c04113e1b985d39f Mon Sep 17 00:00:00 2001 From: ttyS3 Date: Fri, 6 Sep 2024 13:50:00 +0000 Subject: [PATCH] fix(password): update password handling for archives Refactor password handling in archive functions to use &[u8] instead of impl AsRef<[u8]>. Include better error reporting for invalid UTF-8 passwords in 7z archives. --- src/archive/rar.rs | 7 ++----- src/archive/sevenz.rs | 31 ++++++++++++++++--------------- src/archive/zip.rs | 10 ++++------ src/cli/args.rs | 2 +- src/commands/decompress.rs | 2 +- src/commands/list.rs | 2 +- src/commands/mod.rs | 6 +++--- src/error.rs | 3 +++ 8 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/archive/rar.rs b/src/archive/rar.rs index 3d32da6..290916c 100644 --- a/src/archive/rar.rs +++ b/src/archive/rar.rs @@ -11,13 +11,11 @@ use crate::{error::Error, list::FileInArchive, utils::logger::info}; pub fn unpack_archive( archive_path: &Path, output_folder: &Path, - password: Option>, + password: Option<&[u8]>, quiet: bool, ) -> crate::Result { assert!(output_folder.read_dir().expect("dir exists").count() == 0); - let password = password.as_ref().map(|p| p.as_ref()); - let archive = match password { Some(password) => Archive::with_password(archive_path, password), None => Archive::new(archive_path), @@ -49,9 +47,8 @@ pub fn unpack_archive( /// List contents of `archive_path`, returning a vector of archive entries pub fn list_archive( archive_path: &Path, - password: Option>, + password: Option<&[u8]>, ) -> impl Iterator> { - let password = password.as_ref().map(|p| p.as_ref()); let archive = match password { Some(password) => Archive::with_password(archive_path, password), None => Archive::new(archive_path), diff --git a/src/archive/sevenz.rs b/src/archive/sevenz.rs index 623395d..c8be7a1 100644 --- a/src/archive/sevenz.rs +++ b/src/archive/sevenz.rs @@ -20,6 +20,7 @@ use crate::{ Bytes, EscapedPathDisplay, FileVisibilityPolicy, }, }; +use crate::error::Error; pub fn compress_sevenz( files: &[PathBuf], @@ -102,7 +103,7 @@ where pub fn decompress_sevenz( reader: R, output_path: &Path, - password: Option>, + password: Option<&[u8]>, quiet: bool, ) -> crate::Result where @@ -161,17 +162,17 @@ where Ok(true) }; - let password = password.as_ref().map(|p| p.as_ref()); - match password { Some(password) => sevenz_rust::decompress_with_extract_fn_and_password( reader, output_path, - sevenz_rust::Password::from(password.to_str().unwrap()), + sevenz_rust::Password::from(password.to_str().map_err(|_| { + Error::InvalidPassword("7z requires that all passwords are valid UTF-8") + })?), entry_extract_fn, )?, None => sevenz_rust::decompress_with_extract_fn(reader, output_path, entry_extract_fn)?, - }; + } Ok(count) } @@ -179,10 +180,9 @@ where /// List contents of `archive_path`, returning a vector of archive entries pub fn list_archive( archive_path: &Path, - password: Option>, + password: Option<&[u8]>, ) -> impl Iterator> { let reader = fs::File::open(archive_path).unwrap(); - let password = password.as_ref().map(|p| p.as_ref()); let mut files = Vec::new(); @@ -195,15 +195,16 @@ pub fn list_archive( }; match password { - Some(password) => sevenz_rust::decompress_with_extract_fn_and_password( - reader, - ".", - sevenz_rust::Password::from(password.to_str().unwrap()), - entry_extract_fn, - ) - .unwrap(), + Some(password) => { + sevenz_rust::decompress_with_extract_fn_and_password( + reader, + ".", + sevenz_rust::Password::from(password.to_str().unwrap()), + entry_extract_fn, + ).unwrap() + }, None => sevenz_rust::decompress_with_extract_fn(reader, ".", entry_extract_fn).unwrap(), - }; + } files.into_iter() } diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 339d6ba..5b99a02 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -32,7 +32,7 @@ use crate::{ pub fn unpack_archive( mut archive: ZipArchive, output_folder: &Path, - password: Option>, + password: Option<&[u8]>, quiet: bool, ) -> crate::Result where @@ -42,10 +42,8 @@ where let mut unpacked_files = 0; - let password = password.as_ref().map(|p| p.as_ref().to_owned()); - for idx in 0..archive.len() { - let mut file = match password.clone() { + let mut file = match password { Some(password) => archive .by_index_decrypt(idx, password.to_owned().as_bytes()) .unwrap() @@ -108,7 +106,7 @@ where /// List contents of `archive`, returning a vector of archive entries pub fn list_archive( mut archive: ZipArchive, - password: Option>, + password: Option<&[u8]>, ) -> impl Iterator> where R: Read + Seek + Send + 'static, @@ -122,7 +120,7 @@ where } } - let password = password.as_ref().map(|p| p.as_ref().to_owned()); + let password = password.map(|p| p.to_owned()); let (tx, rx) = mpsc::channel(); thread::spawn(move || { diff --git a/src/cli/args.rs b/src/cli/args.rs index b001c2a..dcdf768 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -43,7 +43,7 @@ pub struct CliArgs { /// decompress or list with password #[arg(short = 'p', long = "password", global = true)] - pub password: Option, + pub password: Option, // Ouch and claps subcommands #[command(subcommand)] diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index e4912ce..55086a2 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -36,7 +36,7 @@ pub fn decompress_file( output_file_path: PathBuf, question_policy: QuestionPolicy, quiet: bool, - password: Option<&str>, + password: Option<&[u8]>, ) -> crate::Result<()> { assert!(output_dir.exists()); let input_is_stdin = is_path_stdin(input_file_path); diff --git a/src/commands/list.rs b/src/commands/list.rs index 1582fc9..922ca33 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -21,7 +21,7 @@ pub fn list_archive_contents( formats: Vec, list_options: ListOptions, question_policy: QuestionPolicy, - password: Option<&str>, + password: Option<&[u8]>, ) -> crate::Result<()> { let reader = fs::File::open(archive_path)?; diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 2a1d13e..bce640f 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -5,7 +5,7 @@ mod decompress; mod list; use std::{ops::ControlFlow, path::PathBuf}; - +use std::os::unix::prelude::OsStrExt; use rayon::prelude::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; use utils::colors; @@ -188,7 +188,7 @@ pub fn run( output_file_path, question_policy, args.quiet, - args.password.as_deref(), + args.password.as_deref().map(|str|str.as_bytes()), ) }) } @@ -227,7 +227,7 @@ pub fn run( formats, list_options, question_policy, - args.password.as_deref(), + args.password.as_deref().map(|str|str.as_bytes()), )?; } diff --git a/src/error.rs b/src/error.rs index a54d0a2..7e7a0a8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -39,6 +39,8 @@ pub enum Error { /// Recognised but unsupported format // currently only RAR when built without the `unrar` feature UnsupportedFormat { reason: String }, + /// Invalid password provided + InvalidPassword(&'static str), } /// Alias to std's Result with ouch's Error @@ -148,6 +150,7 @@ impl fmt::Display for Error { Error::UnsupportedFormat { reason } => { FinalError::with_title("Recognised but unsupported format").detail(reason.clone()) } + Error::InvalidPassword(reason) => FinalError::with_title("Invalid password").detail(*reason), }; write!(f, "{err}")