diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp index 9eba6315e91..f82b731d891 100644 --- a/src/cpp/rtps/xmlparser/XMLParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLParser.cpp @@ -1771,39 +1771,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 tags_present; + + /* + * The only allowed elements are and + * - The min occurrences of are 0, and its max is 1; look for it. + * - The min occurrences of 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) { + EPROSIMA_LOG_ERROR(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 + { + EPROSIMA_LOG_ERROR(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) + // is not present, but that's OK + if (nullptr == p_aux0) { - EPROSIMA_LOG_ERROR(XMLPARSER, "Not found '" << RTPS << "' tag"); - return XMLP_ret::XML_ERROR; + return XMLP_ret::XML_OK; } - tinyxml2::XMLElement* p_aux0 = nullptr; - const char* name = nullptr; - - std::unordered_map tags_present; - - for (p_aux0 = p_element->FirstChildElement(); p_aux0 != nullptr; p_aux0 = p_aux0->NextSiblingElement()) + // Check contents of + for (p_aux0 = p_aux0->FirstChildElement(); p_aux0 != nullptr; p_aux0 = p_aux0->NextSiblingElement()) { name = p_aux0->Name(); - if (tags_present[name]) + if (tags_present.count(name) != 0) { EPROSIMA_LOG_ERROR(XMLPARSER, - "Duplicated element found in 'rtpsParticipantAttributesType'. Name: " << name); + "Duplicated element found in 'rtpsParticipantAttributesType'. Tag: " << name); return XMLP_ret::XML_ERROR; } - tags_present[name] = true; + tags_present.emplace(name); if (strcmp(name, ALLOCATION) == 0) { diff --git a/test/unittest/xmlparser/XMLParserTests.cpp b/test/unittest/xmlparser/XMLParserTests.cpp index 2c58d8fb315..d210e092042 100644 --- 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) @@ -1611,10 +1613,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) { @@ -1636,12 +1637,6 @@ TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses) "; char xml[500]; - // Misssing rtps tag - sprintf(xml, xml_p, "0"); - 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, ""); ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml)); @@ -1759,6 +1754,7 @@ TEST_F(XMLParserTests, parseXMLProfilesRoot) // , , , , and are read as xml child elements of the // root element. std::vector elements_ok { + "participant", "publisher", "data_writer", "subscriber", @@ -1776,7 +1772,6 @@ TEST_F(XMLParserTests, parseXMLProfilesRoot) std::vector 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 new file mode 100644 index 00000000000..6c8f945c202 --- /dev/null +++ b/test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml @@ -0,0 +1,14 @@ + + + + + 1 + 2 + + + + + + + + diff --git a/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml b/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml new file mode 100644 index 00000000000..ef1729dde01 --- /dev/null +++ b/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + 1 + + + + 1 + + + +