Refs #22872: Apply suggested changes

Signed-off-by: Carlosespicur <[email protected]>

Author: Carlosespicur

include/fastdds/dds/core/policy/QosPolicies.hpp

@@ -2734,6 +2734,15 @@ class WireProtocolConfigQos : public QosPolicy
     void easy_mode(
             std::string ip)
     {
+        // Check if the input is a valid IP
+        if (!rtps::IPLocator::isIPv4(ip))
+        {
+            EPROSIMA_LOG_ERROR(
+                WIREPROTOCOLQOS, "Invalid IP address format for ROS 2 Easy Mode. It must be an IPv4 address.");
+
+            return;
+        }
+
         easy_mode_ = ip;
     }
 

resources/xsd/fastdds_profiles.xsd

@@ -131,6 +131,7 @@
             ├ builtin                              [0~1],
             ├ port                                 [0~1],
             ├ participantID                        [int32],
+            ├ easy_mode_ip                         [string],
             ├ userTransports                       [0~1],
             |   └ transport_id                     [1~*] [string],
             ├ useBuiltinTransports                 [bool],
@@ -172,6 +173,7 @@
                         <xs:element name="builtin" type="builtinAttributesType" minOccurs="0" maxOccurs="1"/>
                         <xs:element name="port" type="portType" minOccurs="0" maxOccurs="1"/>
                         <xs:element name="participantID" type="int32" minOccurs="0" maxOccurs="1"/>
+                        <xs:element name="easy_mode_ip" type="string" minOccurs="0" maxOccurs="1"/>
                         <xs:element name="userTransports" minOccurs="0" maxOccurs="1">
                             <xs:complexType>
                                 <xs:sequence>

src/cpp/rtps/attributes/ServerAttributes.cpp

@@ -73,6 +73,20 @@ const std::string& ros_easy_mode_env()
 {
     static std::string ip_value;
     SystemInfo::get_env(ROS2_EASY_MODE_URI, ip_value);
+
+    if (!ip_value.empty())
+    {
+        // Check that the value is a valid IPv4 address
+        if (!IPLocator::isIPv4(ip_value))
+        {
+            EPROSIMA_LOG_ERROR(
+                SERVERATTRIBUTES,
+                "Invalid format: Easy Mode IP must be a valid IPv4 address. "
+                "Ignoring " << ROS2_EASY_MODE_URI << " value.");
+
+            ip_value = "";
+        }
+    }
     return ip_value;
 }
 

src/cpp/rtps/attributes/ServerAttributes.hpp

@@ -198,7 +198,8 @@ const std::string& ros_discovery_server_env();
 
 /**
  * Get the value of environment variable ROS2_EASY_MODE_URI
- * @return The value of environment variable ROS2_EASY_MODE_URI. Empty string if the variable is not defined.
+ * @return The value of environment variable ROS2_EASY_MODE_URI.
+ * Empty string if the variable is not defined or does not have a valid IPv4 format.
  */
 const std::string& ros_easy_mode_env();
 

src/cpp/xmlparser/XMLParser.cpp

@@ -2450,10 +2450,20 @@ XMLP_ret XMLParser::fillDataNode(
         else if (strcmp(name, EASY_MODE_IP) == 0)
         {
             // easy_mode_ip - stringType
-            if (XMLP_ret::XML_OK != getXMLString(p_aux0, &participant_node.get()->rtps.easy_mode_ip, ident))
+            std::string str_aux;
+            if (XMLP_ret::XML_OK != getXMLString(p_aux0, &str_aux, ident))
             {
                 return XMLP_ret::XML_ERROR;
             }
+
+            // Check that the string is a valid IPv4 address
+            if (!fastdds::rtps::IPLocator::isIPv4(str_aux))
+            {
+                EPROSIMA_LOG_ERROR(XMLPARSER, "'easy_mode_ip' is not a valid IPv4 address.");
+                return XMLP_ret::XML_ERROR;
+            }
+
+            participant_node.get()->rtps.easy_mode_ip = str_aux;
         }
         else if (strcmp(name, FLOW_CONTROLLER_DESCRIPTOR_LIST) == 0)
         {

test/blackbox/common/DDSBlackboxTestsDSEasyMode.cpp

@@ -361,3 +361,78 @@ TEST(DSEasyMode, easy_discovery_mode_env_inconsistent_ip)
     stop_background_servers();
 #endif // _WIN32
 }
+
+/**
+ * Check that the environment variable ROS2_EASY_MODE is ignored if
+ * it does not have a valid IPv4 format.
+ */
+TEST(DSEasyMode, easy_discovery_mode_env_invalid)
+{
+    #ifndef _WIN32 // The feature is not supported on Windows yet
+    PubSubWriter<HelloWorldPubSubType> writer(TEST_TOPIC_NAME);
+    PubSubReader<HelloWorldPubSubType> reader(TEST_TOPIC_NAME);
+
+    // Set ROS2_EASY_MODE to an invalid string value
+    set_easy_discovery_mode_env("Foo");
+
+    std::atomic<bool> writer_background_ds_discovered(false);
+    std::atomic<bool> reader_background_ds_discovered(false);
+
+    writer.set_on_discovery_function(
+        [&writer_background_ds_discovered](
+            const eprosima::fastdds::rtps::ParticipantBuiltinTopicData& data,
+            eprosima::fastdds::rtps::ParticipantDiscoveryStatus)
+        {
+            if (data.participant_name == "DiscoveryServerAuto")
+            {
+                writer_background_ds_discovered.store(true);
+            }
+            return true;
+        });
+    writer.init();
+
+    reader.set_on_discovery_function(
+        [&reader_background_ds_discovered](const eprosima::fastdds::rtps::ParticipantBuiltinTopicData& data,
+        eprosima::fastdds::rtps::ParticipantDiscoveryStatus)
+        {
+            if (data.participant_name == "DiscoveryServerAuto")
+            {
+                reader_background_ds_discovered.store(true);
+            }
+            return true;
+        });
+    reader.init();
+
+    ASSERT_TRUE(writer.isInitialized());
+    ASSERT_TRUE(reader.isInitialized());
+
+    // Wait for endpoint discovery first
+    writer.wait_discovery();
+    reader.wait_discovery();
+
+    // Check that no Background DS was discovered,
+    // only the other reader or writer
+    ASSERT_GE(writer.get_participants_matched(), 1u);
+    ASSERT_GE(reader.get_participants_matched(), 1u);
+    ASSERT_FALSE(writer_background_ds_discovered.load());
+    ASSERT_FALSE(reader_background_ds_discovered.load());
+#endif // _WIN32
+}
+
+/**
+ * Check that easy mode configuration is ignored when setting it
+ * with code using WireProtocolConfigQos.
+ *
+ * Note: This test only checks that the configuration is ignored.
+ * Probably it should be extended similarly to easy_discovery_mode_env_invalid
+ * when Configuring Easy Mode via c++ and XML is implemented.
+ */
+TEST(DSEasyMode, wire_protocol_qos_params_invalid)
+{
+    eprosima::fastdds::dds::WireProtocolConfigQos wire_protocol_qos;
+
+    // Try to set easy mode IP using an invalid IP address
+    wire_protocol_qos.easy_mode("Foo");
+
+    ASSERT_TRUE(wire_protocol_qos.easy_mode().empty());
+}

test/unittest/xmlparser/XMLParserTests.cpp

@@ -1813,6 +1813,7 @@ TEST_F(XMLParserTests, parseLogConfig)
  * 2. Check missing DomainId value in tag
  * 3. Check bad values for all attributes
  * 4. Check a non existant attribute tag
+ * 5. Check a non valid Easy Mode IP
  */
 TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses)
 {
@@ -1885,6 +1886,13 @@ TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses)
             EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParserTest::fillDataNode_wrapper(titleElement, *participant_node));
 
         }
+
+        // Check invalid easy_mode_ip value (not a valid IPv4 address)
+        std::string invalid_easy_mode_ip = "<easy_mode_ip>Foo</easy_mode_ip>";
+        snprintf(xml, xml_len, xml_p, invalid_easy_mode_ip.c_str());
+        ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml));
+        titleElement = xml_doc.RootElement();
+        EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParserTest::fillDataNode_wrapper(titleElement, *participant_node));
     }
 }
 

versions.md

@@ -17,7 +17,7 @@ Forthcoming
   * New `ROS2_EASY_MODE` environment variable
   * Extended CLI ``discovery`` command to handle Fast DDS daemon.
   * Added P2P builtin transport.
-  * Added new `easy_mode_ip` XML tag and `easy_mode_` member in `WireProtocolConfigQos` for Discovery Server IP configuration. 
+  * Added new `easy_mode_ip` XML tag and `easy_mode` setter in `WireProtocolConfigQos` for Easy Mode IP configuration through XML and code.
 
 Version v3.1.0
 --------------