Skip to content

properly implemented Accept-Encoding:gzip (and changed windows curl binary to haxx one with zlib support) #74

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

Closed
wants to merge 12 commits into from

Conversation

cielavenir
Copy link

Tested on Windows as well.

However, unfortunately it turned out our libcurl binary does not enable zlib. I needed to use https://curl.haxx.se/windows/ for testing.

@rdiankov
Copy link
Member

@cielavenir please update the PATCH version in CMakeLists.txt thanks

@cielavenir
Copy link
Author

Done. Sorry to be late.

The issue is that when we set CURL_OPTION_SETTER(_curl, CURLOPT_ACCEPT_ENCODING, "gzip, deflate"); using libcurl without libz support, transparent decompression does not work and gzipped data will be returned, which cannot be parsed by controllerclientcpp.

I needed to replace whole libcurl msvc binary. I cleaned up openssl include as well, which is not read by controllerclientcpp build system.

@cielavenir cielavenir changed the title properly implemented Accept-Encoding:gzip properly implemented Accept-Encoding:gzip (and changed windows curl binary to haxe one with zlib support) Nov 18, 2020
@cielavenir cielavenir changed the title properly implemented Accept-Encoding:gzip (and changed windows curl binary to haxe one with zlib support) properly implemented Accept-Encoding:gzip (and changed windows curl binary to haxx one with zlib support) Nov 18, 2020
@rdiankov
Copy link
Member

rdiankov commented May 1, 2022

necessary? (@ziyan )

@cielavenir
Copy link
Author

@ziyan currently controllerclientcpp does not use zlib

@ziyan
Copy link
Member

ziyan commented May 2, 2022

I guess controllerclientcpp doesn't request compressed response from the server currenlty. But is that a problem @cielavenir I am curious where this PR is needed

@cielavenir
Copy link
Author

Let's move to #108

@cielavenir cielavenir closed this May 2, 2022
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.

3 participants