-
Notifications
You must be signed in to change notification settings - Fork 149
[FIXED] SSL callback #858
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
[FIXED] SSL callback #858
Conversation
Remove openssl dependency in nats.h.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #858 +/- ##
==========================================
+ Coverage 68.71% 70.35% +1.64%
==========================================
Files 39 47 +8
Lines 15207 15429 +222
Branches 3143 3167 +24
==========================================
+ Hits 10449 10855 +406
+ Misses 1700 1527 -173
+ Partials 3058 3047 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Except for a nit, LGTM. But I don't remember a case where we had to remove an API, but deprecating the existing one is not possible since it uses an SSL structure in the definition, so I guess we have no choice. Will let @levb decide if we go with this approach.
src/conn.c
Outdated
SSL_set_verify(ssl, SSL_VERIFY_PEER, nc->opts->sslCtx->callback != NULL ? nc->opts->sslCtx->callback : _collectSSLErr); | ||
{ | ||
if (nc->opts->sslCtx->callback != NULL) | ||
nc->opts->sslCtx->callback(ssl); |
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.
nit: I guess it should be (void*)ssl
to comply with the callback definition.
This is to comply with the callback definition.
# Conflicts: # src/conn.c # src/nats.h # src/natsp.h
@lev @kozlovic @ckasabula
|
@mtmk If you don't mind, I would like to close this one in favor of #908. We could add a callback equivalent of what you did here in the future, but since this PR would be a breaking change anyway, I think that the proposed PR #908 is a bit less. I have documented what the users would need to do for compilation to work with the new version, and already compiled applications would work well without any change. |
Remove openssl dependency in nats.h.