-
-
Notifications
You must be signed in to change notification settings - Fork 417
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
Explicit TLS configuration setup #1194
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall; this definitely bites people on a regular basis.
Seems like there's two identically named commits for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only skimmed it.
So one of the gotchas with the previous configs was that some ::default()
methods made rustls load TLS root certs from the system, which turned out to be very expensive (100ms+). Sometimes those even had not been used and where overwritten by custom config later on. Is this now better/worse/the-same with these changes?
I think this is now the same or better. No |
They're not identically named, one is for |
c7087d5
to
f031328
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's pretty great how much code this saves, while simultaneously simplifying the API.
They're not identically named
Oops, reading is hard.
f031328
to
9cba8b7
Compare
9cba8b7
to
365a426
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
Extracts the parts of #1150 that don't rely on rustls 0.20 (now released) and the rustls-native-certs update (not yet done). Arguably should have done this sooner to avoid the rebase, but let's get it done now?