sagebind / isahc

The practical HTTP client that is fun to use.
https://docs.rs/isahc
MIT License
705 stars 61 forks source link

Creating a request with `Body::empty()` sends a request with `Body(0)` #452

Open JoseSFOc opened 1 month ago

JoseSFOc commented 1 month ago
isahc = "1.7.2"
rustc 1.79.0 (129f3b996 2024-06-10)
isahc/1.7.2 (features:default,encoding-rs,http2,mime,static-curl,text-decoding) libcurl/8.8.0-DEV SecureTransport zlib/1.2.12 nghttp2/1.61.0

Hello!

My team is currently working on a system that has to interact with many different APIs, we migrated from reqwest for some issues with the library that were solved by isahc.

We've now encountered a problem where some APIs don't accept a request with an empty body as it would do with no body at all. We're creating the request using the following method:

fn set_body(
    builder: isahc::http::request::Builder,
    body: Option<serde_json::Value>,
) -> Result<isahc::Request<Body>, Error> {
    if let Some(body) = body {
        let body_str = body.as_str().unwrap_or_default();
        builder.body(body_str.into()).map_err(Error::IsahcHttpError)
    } else {
        builder.body(Body::empty()).map_err(Error::IsahcHttpError)
    }
}

As the documentation says that Body::empty() gives a request with the absence of a body -> Body::empty()

But when displaying the log information for the created request we obtain this (obscuring some sensible information):

Request { method: POST, uri: ***, version: HTTP/1.1, headers: {"x-api-key": "***", "x-nonce": "1720526846267", "api-signature": "***"}, body: Body(0) }

While reqwest generates the following:

RequestBuilder { method: POST, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("***")), port: None, path: "***", query: None, fragment: None }, headers: {"x-api-key": "***", "x-nonce": "1720525011723", "api-signature": "***"} }

As in the previous example, using Body::empty() is creating a body in the request with length zero, is there any other way to create a request with no body? Using Body::empty() should create a empty body instead of no body in the request?

Thanks!

JoseSFOc commented 1 month ago

After some tweaking and checking in the codebase, I manage to fix this issue for all the APIs that failed.

The change was made in isahc::HttpClient::create_easy_handle and it's as follows:

1093        // Setting the post fields to `&[]` fixes the issue with some APIs not accepting
1094        // POST request with undefined body
1095        if !has_body {
1096            easy.post_fields_copy(&[])?;
1097        }

This makes the body empty and not undefined, fixing the rejection of some APIs.

I made this change myself and ran all the tests available in the repo, succeeding in all of them. As I'm not allowed to create a new branch, ergo I cannot create a new PR for this fix. I'll wait for @sagebind to see this issue and offer a solution for this.