Skip to content

Conversation

@bdenhollander
Copy link
Contributor

@bdenhollander bdenhollander commented Jul 7, 2023

Streams less than 4096 bytes are sometimes reported as failed reads because stream_.avail_in is not reduced to 0. The next iteration of the loop finds prev_avail_in == strm_.avail_in and return false. ret = inflate(...) returns Z_STREAM_END on the first iteration of the loop indicating that inflate is finished. This fix prevents the second iteration of the loop from failing.

Relevant zlib.h source code:
https://github.com/madler/zlib/blob/04f42ceca40f73e2978b50e93806c2a18c1281fc/zlib.h#L462-L464

Streams less than 4096 bytes are sometimes reported as failed reads because stream_.avail_in is not reduced to 0. The next iteration of the loop finds `prev_avail_in == strm_.avail_in` and return false. `ret = inflate(...)` returns Z_STREAM_END on the first iteration of the loop indicating that inflate is finished. This fix prevents the second iteration of the loop from failing.
@yhirose
Copy link
Owner

yhirose commented Jul 7, 2023

@bdenhollander could you please include at least one unit test in test/test.cc as I typically request from contributors when they send me a pull request? Thank you!

- Add unit tests for raw deflate that illustrates the decompression failure when there are extra trailing bytes
@bdenhollander
Copy link
Contributor Author

@yhirose Writing the unit test helped narrow down the original source of the problem in my code that uses cpp-httplib, an incorrect Content-Length was including garbage after the end of the data I had compressed. Fetching the file with the extra bytes via browser fetch was successfully skipping the extra bytes but cpp-httplib couldn't handle them.

@yhirose
Copy link
Owner

yhirose commented Jul 7, 2023

but cpp-httplib couldn't handle them

Does your fix handle the above issue as well, right?

@bdenhollander
Copy link
Contributor Author

but cpp-httplib couldn't handle them

Does your fix handle the above issue as well, right?

Yes, that's correct. The second unit test recreates the scenario where there are extra bytes after the end of the Deflate compressed data and passes on this branch. The same test on the master version of httplib.h will fail.

@yhirose yhirose merged commit ee62523 into yhirose:master Jul 11, 2023
@yhirose
Copy link
Owner

yhirose commented Jul 11, 2023

@bdenhollander thanks for your fine contribution!

@bdenhollander bdenhollander deleted the patch-1 branch July 12, 2023 11:34
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

Successfully merging this pull request may close these issues.

2 participants