From eb9a99fff3ea912fdcee5f05e1e83ba69a1c9e77 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Thu, 11 Nov 2021 11:36:26 +0200 Subject: [PATCH 1/3] Create util for clearing a path --- src/archive/tar.rs | 10 ++-------- src/archive/zip.rs | 14 ++++---------- src/utils/fs.rs | 23 +++++++++++++++++++++-- src/utils/mod.rs | 2 +- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index e0265fd..97159fc 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -31,17 +31,11 @@ pub fn unpack_archive( let mut file = file?; let file_path = output_folder.join(file.path()?); - if file_path.exists() && !utils::user_wants_to_overwrite(&file_path, question_policy)? { + if utils::clear_path(&file_path, question_policy)?.is_none() { + // User doesn't want to overwrite continue; } - if file_path.is_dir() { - // ToDo: Maybe we should emphasise that `file_path` is a directory and everything inside it will be gone? - fs::remove_dir_all(&file_path)?; - } else if file_path.is_file() { - fs::remove_file(&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 6a46b31..5d383b6 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -15,8 +15,8 @@ use crate::{ info, list::FileInArchive, utils::{ - cd_into_same_dir_as, concatenate_os_str_list, dir_is_empty, get_invalid_utf8_paths, strip_cur_dir, to_utf, - user_wants_to_overwrite, Bytes, + cd_into_same_dir_as, clear_path, concatenate_os_str_list, dir_is_empty, get_invalid_utf8_paths, strip_cur_dir, + to_utf, Bytes, }, QuestionPolicy, }; @@ -39,17 +39,11 @@ where }; let file_path = into.join(file_path); - if file_path.exists() && !user_wants_to_overwrite(&file_path, question_policy)? { + if clear_path(&file_path, question_policy)?.is_none() { + // User doesn't want to overwrite continue; } - if file_path.is_dir() { - // ToDo: Maybe we should emphasise that `file_path` is a directory and everything inside it will be gone? - fs::remove_dir_all(&file_path)?; - } else if file_path.is_file() { - fs::remove_file(&file_path)?; - } - check_for_comments(&file); match (&*file.name()).ends_with('/') { diff --git a/src/utils/fs.rs b/src/utils/fs.rs index 875619e..9d47ca9 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -9,8 +9,8 @@ use std::{ use fs_err as fs; -use super::to_utf; -use crate::{extension::Extension, info}; +use super::{to_utf, user_wants_to_overwrite}; +use crate::{extension::Extension, info, QuestionPolicy}; /// Checks if given path points to an empty directory. pub fn dir_is_empty(dir_path: &Path) -> bool { @@ -19,6 +19,25 @@ pub fn dir_is_empty(dir_path: &Path) -> bool { dir_path.read_dir().map(is_empty).unwrap_or_default() } +/// Remove `path` asking the user to overwrite if necessary +/// `Ok(Some(())` means the path is clear, +/// `Ok(None)` means the user doesn't want to overwrite +/// `Err(_)` is an error +// ToDo: Actual type to translate the above might be clearer? +pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result> { + if path.exists() && !user_wants_to_overwrite(path, question_policy)? { + return Ok(None); + } + + if path.is_dir() { + fs::remove_dir_all(path)?; + } else if path.is_file() { + fs::remove_file(path)?; + } + + Ok(Some(())) +} + /// Creates a directory at the path, if there is nothing there. pub fn create_dir_if_non_existent(path: &Path) -> crate::Result<()> { if !path.exists() { diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 7e948b7..4c09cd5 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -9,7 +9,7 @@ mod fs; mod question; pub use formatting::{concatenate_os_str_list, nice_directory_display, strip_cur_dir, to_utf, Bytes}; -pub use fs::{cd_into_same_dir_as, create_dir_if_non_existent, dir_is_empty, try_infer_extension}; +pub use fs::{cd_into_same_dir_as, clear_path, create_dir_if_non_existent, dir_is_empty, try_infer_extension}; pub use question::{ create_or_ask_overwrite, user_wants_to_continue_decompressing, user_wants_to_overwrite, QuestionPolicy, }; From d1d781dded5c17a26b45711266cb82fdfc096c35 Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Thu, 11 Nov 2021 11:36:40 +0200 Subject: [PATCH 2/3] Clear path before creating a dir --- src/commands.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/commands.rs b/src/commands.rs index 1559ec9..235792c 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -342,6 +342,10 @@ fn decompress_file( // // Any other Zip decompression done can take up the whole RAM and freeze ouch. if formats.len() == 1 && *formats[0].compression_formats == [Zip] { + if utils::clear_path(output_dir, question_policy)?.is_none() { + // User doesn't want to overwrite + return Ok(()); + } utils::create_dir_if_non_existent(output_dir)?; let zip_archive = zip::ZipArchive::new(reader)?; let _files = crate::archive::zip::unpack_archive(zip_archive, output_dir, question_policy)?; @@ -370,6 +374,10 @@ fn decompress_file( reader = chain_reader_decoder(format, reader)?; } + if utils::clear_path(&output_path, question_policy)?.is_none() { + // User doesn't want to overwrite + return Ok(()); + } utils::create_dir_if_non_existent(output_dir)?; let files_unpacked; From c33d896743137df3186b5a9096e7ee947db1087a Mon Sep 17 00:00:00 2001 From: Spyros Roum Date: Thu, 11 Nov 2021 15:58:51 +0200 Subject: [PATCH 3/3] Change `clear_path` to return `Result` So `Ok(true)` means the path is clear while `Ok(false)` means the user doesn't want to overwrite --- src/archive/tar.rs | 2 +- src/archive/zip.rs | 2 +- src/commands.rs | 4 ++-- src/utils/fs.rs | 16 ++++++++-------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 97159fc..a42108c 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -31,7 +31,7 @@ pub fn unpack_archive( let mut file = file?; let file_path = output_folder.join(file.path()?); - if utils::clear_path(&file_path, question_policy)?.is_none() { + if !utils::clear_path(&file_path, question_policy)? { // User doesn't want to overwrite continue; } diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 5d383b6..c71be89 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -39,7 +39,7 @@ where }; let file_path = into.join(file_path); - if clear_path(&file_path, question_policy)?.is_none() { + if !clear_path(&file_path, question_policy)? { // User doesn't want to overwrite continue; } diff --git a/src/commands.rs b/src/commands.rs index 235792c..5a2dcee 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -342,7 +342,7 @@ fn decompress_file( // // Any other Zip decompression done can take up the whole RAM and freeze ouch. if formats.len() == 1 && *formats[0].compression_formats == [Zip] { - if utils::clear_path(output_dir, question_policy)?.is_none() { + if !utils::clear_path(output_dir, question_policy)? { // User doesn't want to overwrite return Ok(()); } @@ -374,7 +374,7 @@ fn decompress_file( reader = chain_reader_decoder(format, reader)?; } - if utils::clear_path(&output_path, question_policy)?.is_none() { + if !utils::clear_path(&output_path, question_policy)? { // User doesn't want to overwrite return Ok(()); } diff --git a/src/utils/fs.rs b/src/utils/fs.rs index 9d47ca9..034c3aa 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -19,14 +19,14 @@ pub fn dir_is_empty(dir_path: &Path) -> bool { dir_path.read_dir().map(is_empty).unwrap_or_default() } -/// Remove `path` asking the user to overwrite if necessary -/// `Ok(Some(())` means the path is clear, -/// `Ok(None)` means the user doesn't want to overwrite -/// `Err(_)` is an error -// ToDo: Actual type to translate the above might be clearer? -pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result> { +/// Remove `path` asking the user to overwrite if necessary. +/// +/// * `Ok(true)` means the path is clear, +/// * `Ok(false)` means the user doesn't want to overwrite +/// * `Err(_)` is an error +pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result { if path.exists() && !user_wants_to_overwrite(path, question_policy)? { - return Ok(None); + return Ok(false); } if path.is_dir() { @@ -35,7 +35,7 @@ pub fn clear_path(path: &Path, question_policy: QuestionPolicy) -> crate::Result fs::remove_file(path)?; } - Ok(Some(())) + Ok(true) } /// Creates a directory at the path, if there is nothing there.