Skip to content

fix(compress): a negative --level gave maximum compression - #1046

Merged
marcospb19 merged 1 commit into
ouch-org:mainfrom
VXNCXNX:fix/negative-compression-level
Aug 16, 2026
Merged

fix(compress): a negative --level gave maximum compression#1046
marcospb19 merged 1 commit into
ouch-org:mainfrom
VXNCXNX:fix/negative-compression-level

Conversation

@VXNCXNX

@VXNCXNX VXNCXNX commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

What's broken

A negative --level silently gives maximum compression, which is the opposite of what was asked for.

$ ouch compress big.txt o.gz --level=0  -y ; stat -c%s o.gz
1012168
$ ouch compress big.txt o.gz --level=9  -y ; stat -c%s o.gz
3473
$ ouch compress big.txt o.gz --level=-1 -y ; stat -c%s o.gz
3473          <- asked for below-minimum, got the maximum
$ ouch compress big.txt o.gz --level=-32768 -y ; stat -c%s o.gz
3473

The fix

level is an Option<i16>, and it was cast before being clamped:

gzp::Compression::new((l as u32).clamp(0, 9))

-1i16 as u32 is 4294967295, so clamp(0, 9) returns 9. Clamping first and casting after gives the intended result:

gzp::Compression::new(l.clamp(0, 9) as u32)

Brotli in the same match already does exactly this, level.unwrap_or(default_level).clamp(0, 11) as u32, which is what the other arms were meant to look like.

Six arms changed: Gzip, Bzip, Lzma, Xz, Lzip and Snappy.

Zstd is deliberately untouched. It casts to i32, which preserves the sign for an i16, then clamps against min_c_level() and max_c_level(). Negative zstd levels are a real feature and still work:

$ ouch compress big.txt o.zst --level=-1 -y   # still a genuine negative level

Bzip3, Lz4 and the container formats ignore level entirely.

After:

level before after
0 1012168 1012168
9 3473 3473
-1 3473 1012168
-32768 3473 1012168

Verification

negative_compression_level_is_not_maximum in tests/integration.rs asserts --level=-1 and --level=9 do not produce the same output. With the change reverted it fails with both sizes equal.

The reason the existing coverage missed this: the single_file proptest right above it already drives -l, but its strategy is 0i16..12, so it never generates a negative.

cargo test passes, no snapshot changed and none was regenerated, and cargo fmt -- --check and cargo clippy --all-targets are clean.

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.
@marcospb19
marcospb19 merged commit 410337a into ouch-org:main Aug 16, 2026
18 checks passed
@marcospb19

Copy link
Copy Markdown
Member

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants