Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/commands/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub fn compress_files(
// instead of the regular default that flate2 uses
let parz: ParCompress<gzp::deflate::Gzip, _> = 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")
Expand All @@ -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"))]
Expand All @@ -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));
Expand All @@ -95,15 +95,15 @@ 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())
}
Snappy => Box::new({
let parz: ParCompress<gzp::snap::Snap, _> = 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")
Expand Down
27 changes: 27 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down