Allow participant profiles with no rtps tag (#3771) (#3781)

* Refs #19315: Allow participant profiles with no rtps tag Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19315: Apply suggestions Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19315: Apply suggestions 2 Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> --------- Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> (cherry picked from commit 9bcd313) Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
eProsima · Sep 9, 2023 · c27a563 · c27a563
1 parent c70c904
commit c27a563
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 22 deletions.
diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp
@@ -1756,29 +1756,64 @@ XMLP_ret XMLParser::fillDataNode(
     addAllAttributes(p_profile, participant_node);
 
     uint8_t ident = 1;
-    tinyxml2::XMLElement* p_element = p_profile->FirstChildElement(DOMAIN_ID);
-    if (nullptr != p_element)
+    tinyxml2::XMLElement* p_element;
+    tinyxml2::XMLElement* p_aux0 = nullptr;
+    const char* name = nullptr;
+    std::set<std::string> tags_present;
+
+    /*
+     * The only allowed elements are <domainId> and <rtps>
+     *   - The min occurrences of <domainId> are 0, and its max is 1; look for it.
+     *   - The min occurrences of <rtps> are 0, and its max is 1; look for it.
+     */
+    for (p_element = p_profile->FirstChildElement(); p_element != nullptr; p_element = p_element->NextSiblingElement())
     {
-        // domainId - uint32Type
-        if (XMLP_ret::XML_OK != getXMLUint(p_element, &participant_node.get()->domainId, ident))
+        name = p_element->Name();
+        if (tags_present.count(name) != 0)
         {
+            logError(XMLPARSER, "Duplicated element found in 'participant'. Tag: " << name);
+            return XMLP_ret::XML_ERROR;
+        }
+        tags_present.emplace(name);
+
+        if (strcmp(p_element->Name(), DOMAIN_ID) == 0)
+        {
+            // domainId - uint32Type
+            if (XMLP_ret::XML_OK != getXMLUint(p_element, &participant_node.get()->domainId, ident))
+            {
+                return XMLP_ret::XML_ERROR;
+            }
+        }
+        else if (strcmp(p_element->Name(), RTPS) == 0)
+        {
+            p_aux0 = p_element;
+        }
+        else
+        {
+            logError(XMLPARSER, "Found incorrect tag '" << p_element->Name() << "'");
             return XMLP_ret::XML_ERROR;
         }
     }
+    tags_present.clear();
 
-    p_element = p_profile->FirstChildElement(RTPS);
-    if (nullptr == p_element)
+    // <rtps> is not present, but that's OK
+    if (nullptr == p_aux0)
     {
-        logError(XMLPARSER, "Not found '" << RTPS << "' tag");
-        return XMLP_ret::XML_ERROR;
+        return XMLP_ret::XML_OK;
     }
 
-    tinyxml2::XMLElement* p_aux0 = nullptr;
-    const char* name = nullptr;
-    for (p_aux0 = p_element->FirstChildElement(); p_aux0 != nullptr; p_aux0 = p_aux0->NextSiblingElement())
+    // Check contents of <rtps>
+    for (p_aux0 = p_aux0->FirstChildElement(); p_aux0 != nullptr; p_aux0 = p_aux0->NextSiblingElement())
     {
         name = p_aux0->Name();
 
+        if (tags_present.count(name) != 0)
+        {
+            logError(XMLPARSER, "Duplicated element found in 'rtpsParticipantAttributesType'. Tag: " << name);
+            return XMLP_ret::XML_ERROR;
+        }
+        tags_present.emplace(name);
+
         if (strcmp(name, ALLOCATION) == 0)
         {
             // allocation

diff --git a/test/unittest/xmlparser/XMLParserTests.cpp b/test/unittest/xmlparser/XMLParserTests.cpp
@@ -61,6 +61,8 @@ TEST_F(XMLParserTests, regressions)
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/14456_profile_bin.xml", root));
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/15344_profile_bin.xml", root));
     EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/18395_profile_bin.xml", root));
+    EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/simple_participant_profiles_nok.xml", root));
+    EXPECT_EQ(XMLP_ret::XML_OK, XMLParser::loadXML("regressions/simple_participant_profiles_ok.xml", root));
 }
 
 TEST_F(XMLParserTests, NoFile)
@@ -1315,10 +1317,9 @@ TEST_F(XMLParserTests, parseLogConfig)
 /*
  * This test checks the return of the negative cases of the fillDataNode given a ParticipantAttributes DataNode
  * 1. Check passing a nullptr as if the XMLElement was wrongly parsed above
- * 2. Check missing required rtps tag
- * 3. Check missing DomainId value in tag
- * 4. Check bad values for all attributes
- * 5. Check a non existant attribute tag
+ * 2. Check missing DomainId value in tag
+ * 3. Check bad values for all attributes
+ * 4. Check a non existant attribute tag
  */
 TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses)
 {
@@ -1340,12 +1341,6 @@ TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses)
                 ";
         char xml[500];
 
-        // Misssing rtps tag
-        sprintf(xml, xml_p, "<domainId>0</domainId>");
-        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));
-
         // Misssing DomainId Value
         sprintf(xml, xml_p, "<domainId></domainId><rtps></rtps>");
         ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml));
@@ -1463,6 +1458,7 @@ TEST_F(XMLParserTests, parseXMLProfilesRoot)
     // <topic>, <requester>, <replier>, <types>, and <log> are read as xml child elements of the <profiles>
     // root element.
     std::vector<std::string> elements_ok {
+        "participant",
         "publisher",
         "data_writer",
         "subscriber",
@@ -1480,7 +1476,6 @@ TEST_F(XMLParserTests, parseXMLProfilesRoot)
 
     std::vector<std::string> elements_error {
         "library_settings",
-        "participant",
         "requester",
         "replier"
     };

diff --git a/test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml b/test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dds xmlns="http://www.eprosima.com/XMLSchemas/fastRTPS_Profiles">
+    <profiles>
+        <participant profile_name="duplicated_domainid_participant">
+            <domainId>1</domainId>
+            <domainId>2</domainId>
+        </participant>
+
+        <participant profile_name="duplicated_rtps_participant">
+            <rtps></rtps>
+            <rtps></rtps>
+        </participant>
+    </profiles>
+</dds>
diff --git a/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml b/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dds xmlns="http://www.eprosima.com/XMLSchemas/fastRTPS_Profiles">
+    <profiles>
+        <participant profile_name="empty_participant">
+        </participant>
+
+        <participant profile_name="only_rtps_participant">
+            <rtps></rtps>
+        </participant>
+
+        <participant profile_name="only_rtps_participant">
+            <rtps/>
+        </participant>
+
+        <participant profile_name="only_domainid_participant">
+            <domainId>1</domainId>
+        </participant>
+
+        <participant profile_name="complete_participant">
+            <domainId>1</domainId>
+            <rtps></rtps>
+        </participant>
+    </profiles>
+</dds>