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.

Sign up for GitHub

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

Jump to bottom

Opt into ringless behavior #2301

Merged
merged 8 commits into from
Jun 4, 2024
Merged

Opt into ringless behavior #2301

merged 8 commits into from
Jun 4, 2024

Conversation

asonix
Copy link
Contributor

@asonix asonix commented Jun 3, 2024

Add a feature rustls-tls-no-provider to build reqwest with rustls but without a default provider. This will deliberately panic if no global provider is set. The feature is "additive" with the rustls-tls feature in the sense that it will simply default to ring rather than panicking if both are enabled

This is accomplished by conditionally enabling ring features on dependencies through a new internal __rustls-ring feature, which is enabled in all cases where ring was previously used

@asonix
Copy link
Contributor Author

asonix commented Jun 3, 2024

I should have expected those test failures :p I'll work on that

@asonix
Copy link
Contributor Author

asonix commented Jun 3, 2024

Making all the tests pass for -no-provider ended up adding lines to basically every test :/ is that alright?

@seanmonstar
Copy link
Owner

Making all the tests pass for -no-provider ended up adding lines to basically every test :/ is that alright?

That doesn't sound great... The feature probably just shouldn't be tested in most cases.

@asonix
Copy link
Contributor Author

asonix commented Jun 3, 2024

Making all the tests pass for -no-provider ended up adding lines to basically every test :/ is that alright?

That doesn't sound great... The feature probably just shouldn't be tested in most cases.

Would you prefer disabling whole test files for -no-provider or is there another preferred way to do that?

@seanmonstar
Copy link
Owner

That's probably fine. We don't need to run all the tests with the feature, we probably just need that code path to run once...

@asonix
Copy link
Contributor Author

asonix commented Jun 3, 2024

I opted to ignore the test files & test modules, but let the doctests run

@asonix
Copy link
Contributor Author

asonix commented Jun 3, 2024

I could be more granular with the test filters, e.g. allow running if -no-provider is enabled as long as rustls-tls is also enabled

@asonix
Copy link
Contributor Author

asonix commented Jun 3, 2024

not sure what's up with windows Unexpected EOF during handshake, none of the rustls stuff is enabled for the test that failed

seanmonstar
seanmonstar approved these changes Jun 4, 2024
View reviewed changes
Copy link
Owner

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

The badssl tests are a little flaky, depending on a server we don't control. I restarted the jobs and they're all green now.

@seanmonstar seanmonstar merged commit a75b26a into seanmonstar:master Jun 4, 2024
36 checks passed
kodiakhq bot pushed a commit to pdylanross/fatigue that referenced this pull request Jun 18, 2024
@dependabot
chore: Bump reqwest from 0.12.4 to 0.12.5 (#297)
41210ef 
Bumps reqwest from 0.12.4 to 0.12.5.

Release notes
Sourced from reqwest's releases.

v0.12.5
What's Changed

Add http3 feature back, still requiring reqwest_unstable.
Add rustls-tls-no-provider Cargo feature to use rustls without a crypto provider.
Add blocking::ClientBuilder::dns_resolver() method to change DNS resolver in blocking client.
Fix Accept-Encoding header combinations.
Fix http3 resolving IPv6 addresses.
Internal: upgrade to rustls 0.23.

New Contributors

@​bnjmnjrk made their first contribution in seanmonstar/reqwest#2253
@​d-e-s-o made their first contribution in seanmonstar/reqwest#2257
@​mger1 made their first contribution in seanmonstar/reqwest#2260
@​nickguletskii made their first contribution in seanmonstar/reqwest#2268
@​lenstr made their first contribution in seanmonstar/reqwest#2271
@​torokati44 made their first contribution in seanmonstar/reqwest#2274
@​jayvdb made their first contribution in seanmonstar/reqwest#2284
@​adamaq01 made their first contribution in seanmonstar/reqwest#2288
@​asonix made their first contribution in seanmonstar/reqwest#2301

Full Changelog: seanmonstar/reqwest@v0.12.4...v0.12.5



Changelog
Sourced from reqwest's changelog.

v0.12.5

Add blocking::ClientBuilder::dns_resolver() method to change DNS resolver in blocking client.
Add http3 feature back, still requiring reqwest_unstable.
Add rustls-tls-no-provider Cargo feature to use rustls without a crypto provider.
Fix Accept-Encoding header combinations.
Fix http3 resolving IPv6 addresses.
Internal: upgrade to rustls 0.23.




Commits

29d4cff v0.12.5
a7880d6 tests: fix http3 tests
c32d877 tests: enable http3 content-length test
ce3b30e http3: send content-length if known
404df59 test: add http3 test server support
e5ce0b5 fix: don't enable hyper-rustls/http2 unless http2 is already enabled
ccb5e40 Don't compile hyper-tls with native-roots unless rustls-tls-native-roots is e...
c56fbae tests: remove a proxy parse error check
8cc7cd4 msrv: pin url
695bc04 fix: http3 resolving ipv6 addresses (#2305)
Additional commits viewable in compare view




Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
@rami3l rami3l mentioned this pull request Jun 22, 2024
Merged
Nutomic pushed a commit to Nutomic/reqwest that referenced this pull request Nov 7, 2024
@asonix @Nutomic
feat: add cargo feature to use rustls without ring (seanmonstar#2301)
f5b3469
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanmonstar seanmonstar seanmonstar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asonix @seanmonstar