Skip to content
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

Incorrect content-length header in reply to HEAD request #730

Closed
mdickopp opened this issue Jan 30, 2022 · 5 comments · Fixed by #734
Closed

Incorrect content-length header in reply to HEAD request #730

mdickopp opened this issue Jan 30, 2022 · 5 comments · Fixed by #734
Assignees
Labels
A-axum C-bug Category: This is a bug.

Comments

@mdickopp
Copy link

Bug Report

Version

axum v0.4.4
axum-core v0.1.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

The content-length header in the reply to a simple HEAD request is set to 0 instead of the length of the reply body to the corresponding GET request.

Steps to reproduce

Run the axum examle program:

use axum::{routing::get, Router};

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

Send a HTTP/1.1 HEAD request for /. I used the the following command on the Linux command line:

printf 'HEAD / HTTP/1.1\r\n\r\n' | nc 127.0.0.1 3000

Observed behavior

The output contains the line:

content-length: 0

Expected behavior

The output contains the line:

content-length: 13

When a web server sends a content-length header in reply to a HEAD request, its value should be the same as would have been used in the reply to the same request using the GET method.

@davidpdrsn
Copy link
Member

Not quite sure whats going on here, but axum doesn't remove content-lengths from responses to HEAD requests:

use axum::{
    http::Request,
    middleware::{self, Next},
    response::{IntoResponse, Response, Headers},
    routing::get,
    Router,
};
use std::net::SocketAddr;

#[tokio::main]
async fn main() {
    let app = Router::new()
        .route("/", get(handler))
        .layer(middleware::from_fn(debug));

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap();
}

async fn handler() -> impl IntoResponse {
    let body = "Hello, World!";
    let headers = Headers([("content-length", body.len())]);
    (headers, body)
}

async fn debug<B>(request: Request<B>, next: Next<B>) -> Response {
    dbg!(&request.method());
    let res = next.run(request).await.into_response();
    dbg!(&res.headers());
    res
}

And then

❯ curl -v -I localhost:3000
*   Trying ::1...
* TCP_NODELAY set
* Connection failed
* connect to ::1 port 3000 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 3000 (#0)
> HEAD / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.64.1
> Accept: */*
>
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< content-type: text/plain; charset=utf-8
content-type: text/plain; charset=utf-8
< content-length: 13
content-length: 13
< date: Sun, 30 Jan 2022 12:15:36 GMT
date: Sun, 30 Jan 2022 12:15:36 GMT

<
* Connection #0 to host localhost left intact
* Closing connection 0

If you make a response that just returns &'static str (without any additional headers from your handler) its actually hyper that inserts the content-length. axum doesn't as seen here. Maybe we should though, but thats a separate question.

So it appears that hyper's automatic content-length stuff doesn't happen on HEAD requests which might be a bug.

It would be worth it to experiment with using straight hyper so without involving axum.

@davidpdrsn
Copy link
Member

@seanmonstar maybe you know whats going on here.

@mdickopp
Copy link
Author

I could not reproduce this issue with plain hyper, but while experimenting, I noticed that axum removes the response body for HEAD requests, and hyper does as well. Maybe it is related to this double stripping, i.e., when hyper adds the content-length header, the body has already been stripped?

When I replace this code with Poll::Ready(Ok(res)), everything seems to work fine. The header is content-length: 13, and the body is empty (presumably because it is stripped by hyper).

@davidpdrsn
Copy link
Member

If we change axum to set a content-length in the IntoResponse impls then it works correctly:

--- a/axum-core/src/response/mod.rs
+++ b/axum-core/src/response/mod.rs
@@ -272,11 +272,16 @@ impl IntoResponse for String {

 impl IntoResponse for Cow<'static, str> {
     fn into_response(self) -> Response {
+        let len = self.len();
         let mut res = Response::new(boxed(Full::from(self)));
         res.headers_mut().insert(
             header::CONTENT_TYPE,
             HeaderValue::from_static(mime::TEXT_PLAIN_UTF_8.as_ref()),
         );
+        res.headers_mut().insert(
+            header::CONTENT_LENGTH,
+            HeaderValue::from_str(&len.to_string()).unwrap(),
+        );
         res
     }
 }

That feels like the most correct solution to me. Then things work both with and without hyper.

@mdickopp
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-axum C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants