Enable HTTP compression support on all platforms #1322
Conversation
| #define CPPREST_HTTP_COMPRESSION | ||
| #endif // !defined(CPPREST_EXCLUDE_COMPRESSION) | ||
| #endif // defined(TARGET_OS_MAC) | ||
| #elif defined(_WIN32) && (!defined(WINAPI_FAMILY) || WINAPI_PARTITION_DESKTOP) |
There was a problem hiding this comment.
This condition always returns true, because if WINAPI_FAMILY is defined, then WINAPI_PARTITION_DESKTOP is just a constant.
Perhaps the author wanted to write WINAPI_FAMILY_PARTITION (WINAPI_PARTITION_DESKTOP), but I do not know why this condition is necessary since UWP applications should also work
| @@ -17,18 +17,9 @@ | |||
| // it. CPPREST_EXCLUDE_BROTLI is set if we want to explicitly disable Brotli compression support. | |||
| // CPPREST_EXCLUDE_WEBSOCKETS is a flag that now essentially means "no external dependencies". TODO: Rename | |||
|
|
|||
| #if __APPLE__ | |||
| #include "TargetConditionals.h" | |||
| #if defined(TARGET_OS_MAC) | |||
There was a problem hiding this comment.
If I googled correctly, TARGET_OS_MAC includes all Apple platforms https://stackoverflow.com/a/49560690
I don't know why we need this condition either
|
Related to PR #964 (just found it) |
|
On Ubuntu 16: |
KindDragon
force-pushed
the
patch-1
branch
6 times, most recently
from
fd62468 to
5c4ff82
Compare
KindDragon
changed the title
KindDragon
changed the title
azure-pipelines.yml
Outdated
| mkdir build.debug | ||
| cd build.debug | ||
| /usr/local/bin/cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug .. | ||
| /usr/local/bin/cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCPPREST_EXCLUDE_COMPRESSION=ON .. |
There was a problem hiding this comment.
Is this not a serious breaking change?
|
To clarify: the feature is turned off because tests fail; I don't believe turning the feature on in code, and then disabling all testing of it, is an acceptable change. If you can debug into it and fix it so that tests pass for the platforms on which you're enabling the feature, then of course we'd be happy to merge that, but I don't think I can merge this in this form. |
To can set CPPREST_EXCLUDE_COMPRESSION=ON to disable compression
…er to detect which one failed
|
Hi. I installed Ubuntu 16 and checked, this issue only happens with zlib 1.2.8. zlib 1.2.9 and better works well. Test uri_address.compress_and_decompress_gzip failing only with tuples |
|
Found on the web similar issue https://npm.community/t/zlib-invalid-distance-too-far-back/5577 |
|
Then I think we need some form of detection for those to turn the feature off for those versions; we can't merge something that causes tests to fail in a supported configuration. |
|
I spent a couple of hours, but found out why it didn't work with 1.2.8 |
|
Awesome, thanks for your contribution! |
BillyONeal
changed the title
You can set CPPREST_EXCLUDE_COMPRESSION=ON to disable compression
Fix #1165