Reqwest doesn't protect agains incorrect usage of HTTP verbs

[Pzixel]

Most of existing HTTP client implementation have safeguards against incorrect usage of HTTP verbs. For example, following JS code:

fetch('https://jsonplaceholder.typicode.com/posts/1', {
    method: 'GET',
    body: JSON.stringify({a: 1}),
}).then((response) => {
    console.log(response);
});

Raises an error: TypeError: Request with GET/HEAD method cannot have body.

Most of clients in other languages do this as well.

It makes sense if following Rust code was also producing an error:

use url::Url;

#[tokio::main]
async fn main() {
    let url = Url::parse("https://jsonplaceholder.typicode.com/posts/1").unwrap();
    let client = reqwest::Client::new();
    let res = client.get(url).json(&serde_json::json!({"a": 1})).send().await.unwrap().text().await.unwrap();
    println!("{}", res);
}

The specification is pretty much against allowing such code to be executed:

9.3.1. GET A client SHOULD NOT generate content in a GET request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported. An origin server SHOULD NOT rely on private agreements to receive content, since participants in HTTP communication are often unaware of intermediaries along the request chain.

While it might be valuable to keep a backdoor for extremely rare cases where server violates HTTP semantic and requires such requests, I think it is worth making this an error by-default.

I can work on an implementation if overall idea is supported by crate maintainers.

[DenuxPlays]

I dont think this is a good idea.
It isn't forbidden just discouraged because it can lead to undefined behavior.

RFC9110

A client SHOULD NOT generate content in a GET request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported. An origin server SHOULD NOT rely on private agreements to receive content, since participants in HTTP communication are often unaware of intermediaries along the request chain

It SHOULD NOT.

Every http verb (even custom ones) can contain a body.

And it is acutally used like in opensearch or elastic where queries can very fast reach the "soft limit" of 255 bytes for the url when you are using query parameters.

It is not recommended because proxies may cache GET results and ignore the body or discard the body completely because GET is Idempotent by the definition.

But in cases like with elastic where you can guarantee that the service and proxies (mostly none) can handle it there is no reason to not allow it.

While it might be valuable to keep a backdoor for extremely rare cases where server violates HTTP semantic and requires such requests, I think it is worth making this an error by-default.

I would not consider elastic etc. extremely rare cases as they are most established and widely used search services.
Actually the opensearch crate in rust uses reqwest to make http calls.

TLDR:
It is not forbidden to use a body just not recommended.
Making this an error would actually be an error in the http implementation and would make this crate useless when trying to reach services like elastic- or opensearch.

[DenuxPlays]

Maybe a opt-in feature flag would be reasonable but IMO this is useless maintenance of code.

[Pzixel]

It SHOULD NOT.

So this is not ideal if client does something by default that spec says it shouldn't be doing, right?

I'm talking about defaults, having some backdoor for such cases can be allowed. But the default should listen to SHOULD NOT part of the spec

[DenuxPlays]

The spec clearly stats that it can be done
So maybe a Feature flag can be added to disallow it but I dont think that the default should be changed.

EVERY http method can contain a Body Even GET so to be able to do that I would Need to enable a Feature?
Does not Sound right

And if js disables this I think thats a reason we should not do the same.
I would Not take adivce from a Language that cannot Even de-/serialize all primitives

[Pzixel]

And if js disables this I think thats a reason we should not do the same.
I would Not take adivce from a Language that cannot Even de-/serialize all primitives

C#: HttpClient.Get doesn't accept Body parameter

Java: HttpClient.GET() doesn't accept Body parameter

Go: net.http.Get doesn't accept Body parameter

... and others I won't list now

All major libraries in all major languages protect against such usages. Therer are backdoor hatches that allow this for rare cases when it is required, but the overall consensus is that the lib API should protect against this by default.

[DenuxPlays]

My question

When You Need this do You Need an external Library to handle this?

Because Java does not have Feature Flags so it is impossible to send reqeusts to elastic with the std Client?

Again These are all Bugs because it is allowed and if you know the routes, middlemans and Service you absolutly can send a GET with a Body without a problem.

[DenuxPlays]

But the question it why implementing this restriction?

If a user makes a get and the Service doesnt Support it he will get an error code
No Need for compile time checking

And again the following Services REQUIRE sending a Body with GET

• elastic
• opensearch
• solr

[Pzixel]

Because Java does not have Feature Flags so it is impossible to send reqeusts to elastic with the std Client?

it is possible, you just need to be more explicit about this, i.e. use method("GET", body) instead of just GET(). Same with C# - they all allow doing this, but is is discouraged and you need to be explicit that this is exactly what you want.

I never proposed to ban such cases entirely - and I'm sorry if I made such an impression - I'm just talking about defaults. I had already several bugs in our existing codebase with erroneous get where should have been post, so when we found another one I decided to open an issue to see if anyone shares the sentiment that we can do something about it, while making sure that cases where it is required are still expressible.

[DenuxPlays]

IMO When you have something Like this You Need more Tests Not an error.

If it is still possible it is ok but I am still against it.
I don't want to be more explicit to do something that is allowed.

But I am not a maintainer
He will has to decide

[seanmonstar]

That's actually what I was most curious about: if people commonly make the mistake.

We could make it harder at compile time, by introducing a RequestBuilderWithoutBody or something that is returned for the simple .get() helpers, while still allowing .method() to give you the full one.

But we have to consider if the increase to surface area complexity of the API is worth it. I'm undecided.

[Pzixel]

@seanmonstar yes, I understand the tradeoffs, this is why I stated this in my original post: if you are overall in support of this I can try working on a more detailed design or even an impl, but if you aren't comfortable with the change I will know about it before I am heavily invested into this.