Bugfix/protocol handler memory issues (#2270)

* fix: memory leak in ProtocolHandlerImpl::NotifySessionStarted()

This is detected by valgrind.

* fix: buffer over-run in ProtocolPayloadTest

This is an issue of unit test implementation and only affects
unit testing.

* fix: invalid memory accesses in ProtocolHandlerImplTest

These affect unit testing only.
Invalid memory accesses occurred because the mock class didn't
configure protocol version field properly.

* fix: valgrind warning in IncomingDataHandlerTest

This is not actually an issue. valgrind reported a warning
since the memory area was not initialized.

Co-authored-by: Collin <iCollin@users.noreply.github.com>

4 files changed, 19 insertions, 8 deletions

diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index ace27fb307..b502a924af 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -1835,7 +1835,11 @@ void ProtocolHandlerImpl::NotifySessionStarted(
   }
 
   std::shared_ptr<BsonObject> start_session_ack_params(
-      new BsonObject(), [](BsonObject* obj) { bson_object_deinitialize(obj); });
+      new BsonObject(),
+      [](BsonObject* obj) {
+        bson_object_deinitialize(obj);
+        delete obj;
+      });
   bson_object_initialize_default(start_session_ack_params.get());
   // when video service is successfully started, copy input parameters
   // ("width", "height", "videoProtocol", "videoCodec") to the ACK packet
diff --git a/src/components/protocol_handler/test/incoming_data_handler_test.cc b/src/components/protocol_handler/test/incoming_data_handler_test.cc
index 3f1b28cbe2..c9bfa748dd 100644
--- a/src/components/protocol_handler/test/incoming_data_handler_test.cc
+++ b/src/components/protocol_handler/test/incoming_data_handler_test.cc
@@ -55,7 +55,14 @@ class IncomingDataHandlerTest : public ::testing::Test {
     some_data_size = 4;
     some_data2_size = 512;
     some_data = new uint8_t[some_data_size];
+    for (size_t i = 0; i < some_data_size; i++) {
+      // put some data in the buffer
+      some_data[i] = i % 256;
+    }
     some_data2 = new uint8_t[some_data2_size];
+    for (size_t i = 0; i < some_data2_size; i++) {
+      some_data2[i] = i % 256;
+    }
     protov1_message_id = 0x0;
     some_message_id = 0xABCDEF0;
     some_session_id = 0xFEDCBA0;
@@ -254,9 +261,8 @@ TEST_F(IncomingDataHandlerTest, MixedPayloadData_TwoConnections) {
   for (FrameList::const_iterator it = actual_frames.begin();
        it != actual_frames.end();
        ++it, ++it_exp) {
-    // TODO(EZamakhov): investigate valgrind warning (unitialized value)
-    EXPECT_EQ(**it, **it_exp)
-        << "Element number " << std::distance(mobile_packets.begin(), it_exp);
+    EXPECT_EQ(**it, **it_exp) << "Element number "
+                              << std::distance(mobile_packets.begin(), it_exp);
   }
 }
 
diff --git a/src/components/protocol_handler/test/protocol_handler_tm_test.cc b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
index b87e31774b..312ec88930 100644
--- a/src/components/protocol_handler/test/protocol_handler_tm_test.cc
+++ b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
@@ -3554,7 +3554,7 @@ TEST_F(ProtocolHandlerImplTest,
   // Expect check connection with ProtocolVersionUsed
   EXPECT_CALL(session_observer_mock,
               ProtocolVersionUsed(connection_id, session_id, _))
-      .WillOnce(Return(true));
+      .WillOnce(DoAll(SetArgReferee<2>(PROTOCOL_VERSION_1), Return(true)));
   // Expect send End Service
   EXPECT_CALL(transport_manager_mock,
               SendMessageToDevice(ExpectedMessage(FRAME_TYPE_CONTROL,
@@ -3593,7 +3593,7 @@ TEST_F(ProtocolHandlerImplTest, SendHeartBeat_Successful) {
   // Expect check connection with ProtocolVersionUsed
   EXPECT_CALL(session_observer_mock,
               ProtocolVersionUsed(connection_id, session_id, _))
-      .WillOnce(Return(true));
+      .WillOnce(DoAll(SetArgReferee<2>(PROTOCOL_VERSION_3), Return(true)));
   // Expect send HeartBeat
   EXPECT_CALL(
       transport_manager_mock,
diff --git a/src/components/protocol_handler/test/protocol_payload_test.cc b/src/components/protocol_handler/test/protocol_payload_test.cc
index e89e6c7395..84f6e2b2c5 100644
--- a/src/components/protocol_handler/test/protocol_payload_test.cc
+++ b/src/components/protocol_handler/test/protocol_payload_test.cc
@@ -242,8 +242,9 @@ TEST(ProtocolPayloadTest, ExtractProtocolWithJSONWithDataWithWrongPayloadSize) {
   prot_payload_test.json = expect_output_json_string;
   prot_payload_test.header.json_size = prot_payload_test.json.length();
 
-  const size_t data_for_sending_size =
-      PROTOCOL_HEADER_V2_SIZE + prot_payload_test.json.length();
+  const size_t data_for_sending_size = PROTOCOL_HEADER_V2_SIZE +
+                                       prot_payload_test.json.length() +
+                                       prot_payload_test.data.size();
   uint8_t* data_for_sending = new uint8_t[data_for_sending_size];
   prepare_data(data_for_sending, prot_payload_test);