From ffca0ce6d2a6096f7aaaf63ec0b8107b13953a28 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:36:54 +0100 Subject: [PATCH] Fix comparison in `is_update_allowed` (#5414) (#5415) * 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 (cherry picked from commit 6b74b0db937cc33f78691b206a82b62f49c5ef81) Co-authored-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 0a75a6a84b9..2c78ff47841 100644 --- a/src/cpp/rtps/builtin/data/ReaderProxyData.cpp +++ b/src/cpp/rtps/builtin/data/ReaderProxyData.cpp @@ -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)) diff --git a/src/cpp/rtps/builtin/data/WriterProxyData.cpp b/src/cpp/rtps/builtin/data/WriterProxyData.cpp index 225f5c10bf0..384cc4d1983 100644 --- a/src/cpp/rtps/builtin/data/WriterProxyData.cpp +++ b/src/cpp/rtps/builtin/data/WriterProxyData.cpp @@ -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)) diff --git a/test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp b/test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp index 88769af870d..77ca815d66e 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