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

Cargo.toml: allow users to specify tls implementation #13

Merged
merged 2 commits into from
Mar 24, 2024

Conversation

dspicher
Copy link
Contributor

@dspicher dspicher commented Feb 3, 2024

We allow the user to choose between the default
native-tls and rustls TLS implementations for the
reqwest dependency. The default behaviour remains
activating the previously used, default native-tls implementation, thus not breaking any clients.

Closes #11.

@dspicher
Copy link
Contributor Author

Hey @lpotthast would it be possible to get a review on these two open PRs?

@Tockra
Copy link
Contributor

Tockra commented Feb 21, 2024

@lpotthast I'm interest too

@lpotthast
Copy link
Owner

Tanks for the PR! Looking at the current feature declarations in https://github.com/seanmonstar/reqwest/blob/master/Cargo.toml, a few questions came to mind:

  1. The newest reqwest update introduced more default features. We should probably add them back manually. What are your thoughts? (see: seanmonstar/reqwest@v0.11.27...v0.12.0#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542)
  2. There does not seem to be a "rustls" feature (anymore), activating "rustls-tls" should be sufficient.
  3. If we provide "default-tls", we should probably name the second feature "rustls-tls", to match the schema and to closely match reqwests feature names.

reqwest now takes care itself of activating the
optional `serde_json` dependency under the already
activated `json` feature [1].

[1] https://github.com/seanmonstar/reqwest/blob/v0.12.0/CHANGELOG.md#v0120
@dspicher
Copy link
Contributor Author

Thanks for the review! I opened #19 as a preceding PR to update reqwest.

We allow the user to choose between the default
native-tls and rustls TLS implementations for the
reqwest dependency. The default behaviour remains
activating the previously used, default native-tls
implementation, thus not breaking any clients.
@dspicher
Copy link
Contributor Author

  1. ...

Done.

  1. ...

Done.

  1. ...

Done.

@lpotthast
Copy link
Owner

Looks good! Thanks for the quick changes.

@lpotthast lpotthast merged commit 907156a into lpotthast:main Mar 24, 2024
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.

Inability to choose TLS implementation
3 participants