fix: When only zopfli is available, decompression of deflate should not be possible#348
Conversation
…ot be possible Signed-off-by: Derzsi Dániel <daniel@tohka.us>
Pr0methean
left a comment
There was a problem hiding this comment.
Looks good in principle, but please use the dependency implicit feature to check for flate2, because more than one explicitly-defined feature flag sets it depending on the choice of back end.
| pub(crate) enum Decompressor<R: io::BufRead> { | ||
| Stored(R), | ||
| #[cfg(feature = "_deflate-any")] | ||
| #[cfg(feature = "deflate-flate2")] |
There was a problem hiding this comment.
Change this to feature = "flate2" throughout, so that deflate-zlib will also enable it.
There was a problem hiding this comment.
It should be fine. deflate-zlib implies the deflate-flate2 feature. Tested with only features = ["deflate-zlib"] set as well.
- git clone https://github.com/darktohka/zip-flate2-zopfli-feature-repro
- cd zlib-only
- cargo build
I don't believe any changes are required. When Can you please confirm? |
|
@darktohka This looks good. |
…ot be possible (zip-rs#348) * fix: When only zopfli is available, decompression of deflate should not be possible Signed-off-by: Derzsi Dániel <daniel@tohka.us> * fix: Rollback flate2 feature as flate2/rust_backend is enough --------- Signed-off-by: Derzsi Dániel <daniel@tohka.us>
Fixes #347.
Zopfli is a compression only algorithm. This means that it cannot be used to decompress DEFLATE streams. However, the code currently implies this, and tries to build flate2-related code paths because zopfli sets
_deflate_any.However, unsetting
_deflate_anyis not enough: zopfli can still compress DEFLATE streams even if it can't decompress them.So to make sure everything is still OK, allow decompression of DEFLATE only when
flate2is active, but allow compression both when onlyzopfliis active (in this case, shift the compression level by 9 so the same functionality takes place as when flate2 is enabled), and when bothzopfliandflate2are active (in this case, useflate2when compression level is less than 9, andzopfliwhen compression level is higher than 9, as the original intent).The
if cfg!(...)paradigm only works if both conditional blocks can compile regardless of the config options, so instead we must use#[cfg[...]]to compile out the parts that can't compile whenflate2is not available.As for the compression level ranges:
Tests have been modified: some tests require writing DEFLATE zip files as well. Those tests are disabled when DEFLATE can't be compressed, only uncompressed. Two doctests require writing DEFLATE zip files as well, so these doctests are now protected with the config guard.
The CI workflow has also been extended to build two more variants of the crate: one with only
flate2enabled and one with onlyzopflienabled.The CI/CD pipeline builds fine: https://github.com/darktohka/zip2/actions/runs/14960898954
To test this pull request:
https://github.com/darktohka/zip-flate2-zopfli-feature-reprocargo build==> builds fine