Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

Inflate with window bits 47 doesn't correctly inflate #34

Open
stephentoub opened this issue Aug 19, 2021 · 4 comments
Open

Inflate with window bits 47 doesn't correctly inflate #34

stephentoub opened this issue Aug 19, 2021 · 4 comments

Comments

@stephentoub
Copy link

stephentoub commented Aug 19, 2021

https://github.com/dotnet/runtime is using this implementation of zlib.

When trying to switch its ZLibStream's decompression to specify window bits == 47 instead of window bits == 15:
stephentoub/runtime@a11d948
lots of tests start to fail when trying to roundtrip zlib data. These failures only occur on Windows where this implementation of zlib is being used; on Linux where the distro's zlib is being used, all tests pass successfully.

The failure appears to stem from the checksum computation at the end of the input, failing at
https://github.com/jtkukunas/zlib/blob/bf29715726fe22390b59b2cbf9c935179ef49aed/inflate.c#L1273-L1274

@stephentoub stephentoub changed the title Inflate with with window bits 47 doesn't correctly inflate Inflate with window bits 47 doesn't correctly inflate Aug 19, 2021
@jtkukunas
Copy link
Contributor

jtkukunas commented Aug 29, 2022

Thanks for reporting. This should be fixed in v1.2.12_jtk.2

Once the paperwork goes through, I'll open a PR on dotnet/runtime

@AraHaan
Copy link

AraHaan commented Jun 23, 2023

I been wondering what is the difference between this version of zlib and madler's version (other than for speed). I been manually p/invoking madler's zlib in all Windows, linux, and macos due to a few reasons:

  • lack of control of manually providing certain compression levels (due to argument out of range that cannot be used with the CompressionLevel enum, think lv 7 and lv 8, and the real lv 3~4 (could be more that I have yet to fully remember off the top of my head).
  • other reasons, I prefer the latest patch of zlib from madler's as well which could include much needed bugfixes before this version could possibly get them (or even rare security issues like using the insecure version snprintf for example which does not do length checking and so could be exploited).
  • I been running my code as an external nuget package (ZlibSharp) first before I even think about considering migrating the code to dotnet/runtime as a new library (System.IO.Compression.Zlib (and having the existing streams internally use it as it is entirely span based, I think it could increase performance in the System.IO.Compression streams not to mention shortens them significantly).
  • I consume the native zlib libraries as nuget packages that are carefully packed in a way that only .NET projects can consume them. However, I would be willing to consider having my code migrate to this version of zlib if this repository could start doing similar ways of packing it and publishes it to nuget.org so any .NET project could manually consume them (I usually pack x86, x64, ARM (optionally), ARM64 builds into the same package and name each package by the operating system's name, e.g. linux, win, osx which made it possible for my code to work on all architectures that .NET runs on for those operating systems.

@jtkukunas
Copy link
Contributor

@AraHaan If you look at the git history, you'll notice that each release is organized as a madler zlib snapshot and then a series of patches on top of that. When there's a new madler release, the patches get rebased on top of that. So it's very easy to see the changes that we've made. Git can also provide a nice diff for you.

  • We support the same levels/parameters as madler zlib. You can file an issue if something isn't working
  • Typically, we release more frequently than madler zlib. Historically, a lot of the madler changes sit in his dev branch for a long time waiting for a release. If any of those changes are important, we will typically do a release rebased on top of the dev branch .
  • I'll have to look into nuget. Is there something special with how it's packaged there? or is it just the integration that you want?

@AraHaan
Copy link

AraHaan commented Jun 29, 2023

@AraHaan If you look at the git history, you'll notice that each release is organized as a madler zlib snapshot and then a series of patches on top of that. When there's a new madler release, the patches get rebased on top of that. So it's very easy to see the changes that we've made. Git can also provide a nice diff for you.

* We support the same levels/parameters as madler zlib. You can file an issue if something isn't working

* Typically, we release more frequently than madler zlib. Historically, a lot of the madler changes sit in his dev branch for a long time waiting for a release. If any of those changes are important, we will typically do a release rebased on top of the dev branch .

* I'll have to look into nuget. Is there something special with how it's packaged there? or is it just the integration that you want?

Doing nuget releases of prebuilt binaries actually helps out with the consumption of the library in .NET.

Feel free to look at how I packaged the zlib libraries in the pull request I opened on madler/zlib to add vs2022 project files into it to get more details on how I managed to add nuget support on it (basically I have VS2022 build the library in x86, x64, ARM, and ARM64 and then run the packing code to pack them into a single windows specific nuget package, and similar for systems like mac and linux as well (as long as you know how to cross compile zlib for those architectures that is it becomes a non-issue).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants