Frommi / miniz_oxide

Rust replacement for miniz
MIT License
168 stars 48 forks source link

Zlib succes while miniz_oxide fails #143

Closed catenacyber closed 5 months ago

catenacyber commented 6 months ago

Using code

use flate2::read::DeflateDecoder;
use std::io::prelude::*;

fn main() {
    let mut v1 = Vec::new();
    v1.extend_from_slice(&[0xf2, 0x48, 0xcd, 0xc9, 0xc9, 0x07, 0x00]);
    v1.extend_from_slice(&[0, 0, 0xFF, 0xFF]);
    let mut d = DeflateDecoder::new(&v1[..]);
    let mut s = String::new();
    d.read_to_string(&mut s).unwrap();
    println!("{}", s);
}

And Cargo.toml

[dependencies]
flate2 = { version = "1.0.28", default-features = true }

panicks

thread 'main' panicked at src/main.rs:10:30:
called `Result::unwrap()` on an `Err` value: Custom { kind: InvalidInput, error: "corrupt deflate stream" }

While Cargo.toml

[dependencies]
flate2 = { version = "1.0.28", features = ["zlib"], default-features = false }

succeeds in printing Hello

The data from this example comes from https://datatracker.ietf.org/doc/html/rfc7692#section-7.2.3.1

One fix may be in inflate_loop

Instead of

if (status as i32) < 0 {
            return Err(MZError::Data);
        }

add a check before like

        if (status == TINFLStatus::FailedCannotMakeProgress) && orig_in_len == 0 {
            return Err(MZError::Buf);
        }

Original report https://github.com/rust-lang/flate2-rs/issues/389

oyvindln commented 5 months ago

Hm, need to see what's done in C miniz and internally in zlib in this situation, and where the error is handled in those cases - that is it's called with the input having a complete block lacking the final flag but with the caller indicating there is no more data after that. Would have thought miniz would emulate what zlib did and as such we would have ported that behavior over and thus this would need to be fixed here but could be wrong.

Might take some time before I get around to this if no one else does.

catenacyber commented 5 months ago

Thanks. I do not trust myself to find the good solution (managed to find the above patch but do not trust it to solve the problem the right way zlib does)

oyvindln commented 5 months ago

I think the issue is that miniz_oxide erroneously ends up returning MZError::Data in this situation, rather than of MZError::Buf which is the equivalent error zlib and original miniz does (since this is technically an error after all), which flate2 will then ignore (while MZError::Data causes it to error out) and return the decoded data.

So, something like your suggested fix is probably the right approach yeah, will try to sort it when I have time.

catenacyber commented 5 months ago

Thanks. Would you know when will there be a patch release with this fix ?

oyvindln commented 5 months ago

I can do another one soon, was mainly holding off a bit in case @kornelski was working on any more PRs