CompressionLayer: accept-encoding header parsed incorrectly

[mdickopp]

Bug Report

Version

tower-http v0.2.1

Platform

Debian Linux 12 (“bookworm”)
Linux feynman 5.15.0-3-amd64 #1 SMP Debian 5.15.15-1 (2022-01-18) x86_64 GNU/Linux
rustc 1.58.1 (db9d1b20b 2022-01-20)

Description

When using CompressionLayer, the accept-encoding header sent by the client is not parsed correctly (i.e., according to RFC 7231, sections 5.3.1 and 5.3.4). The following program demonstrates the issues (using axum v0.4.5):

use axum::{routing::get, Router};
use tower_http::compression::{predicate::SizeAbove, CompressionLayer};

#[tokio::main]
async fn main() {
    let app = Router::new()
        .route("/", get(|| async { "" }))
        .layer(CompressionLayer::new().compress_when(SizeAbove::new(0)));
    axum::Server::bind(&"0.0.0.0:3000".parse().unwrap())
        .serve(app.into_make_service())
        .await
        .unwrap();
}

Case 1: Uppercase encodings and qvalues are not parsed (fixed by #220)

Encodings and qvalues are case-insensitive, i.e. the server should understand them whether they are lowercase, uppercase, or a mixture of both.

Command	Expected Encoding	Observed Encoding
curl -I -H 'accept-encoding: GZIP' http://127.0.0.1:3000/	gzip	(none)
curl -I -H 'accept-encoding: gZiP' http://127.0.0.1:3000/	gzip	(none)
curl -I -H 'accept-encoding: gzip;q=0.5, br;Q=0.8' http://127.0.0.1:3000/	br	gzip

Case 2: Spaces before and after semicolon are not parsed (fixed by #220)

Space and horizontal tab characters are allowed before and after the semicolon separating the encoding from the qvalue.

Command	Expected Encoding	Observed Encoding
curl -I -H 'accept-encoding: gzip;q=0.5, br; q=0.8' http://127.0.0.1:3000/	br	gzip
curl -I -H 'accept-encoding: gzip;q=0.5, br ;q=0.8' http://127.0.0.1:3000/	br	gzip
curl -I -H 'accept-encoding: gzip;q=0.5, br ; q=0.8' http://127.0.0.1:3000/	br	gzip

Case 3: Invalid qvalues are accepted (fixed by #220)

Qvalues are expected to have exactly 1 digit before and not more than 3 digits after the decimal point.

Command	Expected Encoding	Observed Encoding	Explanation
curl -I -H 'accept-encoding: gzip;q=00.5' http://127.0.0.1:3000/	gzip	(none)	More than 1 digit before decimal point
curl -I -H 'accept-encoding: gzip;q=0.5000' http://127.0.0.1:3000/	gzip	(none)	More than 3 digits after decimal point
curl -I -H 'accept-encoding: gzip;q=.5' http://127.0.0.1:3000/	gzip	(none)	Missing digit before decimal point

Case 4: Request not rejected if client rejects identity encoding

If the client explicitly rejects the identity encoding or the wildcard encoding *, and accepts no encodings supported by the server, the request should be rejected.

Command	Expected HTTP Status Code	Observed HTTP Status Code
curl -I -H 'accept-encoding: identity;q=0' http://127.0.0.1:3000/	406 Not Acceptable	200 OK
curl -I -H 'accept-encoding: *;q=0' http://127.0.0.1:3000/	406 Not Acceptable	200 OK

[davidpdrsn]

Thanks for the detailed breakdown!

[mdickopp]

When I looked at the source code (to see if I can implement a fix), I found another case that is incorrect:

Case 5 (fixed by #220)

Command	Expected Encoding	Observed Encoding
curl -I -H 'accept-encoding: gzip;q=0.995, br;q=0.999' http://127.0.0.1:3000/	br	gzip

[mdickopp]

There's another case:

Case 6

Command	Expected Encoding	Observed Encoding
curl -I -H 'accept-encoding: *;q=0.8, gzip;q=0.5' http://127.0.0.1:3000/	no encoding (identity) or any encoding supported by the server other than gzip	gzip

[mdickopp]

I will work on a fix for this issue.

[davidpdrsn]

@mdickopp Thanks for your work on #220! Would you mind editing your comments here with the cases that aren't handled yet? Should make it easier for people to pick up and help.

[mdickopp]

Sure. Cases 4 and 6 are currently unresolved.

[bouk]

Another related issue:

grpc-web relies on the normal Content-Encoding headers for compression. There's a default predicate in CompressionLayer to avoid compressing application/grpc requests which unfortunately also filters out application/grpc-web requests because the predicate just checks if there's a prefix match:

tower-http/tower-http/src/compression/predicate.rs

Lines 220 to 233 in e8eb549

	impl Predicate for NotForContentType {
	fn should_compress<B>(&self, response: &http::Response<B>) -> bool
	where
	B: Body,
	{
	if let Some(except) = &self.exception {
	if content_type(response) == except.as_str() {
	return true;
	}
	}
	!content_type(response).starts_with(self.content_type.as_str())
	}
	}

This means I need to either remove this predicate or not have compression for grpc-web requests