Content-Length is not set when using compression

[mladedav]

• I have looked for existing issues (including closed) about this

Bug Report

Version

0.7.9 and main

Platform

All

Crates

axum

Description

When the following code is run and a request is made against / with no accept-encoding header, we used to send a response with content-length but we're instead sending transfer-encoding: chunked now. If compression is applied (e.g. accept-encoding: gzip is set for the request to the example code here), we use transfer-encoding: chunked in the current and previous versions.

use std::future::ready;

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

#[tokio::main]
async fn main() {
    let app = Router::new()
        .route("/", get(|| ready("\n")).layer(CompressionLayer::new()));

    let listener = tokio::net::TcpListener::bind("127.0.0.1:3000")
        .await
        .unwrap();
    axum::serve(listener, app).await.unwrap();
}

This was flagged by crates-io. I would expect most clients to handle this change without any issues but there theoretically could be some that would reject any response without content-length set so I'm not sure if the upgrade would be completely safe. If we assume that all clients should be able to handle this, we can close this as not a bug.

The most likely change that had an impact on this behavior would be #2897. I did not look closer into the content-length setting code yet.

This change is most likely not just about compression but possibly any layer that wraps the body.

As a follow up, I think we should consider (in a separate issue if it doesn't turn out to be the easiest solution) whether we could set content-length for compressed responses where the whole body is already present.

[mladedav]

@SabrinaJewson pinging since you made the patch regarding content length and layers. If you have any idea how to make this work, that would be appreciated, if not, I'll try to dig into it when I get some time.

[jplatte]

I thought I had read that content-length referred to the un-encoded form when the response body is compressed, however I found this sentence on MDN:

When the Content-Encoding header is present, other metadata (e.g., Content-Length) refer to the encoded form of the data, not the original resource, unless explicitly stated.

Actually looking at previous tower-http issues, I found that CompressionService should already have been removing the Content-Length previously, because with the streaming compression implemented there, it's impossible to know up-front how big the content is going to be.

So maybe this actually fixed a bug for crates.io? It seems weird, do HTTP clients not raise an error when they receive fewer body bytes than the Content-Length indicates? More questions than answers here really. cc @Turbo87

[mladedav]

There never was a scenario when less bytes were sent than declared in the content length header.

When compression occurred, it used chunked encoding. As far as I can tell the only difference is when there is a compression layer that does nothing because of a missing accept encoding header in the request. In that case, we used to send the content length, but we do not anymore.

[jplatte]

Oh, I see. Interesting! That sounds like it may be fixable by adding a size hint to tower-http's compression body if it's in no-compression mode?

[SabrinaJewson]

I agree with jplatte, so I opened tower-rs/tower-http#531

[mladedav]

So the fix for anyone looking for the original behavior should be to just wait for new tower-http release and upgrade that along with axum.

I'll probably try this with crates.io when I get some time to see if there are any other layers there that have similar effect and also need the size hint.

[jplatte]

New version of tower-http is out (thanks to Sean). Does that fix the issue?

[Turbo87]

rust-lang/crates.io#9994 updated crates.io to the new tower-http release, and with that rust-lang/crates.io#9969 (the axum update) is now green too. big thank you to everyone involved! :)