Partial fix for #1148 by adding deflate content encoding support...

[kiplingw]

Partial fix for #1148 by adding deflate content encoding support...

[codecov-commenter]

Codecov Report

Patch coverage: 44.44% and project coverage change: -0.23% :warning:

Comparison is base (494e1bc) 78.16% compared to head (4ef74e8) 77.93%.

:exclamation: Current head 4ef74e8 differs from pull request most recent head b0b7d30. Consider uploading reports for the commit b0b7d30 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1151 +/- ## ========================================== - Coverage 78.16% 77.93% -0.23% ========================================== Files 53 53 Lines 6852 6872 +20 ========================================== Hits 5356 5356 - Misses 1496 1516 +20 ``` | [Files Changed](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio) | Coverage Δ | | |---|---|---| | [include/pistache/http\_headers.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlcnMuaA==) | `80.55% <ø> (ø)` | | | [src/common/http\_header.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlci5jYw==) | `91.66% <20.00%> (-0.98%)` | :arrow_down: | | [include/pistache/http\_header.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlci5o) | `87.87% <28.57%> (-2.75%)` | :arrow_down: | | [src/common/http.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwLmNj) | `74.39% <33.33%> (-0.27%)` | :arrow_down: | | [include/pistache/http.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwLmg=) | `86.11% <100.00%> (ø)` | | | [src/common/http\_headers.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlcnMuY2M=) | `76.69% <100.00%> (+0.17%)` | :arrow_up: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/pistacheio/pistache/pull/1151/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pistacheio)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Tachi107]

And I've just noticed that the Debian testing clang package changed something in a manner that breaks our CI again... Going to investigate that after merging this patch.

[kiplingw]

Some considerations about setCompression():

1. Having it as part of `ResponseWriter()` is more flexible than having a compression option in `Http::Endpoint::Options`, and since Pistache is not an httpd but a toolkit to build REST APIs, where each route can have its quirks, the ability to specify compression per-route (and per-request) is probably essential. For the record, nginx and Apache2 allow enabling compression as a global setting, much like the `Http::Endpoint::Options`. So well done with that!

I agree. There's a lot of situations where the user will want compression on some endpoints and not on others. For instance an endpoint that spits out lots of JSON will compress nicely whereas an endpoint that spits out compressed audio or video is just going to waste CPU cycles trying to compress what won't compress well. This gives them more control.

2. In my opinion, giving responsibility to the user to set the correct `Content-Encoding` header is undesirable. The [HTTP RFC](https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding) is pretty clear:
   > If one or more encodings have been applied to a representation, the sender that applied the encodings MUST generate a Content-Encoding header field that lists the content codings in the order in which they were applied. Note that the coding named "identity" is reserved for its special role in [Accept-Encoding](https://www.rfc-editor.org/rfc/rfc9110#field.accept-encoding) and thus SHOULD NOT be included.

   There's no scenario where letting the user choose the `Content-Encoding` header for compression makes sense, so it should be handled inside the `setCompression()` function to make mistakes harder.

I was on the fence about this, but since you pointed out the RFC there's really only one correct expected behaviour for the server if it returns a compressed encoding. Fixed in today's PR.

[dennisjenkins75]

[personal opinion] Please avoid adding a dependency on an external C++ zlib wrapper library.

[kiplingw]

[personal opinion] Please avoid adding a dependency on an external C++ zlib wrapper library.

We already removed that.

[Tachi107]

And one more thing: if you could squash your commits so that commitlint passes it'd be great

[kiplingw]

And one more thing: if you could squash your commits so that commitlint passes it'd be great

I always forget how to do this. Can you remind me the command? I know what it does, I just can't remember the CLI for this whole PR.

[Tachi107]

I always forget how to do this. Can you remind me the command? I know what it does, I just can't remember the CLI for this whole PR.

It's git rebase --interactive <first-commit-parent>, and in this case the parent of this feature branch is master, so you can do:

git rebase --interactive master

That'll open a text editor with something like this:

pick 6e4135d Partial fix for #1148 by adding gzip content encoding support...
pick 02e6255 contentEncoding renamed => contentEncoding_ to be consistent with class attribute naming convention...
pick 9174955 Fixed some comments in content encoding unit test...
pick 63cd6ed {meson.build, src/meson.build}: This PR is actually more correctly, as Andrea rightfully pointed out, implements support for deflate and not gzip, as similar as they may be... include/pistache/http.h: Added a setCompressionDeflateLevel() so user can change zlib's compression level... meson.build: Remove dependency on libhpptools-dev package which is not well maintained and use zlib directly... version.txt: Bumped versioning metadata...
pick 0501399 src/common/http.cc: setCompression() now sets Content-Encoding header automatically, consistent with RFC 9110... {src/common/http.cc,include/pistache/http.h}: Use in-class initializer for contentEncoding_ and contentEncodingDeflateLevel_ members... include/pistache/{http,http_header,http_headers}.h: Ran clang-format... tests/http_server_test.cc: Ran clang-format...
pick 57a7560 include/pistache/http.h: Clarify that Content-Encoding automatically set by setCompression()...

From there, you can change the "pick" words into "squash" (or "s" for short) of all the commits you want to squash, leaving pick only on the top one. Like this:

pick 6e4135d Partial fix for #1148 by adding gzip content encoding support...
squash 02e6255 contentEncoding renamed => contentEncoding_ to be consistent with class attribute naming convention...
squash 9174955 Fixed some comments in content encoding unit test...
squash 63cd6ed {meson.build, src/meson.build}: This PR is actually more correctly, as Andrea rightfully pointed out, implements support for deflate and not gzip, as similar as they may be... include/pistache/http.h: Added a setCompressionDeflateLevel() so user can change zlib's compression level... meson.build: Remove dependency on libhpptools-dev package which is not well maintained and use zlib directly... version.txt: Bumped versioning metadata...
squash 0501399 src/common/http.cc: setCompression() now sets Content-Encoding header automatically, consistent with RFC 9110... {src/common/http.cc,include/pistache/http.h}: Use in-class initializer for contentEncoding_ and contentEncodingDeflateLevel_ members... include/pistache/{http,http_header,http_headers}.h: Ran clang-format... tests/http_server_test.cc: Ran clang-format...
squash 57a7560 include/pistache/http.h: Clarify that Content-Encoding automatically set by setCompression()...

Now you can save and close the file. This will fire up another text editor from where you can edit the commit message of the single commit you'll leave on the branch. So you can write something like:

# This is a combination of 6 commits.
# This is the 1st commit message:

feat: add deflate content encoding support

This patch adds "Content-Encoding: deflate" support by using zlib. Users
can enable compression per-route and specify the compression level too.
Partial fix for #1148

# This is the commit message #2:

# This is the commit message #3:

# This is the commit message #4:

# This is the commit message #5:

# This is the commit message #6:

Be sure to remove all the unwanted non-comment lines! And after saving and closing the file, you'll rebase will be complete.

[kiplingw]

Ok done. Let me know how that looks. After saving the commit log I ran git pull and then git push. Did I do it correctly?

@dennisjenkins75, also please have a look over if you can and test it out.

[Tachi107]

Ok done. Let me know how that looks

I believe you forgot to force-push

[kiplingw]

Ok done. Let me know how that looks

I believe you forgot to force-push

I tried that:

$ git push --force
Everything up-to-date

[Tachi107]

I tried that:

$ git push --force
Everything up-to-date

Maybe you didn't actually perform the rebase? What do git show and git log print?

[Tachi107]

It's probably GitHub's fault. My other PR isn't showing new commits too, and today GitHub had issues with pull requests: https://www.githubstatus.com/incidents/l59z35rhzdky

[kiplingw]

Ok I think I got it to work this time, albeit the squash commit message could have been cleaner.

[kiplingw]

So I just realized that there is a problem. The setCompressionDeflateLevel() method in the ResponseWriter in http.h is only publicly available if PISTACHE_USE_CONTENT_ENCODING_DEFLATE is defined by the preprocessor. It is defined during compile time of the library, but not for the user.

Before anyone tries to fix this, we should probably talk about how. @Tachi107?

[Tachi107]

So I just realized that there is a problem. The setCompressionDeflateLevel() method in the ResponseWriter in http.h is only publicly available if PISTACHE_USE_CONTENT_ENCODING_DEFLATE is defined by the preprocessor. It is defined during compile time of the library, but not for the user.

Before anyone tries to fix this, we should probably talk about how. @Tachi107?

Yes, I thought about this problem when working on my patch too. I was thinking about a more flexible method, where the user passes in a custom function or something similar which mutates the compression level, but I have nothing concrete yet.

[kiplingw]

Yeah it needs some thinking. Or perhaps having some kind of generic way to set the compression level because the different schemes have different scales, such as a normalized 0 to 1 floating point range.