Frommi / miniz_oxide

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

Introduce reset policy to control InflateState::reset #91

Closed DoumanAsh closed 3 years ago

DoumanAsh commented 3 years ago

This change introduce customization to reset, allowing user to decide how to perform reset. Following approaches are introduced:

Also corrected reset tests to actually re-perform decompression to test that reset is ok.

Closes #89

P.s. honestly the whole API needs re-work, but meanwhile it would be useful to allow user reset as they see fit.

oyvindln commented 3 years ago

I'm thinking this approach is a bit nicer than the other PR #92. And yeah, API needs rework. Only thing I would add is a warning to the comment of the potential risk of not zeroing data when dealing with untrusted input. (Haven't found any bug that would end up giving out a read from old data through testing or fuzzing so it's likely not normally an issue, but it's not being verified statically as of now.)

Does this one work fine for your needs @Byron ?

Byron commented 3 years ago

I absolutely love it, that will work perfectly! Thanks so much!

Byron commented 3 years ago

For gitoxide, using miniz_oxide as fast and safe pure-Rust implementation will always be the way to go. However, after having seen this unscientific comparison it's clear that for compression performance, gitoxide eventually needs access to zlib-ng at least behind a feature toggle. They are now investigating ways to speed up a particular git requirement that involves finding the end of a compressed stream as quickly as possible, and I thought you folks might be interested in taking a look as well.

DoumanAsh commented 3 years ago

@oyvindln Updated, let me know if you need anything fix

Shnatsel commented 3 years ago

If you want maximum zlib performance on a bounded set of data, you should look into libdeflate. It has Rust bindings too: https://crates.io/crates/libdeflater