Fix quality parse error in Accept-Encoding HTTP header

[arthurlm]

PR Type

Bug Fix

PR Checklist

• Tests for the changes have been added / updated.
• Documentation comments have been added / updated.
• A changelog entry has been made for the appropriate packages.
• Format code with the latest stable rustfmt.
• (Team) Label with affected crates and semver status.

Overview

HTTP header Accept-Encoding was not correctly parsed.

Other changes:

• Better respect of client algorithms choices.
• Follow MDN spec about default qfactor value.
• It client need compressed response, now server send 406 HTTP error if there is no available algorithms.
• Make ContentEncoding::parse fail instead of returning default value.

Issue open to discussion

I have change CompressMiddleware trait requirements.

- B: MessageBody
+ B: MessageBody + From<String>

Indeed, I have not been able to use the Either trait correctly to answer available compress algorithm when we have to answer NotAcceptable status code.

So the only way I have been able to work with is adding From<String> requirement to B.

This make test common_combinations fail (see: dade818). So I have disable it ...
If anyone has a good idea to fix this, I will be happy to edit my PR.

Closes #2322

[robjtede]

nice change, thanks for the contribution

[robjtede]

I was able to sort out the problems you outlined in the description. Indeed adding the From<String> bound would have been unacceptable.

[istr]

While #2322 is a valid bug report, I would tend to say that some of the additional changes made with this PR are a regression over the previous version.

• Neither the HTTP RFC nor the (only informative) MDN "spec" state that the order of equally weighted options has any significance.
• Hence it is still up to the server to decide on the best option for equally weighted preferences. gzip,br,zstd, br,zstd,gzip, and zstd,gzip,br are completely equivalent (in each of the cases the client gives an implict q=1 for each option).
• Given this, the test cases

  actix-web/actix-web/tests/compression.rs

  Line 94 in 06c3513

  async fn negotiate_encoding_gzip() {

  actix-web/actix-web/tests/compression.rs

  Line 123 in 06c3513

  async fn negotiate_encoding_br() {

  actix-web/actix-web/tests/compression.rs

  Line 152 in 06c3513

  async fn negotiate_encoding_zstd() {

  are not only misleading. I would say they are plain wrong (ymmv).
• Instead of this, the server should have a deterministic preference. These tests should verify that the server determines the preference of equally weighted options, regardless of the order in which the client sent them.
• You could still argue that the server's preference is based on the order of equally weighted options. Still, like with TLS handshake it is desirable that the server will go for the best option at hand.
• It was a good idea to have a built-in server preference, however the implementation was questionable:

    /// Default Q-factor (quality) value.
    #[inline]
    pub fn quality(self) -> f64 {
        match self {
            ContentEncoding::Br => 1.1,
            ContentEncoding::Gzip => 1.0,
            ContentEncoding::Deflate => 0.9,
            ContentEncoding::Identity | ContentEncoding::Auto => 0.1,
            ContentEncoding::Zstd => 0.0,
        }
    }

@robjtede could we try to recover the old server-side preference behavior? Or add an option to pass the preferred order to middleware::Compress somehow?