From fa2af20814850eebdfc0ef162e013f59b5d8873c Mon Sep 17 00:00:00 2001 From: Crypto-Spartan Date: Tue, 4 Jan 2022 22:45:24 +0000 Subject: [PATCH 1/4] fix warning for zip with additional formats --- src/commands.rs | 81 +++++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index fe96eb70a..fa09199c9 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -158,8 +158,14 @@ pub fn run(args: Opts, question_policy: QuestionPolicy) -> crate::Result<()> { } let compress_result = compress_files(files, formats, output_file, &output_path, question_policy); - // If any error occurred, delete incomplete file - if compress_result.is_err() { + if let Ok(true) = compress_result { + // this is only printed once, so it doesn't result in much text. On the other hand, + // having a final status message is important especially in an accessibility context + // as screen readers may not read a commands exit code, making it hard to reason + // about whether the command succeeded without such a message + info!(accessible, "Successfully compressed '{}'.", to_utf(output_path)); + } else { + // If Ok(false) or Err() occurred, delete incomplete file // Print an extra alert message pointing out that we left a possibly // CORRUPTED FILE at `output_path` if let Err(err) = fs::remove_file(&output_path) { @@ -168,12 +174,6 @@ pub fn run(args: Opts, question_policy: QuestionPolicy) -> crate::Result<()> { eprintln!(" Compression failed and we could not delete '{}'.", to_utf(&output_path),); eprintln!(" Error:{reset} {}{red}.{reset}\n", err, reset = *colors::RESET, red = *colors::RED); } - } else { - // this is only printed once, so it doesn't result in much text. On the other hand, - // having a final status message is important especially in an accessibility context - // as screen readers may not read a commands exit code, making it hard to reason - // about whether the command succeeded without such a message - info!(accessible, "Successfully compressed '{}'.", to_utf(output_path)); } compress_result?; @@ -285,7 +285,7 @@ fn compress_files( output_file: fs::File, output_dir: &Path, question_policy: QuestionPolicy, -) -> crate::Result<()> { +) -> crate::Result { // The next lines are for displaying the progress bar // If the input files contain a directory, then the total size will be underestimated let (total_input_size, precise) = files @@ -353,16 +353,18 @@ fn compress_files( writer.flush()?; } Zip => { - eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); - eprintln!( - "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ - \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ - \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" - ); + if formats.len() > 1 { + eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); + eprintln!( + "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ + \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ + \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" + ); - // give user the option to continue compressing after warning is shown - if !user_wants_to_continue(output_dir, question_policy, QuestionAction::Compression)? { - return Ok(()); + // give user the option to continue compressing after warning is shown + if !user_wants_to_continue(output_dir, question_policy, QuestionAction::Compression)? { + return Ok(false); + } } let mut vec_buffer = io::Cursor::new(vec![]); @@ -392,7 +394,7 @@ fn compress_files( } } - Ok(()) + Ok(true) } // Decompress a file @@ -515,16 +517,18 @@ fn decompress_file( }; } Zip => { - eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); - eprintln!( - "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ - \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ - \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" - ); + if formats.len() > 1 { + eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); + eprintln!( + "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ + \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ + \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" + ); - // give user the option to continue decompressing after warning is shown - if !user_wants_to_continue(input_file_path, question_policy, QuestionAction::Decompression)? { - return Ok(()); + // give user the option to continue decompressing after warning is shown + if !user_wants_to_continue(input_file_path, question_policy, QuestionAction::Decompression)? { + return Ok(()); + } } let mut vec = vec![]; @@ -612,16 +616,19 @@ fn list_archive_contents( let files: Box>> = match formats[0] { Tar => Box::new(crate::archive::tar::list_archive(tar::Archive::new(reader))), Zip => { - eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); - eprintln!( - "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ - \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ - \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" - ); - // give user the option to continue decompressing after warning is shown - if !user_wants_to_continue(archive_path, question_policy, QuestionAction::Decompression)? { - return Ok(()); + if formats.len() > 1 { + eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); + eprintln!( + "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ + \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ + \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" + ); + + // give user the option to continue decompressing after warning is shown + if !user_wants_to_continue(archive_path, question_policy, QuestionAction::Decompression)? { + return Ok(()); + } } let mut vec = vec![]; From e816ec1f8f53f3d51399ee211b0e96bebda05c5e Mon Sep 17 00:00:00 2001 From: Crypto-Spartan Date: Tue, 4 Jan 2022 23:01:32 +0000 Subject: [PATCH 2/4] use rustfmt --- src/commands.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/commands.rs b/src/commands.rs index fa09199c9..c1f71e88a 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -616,7 +616,6 @@ fn list_archive_contents( let files: Box>> = match formats[0] { Tar => Box::new(crate::archive::tar::list_archive(tar::Archive::new(reader))), Zip => { - if formats.len() > 1 { eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); eprintln!( From 923a0601eb085a232f150660877b26102f029112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Thu, 3 Feb 2022 16:02:48 -0300 Subject: [PATCH 3/4] Update compress_files docs --- src/commands.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/commands.rs b/src/commands.rs index c1f71e88a..b6f1ebbd2 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -279,6 +279,8 @@ pub fn run(args: Opts, question_policy: QuestionPolicy) -> crate::Result<()> { // files are the list of paths to be compressed: ["dir/file1.txt", "dir/file2.txt"] // formats contains each format necessary for compression, example: [Tar, Gz] (in compression order) // output_file is the resulting compressed file name, example: "compressed.tar.gz" +// +// Returns Ok(true) if compressed all files successfully, and Ok(false) if user opted to skip files fn compress_files( files: Vec, formats: Vec, From 26138e0f0600583ce524c9ae02a127375d9fdc99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Thu, 3 Feb 2022 18:00:02 -0300 Subject: [PATCH 4/4] Reuse zip warning message --- src/commands.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 1264efd79..1458d32ed 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -29,6 +29,13 @@ use crate::{ warning, Opts, QuestionAction, QuestionPolicy, Subcommand, }; +// Message used to advice the user that .zip archives have limitations that require it to load everything into memory at once +// and this can lead to out-of-memory scenarios for archives that are big enough. +const ZIP_IN_MEMORY_LIMITATION_WARNING: &str = + "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz) +\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory. +\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!"; + // Used in BufReader and BufWriter to perform less syscalls const BUFFER_CAPACITY: usize = 1024 * 64; @@ -300,7 +307,8 @@ fn compress_files( .iter() .map(|f| (f.metadata().expect("file exists").len(), f.is_file())) .fold((0, true), |(total_size, and_precise), (size, precise)| (total_size + size, and_precise & precise)); - //NOTE: canonicalize is here to avoid a weird bug: + + // NOTE: canonicalize is here to avoid a weird bug: // > If output_file_path is a nested path and it exists and the user overwrite it // >> output_file_path.exists() will always return false (somehow) // - canonicalize seems to fix this @@ -364,11 +372,7 @@ fn compress_files( Zip => { if formats.len() > 1 { eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); - eprintln!( - "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ - \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ - \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" - ); + eprintln!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING); // give user the option to continue compressing after warning is shown if !user_wants_to_continue(output_dir, question_policy, QuestionAction::Compression)? { @@ -529,11 +533,7 @@ fn decompress_file( Zip => { if formats.len() > 1 { eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); - eprintln!( - "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ - \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ - \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" - ); + eprintln!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING); // give user the option to continue decompressing after warning is shown if !user_wants_to_continue(input_file_path, question_policy, QuestionAction::Decompression)? { @@ -628,11 +628,7 @@ fn list_archive_contents( Zip => { if formats.len() > 1 { eprintln!("{orange}[WARNING]{reset}", orange = *colors::ORANGE, reset = *colors::RESET); - eprintln!( - "\tThere is a limitation for .zip archives with extra extensions. (e.g. .zip.gz)\ - \n\tThe design of .zip makes it impossible to compress via stream, so it must be done entirely in memory.\ - \n\tBy compressing .zip with extra compression formats, you can run out of RAM if the file is too large!" - ); + eprintln!("{}", ZIP_IN_MEMORY_LIMITATION_WARNING); // give user the option to continue decompressing after warning is shown if !user_wants_to_continue(archive_path, question_policy, QuestionAction::Decompression)? {