search

alexcrichton / curl-rust

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

Incorrect comment for CURL_HTTP_VERSION_3 #519

Closed jcamiel closed 10 months ago

jcamiel commented 10 months ago

To use HTTP 3 we can use the following code:

self.handle.http_version(HttpVersion::V3);

HttpVersion::V3 is defined as as https://github.com/alexcrichton/curl-rust/blob/ff6ad21cc1034826b2ab3f8be0653d8c446e1bdc/src/easy/handler.rs#L455-L465

And curl_sys::CURL_HTTP_VERSION_3 is defined here:

https://github.com/alexcrichton/curl-rust/blob/ff6ad21cc1034826b2ab3f8be0653d8c446e1bdc/curl-sys/lib.rs#L663-L665

The comment of this flag states that

Makes use of explicit HTTP/3 without fallback.

Whereas in curl.h the corresponding flag is defined

https://github.com/curl/curl/blob/9bca45dba81d77de06757ce13fc9193e09e010b5/include/curl/curl.h#L2272-L2274

/ Use HTTP/3, fallback to HTTP/2 or HTTP/1 if needed. For HTTPS only. For HTTP, this option makes libcurl return error. /

libcurl seems to fallback when using this flag whereas comments in crates curl imply that there is no fallback...

sagebind commented 10 months ago

Our doc comment was lifted straight from the libcurl man page, but it looks like it actually changed behaviors in version 7.88 when CURL_HTTP_VERSION_3ONLY was added: https://github.com/curl/curl/commit/a56d2b0b94d2f6583ac0fd7605e4e4d1644c2ba9. When we initially added this, CURL_HTTP_VERSION_3 didn't allow any fallback.

But we can update this now. Maybe worth noting in the doc comment that the behavior varies between version >=7.66,<7.88 and >=7.88, depending on which version of libcurl you are linked to.

jcamiel commented 10 months ago

I can propose a PR with an updated comment if that help,

jcamiel commented 10 months ago

PR here => https://github.com/alexcrichton/curl-rust/pull/521 (relevant curl PR is https://github.com/curl/curl/pull/10264)