diff --git a/CHANGELOG.md b/CHANGELOG.md index 18bc00df6..48f413cbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ Categories Used: ## [Unreleased](https://github.com/ouch-org/ouch/compare/0.8.1...HEAD) +### Bug Fixes + +- Decompressing with `--format` no longer writes the output over the input file (https://github.com/ouch-org/ouch/issues/442). + ### Tweaks - Releases: sign assets with cosign instead of GitHub artifact attestations diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 682c0c9ce..f055ffbac 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -185,7 +185,24 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic let file_name = path.file_name().ok_or_else(|| Error::Custom { reason: FinalError::with_title(format!("{} does not have a file name", PathFmt(path))), })?; - files_output_paths.push(file_name.into()); + // Strip as many trailing known extensions as the format has, so the output + // name never equals the input name (which would overwrite the input file). + let output_file_name = match <[u8] as ByteSlice>::from_os_str(file_name) { + Some(bytes) if !is_path_stdin(path) => { + let stripped = extension::strip_known_extensions_from_name(bytes, format.len()); + if stripped == bytes { + // Nothing was stripped, so keep the name distinct from the input + utils::append_ascii_suffix_to_os_str(file_name, "-output") + } else { + stripped + .to_os_str() + .expect("stripped bytes came from an OsStr") + .to_owned() + } + } + _ => file_name.to_owned(), + }; + files_output_paths.push(output_file_name.into()); files_extensions.push(format.clone()); } } else { diff --git a/src/extension.rs b/src/extension.rs index 3286d2f4b..2eedae29e 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -188,6 +188,20 @@ fn split_extension_at_end(name: &[u8]) -> Option<(&[u8], Extension)> { Some((new_name, ext)) } +/// Remove up to `max_count` trailing known extensions from a file name. +/// +/// Used by `--format`, where the extensions in the path are not parsed, so the output name +/// still has to be derived from the input name to avoid overwriting the input file. +pub fn strip_known_extensions_from_name(mut name: &[u8], max_count: usize) -> &[u8] { + for _ in 0..max_count { + let Some((new_name, _)) = split_extension_at_end(name) else { + break; + }; + name = new_name; + } + name +} + pub fn parse_format_flag(text: &str) -> Result> { let extensions: Vec = text .split('.') diff --git a/tests/integration.rs b/tests/integration.rs index daab9a655..5c422d882 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1939,3 +1939,44 @@ fn decompress_conflict_with_dev_null_stdin_exits_nonzero() { "decompress with an unresolvable conflict on /dev/null stdin must exit non-zero" ); } + +/// Decompressing with `--format` must not write the output over the input file (issue #442). +#[test] +fn decompress_with_format_flag_does_not_overwrite_input() { + let (_tempdir, dir) = testdir().unwrap(); + + let source = dir.join("file"); + fs::write(&source, "hello").unwrap(); + let input = dir.join("file.zst.zst.zst"); + + crate::utils::cargo_bin() + .current_dir(dir) + .args(["compress", "--yes"]) + .arg(&source) + .arg(&input) + .assert() + .success(); + + let archive_before = fs::read(&input).unwrap(); + + // Only the outermost `.zst` is undone, so the output name must be `file.zst.zst`. + // The exit status is not asserted here: when the output path collides with the input, + // the run truncates the input and then fails, and the assertions below must be the + // ones that report it. + let _ = crate::utils::cargo_bin_command() + .current_dir(dir) + .args(["decompress", "--yes", "--here", "--format", "zst"]) + .arg(&input) + .status() + .unwrap(); + + assert_eq!( + fs::read(&input).unwrap(), + archive_before, + "the input archive must not be overwritten by its own decompressed output" + ); + assert!( + dir.join("file.zst.zst").exists(), + "output should drop one known extension from the input name" + ); +}