Fairing returns incorrect Content-Length for status 204

[koenbot]

Rocket Version

0.5.1

Operating System

Windows 10

Rust Toolchain Version

rustc 1.79.0 (129f3b996 2024-06-10)

What happened?

When adding a fairing to handle CORS requests, the Content-Length header is not updated. When you run the example server below and then check the returned headers, e.g. with Python

r = requests.options("http://127.0.0.1:8000/login")
r.headers

it gives

{'content-type': 'text/html; charset=utf-8', 'server': 'Rocket', 'x-frame-options': 'SAMEORIGIN', 'x-content-type-options': 'nosniff', 'permissions-policy': 'interest-cohort=()', 'access-control-allow-methods': 'POST, GET, PATCH, DELETE', 'access-control-allow-headers': '*', 'access-control-allow-origin': 'http://localhost:8080', 'access-control-allow-credentials': 'true', 'content-length': '435', 'date': 'Sun, 07 Jul 2024 19:25:33 GMT'}

I suspect the 'content-length': '435' comes from the length of the default 404 page in Rocket. However for a 204 No Content the RFC says

A server MUST NOT send a Content-Length header field in any response
with a status code of 1xx (Informational) or 204 (No Content)

There is no way to manually set this header in Rocket and trying to override it in the Fairing results in a runtime crash.

It's a really minor thing and browsers don't seem to care, but I found it strange that an OPTIONS request returned a header with Content-Length: 435 so I dug a bit deeper.

I could manually add an OPTIONS route for each existing endpoint to avoid the internal redirect to the default 404 page, but

1. the above does seem like a bug
2. maybe there's a better way?

Note, when trying to find these headers via the Rocket test Client I don't get all the actual headers (e.g. Date and Content-Length are not included), I'm probably doing something wrong.

#[test]
fn test_options() {
    let client = Client::tracked(rocket()).unwrap();
    let response = client.options("/login").dispatch();
    println!("{:?}", response.headers());
}

Test Case

#[macro_use] extern crate rocket;

use rocket::http::{Header, Method, Status};
use rocket::{Request, Response};
use rocket::fairing::{Fairing, Info, Kind};

pub struct CORS;

#[rocket::async_trait]
impl Fairing for CORS {
    fn info(&self) -> Info {
        Info {
            name: "Add CORS headers to responses",
            kind: Kind::Response
        }
    }

    async fn on_response<'r>(&self, request: &'r Request<'_>, response: &mut Response<'r>) {
        if request.method() == Method::Options {
            response.set_status(Status::NoContent);
            response.set_header(Header::new("Access-Control-Allow-Methods", "POST, GET, PATCH, DELETE"));
            response.set_header(Header::new("Access-Control-Allow-Headers", "*"));
        }
        response.set_header(Header::new("Access-Control-Allow-Origin", "http://localhost:8080"));
        response.set_header(Header::new("Access-Control-Allow-Credentials", "true"));
        // response.set_header(Header::new("Content-Length", "0")); // adding this causes a runtime crash
    }
}

#[post("/login")]
pub async fn login() -> String {
    "logged in".to_string()
}

#[launch]
fn rocket() -> _ {
    rocket::build()
        .mount("/", routes![login])
        .attach(CORS)
}

Log Output

{'content-type': 'text/html; charset=utf-8', 'server': 'Rocket', 'x-frame-options': 'SAMEORIGIN', 'x-content-type-options': 'nosniff', 'permissions-policy': 'interest-cohort=()', 'access-control-allow-methods': 'POST, GET, PATCH, DELETE', 'access-control-allow-headers': '*', 'access-control-allow-origin': 'http://localhost:8080', 'access-control-allow-credentials': 'true', 'content-length': '435', 'date': 'Sun, 07 Jul 2024 19:25:33 GMT'}

Additional Context

No response

System Checks

• My bug report relates to functionality.
• I have tested against the latest Rocket release or a recent git commit.
• I have tested against the latest stable rustc toolchain.
• I was unable to find this issue previously reported.

[the10thWiz]

The issue here is that the response still has a body - the client is just ignoring it. You can remove it by replacing it with an empty body: response.set_sized_body(0, std::io::Cursor::new(b""));. This will cause Rocket to emit a Content-Length: 0 header.

Second - I believe the Date and Content-Length headers are not getting set because the code to do so is outside the test module - it runs after all user code has already been run, right before Rocket actually sends the Response.

For Date, this can be considered a feature, since it means the Response doesn't depend on the current time (unless you add it in via user code). For Content-Length, this can be considered an implementation detail. Retrieving the Response body will give you the content length (even if the body is sent chunked).

[koenbot]

The OPTIONS response doesn't have a body though, r.content shows that it is empty (correctly). I think that the body content is still present internally when the content-length is calculated, and then the body is removed when Rocket actually sends the Response. Similar to how HEAD works, except that OPTIONS should be different.

That said, adding the set_sized_body() line of code does work, thanks! It's just strange to have to do this manually.

Edit - and also to be pedantic, there really shouldn't be a Content-Length header at all...

[the10thWiz]

It's either your client or Hyper that's removing the body. If you read the body in the test, it's still present.

[koenbot]

You make me doubt my client lol! Both Python and browsers show an empty body.

I just ran a Wireshark trace and it also gives an empty body. I see that the content-length is calculated in lifecycle.rs and the body isn't removed there. So I guess it must be Hyper that's removing the body? And keeping the Content-Length header and value even though it shouldn't?

[koenbot]

There's an open issue on this on Hyper: hyperium/hyper#2427 with a pending 3-year old PR. So yeah clearly not a Rocket thing.

[the10thWiz]

I would assume so. Hyper isn't really a web framework like Rocket - it doesn't have nearly as many checks and so forth.

I think this issue should be handled by Rocket - we can add a check for a status of No Content, and strip/warned about.

[SergioBenitez]

@koenbot

There is no way to manually set this header in Rocket and trying to override it in the Fairing results in a runtime crash.

Rocket should never crash. Can you describe what you're observing? I cannot trigger a crash.

I think this issue should be handled by Rocket - we can add a check for a status of No Content, and strip/warned about.

Yes, we can handle this ourselves, and I've written up a commit to do so. I'll push it once I test it a bit more.

I will say I did expect hyper to handle this, as it also (ostensibly) handles a few other similar cases. It's unfortunate that the issue has gone unfixed for years in hyper.