Skip to content

Enable HTTP compression support on all platforms #1322

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 3 commits into from
Feb 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions Release/src/http/common/http_compression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#if !defined(CPPREST_EXCLUDE_COMPRESSION)
#define CPPREST_HTTP_COMPRESSION
#endif // !defined(CPPREST_EXCLUDE_COMPRESSION)
#endif // defined(TARGET_OS_MAC)
#elif defined(_WIN32) && (!defined(WINAPI_FAMILY) || WINAPI_PARTITION_DESKTOP)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#if !defined(CPPREST_EXCLUDE_WEBSOCKETS) && !defined(CPPREST_EXCLUDE_COMPRESSION)
#define CPPREST_HTTP_COMPRESSION
#endif // !defined(CPPREST_EXCLUDE_WEBSOCKETS) && !defined(CPPREST_EXCLUDE_COMPRESSION)
#endif

#if defined(CPPREST_HTTP_COMPRESSION)
#include <zlib.h>
Expand Down Expand Up @@ -277,7 +268,7 @@ class gzip_compressor : public zlib_compressor_base
class gzip_decompressor : public zlib_decompressor_base
{
public:
gzip_decompressor() : zlib_decompressor_base(16) // gzip auto-detect
gzip_decompressor() : zlib_decompressor_base(31) // 15 is MAX_WBITS in zconf.h; add 16 for gzip
{
}
};
Expand Down
14 changes: 13 additions & 1 deletion Release/tests/functional/http/client/compression_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,19 +376,31 @@ SUITE(compression_tests)
}
}

TEST_FIXTURE(uri_address, compress_and_decompress)
TEST_FIXTURE(uri_address, compress_and_decompress_fake)
{
compress_test(nullptr, nullptr); // FAKE
}

TEST_FIXTURE(uri_address, compress_and_decompress_gzip)
{
if (builtin::algorithm::supported(builtin::algorithm::GZIP))
{
compress_test(builtin::get_compress_factory(builtin::algorithm::GZIP),
builtin::get_decompress_factory(builtin::algorithm::GZIP));
}
}

TEST_FIXTURE(uri_address, compress_and_decompress_deflate)
{
if (builtin::algorithm::supported(builtin::algorithm::DEFLATE))
{
compress_test(builtin::get_compress_factory(builtin::algorithm::DEFLATE),
builtin::get_decompress_factory(builtin::algorithm::DEFLATE));
}
}

TEST_FIXTURE(uri_address, compress_and_decompress_brotli)
{
if (builtin::algorithm::supported(builtin::algorithm::BROTLI))
{
compress_test(builtin::get_compress_factory(builtin::algorithm::BROTLI),
Expand Down