Fix comparison in is_update_allowed (#5414) (#5415)

* Refs #19927. Add unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19927. Fix issue in ReaderProxyData. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19927. Fix issue in WriterProxyData. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 6b74b0d) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
eProsima · Dec 11, 2024 · ffca0ce · ffca0ce
1 parent 991e265
commit ffca0ce
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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