-
Notifications
You must be signed in to change notification settings - Fork 143
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
Implement SSL_CTX_set_cert_cb #456
Conversation
@Lukasa Thanks so much for the comments earlier. I've since changed quite a bit now that I have a better understanding of whats going on. I was able to create a test that at least ensures the handshake is completing successfully, where as before I was never signaling to the connection that our async lookup was complete. Im sure there's still a lot more testing and error handling we can do but I wanted to pause here and wait for more feedback. I also want to call out that I'm concerned about this API returning a new |
Co-authored-by: Cory Benfield <[email protected]>
Co-authored-by: Cory Benfield <[email protected]>
@Lukasa I'm having problems getting CI to pass, it's failing to compile with a |
So you can use the docker-compose files in the repo to generate a simulacrum of the CI (for now, until we cut over to GHA). The steps are:
That latter command reveals the issue:
|
Thanks @Lukasa I think this is ready for another review. |
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, really nice job here @AndrewBarba! Thanks for persevering with the reviews, I think it left this patch in a great state. We're good to go.
This is a first pass at reviving #311 in order to fix #310
Credit to @TechnikEmpire, I did my best to address the comments in the original PR from @Lukasa
If the approach looks okay I'll move to testing