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

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

Closed
sfackler opened this issue May 1, 2024 · 2 comments
Closed

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

sfackler opened this issue May 1, 2024 · 2 comments
Labels
C-bug Category: bug. Something is wrong. This is bad!

Comments

@sfackler
Copy link
Contributor

sfackler commented May 1, 2024

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))
@sfackler sfackler added the C-bug Category: bug. Something is wrong. This is bad! label May 1, 2024
@seanmonstar
Copy link
Member

@sfackler
Copy link
Contributor Author

sfackler commented May 1, 2024

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.

@sfackler sfackler closed this as completed May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug. Something is wrong. This is bad!
Projects
None yet
Development

No branches or pull requests

2 participants