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

411 error when making a body-less PUT request through Google's Https load balancer #2240

Closed
Thelonedevil opened this issue Apr 7, 2024 · 9 comments

Comments

@Thelonedevil
Copy link

Between 0.12.2 and 0.12.3 a change was made that meant sending a request with no body no longer sends the content length header. This seems to be because of #2207. This would be correct for GET requests I believe, but it seems to also be affecting PUT requests at least, and looking at the changes it is probably affecting ALL http methods.

It seems that Google's Https load balancer will reject any requests that do not have a chunked body (so unchunked or no body) that do not have the content length header (or that header not having a valid number, not sure which rule is actually being hit now). I do not know enough about the relevant standards to say confidently who is actually in the wrong here. However my understanding of this paragraph is that a user-agent (Reqwest in this case) SHOULD be sending the Content-Length header even if the value is 0, unless the METHOD (GET for example) should never have a body. This implies to me that reqwest is the one at fault for not being HTTP/1.1 compliant. It does seem a bit silly to have users manually set the content-length to 0 when not providing a body for a non-GET request.

I ran into this when adding reqwest to a project in a workspace after 0.12.3 released which then caused a different project in the workspace to suddenly stop working (as cargo will use the highest version of a dep for all projects in a workspace) as the Spotify API was suddenly and without warning returning the Google load balancer error page with status 411 Length Required. For now I have just downgraded explicitly to 0.12.2 across the workspace.

@paolobarbolini
Copy link
Contributor

paolobarbolini commented Apr 9, 2024

I've tried doing some comparisons and curl also doesn't seem to set the Content-Length either in some cases. Could the difference be with HTTP/2?

curl v8.7.1 test with no body

curl -X POST http://127.0.0.1:8080/ping
POST /ping HTTP/1.1
Host: 127.0.0.1:8080
User-Agent: curl/8.7.1
Accept: */*

curl v8.7.1 test with empty body

curl -X POST http://127.0.0.1:8080/ping --json ''
POST /ping HTTP/1.1
Host: 127.0.0.1:8080
User-Agent: curl/8.7.1
Content-Type: application/json
Accept: application/json
Content-Length: 0

reqwest v0.12.3 and v0.11.27 test with no body

#[tokio::main]
async fn main() {
    let client = reqwest::Client::new();

    client
        .post("http://127.0.0.1:8080/ping")
        .send()
        .await
        .unwrap();
}
POST /ping HTTP/1.1
accept: */*
host: 127.0.0.1:8080

reqwest v0.12.3 and v0.11.27 test with empty body

#[tokio::main]
async fn main() {
    let client = reqwest::Client::new();

    client
        .post("http://127.0.0.1:8080/ping")
        .body("")
        .send()
        .await
        .unwrap();
}
POST /ping HTTP/1.1
accept: */*
host: 127.0.0.1:8080

@cxw620
Copy link
Contributor

cxw620 commented Apr 11, 2024

Changing Inner::Reusable(Bytes) to be Reusable(Option<Bytes>) may help.

According to RFC9110:

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

@seanmonstar
Copy link
Owner

This will be fixed in the underlying hyper dependency: hyperium/hyper#3630

@seanmonstar
Copy link
Owner

The fix in hyper is part of v1.3.0, just release. So running a cargo update -p hyper should do it.

@seanmonstar
Copy link
Owner

On further thought, I'm going to revert this: hyperium/hyper#3633

While it is a SHOULD, and possibly I might add it directly to reqwest, it's worth noting that the spec (both RFC 7230 and 9112) explicitly point out that a request message with no transfer-encoding nor content-length header are defined as having no body. A server having issues with that is a misbehaving server.

@seanmonstar seanmonstar closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2024
@teohhanhui
Copy link

teohhanhui commented Apr 17, 2024

While it is a SHOULD, and possibly I might add it directly to reqwest, it's worth noting that the spec (both RFC 7230 and 9112) explicitly point out that a request message with no transfer-encoding nor content-length header are defined as having no body. A server having issues with that is a misbehaving server.

I'd argue that "SHOULD" means it should be the default. Otherwise it's not a well-behaved client, even if it's technically in compliance with the spec.

For curl's use they could disable that behaviour.

@lucacasonato
Copy link

lucacasonato commented Jun 6, 2024

I want to add another two cents here. The web fetch spec requires that implementations add content-length: 0 to empty POST and PUT requests (because web reality requires this for compatibility). See https://fetch.spec.whatwg.org/#http-network-or-cache-fetch and https://chromium.googlesource.com/chromium/src/+/7baede7674f02aaae25a4f471a672a1482cd3983/net/http/http_network_transaction.cc#1084. I would urge you to reconsider your stance here, particularly because since #2207 it is not possible anymore to override the content-length to be explicitly 0 for POST and PUT requests. We are doing this in Deno here, but this does not work in reqwest 0.12.3 and above anymore. This effectively blocks us from upgrading to reqwest 0.12.3 and above :(

EDIT: retracted, I was misreading our tests, and reqwest now adds content-length: 0 explicitly to empty PATCH requests, which it did not previously. The fetch spec does not suggest to do this. That specific change should probably reverted, and the content-length: 0 should only apply to POST and PUT?

@seanmonstar
Copy link
Owner

It should be possible to override by providing your own header. If not, that's a bug that should be fixed.

Also, PATCH behavior is different? If you have details of how it worked before and after, want to put them in a new issue?

@lucacasonato
Copy link

Yeah, you're right @seanmonstar. We were on 1.3.0 of hyper - after upgrading to 1.3.1 everything should work as expected again. I'll open another issue in case we run into anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants