From 6b74b0db937cc33f78691b206a82b62f49c5ef81 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 25 Nov 2024 17:16:09 +0100 Subject: [PATCH] Fix comparison in `is_update_allowed` (#5414) * Refs #19927. Add unit test. Signed-off-by: Miguel Company * Refs #19927. Fix issue in ReaderProxyData. Signed-off-by: Miguel Company * Refs #19927. Fix issue in WriterProxyData. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company --- src/cpp/rtps/builtin/data/ReaderProxyData.cpp | 2 +- src/cpp/rtps/builtin/data/WriterProxyData.cpp | 2 +- .../builtin/BuiltinDataSerializationTests.cpp | 65 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/builtin/data/ReaderProxyData.cpp b/src/cpp/rtps/builtin/data/ReaderProxyData.cpp index 7aba95c8146..e3c2f1b6d11 100644 --- a/src/cpp/rtps/builtin/data/ReaderProxyData.cpp +++ b/src/cpp/rtps/builtin/data/ReaderProxyData.cpp @@ -1205,7 +1205,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)) diff --git a/src/cpp/rtps/builtin/data/WriterProxyData.cpp b/src/cpp/rtps/builtin/data/WriterProxyData.cpp index 2c2d4517f25..31b1dcb14bb 100644 --- a/src/cpp/rtps/builtin/data/WriterProxyData.cpp +++ b/src/cpp/rtps/builtin/data/WriterProxyData.cpp @@ -1217,7 +1217,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)) diff --git a/test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp b/test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp index 16602d2f3e8..3dcd144c2dc 100644 --- a/test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp +++ b/test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp @@ -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