vapor/vapor

Incorrect Content-Length header for HEAD requests

tib opened this issue · 2 comments

tib commented

Hello,

I strongly believe that I've discovered an issue related to HTTP HEAD requests.
Consider the following route handler:

app.on(.HEAD, "test") { req async throws -> Response in
    .init(
        status: .ok,
        version: req.version,
        headers: [
            "Content-Length": "42",
            "foo": "bar",
        ],
        body: .empty
    )
}

Now if you run a basic Vapor application using the snippet above and send the following curl request against the server:

curl -i --head http://localhost:8080/test  

According to the HTTP specification, you should get back 42 as a value for the Content-Length header, but since the body is empty, Vapor returns 0 as the header value.

This works as expected when I use other web frameworks, such as Hummingbird.

Anyway, thank you very much if you take a look at this issue, I'm happy to submit a PR if somebody gives me some guidenance where to fix this problem in the codebase. 🙏

Yep, this is a real issue. We currently rely on NIOHTTP1 to provide framing headers (including Content-Length), and as described by apple/swift-nio#2508, NIO currently can't handle this case properly. (Big thanks to @Lukasa for helping clarify this for me!)

What we can do is handle this in Vapor's part of the channel pipeline. I'll try to get a PR up for it shortly.

Belated correction: This is not an NIO issue at all. NIO actually does the right thing already here, and I misread where the problem was happening. It's actually an issue with how Vapor's Response is implemented. If you write your route like this instead, it works as intended:

app.on(.HEAD, "test") { req async in
    let res = Response(version: req.version, headers: ["foo": "bar"])
    res.headers.replaceOrAdd(name: .contentLength, value: "42")
    return res
}

(Note: The only critical change here is setting the content length separately; the other differences were just me removing redundancy.)

$ curl -i --head http://localhost:8080/test
HTTP/1.1 200 OK
foo: bar
content-length: 42
connection: keep-alive
date: Wed, 11 Oct 2023 17:09:23 GMT

There's nothing I can do about the awkwardness of this without breaking existing public API, unfortunately, so I have to close this issue as a wontfix 😕.