support rustls #97
support rustls #97
Conversation
|
Looks great; thanks for your contribution. I will release the next minor release with breaking changes and disable by default TLS or use rustls as a default backend for TLS. Sadly, hyper-rustls doesn't support hyper v1 yet: rustls/hyper-rustls#234 |
| .with_native_roots() | ||
| .https_or_http() | ||
| .enable_http2() | ||
| .build(); |
There was a problem hiding this comment.
It seems that we should use wrap_connector here to provide the configured connector above (with set_keepalive and possibly other options in the future).
There was a problem hiding this comment.
We'll also need to disable the enforcement of http on the connector here (that only happens in the native-tls connector as of now). I've opened a PR with all these changes here: #102.
| @@ -71,6 +71,13 @@ impl Default for Client { | |||
| connector | |||
| }); | |||
|
|
|||
| #[cfg(feature = "rustls")] | |||
There was a problem hiding this comment.
I think we should explicitly specify the priority of features (both in the code and readme) if both are enabled (because they can be enabled in different parts of the resulting program):
#[cfg(all(feature = "rustls", not(feature = "tls")))]
|
Now that hyper-rustls has been updated to support Hyper v1, is there anything I can do to help get this merged? |
|
(I think) I made all the requested changes in a branch in my fork (https://github.com/cole-h/clickhouse.rs/tree/rustls); here's the commit I made on top of this PR's changes: cole-h@612bd42 Feel free to apply that to this PR; I'd also be happy to open a PR from my branch that includes and replaces this PR, just let me know! |
|
Hey, any chance to merge this? Thanks. |
For certain environments, users have to avoid native-tls. Thus using rustls would be a good alternative.