Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behaviour between miniz and zlib in inflate #80

Open
drglove opened this issue Oct 23, 2017 · 5 comments
Open

Inconsistent behaviour between miniz and zlib in inflate #80

drglove opened this issue Oct 23, 2017 · 5 comments

Comments

@drglove
Copy link

drglove commented Oct 23, 2017

If inflate returns Z_OK or Z_BUF_ERROR, and avail_out is not zero, then avail_in must be guaranteed to be zero. I have some data I compressed with deflate using miniz that, when trying to inflate it, does not follow this. This guarantee was confirmed by Mark Adler himself in this Stack Overflow post.

I have enabled the following preprocessor defines in miniz.h:

  • MINIZ_NO_STDIO
  • MINIZ_NO_TIME
  • MINIZ_NO_ARCHIVE_APIS
  • MINIZ_NO_ARCHIVE_WRITING_APIS

My inflate and deflate loop are copied directly from the zpipe.c example in zlib, with the only difference being that I use a 512KB input buffer for reading from disk, and a 2MB output buffer for inflated data to be written to disk. This includes using Z_NO_FLUSH. I've included my source for testing and the deflated data that I am trying to inflate. Note that zlib inflates this fine.

zlib-test.zip

It seems to me that in mz_inflate, if m_dict_avail on the stream is non-zero, then avail_in is not being properly updated, or avail_out is being incorrectly updated. This might be a naive picture though. It is strange though that avail_out is updated there, but avail_in is not. The result is that one of the calls to inflate near the stream end, avail_out is non-zero, and avail_in is also non-zero.

Changing the termination condition of the inner do-while loop from avail_out == 0 (the one recommended by zlib) to avail_out == 0 || avail_in > 0 successfully decompresses the stream. It appears that calling inflate again after the stream has gotten into such a state is safe, at least with my data stream.

@randy408
Copy link
Contributor

randy408 commented Dec 29, 2020

I have encountered a related issue, miniz can enter a state where inflate() keeps returning Z_OK while avail_out is zero, if a loop is set up to check for Z_OK before anything else it can cause an infinite loop: https://github.com/randy408/libspng/blob/9c35dc3ac180f0c486b1792c0f424718c3d07cd6/spng/spng.c#L787-L793

Although the official zlib documentation does say inflate() can set avail_out to zero while returning Z_OK, zlib will also return Z_BUF_ERROR if avail_out is not updated by the user, the latter is what miniz doesn't do.

This was worked around by checking for unexpected return values up front before checking the value of avail_out/avail_in, ignoring Z_BUF_ERROR: randy408/libspng@c1f447e

@wdlkmpx
Copy link

wdlkmpx commented May 13, 2021

Testing miniz with unshield I see some tests fail because of inconsistencies between miniz and zlib inflate
So it's not exactly a drop-in replacement ...

@wdlkmpx
Copy link

wdlkmpx commented May 13, 2021

Guys you know what's going on and how to fix it, could you produce a patch to fix this miniz bug?
It's not a zlib inflate drop-in replacement until this bug is fixed

@drglove
Copy link
Author

drglove commented May 13, 2021

Guys you know what's going on and how to fix it

I do not know how to fix this within miniz, only how to workaround it as a client of miniz (as described in my initial post).

@wdlkmpx
Copy link

wdlkmpx commented May 14, 2021

Thanks @drglove for providing zlib-test.zip

I created an automated test procedure for linux (and unix) systems that includes miniz.c/h.
zlib-miniz__inflate-test-linux.zip

Just run ztest.sh.. binaries are compiled, logs are generated, make clean to remove generated files.

Challenge: fix inflate() in miniz.c without editing zlib-test.c

# ./ztest.sh 
rm -f inflate.blob zlib miniz *.log *.diff
gcc -g -Wall -o zlib  zlib-test.c -DUSE_ZLIB -lz
gcc -g -Wall -o miniz zlib-test.c miniz.c

* zlib
inflate.blob: OK

* miniz
./ztest.sh: line 7: 14644 Aborted                 ./${i} 2> ${i}.log
inflate.blob: FAILED
sha256sum: WARNING: 1 of 1 computed checksums did NOT match

log files:

  • zlib.log
  • miniz.log
  • zzz_test.diff (red = zlib, green = miniz)

miniz: as we can see, at some point both avail_in and avail_out are non-zero

--- zlib.log	2021-05-14 12:06:28.000000000 +0800
+++ miniz.log	2021-05-14 12:06:29.000000000 +0800
@@ -44,17 +44,17 @@
 total_in: 4718592
 total_out: 15218849
 [10]
-avail_in: 10636
+avail_in: 5913
 avail_out: 0
-total_in: 5232244
+total_in: 5236967
 total_out: 17316001
 [11]
-avail_in: 0
-avail_out: 2053873
-total_in: 5242880
-total_out: 17359280
+avail_in: 5913
+avail_out: 2078881
+total_in: 5236967
+total_out: 17334272
 [12]
 avail_in: 0
-avail_out: 947632
-total_in: 5527204
-total_out: 18508800
+avail_out: 947639
+total_in: 5521291
+total_out: 18483785

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

No branches or pull requests

3 participants