hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.07k stars 1.55k forks source link

Requests with empty bodies don't set a content-length header #3654

Closed sfackler closed 1 month ago

sfackler commented 1 month ago

Version 1.3.1, 0.14.28

Platform

Darwin xxx 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000 arm64

Description When sending a request with an empty body (specifically a body that returns true from is_end_stream, hyper will not add a content-length header to the request. According to RFC 7230, this is allowed but discouraged:

A user agent SHOULD send a Content-Length in a request message when no Transfer-Encoding is sent and the request method defines a meaning for an enclosed payload body. For example, a Content-Length header field is normally sent in a POST request even when the value is 0 (indicating an empty payload body). A user agent SHOULD NOT send a Content-Length header field when the request message does not contain a payload body and the method semantics do not anticipate such a body.

Python made this change back in 2015: https://bugs.python.org/issue14721

I think the issue is here, where hyper doesn't set a body type when is_end_stream returns true: https://github.com/hyperium/hyper/blob/master/src/proto/h1/dispatch.rs#L317

Here's a failing test for reference:

test! {
    name: client_empty_post_content_length,

    server:
        expected: "\
            POST / HTTP/1.1\r\n\
            host: {addr}\r\n\
            content-length: 0\r\n\
            \r\n\
            ",
        reply: "\
            HTTP/1.1 200 OK\r\n\
            content-length: 0\r\n\
            \r\n\
            ",

    client:
        request: {
            method: POST,
            url: "http://{addr}/",
        },
        response:
            status: OK,
            headers: {},
            body: None,
}
---- client_empty_post_content_length stdout ----
thread 'tcp-server<client_empty_post_content_length>' panicked at tests/client.rs:1374:1:
failed to read request, partially read = "POST / HTTP/1.1\r\nhost: 127.0.0.1:56582\r\n\r\n", error: Resource temporarily unavailable (os error 35)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'client_empty_post_content_length' panicked at tests/client.rs:1374:1:
test: Hyper(hyper::Error(IncompleteMessage))
seanmonstar commented 1 month ago

Just to link up a couple of other references:

sfackler commented 1 month ago

Oh if this is intentional, then it seems fine. I just noticed it when writing some tests. It looks like RFC 9112 omits the SHOULD that was in 7230.