From 835d044d68e0d63dab3b2a80fdb256c0240eec86 Mon Sep 17 00:00:00 2001 From: VXNCXNX Date: Sat, 15 Aug 2026 14:54:31 +0000 Subject: [PATCH] fix(compress): clamp compression level before casting to u32 Negative compression levels were cast to u32 first, wrapping negative values to large unsigned integers. Clamping after the cast resulted in maximum compression instead of minimum. Fix: clamp on the i16 value before casting to u32. Add regression test to prevent silent wrap-around. --- src/commands/compress.rs | 12 ++++++------ tests/integration.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) 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(