Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'h1_client_rustls' feature using 'async-tls' #53

Merged
merged 1 commit into from
Dec 4, 2020
Merged

Add 'h1_client_rustls' feature using 'async-tls' #53

merged 1 commit into from
Dec 4, 2020

Conversation

JEnoch
Copy link
Contributor

@JEnoch JEnoch commented Dec 4, 2020

Following discussions in http-rs/surf#40 and http-rs/surf#43, here my proposal to add a "h1_client_rustls" feature.

It makes http-client to depend on async-tls instead of async-native-tls. It seems to work fine according to the https_functionality() test I added in h1.rs.
The only inconvenient I see now is that the default feature ("h1_client") must be explicitly deactivated to avoid dependency to async-native-tls. I don't know if it's possible to make Cargo automatically deactivate "h1_client" if "h1_client_rustls" is active.

Copy link
Member

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, we'll have to follow-up with a PR to Surf with full docs.

@Fishrock123
Copy link
Member

The only inconvenient I see now is that the default feature ("h1_client") must be explicitly deactivated to avoid dependency to async-native-tls. I don't know if it's possible to make Cargo automatically deactivate "h1_client" if "h1_client_rustls" is active.

We should do that in the next major, although due to Surf re-exporting http-client we'll have to wait for that. I will create a follow-up issue.

@JEnoch
Copy link
Contributor Author

JEnoch commented Dec 6, 2020

Thanks for the merge @Fishrock123 !

Looks good, we'll have to follow-up with a PR to Surf with full docs.

I can do this PR tomorrow, just exposing a h1_client_rustls feature, if it's fine for you.
What do you mean with "full docs" ? Anything more than adding the feature description here ?

Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Feb 11, 2021
A successor to http-rs#53, this separates out the tls feature flags, which should be nicer.

Fixes: http-rs#54
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Feb 12, 2021
A successor to http-rs#53, this separates out the tls feature flags, which should be nicer.

Fixes: http-rs#54
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Feb 12, 2021
A successor to http-rs#53, this separates out the tls feature flags, which should be nicer.

Fixes: http-rs#54
Fishrock123 added a commit to Fishrock123/http-client that referenced this pull request Feb 12, 2021
A successor to http-rs#53, this separates out the tls feature flags, which should be nicer.

Fixes: http-rs#54
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Mar 1, 2021
This PR is a follow-up of http-rs/http-client#53 and addresses http-rs#40.
It adds a `h1-client-rustls` feature using `http-client/rustls` feature introduced by http-rs/http-client#53.

Co-Authored-By: Julien Enoch <julien.enoch@adlinktech.com>
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Mar 1, 2021
This PR is a follow-up of http-rs/http-client#53 and addresses http-rs#40.
It adds a `h1-client-rustls` feature using `http-client/rustls` feature introduced by http-rs/http-client#53.

Co-Authored-By: Julien Enoch <julien.enoch@adlinktech.com>
Fishrock123 added a commit to Fishrock123/surf that referenced this pull request Mar 1, 2021
This PR is a follow-up of http-rs/http-client#53 and addresses http-rs#40.
It adds a `h1-client-rustls` feature using `http-client/rustls` feature introduced by http-rs/http-client#53.

Co-Authored-By: Julien Enoch <julien.enoch@adlinktech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants