Switch to flate2

[jonpas]

Closes #88.

This pull request switches back to flate2 implementation of (DE)FLATE, from libflate. flate2 is ahead of libflate, ref. #88 (comment).

Also adds default value in cp437.rs since newer rustc versions error on missing _ resolution. Was using older rustc by mistake.

Benchmarks

All benchmarks were ran on release build (cargo build --release) of a program using zip-rs - HEMTT. Code is using BufReader, BufWriter and copy (instead of buffer) - code below.

On Arch Linux 5.1.8. Only zipping in the program was timed (using std::time).

ACE3 Folder Size is is different due to different build process on Windows - irrelevant for this.

Linux

Project	Folder Size	libflate Speed	flate2 Speed	libflate Zip Size	flate2 Zip Size
TACU	12.2 MB	1.5 s	0.5 s	11.2 MB	11.2 MB
TACR	59.2 MB	7.5 s	2.5 s	50.6 MB	50.5 MB
ACE3	205 MB	15 s	7.2 s	143 MB	139 MB

Windows

Project	Folder Size	libflate Speed	flate2 Speed	libflate Zip Size	flate2 Zip Size
TACU	13 MB	2.5 s	0.5 s	11.2 MB	11.1 MB
TACR	59.2 MB	12 s	3 s	50.6 MB	50.4 MB
ACE3	174 MB	30 s	8.5 s	137 MB	135 MB

Benchmark Code

Excerpt from the program, where it is timed.

let now = std::time::Instant::now();

let zippath = Path::new(&zipsubpath);
let file = BufWriter::new(File::create(&zippath).unwrap());

let dir = walkdir::WalkDir::new(&release_dir);
let mut zip = zip::ZipWriter::new(file);
let options = zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Deflated);

// Zip all files and folders in all subdirectories
for entry in dir.into_iter().filter_map(|e| e.ok()) {
    let path = entry.path();
    let name = path.strip_prefix(Path::new(&release_dir)).unwrap();

    // Write file or directory explicitly
    // Some unzip tools unzip files with directory paths correctly, some do not!
    if path.is_file() {
        zip.start_file_from_path(name, options)?;

        let mut f = BufReader::new(File::open(path)?);

        // Copy directly, without any buffer, as we have no use for the intermediate data
        copy(&mut f, &mut zip)?;
    } else if name.as_os_str().len() != 0 {
        // Only if not root! Avoids path spec / warning
        // and mapname conversion failed error on unzip
        zip.add_directory_from_path(name, options)?;
    }
}
zip.finish()?;

println!("{}.{} seconds", now.elapsed().as_secs(), now.elapsed().subsec_nanos());

[mvdnes]

Thank you for your contribution. However, I do not think this is the way to go as some people building for wasm were not able to use flate2.

I was working on a branch for allowing people to choose their own library as needed, but have not found the time to finish it. See the plugin branch in this repository.

Some further questions:

• Why do you re-add the _ in cp437? In the current stable and nightly it works fine without it.
• If I where to merge this, the feature name should still be named deflate' which maps to flate2'. I do not want the external library name to leak unnecessarily.

[jonpas]

flate2 should work for WASM, ref. rust-lang/flate2-rs#161.

Why do you re-add the _ in cp437? In the current stable and nightly it works fine without it.

Because with rustc 1.32.0 (9fda7c223 2019-01-16):
Disregard, I had an older rustc version.

If I where to merge this, the feature name should still be named deflate' which maps to flate2'. I do not want the external library name to leak unnecessarily.

Done.

[Shnatsel]

I am now making an auditing pass over libflate, and its memory safety story is not great. Here are just the bugs I've found:
sile/libflate#16
sile/libflate#29
sile/libflate#31
sile/libflate#33
I'm not a security professional and may miss bugs, so even after I'm done you should not rely on it being correct. So I am very much in favor of this PR.

[Shnatsel]

If you want to make this a non-breaking change, rust backend should be the default so you would not require users to have a C-to-WASM compiler to build it.

AFAIK there is no reason to not use the Rust backend for flate2 in 2019. Last I looked at it, performance was on par with or slightly better than C, and it was in good shape safety-wise.

[Shnatsel]

Upon closer inspection, the safety of the Rust backend for flate2 is not great right now either:
Frommi/miniz_oxide#36
Frommi/miniz_oxide#47 (probably also applies to the C backend)

I'm working with maintainers of both crates on improving them, but I cannot guarantee that they'll be entirely safe after I'm done as I am NOT a security professional.

[jonpas]

Still looks better than libflate.

Also speed of flate2 is more than triple of libflate, unless that changes in libflate, flate2 is a clear winner.

[Shnatsel]

Latest release of miniz_oxide contains no unsafe code. It is now superior to libflate in every way.

[malbarbo]

@mvdnes What is the status of this PR? It would be great if it could be merged. Many crates, like tectonic and reqwest already depends on flate2, so using zip crate together with one the two make the application uses two distinct deflate implementations.

[mvdnes]

Since you made some compelling arguments, I have decided to merge this. Thanks for the contribution!

[malbarbo]

Thank you!