From 5f11e35b3edcc55efee98d9e308ba7a7c19b4420 Mon Sep 17 00:00:00 2001 From: Mario Dominguez Date: Tue, 30 Apr 2024 17:12:23 +0200 Subject: [PATCH] Refs #20945: Fix SecurityTest.discovered_participant_ heap_after_free errors Signed-off-by: Mario Dominguez --- .../SecurityHandshakeProcessTests.cpp | 28 +++++++++++-------- test/unittest/rtps/security/SecurityTests.cpp | 20 +++++++------ test/unittest/rtps/security/SecurityTests.hpp | 3 +- .../SecurityValidationRemoteTests.cpp | 14 ++++++---- 4 files changed, 37 insertions(+), 28 deletions(-) diff --git a/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp b/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp index ce6911bf8e4..ba9588bde2e 100644 --- a/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp +++ b/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp @@ -337,12 +337,13 @@ TEST_F(SecurityTest, discovered_participant_process_message_ok_begin_handshake_r info.guid = participant_data.m_guid; EXPECT_CALL(*participant_.getListener(), onParticipantAuthentication(_, info)).Times(1); - CacheChange_t* kx_change = new CacheChange_t(500); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(500); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); stateless_reader_->listener_->onNewCacheChangeAdded(stateless_reader_, change); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); return_handle(remote_identity_handle); return_handle(handshake_handle); @@ -527,14 +528,15 @@ TEST_F(SecurityTest, discovered_participant_process_message_pending_handshake_re WillOnce(DoAll(SetArgPointee<0>(&remote_identity_handle), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - CacheChange_t* kx_change = new CacheChange_t(500); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(500); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); ParticipantProxyData participant_data; fill_participant_key(participant_data.m_guid); ASSERT_TRUE(manager_.discovered_participant(participant_data)); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); ParticipantGenericMessage message; message.message_identity().source_guid(participant_data.m_guid); @@ -732,12 +734,13 @@ TEST_F(SecurityTest, discovered_participant_process_message_ok_process_handshake info.guid = remote_participant_key; EXPECT_CALL(*participant_.getListener(), onParticipantAuthentication(_, info)).Times(1); - CacheChange_t* kx_change = new CacheChange_t(500); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(500); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); stateless_reader_->listener_->onNewCacheChangeAdded(stateless_reader_, change); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); } TEST_F(SecurityTest, discovered_participant_process_message_process_handshake_reply_new_change_fail) @@ -1131,12 +1134,13 @@ TEST_F(SecurityTest, discovered_participant_process_message_ok_process_handshake info.guid = remote_participant_key; EXPECT_CALL(*participant_.getListener(), onParticipantAuthentication(_, info)).Times(1); - CacheChange_t* kx_change = new CacheChange_t(500); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(500); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); stateless_reader_->listener_->onNewCacheChangeAdded(stateless_reader_, change); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); } int main( diff --git a/test/unittest/rtps/security/SecurityTests.cpp b/test/unittest/rtps/security/SecurityTests.cpp index 2e8cc5ac743..d8743f30de1 100644 --- a/test/unittest/rtps/security/SecurityTests.cpp +++ b/test/unittest/rtps/security/SecurityTests.cpp @@ -245,12 +245,13 @@ void SecurityTest::final_message_process_ok( info.guid = remote_participant_key; EXPECT_CALL(*participant_.getListener(), onParticipantAuthentication(_, info)).Times(1); - CacheChange_t* kx_change = new CacheChange_t(200); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(200); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); stateless_reader_->listener_->onNewCacheChangeAdded(stateless_reader_, change); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); if (final_message_change == nullptr) { @@ -263,17 +264,18 @@ void SecurityTest::final_message_process_ok( } void SecurityTest::expect_kx_exchange( - CacheChange_t* kx_change) + CacheChange_t& kx_change_to_add, + CacheChange_t* kx_change_to_remove) { EXPECT_CALL(*volatile_writer_, new_change(_, _, _)).Times(1).WillOnce( - DoAll(Invoke([kx_change](const std::function& f, ChangeKind_t, InstanceHandle_t) + DoAll(Invoke([&kx_change_to_add](const std::function& f, ChangeKind_t, InstanceHandle_t) { - kx_change->serializedPayload.reserve(f()); + kx_change_to_add.serializedPayload.reserve(f()); }), - Return(kx_change))); - EXPECT_CALL(*volatile_writer_->history_, add_change_mock(kx_change)).Times(1). + Return(&kx_change_to_add))); + EXPECT_CALL(*volatile_writer_->history_, add_change_mock(&kx_change_to_add)).Times(1). WillOnce(Return(true)); - EXPECT_CALL(*volatile_writer_->history_, remove_change_mock(kx_change)).Times(1). + EXPECT_CALL(*volatile_writer_->history_, remove_change_mock(kx_change_to_remove)).Times(1). WillOnce(Return(true)); } diff --git a/test/unittest/rtps/security/SecurityTests.hpp b/test/unittest/rtps/security/SecurityTests.hpp index 60eb25f9560..852cd82145e 100644 --- a/test/unittest/rtps/security/SecurityTests.hpp +++ b/test/unittest/rtps/security/SecurityTests.hpp @@ -125,7 +125,8 @@ class SecurityTest : public ::testing::Test CacheChange_t** final_message_change = nullptr); void expect_kx_exchange( - CacheChange_t* kx_change); + CacheChange_t& kx_change_to_add, + CacheChange_t* kx_change_to_remove); void destroy_manager_and_change( CacheChange_t*& change, diff --git a/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp b/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp index 8169510708d..9ea44500190 100644 --- a/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp +++ b/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp @@ -158,12 +158,13 @@ TEST_F(SecurityTest, discovered_participant_validation_remote_identity_pending_h info.guid = participant_data.m_guid; EXPECT_CALL(*participant_.getListener(), onParticipantAuthentication(_, info)).Times(1); - CacheChange_t* kx_change = new CacheChange_t(500); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(500); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); ASSERT_TRUE(manager_.discovered_participant(participant_data)); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); return_handle(remote_identity_handle); return_handle(handshake_handle); @@ -330,12 +331,13 @@ TEST_F(SecurityTest, discovered_participant_validation_remote_identity_pending_h info.guid = participant_data.m_guid; EXPECT_CALL(*participant_.getListener(), onParticipantAuthentication(_, info)).Times(1); - CacheChange_t* kx_change = new CacheChange_t(500); - expect_kx_exchange(kx_change); + CacheChange_t kx_change_to_add; + CacheChange_t* kx_change_to_remove = new CacheChange_t(500); + expect_kx_exchange(kx_change_to_add, kx_change_to_remove); ASSERT_TRUE(manager_.discovered_participant(participant_data)); - volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change); + volatile_writer_->listener_->onWriterChangeReceivedByAll(volatile_writer_, kx_change_to_remove); destroy_manager_and_change(change);