KAA-1087: [C++ SDK] Bad padding in PKCS7 #1529

RostakaGmfun · 2017-05-13T17:37:36Z

rasendubi

(setq-default indent-tabs-mode nil)

rasendubi · 2017-05-13T17:39:19Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+    boost::asio::deadline_timer connAckTimer_;
+    IKaaDataMultiplexer *multiplexer_ = nullptr;
+    IKaaDataDemultiplexer *demultiplexer_ = nullptr;
+    KeyPair clientKeys_;


clientKeys_ only used in the constructor. There is no need to store them.

rasendubi · 2017-05-13T17:41:18Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+
+    std::deque<std::vector<uint8_t>> requestQueue_;
+
+    std::shared_ptr<RsaEncoderDecoder> encDec_;


Shouldn't be a pointer. It is neither reinitialized, nor shared.

rasendubi · 2017-05-13T17:42:52Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+
+    std::shared_ptr<RsaEncoderDecoder> encDec_;
+
+    std::shared_ptr<IPTransportInfo> currentServer_;


currentServer_ is only used in the constructor. No need to store it.

rasendubi · 2017-05-13T17:44:12Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

-    isConnected_ = true;
+    if (state_ != State::Disconnected) {
+        readFromSocket();
+        }


Indentation.

rasendubi · 2017-05-13T17:48:03Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+    void setPingTimer();
+    void setConnAckTimer();
+
+    void run();


I would say, ChannelConnection should not support run/shutdown. They only complicate things. (Then we don't need Disconnected state.) Constructor is run, destructor is shutdown. RAII FTW!

rasendubi · 2017-05-13T18:11:50Z

client/client-multi/client-cpp/kaa/channel/impl/DefaultOperationTcpChannel.hpp

+
+    boost::asio::io_service io_;
+    boost::asio::io_service::work work_;
+    std::vector<std::thread> ioThreads_;


Should we support more than one IO thread?

rasendubi · 2017-05-13T18:14:46Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+void ChannelConnection::readFromSocket()
+{
+    std::lock_guard<std::mutex> lock(connectionMutex_);
+    KAA_LOG_INFO(boost::format("readFromSocket() %1%") % this);


rasendubi · 2017-05-13T18:15:18Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+        channel_->onServerFailed();
+        return;
+    } else {
+        if (err != boost::asio::error::operation_aborted){


Code style. (){)

rasendubi · 2017-05-13T18:17:21Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

-                                                  clientKeys_.getPrivateKey(),
-                                                  currentServer_->getPublicKey(),
-                                                  context_);
+    KAA_LOG_INFO(boost::format("Channel [%1%] scheduling open connection") % getId());


Does this add anything to the previous line?

rasendubi · 2017-05-13T18:19:36Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

        closeConnection();

        KAA_LOG_TRACE(boost::format("Channel [%1%] stopping IO service: isStopped '%2%'")
-                                                    % getId()
+													% getId()


Indentation.

rasendubi · 2017-05-15T12:07:03Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+    auto data = request.getRawMessage();
+    strand_.post([this, data] {
+        requestQueue_.push_back(data);
+        if (requestQueue_.size() > 1) {


requestQueue_.push_back(data); if (requestQueue_.size() == 1) { sendDataImpl(); }

rasendubi · 2017-05-15T12:07:47Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+                             boost::asio::buffer(reinterpret_cast<const char *>(data.data()), data.size()),
+                             strand_.wrap(boost::bind(&ChannelConnection::onWriteEvent, shared_from_this(),
+                                                      boost::asio::placeholders::error,
+                                                      boost::asio::placeholders::bytes_transferred)));}


Code style. Why closing } is here?

rasendubi · 2017-05-15T12:09:18Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

 }
-
-boost::system::error_code DefaultOperationTcpChannel::sendKaaSync(const std::map<TransportType, ChannelDirection>& transportTypes)
+void ChannelConnection::sendKaaSync(const std::map<TransportType, ChannelDirection>& transportTypes)


Code style: add an empty line before the function.

rasendubi · 2017-05-15T12:18:32Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

-
-            KAA_LOG_ERROR(boost::format("Channel [%1%] failed to process PING: %2%") % getId() % err.message());
-            onServerFailed();
+        if (err != boost::asio::error::operation_aborted && state_ != State::Disconnected) {


You can flatten conditionals now using else if.

rasendubi · 2017-05-15T12:19:21Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

        } else {
-            if (isConnected_) {
-                KAA_LOG_DEBUG(boost::format("Channel [%1%] CONNACK processed") % getId());
+            if (state_ != State::Disconnected) {


Flatten ifs.

rasendubi · 2017-05-15T12:24:35Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

-                                                                % getId()
-                                                                % errorCode.message());
-            onServerFailed();
+                        //TODO: http://jira.kaaproject.org/browse/KAA-1321


// TODO(KAA-1321):

rasendubi · 2017-05-15T15:35:58Z

client/client-multi/client-cpp/impl/channel/impl/DefaultOperationTcpChannel.cpp

+void DefaultOperationTcpChannel::sync(TransportType type)
+{
+    std::lock_guard<std::recursive_mutex> lock(channelGuard_);
+    if (!connection_) {


if (connection_) { connection_->sync(type); } else { ...

rasendubi · 2017-05-15T15:37:11Z

client/client-multi/client-cpp/kaa/channel/impl/DefaultOperationTcpChannel.hpp

@@ -84,91 +85,46 @@ class DefaultOperationTcpChannel : public IDataChannel {
    }

    virtual void setFailoverStrategy(IFailoverStrategyPtr strategy) {
-        failoverStrategy_ = strategy;
+        static_cast<void>(strategy);


Could you also adjust code style?

virtual void setFailoverStrategy(IFailoverStrategyPtr strategy) {

rasendubi · 2017-05-15T15:39:32Z

client/client-multi/client-cpp/kaa/channel/impl/DefaultOperationTcpChannel.hpp

-    bool isPaused_ = false;
-    bool isFailoverInProgress_ = false;
+    std::shared_ptr<ChannelConnection> connection_;
+    std::shared_ptr<IPTransportInfo> currentServer_;


Not sure. The only place it is passed as a reference is

channelManager_.onServerFailed(currentServer_, finalFailoverReason);

rasendubi · 2017-05-24T15:26:41Z

@RostakaGmfun, please port the patch to master

RostakaGmfun added 3 commits May 13, 2017 16:16

KAA-1087: [C++ SDK] Bad padding in PKCS7

dcbe655

KAA-1087: fixes

7692d22

KAA-1087: more fixes

1dadade

rasendubi reviewed May 13, 2017

View reviewed changes

KAA-1087: Review fixes

8014fec

rasendubi reviewed May 15, 2017

View reviewed changes

KAA-1087: More review fixes

77a1c54

rasendubi approved these changes May 24, 2017

View reviewed changes

rasendubi merged commit 4832315 into kaaproject:release-0.10 May 24, 2017

RostakaGmfun mentioned this pull request May 26, 2017

KAA-1087: [C++ SDK] Bad padding in PKCS7 #1530

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAA-1087: [C++ SDK] Bad padding in PKCS7 #1529

KAA-1087: [C++ SDK] Bad padding in PKCS7 #1529

RostakaGmfun commented May 13, 2017

rasendubi left a comment

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

RostakaGmfun May 14, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 13, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi May 15, 2017

rasendubi commented May 24, 2017


		std::deque<std::vector<uint8_t>> requestQueue_;

		std::shared_ptr<RsaEncoderDecoder> encDec_;


		std::shared_ptr<RsaEncoderDecoder> encDec_;

		std::shared_ptr<IPTransportInfo> currentServer_;

KAA-1087: [C++ SDK] Bad padding in PKCS7 #1529

KAA-1087: [C++ SDK] Bad padding in PKCS7 #1529

Conversation

RostakaGmfun commented May 13, 2017

rasendubi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rasendubi commented May 24, 2017