diff --git a/src/commands/compress.rs b/src/commands/compress.rs index 4994fdb34..355c0df98 100644 --- a/src/commands/compress.rs +++ b/src/commands/compress.rs @@ -54,7 +54,7 @@ pub fn compress_files( // instead of the regular default that flate2 uses let parz: ParCompress = ParCompressBuilder::new() .compression_level( - level.map_or_else(Default::default, |l| gzp::Compression::new((l as u32).clamp(0, 9))), + level.map_or_else(Default::default, |l| gzp::Compression::new(l.clamp(0, 9) as u32)), ) .num_threads(logical_thread_count()) .expect("gpz: num_threads must be greater than 0") @@ -63,7 +63,7 @@ pub fn compress_files( }), Bzip => Box::new(bzip2::write::BzEncoder::new( encoder, - level.map_or_else(Default::default, |l| bzip2::Compression::new((l as u32).clamp(1, 9))), + level.map_or_else(Default::default, |l| bzip2::Compression::new(l.clamp(1, 9) as u32)), )), Bzip3 => { #[cfg(not(feature = "bzip3"))] @@ -78,14 +78,14 @@ pub fn compress_files( Lz4 => Box::new(lz4_flex::frame::FrameEncoder::new(encoder).auto_finish()), Lzma => { let options = level.map_or_else(Default::default, |l| { - lzma_rust2::LzmaOptions::with_preset((l as u32).clamp(0, 9)) + lzma_rust2::LzmaOptions::with_preset(l.clamp(0, 9) as u32) }); let writer = lzma_rust2::LzmaWriter::new_use_header(encoder, &options, None)?; Box::new(writer.auto_finish()) } Xz => { let mut options = level.map_or_else(Default::default, |l| { - lzma_rust2::XzOptions::with_preset((l as u32).clamp(0, 9)) + lzma_rust2::XzOptions::with_preset(l.clamp(0, 9) as u32) }); let dict_size = options.lzma_options.dict_size as u64; options.set_block_size(NonZeroU64::new(dict_size)); @@ -95,7 +95,7 @@ pub fn compress_files( } Lzip => { let options = level.map_or_else(Default::default, |l| { - lzma_rust2::LzipOptions::with_preset((l as u32).clamp(0, 9)) + lzma_rust2::LzipOptions::with_preset(l.clamp(0, 9) as u32) }); let writer = lzma_rust2::LzipWriter::new(encoder, options); Box::new(writer.auto_finish()) @@ -103,7 +103,7 @@ pub fn compress_files( Snappy => Box::new({ let parz: ParCompress = ParCompressBuilder::new() .compression_level(gzp::par::compress::Compression::new( - level.map_or_else(Default::default, |l| (l as u32).clamp(0, 9)), + level.map_or_else(Default::default, |l| l.clamp(0, 9) as u32), )) .num_threads(logical_thread_count()) .expect("gpz: num_threads must be greater than 0") diff --git a/tests/integration.rs b/tests/integration.rs index 792ad3d61..daab9a655 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -167,6 +167,33 @@ fn single_file( assert_same_directory(before, after, false); } +/// A negative `--level` must clamp to the minimum compression, not the maximum. +/// +/// Regression test: the level was cast to an unsigned integer before clamping, so +/// `-1i16 as u32` became 4294967295 and `clamp(0, 9)` returned 9, silently giving +/// maximum compression instead of minimum. +#[test] +fn negative_compression_level_is_not_maximum() { + let (_tempdir, dir) = testdir().unwrap(); + let before_file = &dir.join("file"); + // Highly compressible content, so the compression level actually changes the output size + fs::write(before_file, "ouch".repeat(64 * 1024)).unwrap(); + + let compress_with_level = |level: &str, name: &str| { + let archive = &dir.join(name); + ouch!("-A", "c", format!("--level={level}"), before_file, archive); + fs::metadata(archive).unwrap().len() + }; + + let negative = compress_with_level("-1", "negative.gz"); + let maximum = compress_with_level("9", "maximum.gz"); + + assert_ne!( + negative, maximum, + "--level=-1 must not produce the same output as --level=9 (maximum compression)" + ); +} + /// Compress and decompress a single file over stdin. #[proptest(cases = 200)] fn single_file_stdin(