diff --git a/Cargo.lock b/Cargo.lock index fe839b1..9425043 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1106,6 +1106,7 @@ dependencies = [ "libc", "linked-hash-map", "lz4_flex", + "memchr", "num_cpus", "once_cell", "parse-display", diff --git a/Cargo.toml b/Cargo.toml index 17dd326..fc692b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ glob = "0.3.2" infer = "0.16.0" insta = { version = "1.40.0", features = ["filters"] } itertools = "0.14.0" +memchr = "2.7.4" parse-display = "0.9.1" pretty_assertions = "1.4.1" proptest = "1.5.0" diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fd1fc88..4a81d85 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -81,7 +81,7 @@ pub fn run( let parsed_formats = parse_format_flag(&formats)?; (Some(formats), parsed_formats) } - None => (None, extension::extensions_from_path(&output_path)), + None => (None, extension::extensions_from_path(&output_path)?), }; check::check_invalid_compression_with_non_archive_format( @@ -167,7 +167,7 @@ pub fn run( } } else { for path in files.iter() { - let (pathbase, mut file_formats) = extension::separate_known_extensions_from_name(path); + let (pathbase, mut file_formats) = extension::separate_known_extensions_from_name(path)?; if let ControlFlow::Break(_) = check::check_mime_type(path, &mut file_formats, question_policy)? { return Ok(()); @@ -229,7 +229,7 @@ pub fn run( } } else { for path in files.iter() { - let mut file_formats = extension::extensions_from_path(path); + let mut file_formats = extension::extensions_from_path(path)?; if let ControlFlow::Break(_) = check::check_mime_type(path, &mut file_formats, question_policy)? { return Ok(()); diff --git a/src/extension.rs b/src/extension.rs index 76d2426..8a6304e 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -5,7 +5,10 @@ use std::{ffi::OsStr, fmt, path::Path}; use bstr::ByteSlice; use CompressionFormat::*; -use crate::{error::Error, utils::logger::warning}; +use crate::{ + error::{Error, FinalError, Result}, + utils::logger::warning, +}; pub const SUPPORTED_EXTENSIONS: &[&str] = &[ "tar", @@ -188,17 +191,38 @@ pub fn parse_format_flag(input: &OsStr) -> crate::Result> { /// Extracts extensions from a path. /// /// Returns both the remaining path and the list of extension objects. -pub fn separate_known_extensions_from_name(path: &Path) -> (&Path, Vec) { +pub fn separate_known_extensions_from_name(path: &Path) -> Result<(&Path, Vec)> { let mut extensions = vec![]; let Some(mut name) = path.file_name().and_then(<[u8] as ByteSlice>::from_os_str) else { - return (path, extensions); + return Ok((path, extensions)); }; - while let Some((new_name, extension)) = split_extension_at_end(&name) { + while let Some((new_name, extension)) = split_extension_at_end(name) { name = new_name; extensions.insert(0, extension); if extensions[0].is_archive() { + if let Some((_, misplaced_extension)) = split_extension_at_end(name) { + return Err(FinalError::with_title("File extensions are invalid for operation") + .detail(format!( + "The archive extension '.{}' must come before any non-archive extensions, like '.{}'", + extensions[0].display_text, misplaced_extension.display_text + )) + .detail(format!( + "File: '{path:?}' contains '.{}' and '.{}'", + misplaced_extension.display_text, extensions[0].display_text, + )) + .detail(format!("'.{}' is an archive format", extensions[0].display_text)) + .detail(format!( + "'.{}' isn't an archive format", + misplaced_extension.display_text + )) + .hint("You can use `--format` to specify what format to use, examples:") + .hint(" ouch compress 1 2 file --format zip") + .hint(" ouch decompress file --format gz") + .hint(" ouch list archive --format zip") + .into()); + } break; } } @@ -212,13 +236,12 @@ pub fn separate_known_extensions_from_name(path: &Path) -> (&Path, Vec Vec { - let (_, extensions) = separate_known_extensions_from_name(path); - extensions +pub fn extensions_from_path(path: &Path) -> Result> { + separate_known_extensions_from_name(path).map(|(_, extensions)| extensions) } /// Panics if formats has an empty list of compression formats @@ -278,8 +301,8 @@ mod tests { fn test_extensions_from_path() { let path = Path::new("bolovo.tar.gz"); - let extensions: Vec = extensions_from_path(path); - let formats: Vec = flatten_compression_formats(&extensions); + let extensions = extensions_from_path(path).unwrap(); + let formats = flatten_compression_formats(&extensions); assert_eq!(formats, vec![Tar, Gzip]); } @@ -288,30 +311,30 @@ mod tests { /// Test extension parsing for input/output files fn test_separate_known_extensions_from_name() { assert_eq!( - separate_known_extensions_from_name("file".as_ref()), + separate_known_extensions_from_name("file".as_ref()).unwrap(), ("file".as_ref(), vec![]) ); assert_eq!( - separate_known_extensions_from_name("tar".as_ref()), + separate_known_extensions_from_name("tar".as_ref()).unwrap(), ("tar".as_ref(), vec![]) ); assert_eq!( - separate_known_extensions_from_name(".tar".as_ref()), + separate_known_extensions_from_name(".tar".as_ref()).unwrap(), (".tar".as_ref(), vec![]) ); assert_eq!( - separate_known_extensions_from_name("file.tar".as_ref()), + separate_known_extensions_from_name("file.tar".as_ref()).unwrap(), ("file".as_ref(), vec![Extension::new(&[Tar], "tar")]) ); assert_eq!( - separate_known_extensions_from_name("file.tar.gz".as_ref()), + separate_known_extensions_from_name("file.tar.gz".as_ref()).unwrap(), ( "file".as_ref(), vec![Extension::new(&[Tar], "tar"), Extension::new(&[Gzip], "gz")] ) ); assert_eq!( - separate_known_extensions_from_name(".tar.gz".as_ref()), + separate_known_extensions_from_name(".tar.gz".as_ref()).unwrap(), (".tar".as_ref(), vec![Extension::new(&[Gzip], "gz")]) ); } @@ -370,16 +393,7 @@ mod tests { #[test] fn test_extension_parsing_with_multiple_archive_formats() { - assert_eq!( - separate_known_extensions_from_name("file.tar.zip".as_ref()), - ("file.tar".as_ref(), vec![Extension::new(&[Zip], "zip")]) - ); - assert_eq!( - separate_known_extensions_from_name("file.7z.zst.zip.lz4".as_ref()), - ( - "file.7z.zst".as_ref(), - vec![Extension::new(&[Zip], "zip"), Extension::new(&[Lz4], "lz4")] - ) - ); + assert!(separate_known_extensions_from_name("file.tar.zip".as_ref()).is_err()); + assert!(separate_known_extensions_from_name("file.7z.zst.zip.lz4".as_ref()).is_err()); } } diff --git a/tests/integration.rs b/tests/integration.rs index 8950a6a..b43bb08 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -7,8 +7,10 @@ use std::{ path::{Path, PathBuf}, }; +use bstr::ByteSlice; use fs_err as fs; use itertools::Itertools; +use memchr::memmem; use parse_display::Display; use pretty_assertions::assert_eq; use proptest::sample::size_range; @@ -213,7 +215,7 @@ fn multiple_files( let before = &dir.join("before"); let before_dir = &before.join("dir"); fs::create_dir_all(before_dir).unwrap(); - let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); let after = &dir.join("after"); create_random_files(before_dir, depth, &mut SmallRng::from_entropy()); ouch!("-A", "c", before_dir, archive); @@ -240,7 +242,7 @@ fn multiple_files_with_conflict_and_choice_to_overwrite( fs::create_dir_all(after_dir).unwrap(); create_random_files(after_dir, depth, &mut SmallRng::from_entropy()); - let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); ouch!("-A", "c", before_dir, archive); crate::utils::cargo_bin() @@ -281,7 +283,7 @@ fn multiple_files_with_conflict_and_choice_to_not_overwrite( fs::write(after_dir.join("something.txt"), "Some content").unwrap(); fs::copy(after_dir.join("something.txt"), after_backup_dir.join("something.txt")).unwrap(); - let archive = &dir.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &dir.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); ouch!("-A", "c", before_dir, archive); crate::utils::cargo_bin() @@ -392,7 +394,7 @@ fn smart_unpack_with_single_file( }) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); crate::utils::cargo_bin() .arg("compress") @@ -443,7 +445,7 @@ fn smart_unpack_with_multiple_files( .map(|entry| entry.unwrap().path()) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); let output_path = root_path.join("archive"); assert!(!output_path.exists()); @@ -492,7 +494,7 @@ fn no_smart_unpack_with_single_file( .map(|entry| entry.unwrap().path()) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); let output_path = root_path.join("archive"); assert!(!output_path.exists()); @@ -542,7 +544,7 @@ fn no_smart_unpack_with_multiple_files( .map(|entry| entry.unwrap().path()) .collect::>(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); let output_path = root_path.join("archive"); assert!(!output_path.exists()); @@ -590,7 +592,7 @@ fn multiple_files_with_disabled_smart_unpack_by_dir( let dest_files_path = root_path.join("dest_files"); fs::create_dir_all(&dest_files_path).unwrap(); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); crate::utils::cargo_bin() .arg("compress") @@ -707,7 +709,7 @@ fn symlink_pack_and_unpack( files_path.push(symlink_path); - let archive = &root_path.join(format!("archive.{}", merge_extensions(&ext, &extra_extensions))); + let archive = &root_path.join(format!("archive.{}", merge_extensions(ext, &extra_extensions))); crate::utils::cargo_bin() .arg("compress") @@ -894,22 +896,23 @@ fn reading_nested_archives_with_two_archive_extensions_adjacent() { .success(); } - crate::utils::cargo_bin() + let output = crate::utils::cargo_bin() .args(["list", &compressed_path, "--yes"]) .assert() - .success(); + .failure() + .get_output() + .clone(); + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some()); - crate::utils::cargo_bin() + let output = crate::utils::cargo_bin() .args(["decompress", &compressed_path, "--dir", &in_dir("out"), "--yes"]) .assert() - .success(); - - let decompressed_files = glob::glob(&format!("{}/*", in_dir("out"))) - .unwrap() - .collect::, _>>() - .unwrap(); - let decompression_expected = format!("{}/out/{}", temp_dir.path().display(), files[1]); - assert_eq!(decompressed_files, [Path::new(&decompression_expected)]); + .failure() + .get_output() + .clone(); + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some()); } } @@ -941,22 +944,124 @@ fn reading_nested_archives_with_two_archive_extensions_interleaved() { .success(); } - // // TODO: uncomment after fixing the 7z BadSignature error [4, 34, 77, 24, 96, 64] - // crate::utils::cargo_bin() - // .args(["list", &compressed_path, "--yes"]) - // .assert() - // .success(); + let output = crate::utils::cargo_bin() + .args(["list", &compressed_path, "--yes"]) + .assert() + .failure() + .get_output() + .clone(); + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some()); - crate::utils::cargo_bin() + let output = crate::utils::cargo_bin() .args(["decompress", &compressed_path, "--dir", &in_dir("out"), "--yes"]) .assert() - .success(); - - let decompressed_files = glob::glob(&format!("{}/*", in_dir("out"))) - .unwrap() - .collect::, _>>() - .unwrap(); - let decompression_expected = format!("{}/out/{}", temp_dir.path().display(), files[2]); - assert_eq!(decompressed_files, [Path::new(&decompression_expected)]); + .failure() + .get_output() + .clone(); + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some()); + } +} + +#[test] +fn compressing_archive_with_two_archive_formats() { + let archive_formats = ["tar", "zip", "7z"].into_iter(); + + for (first_archive, second_archive) in archive_formats.clone().cartesian_product(archive_formats.rev()) { + let temp_dir = tempdir().unwrap(); + let dir = temp_dir.path().display().to_string(); + + let output = crate::utils::cargo_bin() + .args([ + "compress", + "README.md", + &format!("{dir}/out.{first_archive}.{second_archive}"), + "--yes", + ]) + .assert() + .failure() + .get_output() + .clone(); + + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some()); + + let output = crate::utils::cargo_bin() + .args([ + "compress", + "README.md", + &format!("{dir}/out.{first_archive}.{second_archive}"), + "--yes", + "--format", + &format!("{first_archive}.{second_archive}"), + ]) + .assert() + .failure() + .get_output() + .clone(); + + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find( + stderr.as_bytes(), + b"can only be used at the start of the file extension", + ) + .is_some()); + + crate::utils::cargo_bin() + .args([ + "compress", + "README.md", + &format!("{dir}/out.{first_archive}.{second_archive}"), + "--yes", + "--format", + first_archive, + ]) + .assert() + .success(); + } +} + +#[test] +fn fail_when_compressing_archive_as_the_second_extension() { + for archive_format in ["tar", "zip", "7z"] { + let temp_dir = tempdir().unwrap(); + let dir = temp_dir.path().display().to_string(); + + let output = crate::utils::cargo_bin() + .args([ + "compress", + "README.md", + &format!("{dir}/out.zst.{archive_format}"), + "--yes", + ]) + .assert() + .failure() + .get_output() + .clone(); + + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find(stderr.as_bytes(), b"use `--format` to specify what format to use").is_some()); + + let output = crate::utils::cargo_bin() + .args([ + "compress", + "README.md", + &format!("{dir}/out_file"), + "--yes", + "--format", + &format!("zst.{archive_format}"), + ]) + .assert() + .failure() + .get_output() + .clone(); + + let stderr = output.stderr.to_str().unwrap(); + assert!(memmem::find( + stderr.as_bytes(), + format!("'{archive_format}' can only be used at the start of the file extension").as_bytes(), + ) + .is_some()); } }