search

alexcrichton / curl-rust

Rust bindings to libcurl
MIT License
1.01k stars 234 forks source link

Add an option to set the expect 100 timeout #376

Closed tailrecur closed 3 years ago

tailrecur commented 3 years ago

For larger request bodies, curl will use an Expect header to first validate the request headers before sending the actual body. However, if the server does not reply within CURLOPT_EXPECT_100_TIMEOUT_MS then the server will send the request anyways.

This behaviour can sometimes lead to higher total latency since in the best case, an additional server roundtrip is required and in the worst case, the request is delayed by CURLOPT_EXPECT_100_TIMEOUT_MS.

The best-case scenario is where the request is invalid and the server replies with a 417 Expectation Failed without having to wait for or process the request body at all.

This commit adds the ability to define a custom timeout or override it by setting it to zero.

We could also disable the default curl behaviour of sending the Expect header if the timeout is set to zero. IMO, curl-rust seems like a lower-level library and so I'm not sure if doing this here makes sense.

Related discussions: https://github.com/sagebind/isahc/issues/303 https://github.com/curl/curl/discussions/6635

/cc @sagebind

sagebind commented 3 years ago

We could also disable the default curl behaviour of sending the Expect header if the timeout is set to zero. IMO, curl-rust seems like a lower-level library and so I'm not sure if doing this here makes sense.

Probably not, changing the default curl behavior for this sort of thing is out of scope for curl-rust. We want to keep this crate as relatively neutral, safe bindings to libcurl.

alexcrichton commented 3 years ago

Thanks for the PR! Agreed that we probably shouldn't change the default for now, but seems good to add regardless!