Add tls authentication plugin for cpp client #200
Conversation
merlimat
added
c++
type/feature
|
@saandrews @rdhabalia Please take a look |
| ParamMap paramMap; | ||
| if(!authParamsString.empty()) { | ||
| std::vector<std::string> params; | ||
| boost::algorithm::split(params, authParamsString, boost::is_any_of(",")); |
There was a problem hiding this comment.
Do we have to worry about escaped , or : characters?
There was a problem hiding this comment.
It is unlikely that authentication parameter's keys and values contain "," and ":" now.
Actually, Java's AuthenticationFactory implementation doesn't worry about it.
https://github.com/yahoo/pulsar/blob/2d75e88269e35ed0e9959c2ad678bd349c605436/pulsar-client/src/main/java/com/yahoo/pulsar/client/api/AuthenticationFactory.java#L42
I think we don't have to consider it right now.
What do you think ?
There was a problem hiding this comment.
Ok, then don't worry about it.
| } | ||
| } | ||
|
|
||
| tlsSocket_ = executor->createTlsSocket(socket_, ctx); |
There was a problem hiding this comment.
Shouldn't we close/cleanup this socket in ClientConnection::close() ?
There was a problem hiding this comment.
yush1ga
force-pushed
the
add_tls_authentication_plugin_for_cpp_client
branch
from
3d49cd6
to
a9e2919
Compare
pulsar-client-cpp/lib/Auth.cc
Outdated
| Authentication *auth = NULL; | ||
| void *handle = dlopen(dynamicLibPath.c_str(), RTLD_LAZY); | ||
| if (handle != NULL) { | ||
| { |
There was a problem hiding this comment.
should we remove this extra block(curly braces) here?
| keepAliveTimer_() { | ||
| keepAliveTimer_(), | ||
| isTlsAllowInsecureConnection_(false) { | ||
| if (clientConfiguration.isUseTls()) { |
| } | ||
|
|
||
| tlsSocket_ = executor->createTlsSocket(socket_, ctx); | ||
| } |
There was a problem hiding this comment.
if we have already intitialized tlsSocket then shouldn't we initialize socket_(executor->createSocket()), at line 101 into else block here.?
There was a problem hiding this comment.
I think we should initialize socket_ at line 101 because if we don't do it, createTlsSocket doesn't work with a following error.
Assertion failed: (px != 0), function operator*, file /usr/local/include/boost/smart_ptr/shared_ptr.hpp, line 703.
| } | ||
| } | ||
|
|
||
| template< typename MutableBufferSequence, typename ReadHandler> |
There was a problem hiding this comment.
formatting space after template<
|
@yush1ga : minor comments. looks good to me. |
yush1ga
force-pushed
the
add_tls_authentication_plugin_for_cpp_client
branch
from
a9e2919
to
7e06246
Compare
|
Broker has tls connection test but it doesn't have tls authentication test. |
|
👍 |
…pache#237) add internal connectionReader readAtLeast error information these error information may help to solve apache#200
related to apache#199, keep requestsQueue order using one thread to solve Correlation ID response in disorder.
Motivation
Authentication plugin for C++ client is empty.
Modifications
Result
C++ client can be authenticated and authorized by broker with AuthenticationProviderTls.