From 70c81ed8a480dec1a2868beb704608f74c387306 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Tue, 2 Nov 2021 10:58:58 +0200 Subject: [PATCH 1/4] Fix non-archives overwriting files without asking and error-ing on dirs --- src/commands.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 7467772..25abe96 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -326,8 +326,22 @@ fn decompress_file( Gzip | Bzip | Lzma | Zstd => { reader = chain_reader_decoder(&formats[0].compression_formats[0], reader)?; - // TODO: improve error treatment - let mut writer = fs::File::create(&output_path)?; + let mut writer = match fs::OpenOptions::new().write(true).create_new(true).open(&output_path) { + Ok(w) => w, + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => { + if utils::user_wants_to_overwrite(&output_path, question_policy)? { + if output_path.is_dir() { + // We can't just use `fs::File::create(&output_path)` because it would return io::ErrorKind::IsADirectory + // ToDo: Maybe we should emphasise that `output_path` is a directory and everything inside it will be gone? + fs::remove_dir_all(&output_path)?; + } + fs::File::create(&output_path)? + } else { + return Ok(()); + } + } + Err(e) => return Err(Error::from(e)), + }; io::copy(&mut reader, &mut writer)?; files_unpacked = vec![output_path]; From 8ef1b25b12f021122cf83d94b650e1c722e943e7 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Tue, 2 Nov 2021 12:38:10 +0200 Subject: [PATCH 2/4] Ask to overwrite dirs when decompressing archives --- src/archive/tar.rs | 7 +++++++ src/archive/zip.rs | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 5aadb46..a778728 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -34,6 +34,13 @@ pub fn unpack_archive( continue; } + if file_path.is_dir() { + // We can't just use `fs::File::create(&file_path)` because it would return io::ErrorKind::IsADirectory + // ToDo: Maybe we should emphasise that `file_path` is a directory and everything inside it will be gone? + fs::remove_dir_all(&file_path)?; + fs::File::create(&file_path)?; + } + file.unpack_in(output_folder)?; info!("{:?} extracted. ({})", output_folder.join(file.path()?), Bytes::new(file.size())); diff --git a/src/archive/zip.rs b/src/archive/zip.rs index b62afd7..83de647 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -41,6 +41,13 @@ where continue; } + if file_path.is_dir() { + // We can't just use `fs::File::create(&file_path)` because it would return io::ErrorKind::IsADirectory + // ToDo: Maybe we should emphasise that `file_path` is a directory and everything inside it will be gone? + fs::remove_dir_all(&file_path)?; + fs::File::create(&file_path)?; + } + check_for_comments(&file); match (&*file.name()).ends_with('/') { From 547b8c91e55971401e4408d47d3f10c45b09da54 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Tue, 2 Nov 2021 13:34:04 +0200 Subject: [PATCH 3/4] Extract function --- src/commands.rs | 22 ++++++---------------- src/utils.rs | 24 +++++++++++++++++++++++- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 25abe96..c07015c 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -326,22 +326,12 @@ fn decompress_file( Gzip | Bzip | Lzma | Zstd => { reader = chain_reader_decoder(&formats[0].compression_formats[0], reader)?; - let mut writer = match fs::OpenOptions::new().write(true).create_new(true).open(&output_path) { - Ok(w) => w, - Err(e) if e.kind() == io::ErrorKind::AlreadyExists => { - if utils::user_wants_to_overwrite(&output_path, question_policy)? { - if output_path.is_dir() { - // We can't just use `fs::File::create(&output_path)` because it would return io::ErrorKind::IsADirectory - // ToDo: Maybe we should emphasise that `output_path` is a directory and everything inside it will be gone? - fs::remove_dir_all(&output_path)?; - } - fs::File::create(&output_path)? - } else { - return Ok(()); - } - } - Err(e) => return Err(Error::from(e)), - }; + let writer = utils::create_or_ask_overwrite(&output_path, question_policy)?; + if writer.is_none() { + // Means that the user doesn't want to overwrite + return Ok(()); + } + let mut writer = writer.unwrap(); io::copy(&mut reader, &mut writer)?; files_unpacked = vec![output_path]; diff --git a/src/utils.rs b/src/utils.rs index e8aaf52..e571bf8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -3,13 +3,35 @@ use std::{ cmp, env, ffi::OsStr, + io, path::Component, path::{Path, PathBuf}, }; use fs_err as fs; -use crate::{dialogs::Confirmation, info}; +use crate::{dialogs::Confirmation, info, Error}; + +/// Create the file if it doesn't exist and if it does then ask to overwrite it. +/// If the user doesn't want to overwrite then we return [`Ok(None)`] +pub fn create_or_ask_overwrite(path: &Path, question_policy: QuestionPolicy) -> Result, Error> { + match fs::OpenOptions::new().write(true).create_new(true).open(path) { + Ok(w) => Ok(Some(w)), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => { + if user_wants_to_overwrite(path, question_policy)? { + if path.is_dir() { + // We can't just use `fs::File::create(&path)` because it would return io::ErrorKind::IsADirectory + // ToDo: Maybe we should emphasise that `path` is a directory and everything inside it will be gone? + fs::remove_dir_all(path)?; + } + Ok(Some(fs::File::create(path)?)) + } else { + Ok(None) + } + } + Err(e) => Err(Error::from(e)), + } +} /// Checks if the given path represents an empty directory. pub fn dir_is_empty(dir_path: &Path) -> bool { From 7f5ff0faf1c95705e451bfa7da522eb2f75bc2a0 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Tue, 2 Nov 2021 13:34:38 +0200 Subject: [PATCH 4/4] Fix archives panicking when asked to overwrite file --- src/archive/tar.rs | 4 ++-- src/archive/zip.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index a778728..c71cdbb 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -35,10 +35,10 @@ pub fn unpack_archive( } if file_path.is_dir() { - // We can't just use `fs::File::create(&file_path)` because it would return io::ErrorKind::IsADirectory // ToDo: Maybe we should emphasise that `file_path` is a directory and everything inside it will be gone? fs::remove_dir_all(&file_path)?; - fs::File::create(&file_path)?; + } else if file_path.is_file() { + fs::remove_file(&file_path)?; } file.unpack_in(output_folder)?; diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 83de647..9085129 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -42,10 +42,10 @@ where } if file_path.is_dir() { - // We can't just use `fs::File::create(&file_path)` because it would return io::ErrorKind::IsADirectory // ToDo: Maybe we should emphasise that `file_path` is a directory and everything inside it will be gone? fs::remove_dir_all(&file_path)?; - fs::File::create(&file_path)?; + } else if file_path.is_file() { + fs::remove_file(&file_path)?; } check_for_comments(&file);