Avoid calling setIPv4 in TCPTransportInterface (#5492) (#5893)

* Refs #22469: Regression Tests for IPv6



* Refs #22469: New method copyAddress



* Refs #22469: Fix TCPTransportInterface



* Refs #22469: Apply Review



* Refs #22469: Disable grouping of locators in single channel for TCPv6



* Refs #22469: Uncrustify



* Refs #22469: Apply review



* Refs #22469: Allow to copyIPv4 between two locators



* Refs #22469: Apply Review 2



---------

Signed-off-by: cferreiragonz <[email protected]>
Signed-off-by: Eugenio Collado <[email protected]>
Co-authored-by: Carlos Ferreira González <[email protected]>

Author: mergify[bot]

include/fastrtps/utils/IPLocator.h

@@ -97,11 +97,27 @@ class IPLocator
     RTPS_DllAPI static std::string toIPv4string(
             const Locator_t& locator);
 
-    //! Copies locator's IPv4.
+    /**
+     * @brief Copies locator's IPv4 to a destination array.
+     * @param locator Locator from which to copy the IPv4.
+     * @param dest Destination array where the IPv4 will be copied.
+     * @return true if the copy was successful, false otherwise.
+     */
     RTPS_DllAPI static bool copyIPv4(
             const Locator_t& locator,
             unsigned char* dest);
 
+    /**
+     * @brief Copies locator's IPv4 to a destination locator.
+     *        It only copies the IPv4 part (last 4 bytes), leaving other parts unchanged.
+     * @param locator Locator from which to copy the IPv4.
+     * @param dest Destination locator where the IPv4 will be copied.
+     * @return true if the copy was successful, false otherwise.
+     */
+    RTPS_DllAPI static bool copyIPv4(
+            const Locator_t& locator,
+            Locator_t& dest);
+
     // IPv6
     //! Sets locator's IPv6.
     RTPS_DllAPI static bool setIPv6(
@@ -252,6 +268,16 @@ class IPLocator
             const Locator_t& loc2,
             bool fullAddress = false);
 
+    /**
+     * Copies the whole address from one locator to another.
+     * @param loc1 Locator to copy from.
+     * @param loc2 Locator to copy to.
+     * @return True if the copy was successful.
+     */
+    RTPS_DllAPI static bool copy_address(
+            const Locator_t& loc1,
+            Locator_t& loc2);
+
     //! Checks if a both locators has the same IP address and physical port  (as in RTCP protocol).
     RTPS_DllAPI static bool compareAddressAndPhysicalPort(
             const Locator_t& loc1,

src/cpp/rtps/transport/TCPTransportInterface.cpp

@@ -324,14 +324,18 @@ ResponseCode TCPTransportInterface::bind_socket(
 
     std::vector<fastrtps::rtps::IPFinder::info_IP> local_interfaces;
     // Check if the locator is from an owned interface to link all local interfaces to the channel
-    is_own_interface(channel->locator(), local_interfaces);
-    if (!local_interfaces.empty())
+    // Note: Only applicable for TCPv4 until TCPv6 scope selection is implemented
+    if (channel->locator().kind != LOCATOR_KIND_TCPv6)
     {
-        Locator local_locator(channel->locator());
-        for (auto& interface_it : local_interfaces)
+        is_own_interface(channel->locator(), local_interfaces);
+        if (!local_interfaces.empty())
         {
-            IPLocator::setIPv4(local_locator, interface_it.locator);
-            channel_resources_.insert(decltype(channel_resources_)::value_type{local_locator, channel});
+            Locator local_locator(channel->locator());
+            for (auto& interface_it : local_interfaces)
+            {
+                IPLocator::copy_address(interface_it.locator, local_locator);
+                channel_resources_.insert(decltype(channel_resources_)::value_type{local_locator, channel});
+            }
         }
     }
     return ret;
@@ -1031,14 +1035,18 @@ bool TCPTransportInterface::CreateInitialConnect(
 
     std::vector<fastrtps::rtps::IPFinder::info_IP> local_interfaces;
     // Check if the locator is from an owned interface to link all local interfaces to the channel
-    is_own_interface(physical_locator, local_interfaces);
-    if (!local_interfaces.empty())
+    // Note: Only applicable for TCPv4 until TCPv6 scope selection is implemented
+    if (physical_locator.kind != LOCATOR_KIND_TCPv6)
     {
-        Locator local_locator(physical_locator);
-        for (auto& interface_it : local_interfaces)
+        is_own_interface(physical_locator, local_interfaces);
+        if (!local_interfaces.empty())
         {
-            IPLocator::setIPv4(local_locator, interface_it.locator);
-            channel_resources_[local_locator] = channel;
+            Locator local_locator(physical_locator);
+            for (auto& interface_it : local_interfaces)
+            {
+                IPLocator::copy_address(interface_it.locator, local_locator);
+                channel_resources_[local_locator] = channel;
+            }
         }
     }
 
@@ -1345,7 +1353,8 @@ bool TCPTransportInterface::Receive(
         do
         {
             header_found = receive_header(channel, tcp_header, ec);
-        } while (!header_found && !ec && channel->connection_status());
+        }
+        while (!header_found && !ec && channel->connection_status());
 
         if (ec)
         {

src/cpp/utils/IPLocator.cpp

@@ -178,6 +178,13 @@ bool IPLocator::copyIPv4(
     return true;
 }
 
+bool IPLocator::copyIPv4(
+        const Locator_t& locator,
+        Locator_t& dest)
+{
+    return copyIPv4(locator, &(dest.address[12]));
+}
+
 // IPv6
 bool IPLocator::setIPv6(
         Locator_t& locator,
@@ -911,6 +918,27 @@ bool IPLocator::compareAddress(
     }
 }
 
+bool IPLocator::copy_address(
+        const Locator_t& loc1,
+        Locator_t& loc2)
+{
+    if (loc1.kind != loc2.kind)
+    {
+        return false;
+    }
+
+    if (loc1.kind == LOCATOR_KIND_UDPv4 || loc1.kind == LOCATOR_KIND_TCPv4)
+    {
+        copyIPv4(loc1, loc2);
+        return true;
+    }
+    else if (loc1.kind == LOCATOR_KIND_UDPv6 || loc1.kind == LOCATOR_KIND_TCPv6)
+    {
+        return copyIPv6(loc1, loc2.address);
+    }
+    return false;
+}
+
 bool IPLocator::compareAddressAndPhysicalPort(
         const Locator_t& loc1,
         const Locator_t& loc2)

test/unittest/transport/TCPv4Tests.cpp

@@ -1955,6 +1955,7 @@ TEST_F(TCPv4Tests, client_announced_local_port_uniqueness)
 
     std::this_thread::sleep_for(std::chrono::milliseconds(100));
 
+    EXPECT_GT(receiveTransportUnderTest.get_channel_resources().size(), 2u);
     std::set<std::shared_ptr<TCPChannelResource>> channels_created;
     for (const auto& channel_resource : receiveTransportUnderTest.get_channel_resources())
     {

test/unittest/transport/TCPv6Tests.cpp

@@ -367,7 +367,7 @@ TEST_F(TCPv6Tests, client_announced_local_port_uniqueness)
 
     std::this_thread::sleep_for(std::chrono::milliseconds(100));
 
-    ASSERT_EQ(receiveTransportUnderTest.get_channel_resources().size(), 2u);
+    EXPECT_EQ(receiveTransportUnderTest.get_channel_resources().size(), 2u);
 }
 
 #ifndef _WIN32

test/unittest/utils/LocatorTests.cpp

@@ -766,6 +766,37 @@ TEST_F(IPLocatorTests, copyIPv6)
     ASSERT_EQ(arr[15], 1u);
 }
 
+/*
+ * Check to copy an address
+ */
+TEST_F(IPLocatorTests, copy_address)
+{
+    // Copy IPv4
+    Locator_t locator1(LOCATOR_KIND_UDPv4);
+    Locator_t locator2(LOCATOR_KIND_UDPv4);
+    IPLocator::setIPv4(locator1, ipv4_lo_address);
+    ASSERT_FALSE(IPLocator::compareAddress(locator1, locator2));
+    ASSERT_TRUE(IPLocator::copy_address(locator1, locator2));
+    ASSERT_TRUE(IPLocator::compareAddress(locator1, locator2));
+
+    // Check cannot copy between different kinds
+    locator1.kind = LOCATOR_KIND_UDPv6;
+    ASSERT_FALSE(IPLocator::copy_address(locator1, locator2));
+
+    // Copy IPv6
+    locator2.kind = LOCATOR_KIND_UDPv6;
+    IPLocator::setIPv6(locator1, ipv6_lo_address);
+    ASSERT_FALSE(IPLocator::compareAddress(locator1, locator2));
+    ASSERT_TRUE(IPLocator::copy_address(locator1, locator2));
+    ASSERT_TRUE(IPLocator::compareAddress(locator1, locator2));
+
+    // Check cannot copy between SHM locators
+    locator1.kind = LOCATOR_KIND_SHM;
+    Locator_t locator3(LOCATOR_KIND_SHM);
+    ASSERT_FALSE(IPLocator::copy_address(locator1, locator3));
+    ASSERT_FALSE(IPLocator::compareAddress(locator1, locator3));
+}
+
 /*
  * Check to set ip of any kind
  */