Add rustls support #390
Add rustls support #390
Conversation
src/error.rs
Outdated
| @@ -293,6 +293,8 @@ pub(crate) enum Kind { | |||
| UrlBadScheme, | |||
| #[cfg(feature = "default-tls")] | |||
| Tls(::native_tls::Error), | |||
| #[cfg(feature = "rustls-tls")] | |||
| Tls(::rustls::TLSError), | |||
There was a problem hiding this comment.
In order to support when both default-tls and rustls-tls are enabled, these should probably become two variants, NativeTls and Rustls.
There was a problem hiding this comment.
I don't know if it makes sense to choose TLS backend at runtime, but I'll give it a try.
There was a problem hiding this comment.
Yea, it may not be the best to support both at runtime, and perhaps it should be changed to only 1 or the other, but I believe it'd be a breaking change. native-tls provides a couple features that rustls never will (disabling security), so we cannot add a non-default feature that doesn't have the same methods.
There was a problem hiding this comment.
In order to avoid more complexity and overhead, I would rather support only one.
|
I don't know how to make |
This doesn't sound fantastic, but it at least works: if both features are enabled, the |
This seems to make the API very counterintuitive. Also if we want to use encrypted pkcs12, then we must |
|
@seanmonstar ping? |
src/async_impl/client.rs
Outdated
| @@ -129,20 +213,34 @@ impl ClientBuilder { | |||
| }) | |||
| } | |||
|
|
|||
| /// Use native TLS backend. | |||
| #[cfg(feature = "default-tls")] | |||
| pub fn use_default_tls(mut self, tls: Option<TlsConnectorBuilder>) -> ClientBuilder { | |||
There was a problem hiding this comment.
I'd remove the argument (and on use_rustls), to be conservative. That way, if they have breaking changes, we don't need to break as well just to upgrade.
src/async_impl/client.rs
Outdated
|
|
||
| /// Use rustls TLS backend. | ||
| #[cfg(feature = "rustls-tls")] | ||
| pub fn use_rustls_tls(mut self, tls: Option<rustls::ClientConfig>) -> ClientBuilder { |
There was a problem hiding this comment.
What do you think of the just use_rustls for the name?
There was a problem hiding this comment.
For default-tls, do we named it use_default?
There was a problem hiding this comment.
No, I sort of felt rustls_tls is repetitive, but I don't feel strongly.
close #378