PROTON-2137: Removing ssl init from ssl_server_options default constructor #210
Conversation
rabih-mourad
force-pushed
the
PROTON-2137-cpp-performance-regression-found-in-0.29.0
branch
from
ac8d755
to
c909684
Compare
rabih-mourad
changed the title
|
Weird, I just renamed the commit and travis-ci failed... |
cpp/src/connection_options.cpp
Outdated
| pn_ssl_domain_t* ssl_domain = ssl_server_options.value.impl_ ? ssl_server_options.value.impl_->pn_domain() : NULL; | ||
| if (pn_ssl_init(ssl, ssl_domain, NULL)) { |
There was a problem hiding this comment.
This change is wrong unfortunately.
the comparison with the client case is misleading. pn_ssl_init(ssl, NULL, NULL) will create an ssl connection with the default domain - however the default domain is a client domain so you can't do this if the connection is a server.
So you need to replace he NULL with something like pn_ssl_domain(PN_SSL_MODE_SERVER).
There was a problem hiding this comment.
It is a shame that this wasn't picked up by any tests - I thought we had some ssl server tests.
There was a problem hiding this comment.
@astitcher I replaced NULL by pn_ssl_domain(PN_SSL_MODE_SERVER) like you mentioned
|
@rabih-mourad Could you squash this into a single commit please - then I can rebase and merge it. |
rabih-mourad
force-pushed
the
PROTON-2137-cpp-performance-regression-found-in-0.29.0
branch
from
1d9f152
to
1236c4b
Compare
|
@astitcher squash done. Will we add a test in this pull request or we will do another one for it? |
|
I'm happy to commit this as is. I think testing c++ ssl servers is probably a different issue. |
|
That travis failure was unrelated to the PR. Something changed either on Travis or in Brew (brew.sh) so that openssl .dynlib is no longer found when I think I know what to do about it. |
No description provided.