Repository: Orange-OpenSource/hurl
File: packages/hurl/src/http/client.rs (Rust)

Code before:
                format!("{url}&")
            } else {
                format!("{url}?")
            };
            let s = self.url_encode_params(params);
            format!("{url}{s}")
        }
    }

    /// Sets HTTP method.
    fn set_method(&mut self, method: &Method) -> Result<(), HttpError> {
        self.handle.custom_request(method.to_string().as_str())?;
        Ok(())
    }

    /// Sets HTTP headers.
    fn set_headers(
        &mut self,
        headers: &HeaderVec,
        implicit_content_type: Option<&str>,
        options: &ClientOptions,
    ) -> Result<(), HttpError> {
        let mut list = List::new();

        for header in headers {
            if header.value.is_empty() {
                list.append(&format!("{};", header.name))?;
            } else {
                list.append(&format!("{}: {}", header.name, header.value))?;
            }
        }

        // If request has no `Content-Type` header, we set it if the content type has been set
        // implicitly on this request.
        if !headers.contains_key(CONTENT_TYPE) {
            if let Some(s) = implicit_content_type {
                list.append(&format!("{}: {s}", CONTENT_TYPE))?;
            } else {
                // We remove default `Content-Type` headers added by curl because we want to
                // explicitly manage this header.
                // For instance, with --data option, curl will send a `Content-type: application/x-www-form-urlencoded`
                // header. From <https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html>, we can delete
                // the headers added by libcurl by adding a header with no content.
                list.append(&format!("{}:", CONTENT_TYPE))?;
            }
        }

        // Workaround for libcurl issue <https://github.com/curl/curl/issues/11664>:
        // When Hurl explicitly sets `Expect:` to remove the header, libcurl will generate
        // `SignedHeaders` that include `expect` even though the header is not present, causing
        // some APIs to reject the request.

Review comment (refactor):
What we could do to improve the "testability" of this code => make a utility method on `HeaderVec` to transform a list of headers to a `libcurl` list of headers.

In client.rs:

```rust
impl HeaderVec {
    /// Converts this list of [`Header`] to a lib curl header list.
    fn to_curl_headers(&self) -> Result<List, HttpError> {
        todo!();
    }
}
```

This way we can call this method in `set_headers`:

```rust
    fn set_headers(
        &mut self,
        headers: &HeaderVec,
        implicit_content_type: Option<&str>,
        options: &ClientOptions,
    ) -> Result<(), HttpError> {
        let mut list = headers.to_curl_headers()?;
```

But more importantly, we can add some unit tests in clients.rs now:

```rust
#[cfg(test)]
mod tests {

    #[test]
    fn test_to_curl_headers() {
        let mut headers = HeaderVec::new();
        headers.push(Header::new("foo", "a"));
        headers.push(Header::new("bar", "b"));
        headers.push(Header::new("baz", ""));

        let list = headers.to_curl_headers().unwrap();
        // assert list with expected values
    }
}
```

For the moment, maybe we can keep the method `to_curl_headers` in `client.rs` file. We could have put it in `headers_helpers` also but, as the method exposes a libcurl types `List`, I prefer it stays here. Nothing fixed anyway, we could change it latter!




 

Code after:
                format!("{url}&")
            } else {
                format!("{url}?")
            };
            let s = self.url_encode_params(params);
            format!("{url}{s}")
        }
    }

    /// Sets HTTP method.
    fn set_method(&mut self, method: &Method) -> Result<(), HttpError> {
        self.handle.custom_request(method.to_string().as_str())?;
        Ok(())
    }

    /// Sets HTTP headers.
    fn set_headers(
        &mut self,
        headers: &HeaderVec,
        implicit_content_type: Option<&str>,
        options: &ClientOptions,
    ) -> Result<(), HttpError> {
        let mut list = headers.to_curl_headers()?;

        // If request has no `Content-Type` header, we set it if the content type has been set
        // implicitly on this request.
        if !headers.contains_key(CONTENT_TYPE) {
            if let Some(s) = implicit_content_type {
                list.append(&format!("{}: {s}", CONTENT_TYPE))?;
            } else {
                // We remove default `Content-Type` headers added by curl because we want to
                // explicitly manage this header.
                // For instance, with --data option, curl will send a `Content-type: application/x-www-form-urlencoded`
                // header. From <https://curl.se/libcurl/c/CURLOPT_HTTPHEADER.html>, we can delete
                // the headers added by libcurl by adding a header with no content.
                list.append(&format!("{}:", CONTENT_TYPE))?;
            }
        }

        // Workaround for libcurl issue <https://github.com/curl/curl/issues/11664>:
        // When Hurl explicitly sets `Expect:` to remove the header, libcurl will generate
        // `SignedHeaders` that include `expect` even though the header is not present, causing
        // some APIs to reject the request.
        // Therefore, we only remove this header when not in aws_sigv4 mode.
        if !headers.contains_key(EXPECT) && options.aws_sigv4.is_none() {
            // We remove default Expect headers added by curl because we want to explicitly manage
            // this header.
            list.append(&format!("{}:", EXPECT))?;
        }

        if !headers.contains_key(USER_AGENT) {