Skip to content

Commit

Permalink
Fix comparison in is_update_allowed (#5414) (#5415)
Browse files Browse the repository at this point in the history
* Refs #19927. Add unit test.

Signed-off-by: Miguel Company <[email protected]>

* Refs #19927. Fix issue in ReaderProxyData.

Signed-off-by: Miguel Company <[email protected]>

* Refs #19927. Fix issue in WriterProxyData.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit 6b74b0d)

Co-authored-by: Miguel Company <[email protected]>
  • Loading branch information
mergify[bot] and MiguelCompany authored Dec 11, 2024
1 parent 991e265 commit ffca0ce
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/cpp/rtps/builtin/data/ReaderProxyData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,7 @@ bool ReaderProxyData::is_update_allowed(
if ((m_guid != rdata.m_guid) ||
#if HAVE_SECURITY
(security_attributes_ != rdata.security_attributes_) ||
(plugin_security_attributes_ != rdata.security_attributes_) ||
(plugin_security_attributes_ != rdata.plugin_security_attributes_) ||
#endif // if HAVE_SECURITY
(m_typeName != rdata.m_typeName) ||
(m_topicName != rdata.m_topicName))
Expand Down
2 changes: 1 addition & 1 deletion src/cpp/rtps/builtin/data/WriterProxyData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ bool WriterProxyData::is_update_allowed(
(persistence_guid_ != wdata.persistence_guid_) ||
#if HAVE_SECURITY
(security_attributes_ != wdata.security_attributes_) ||
(plugin_security_attributes_ != wdata.security_attributes_) ||
(plugin_security_attributes_ != wdata.plugin_security_attributes_) ||
#endif // if HAVE_SECURITY
(m_typeName != wdata.m_typeName) ||
(m_topicName != wdata.m_topicName))
Expand Down
65 changes: 65 additions & 0 deletions test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2474,6 +2474,71 @@ TEST(BuiltinDataSerializationTests, deserialization_of_big_parameters)
}
}

/*!
* This is a regression test for redmine issue #19927
*
* It checks that proxy data for readers and writers can only be updated if the security attributes are equal.
*/
TEST(BuiltinDataSerializationTests, security_attributes_update)
{
// Only if security is enabled
#if HAVE_SECURITY

// Test for ReaderProxyData
{
ReaderProxyData original(max_unicast_locators, max_multicast_locators);
original.security_attributes_ = 0x01;
original.plugin_security_attributes_ = 0x02;

ReaderProxyData updated(original);
EXPECT_TRUE(original.is_update_allowed(updated));

updated.security_attributes_ = original.security_attributes_ + 10;
updated.plugin_security_attributes_ = original.plugin_security_attributes_;
EXPECT_FALSE(original.is_update_allowed(updated));

updated.security_attributes_ = original.security_attributes_;
updated.plugin_security_attributes_ = original.plugin_security_attributes_ + 10;
EXPECT_FALSE(original.is_update_allowed(updated));

updated.security_attributes_ = original.plugin_security_attributes_;
updated.plugin_security_attributes_ = original.plugin_security_attributes_;
EXPECT_FALSE(original.is_update_allowed(updated));

updated.security_attributes_ = original.security_attributes_;
updated.plugin_security_attributes_ = original.security_attributes_;
EXPECT_FALSE(original.is_update_allowed(updated));
}

// Test for WriterProxyData
{
WriterProxyData original(max_unicast_locators, max_multicast_locators);
original.security_attributes_ = 0x01;
original.plugin_security_attributes_ = 0x02;

WriterProxyData updated(original);
EXPECT_TRUE(original.is_update_allowed(updated));

updated.security_attributes_ = original.security_attributes_ + 10;
updated.plugin_security_attributes_ = original.plugin_security_attributes_;
EXPECT_FALSE(original.is_update_allowed(updated));

updated.security_attributes_ = original.security_attributes_;
updated.plugin_security_attributes_ = original.plugin_security_attributes_ + 10;
EXPECT_FALSE(original.is_update_allowed(updated));

updated.security_attributes_ = original.plugin_security_attributes_;
updated.plugin_security_attributes_ = original.plugin_security_attributes_;
EXPECT_FALSE(original.is_update_allowed(updated));

updated.security_attributes_ = original.security_attributes_;
updated.plugin_security_attributes_ = original.security_attributes_;
EXPECT_FALSE(original.is_update_allowed(updated));
}

#endif // HAVE_SECURITY
}

} // namespace rtps
} // namespace fastdds
} // namespace eprosima
Expand Down

0 comments on commit ffca0ce

Please sign in to comment.