Skip to content

Fix gzip compression/decompression over 4 GiB data size #1002

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

Merged
merged 2 commits into from
Jul 23, 2021

Conversation

yosh-matsuda
Copy link
Contributor

I found gzip compression feature does not work correctly for large data (over 4 GiB).
It seems that compression should repeat with the correct chunk size until the end of data.
I have checked this fixes the issue with the large-data post request.

The same issue can occur with gzip decompression and has been fixed, but I haven't tested it yet.
Could you review this PR?

@yhirose
Copy link
Owner

yhirose commented Jul 19, 2021

@yosh-matsuda, thanks for the pull request. Could you add at least one unit test to test/test.cc to validate your fix before I start reviewing?

@yosh-matsuda
Copy link
Contributor Author

@yhirose I added the gzip compression and decompression test for large random data. This test fails on the master branch as follows and I checked it will be fixed. Please note that the test requieres about 13GB memory.

Note: Google Test filter = GzipDecompressor.LargeRandomData
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from GzipDecompressor
[ RUN      ] GzipDecompressor.LargeRandomData
test.cc:2913: Failure
Value of: decompressed_data.size()
  Actual: 134217728
Expected: data_size
Which is: 4429185024
[  FAILED  ] GzipDecompressor.LargeRandomData (28526 ms)
[----------] 1 test from GzipDecompressor (28526 ms total)

@yhirose yhirose merged commit 879dd26 into yhirose:master Jul 23, 2021
@yhirose
Copy link
Owner

yhirose commented Jul 23, 2021

@yosh-matsuda, thanks for adding the unit test. I have reviewed the code and looks very good! I personally feel that the zlib usage example page should use this code instead. :)

The only problem is that the unit tests run on 'ubuntu-latest'in GitHub Actions is always killed (and probably on macOS as well)... I am not sure of the exact reason, but it seems like it's due to timeout. (On my iMac with 3.8 GHz Quad-Core Intel i5 and 24GB Memory, this test takes about 3 minutes. So the total time of tests is about 5 minutes.)
image

Fortunately, the unit tests run on Windows works with no problem. So I surround the test with #ifdef _WIN32 ... #endif at ccbddd8, so that we at least run this test on Windows.

Anyway, thanks for the excellent contribution!

ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
* Fix gzip compression/decompression over 4 GiB data size

* Add gzip test for large random data
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