Regression in decompressing bad data after 0.8.5 w/ small input

[milesgranger]

xref: milesgranger/cramjam#211

Seems a decompression error only arrives if the bad input data is over 4 characters long:

This does not raise an error:

    fn decompress_bad_data() {
        let mut decoder = DeflateDecoder::new(b"1".as_slice());
        let mut out = vec![];
        assert!(std::io::copy(&mut decoder, &mut out).is_err());
    }

But this will, 5 bytes at least for me, seems to spark the error, up to 4 bytes will not raise an error.

    fn decompress_bad_data() {
        let mut decoder = DeflateDecoder::new(b"12345".as_slice());
        let mut out = vec![];
        assert!(std::io::copy(&mut decoder, &mut out).is_err());
    }

[oyvindln]

This is the result of an intentional fix to make miniz_oxide act as it was supposed to. A 1 byte puts the decoder at a valid state at the beginning of a deflate stream, it's just incomplete and flate2 gives a Ok(_) result from incomplete streams with no end block rather than a Err. You get the same result if you use the other backends in flate2. If you give it a f byte instead it will return an error.

12345 is invalid in other ways so it errors out regardless whether it's incomplete or not. miniz_oxide was previously incorrectly returning another error code that resulted in flate2 erroring out instead for very short input if called without a finish flush mode in the first call (which is how flate2 calls it) hence the change in behavior.

There isn't currently a way to get the error state directly if using miniz_oxide via flate2 but there should maybe be one to distinguish valid and incomplete data as noted in this discussion.

[milesgranger]

It's difficult to tell for sure, but are you saying this is expected behavior and not an issue?

It seems absurd that trying to decompress b"foo" does return an error but b"bar" does not. Same with zlib b"f" vs b"b"; it's all garbage data and the decompressor should identify it as such.

Trying the same in Python zlib and Go yield expected error results, as does my ISA-L implementation (which was anyway dismissed as a flate2 backend rust-lang/flate2-rs#433) - albeit it probably has many other problems. 😅

[fintelia]

I think your complaint is actually with the flate2 crate rather than miniz_oxide. If you directly use the decompress_to_vec method from this crate you do get a FailedCannotMakeProgress error:

miniz_oxide::inflate::decompress_to_vec(b"bar").unwrap(); // Panics

[milesgranger]

That could be. Opened here b/c holding flate2 at the same version, and only updating miniz_oxide past 0.8.5 created this behavior.

[oyvindln]

Yeah 0.8.6 fixed a bug where it returned the wrong error code on smaller inputs that were incomplete streams, which resulted in flate2 returning an error on them instead of success as was the intention, on longer incomplete streams flate2 would still not error out on incomplete streams even with older miniz_oxide version.

Your issue is with how flate2 treats deflate streams that are decoding fine but are incomplete - it treats those as valid and discards the error for those streams returned by the various backends. As stated earlier, flate2 would need some additions if you want to be able to distinguish incomplete streams from valid ones.