fix: do not overwrite during merge

CHANGELOG.md

@@ -18,6 +18,10 @@ Categories Used:
 
 ## [Unreleased](https://github.com/ouch-org/ouch/compare/0.8.1...HEAD)
 
+### Bug Fixes
+
+- Ask before replacing files when merging into an existing folder instead of overwriting them silently (https://github.com/ouch-org/ouch/pull/1031).
+
 ### Tweaks
 
 - Releases: sign assets with cosign instead of GitHub artifact attestations

src/archive/rar.rs

@@ -2,30 +2,71 @@
 
 use std::path::{Path, PathBuf};
 
+use fs_err as fs;
 use unrar::{
     Archive, ExtractEvent,
     error::{Code, UnrarError, When},
 };
 
 use crate::{
+    QuestionPolicy,
     error::{Error, FinalError, Result},
     info,
     list::{FileInArchive, ListFileType},
-    utils::{BytesFmt, PathFmt, validate_entry_path},
+    utils::{BytesFmt, PathFmt, resolve_extraction_conflict, validate_entry_path},
     warning,
 };
 
-/// Unpacks the archive given by `archive_path` into the folder given by `output_folder`.
-/// Assumes that output_folder is empty
-pub fn unpack_archive(archive_path: &Path, output_folder: &Path, password: Option<&[u8]>) -> Result<u64> {
+/// Unpacks the archive into `output_folder` and asks before replacing files.
+pub fn unpack_archive(
+    archive_path: &Path,
+    output_folder: &Path,
+    password: Option<&[u8]>,
+    question_policy: QuestionPolicy,
+) -> Result<u64> {
+    // Rar reference records need a full extraction pass to resolve.
+    fs::create_dir_all(output_folder)?;
+    let staging = tempfile::Builder::new()
+        .prefix(".ouch-rar-")
+        .tempdir_in(output_folder)?;
+    extract_all(archive_path, staging.path(), password)?;
+    move_into_place(staging.path(), staging.path(), output_folder, question_policy)
+}
+
+/// Move each staged entry into `output_folder` at the same relative path.
+fn move_into_place(root: &Path, dir: &Path, output_folder: &Path, question_policy: QuestionPolicy) -> Result<u64> {
+    let mut files_unpacked = 0;
+    for entry in fs::read_dir(dir)? {
+        let source = entry?.path();
+        let dest = output_folder.join(source.strip_prefix(root).expect("child of staging root"));
+
+        if fs::symlink_metadata(&source)?.is_dir() {
+            std::fs::create_dir_all(&dest).map_err(|err| Error::Custom {
+                reason: FinalError::with_title(format!("failed to create {}", PathFmt(&dest))).detail(err.to_string()),
+            })?;
+            files_unpacked += move_into_place(root, &source, output_folder, question_policy)?;
+        } else if let Some(target) = resolve_extraction_conflict(&dest, question_policy)? {
+            let size = fs::symlink_metadata(&source)?.len();
+            std::fs::rename(&source, &target).map_err(|err| Error::Custom {
+                reason: FinalError::with_title(format!("failed to extract {}", PathFmt(&target)))
+                    .detail(err.to_string()),
+            })?;
+            info!("extracted ({}) {}", BytesFmt(size), PathFmt(&target));
+            files_unpacked += 1;
+        }
+    }
+    Ok(files_unpacked)
+}
+
+/// Extract the whole archive into a staging folder in one pass.
+fn extract_all(archive_path: &Path, output_folder: &Path, password: Option<&[u8]>) -> Result<()> {
     let archive = match password {
         Some(password) => Archive::with_password(archive_path, password),
         None => Archive::new(archive_path),
     };
 
     let archive = archive.open_for_processing()?;
 
-    let mut files_unpacked: u64 = 0;
     let mut first_err: Option<(PathBuf, i32)> = None;
     let mut unsafe_path: Option<(PathBuf, String)> = None;
 
@@ -39,11 +80,7 @@ pub fn unpack_archive(archive_path: &Path, output_folder: &Path, password: Optio
                 true
             }
         }
-        ExtractEvent::Ok { filename, size } => {
-            info!("extracted ({}) {}", BytesFmt(size), PathFmt(&filename));
-            files_unpacked += 1;
-            true
-        }
+        ExtractEvent::Ok { .. } => true,
         ExtractEvent::Err { filename, error_code } => {
             first_err = Some((filename, error_code));
             // Returning false cancels the rest of the extraction so any
@@ -80,7 +117,7 @@ pub fn unpack_archive(archive_path: &Path, output_folder: &Path, password: Optio
         });
     }
     let _status = cb_result?;
-    Ok(files_unpacked)
+    Ok(())
 }
 
 /// List contents of `archive_path`, returning a vector of archive entries

src/archive/sevenz.rs

@@ -12,22 +12,30 @@ use same_file::Handle;
 use sevenz_rust2::ArchiveEntry;
 
 use crate::{
-    Result,
+    QuestionPolicy, Result,
     error::{Error, FinalError},
     info,
     list::{FileInArchive, ListFileType},
     utils::{
         BytesFmt, FileVisibilityPolicy, PathFmt, cd_into_same_dir_as, copy_limited_decompression,
-        ensure_parent_dir_exists, is_same_file_as_output, validate_dest_inside_root, validate_entry_path,
+        ensure_parent_dir_exists, is_same_file_as_output, resolve_extraction_conflict, validate_dest_inside_root,
+        validate_entry_path,
     },
     warning,
 };
 
-pub fn unpack_archive<R>(reader: R, output_path: &Path, password: Option<&[u8]>) -> Result<u64>
+pub fn unpack_archive<R>(
+    reader: R,
+    output_path: &Path,
+    password: Option<&[u8]>,
+    question_policy: QuestionPolicy,
+) -> Result<u64>
 where
     R: Read + Seek,
 {
     let mut files_unpacked = 0;
+    // The closure cannot return an ouch error so it is carried out here.
+    let mut conflict_error = None;
 
     let entry_extract_fn =
         |entry: &ArchiveEntry, reader: &mut dyn Read, path: &PathBuf| -> Result<bool, sevenz_rust2::Error> {
@@ -54,11 +62,20 @@ where
                     fs::create_dir_all(path)?;
                 }
             } else {
-                info!("extracted ({}) {}", BytesFmt(entry.size()), PathFmt(&file_path));
+                let dest = match resolve_extraction_conflict(path, question_policy) {
+                    Ok(Some(dest)) => dest,
+                    Ok(None) => return Ok(true),
+                    Err(err) => {
+                        conflict_error = Some(err);
+                        return Ok(false);
+                    }
+                };
+
+                info!("extracted ({}) {}", BytesFmt(entry.size()), PathFmt(&dest));
 
-                ensure_parent_dir_exists(path)?;
+                ensure_parent_dir_exists(&dest)?;
 
-                let file = fs::File::create(path)?;
+                let file = fs::File::create(&dest)?;
                 let mut writer = BufWriter::new(file);
                 copy_limited_decompression(reader, &mut writer)?;
 
@@ -70,25 +87,31 @@ where
                     Some(ft::FileTime::from_system_time(entry.last_modified_date().into())),
                     Some(ft::FileTime::from_system_time(entry.creation_date().into())),
                 ) {
-                    warning!("could not set timestamps on {}: {e}", PathFmt(&file_path));
+                    warning!("could not set timestamps on {}: {e}", PathFmt(&dest));
                 }
             }
 
             files_unpacked += 1;
             Ok(true) // Always proceed
         };
 
-    match password {
+    let result = match password {
         Some(password) => sevenz_rust2::decompress_with_extract_fn_and_password(
             reader,
             output_path,
             sevenz_rust2::Password::from(password.to_str().map_err(|err| Error::InvalidPassword {
                 reason: err.to_string(),
             })?),
             entry_extract_fn,
-        )?,
-        None => sevenz_rust2::decompress_with_extract_fn(reader, output_path, entry_extract_fn)?,
+        ),
+        None => sevenz_rust2::decompress_with_extract_fn(reader, output_path, entry_extract_fn),
+    };
+
+    // Report the prompt failure instead of the library error it caused.
+    if let Some(err) = conflict_error {
+        return Err(err);
     }
+    result?;
 
     Ok(files_unpacked)
 }

src/archive/tar.rs

@@ -13,21 +13,21 @@ use fs_err as fs;
 use same_file::Handle;
 
 use crate::{
-    Result,
+    QuestionPolicy, Result,
     error::FinalError,
     info,
     list::{FileInArchive, ListFileType},
     utils::{
         self, BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, create_symlink, is_same_file_as_output,
-        read_file_type, sanitize_archive_mode, set_permission_mode, validate_dest_inside_root, validate_entry_path,
-        validate_symlink_target,
+        read_file_type, resolve_extraction_conflict, sanitize_archive_mode, set_permission_mode,
+        validate_dest_inside_root, validate_entry_path, validate_symlink_target,
     },
     warning,
 };
 
 /// Unpacks the archive given by `archive` into the folder given by `into`.
 /// Assumes that output_folder is empty
-pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result<u64> {
+pub fn unpack_archive(reader: impl Read, output_folder: &Path, question_policy: QuestionPolicy) -> Result<u64> {
     let mut archive = tar::Archive::new(reader);
 
     let mut files_unpacked = 0;
@@ -36,6 +36,9 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result<u64> {
     for entry in archive.entries()? {
         let mut entry = entry?;
 
+        // Set when the user renamed a file so the log can show the real path.
+        let mut written = None;
+
         match entry.header().entry_type() {
             tar::EntryType::Symlink => {
                 let raw_path = entry.path()?.into_owned();
@@ -66,7 +69,20 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result<u64> {
                 fs::hard_link(&full_target_path, &full_link_path)?;
             }
             tar::EntryType::Regular | tar::EntryType::GNUSparse => {
-                entry.unpack_in(output_folder)?;
+                let raw_path = entry.path()?.into_owned();
+                let safe_relpath = validate_entry_path(&raw_path)?;
+                let full_path = output_folder.join(&safe_relpath);
+
+                let Some(dest) = resolve_extraction_conflict(&full_path, question_policy)? else {
+                    continue;
+                };
+
+                if dest == full_path {
+                    entry.unpack_in(output_folder)?;
+                } else {
+                    entry.unpack(&dest)?;
+                }
+                written = Some(dest);
             }
             tar::EntryType::Directory => {
                 let original_mode = entry.header().mode()?;
@@ -90,14 +106,15 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result<u64> {
             _ => continue,
         }
 
+        let unpacked_path = match written {
+            Some(path) => path,
+            None => output_folder.join(entry.path()?),
+        };
+
         if entry.header().entry_type().is_dir() {
-            info!("Directory {} created", PathFmt(&output_folder.join(entry.path()?)));
+            info!("Directory {} created", PathFmt(&unpacked_path));
         } else {
-            info!(
-                "extracted ({}) {}",
-                BytesFmt(entry.size()),
-                PathFmt(&output_folder.join(entry.path()?)),
-            );
+            info!("extracted ({}) {}", BytesFmt(entry.size()), PathFmt(&unpacked_path));
         }
         files_unpacked += 1;
     }

src/archive/zip.rs

@@ -17,22 +17,27 @@ use zip::{self, DateTime, ZipArchive, read::ZipFile};
 #[cfg(unix)]
 use crate::utils::sanitize_archive_mode;
 use crate::{
-    Result,
+    QuestionPolicy, Result,
     error::FinalError,
     info, info_accessible,
     list::{FileInArchive, ListFileType},
     utils::{
         BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, cd_into_same_dir_as,
         copy_limited_decompression, create_symlink, ensure_parent_dir_exists, get_invalid_utf8_paths,
-        is_same_file_as_output, pretty_format_list_of_paths, read_file_type, strip_cur_dir, validate_dest_inside_root,
-        validate_symlink_target,
+        is_same_file_as_output, pretty_format_list_of_paths, read_file_type, resolve_extraction_conflict,
+        strip_cur_dir, validate_dest_inside_root, validate_symlink_target,
     },
     warning,
 };
 
 /// Unpacks the archive given by `archive` into the folder given by `output_folder`.
 /// Assumes that output_folder is empty
-pub fn unpack_archive<R>(reader: R, output_folder: &Path, password: Option<&[u8]>) -> Result<u64>
+pub fn unpack_archive<R>(
+    reader: R,
+    output_folder: &Path,
+    password: Option<&[u8]>,
+    question_policy: QuestionPolicy,
+) -> Result<u64>
 where
     R: Read + Seek,
 {
@@ -87,6 +92,16 @@ where
                 let mode = file.unix_mode();
                 let is_symlink = mode.is_some_and(|mode| mode & 0o170000 == 0o120000);
 
+                // Symlink creation fails on its own when the path is taken.
+                let mut resolved = None;
+                if !is_symlink {
+                    let Some(path) = resolve_extraction_conflict(file_path, question_policy)? else {
+                        continue;
+                    };
+                    resolved = Some(path);
+                }
+                let file_path = resolved.as_deref().unwrap_or(file_path);
+
                 if is_symlink {
                     // Symlink targets are arbitrary bytes on Unix, not guaranteed UTF-8; read as bytes.
                     let mut target_bytes = Vec::new();

src/commands/decompress.rs

@@ -206,7 +206,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> {
         Tar => unpack_archive(
             |output_dir| {
                 let reader = LimitedReader::new(create_decoder_up_to_first_extension()?);
-                crate::archive::tar::unpack_archive(reader, output_dir)
+                crate::archive::tar::unpack_archive(reader, output_dir, options.question_policy)
             },
             dir,
         )?,
@@ -251,7 +251,10 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> {
                 ))
             };
 
-            unpack_archive(|output_dir| unpack_fn(reader, output_dir, options.password), dir)?
+            unpack_archive(
+                |output_dir| unpack_fn(reader, output_dir, options.password, options.question_policy),
+                dir,
+            )?
         }
         #[cfg(feature = "unrar")]
         Rar => {
@@ -260,11 +263,21 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> {
                 let mut temp_file = tempfile::Builder::new().prefix(".ouch-rar-").tempfile_in(&dir)?;
                 copy_limited_decompression(create_decoder_up_to_first_extension()?, &mut temp_file)?;
                 Box::new(move |output_dir| {
-                    crate::archive::rar::unpack_archive(temp_file.path(), output_dir, options.password)
+                    crate::archive::rar::unpack_archive(
+                        temp_file.path(),
+                        output_dir,
+                        options.password,
+                        options.question_policy,
+                    )
                 })
             } else {
                 Box::new(|output_dir| {
-                    crate::archive::rar::unpack_archive(options.input_file_path, output_dir, options.password)
+                    crate::archive::rar::unpack_archive(
+                        options.input_file_path,
+                        output_dir,
+                        options.password,
+                        options.question_policy,
+                    )
                 })
             };
 

src/utils/fs.rs

@@ -50,6 +50,21 @@ pub fn resolve_path_conflict(
     }
 }
 
+/// Decide where to extract a file when the path is taken. None means skip it.
+pub fn resolve_extraction_conflict(path: &Path, question_policy: QuestionPolicy) -> Result<Option<PathBuf>> {
+    // Only an existing file clashes. Directories merge and other kinds fail on write.
+    if !path.is_file() {
+        return Ok(Some(path.to_path_buf()));
+    }
+
+    // These choices fit a single file. They are rename or overwrite or skip.
+    match user_wants_to_overwrite(path, question_policy, QuestionAction::Compression)? {
+        FileConflitOperation::Cancel => Ok(None),
+        FileConflitOperation::Rename => Ok(Some(find_available_filename_by_renaming(path)?)),
+        FileConflitOperation::Overwrite | FileConflitOperation::Merge => Ok(Some(path.to_path_buf())),
+    }
+}
+
 pub fn remove_file_or_dir(path: &Path) -> Result<()> {
     if path.is_dir() {
         if let Ok(cwd) = env::current_dir()