diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp index 0826569fad6..9eba6315e91 100644 --- a/src/cpp/rtps/xmlparser/XMLParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLParser.cpp @@ -1390,23 +1390,28 @@ XMLP_ret XMLParser::parseLogConfig( tinyxml2::XMLElement* p_root) { /* - - - - - - - - - - - - - - + + + + + + + + + + */ + + /* + * TODO(eduponz): Uphold XSD validation in parsing + * Even though the XSD above enforces the log tag to have at least one consumer, + * the parsing allows for an empty log tag (e.g. ``). + * This inconsistency is kept to keep a backwards compatible behaviour. + * In fact, test XMLParserTests.parseXMLNoRoot even checks that an empty log tag + * is valid. */ XMLP_ret ret = XMLP_ret::XML_OK; + tinyxml2::XMLElement* p_aux0 = p_root->FirstChildElement(LOG); if (p_aux0 == nullptr) { @@ -1414,31 +1419,37 @@ XMLP_ret XMLParser::parseLogConfig( } tinyxml2::XMLElement* p_element = p_aux0->FirstChildElement(); - const char* tag = nullptr; - while (nullptr != p_element) + + while (ret == XMLP_ret::XML_OK && nullptr != p_element) { - if (nullptr != (tag = p_element->Value())) + const char* tag = p_element->Value(); + if (nullptr != tag) { if (strcmp(tag, USE_DEFAULT) == 0) { - bool use_default = true; - std::string auxBool = p_element->GetText(); - if (std::strcmp(auxBool.c_str(), "FALSE") == 0) + if (nullptr == p_element->GetText()) { - use_default = false; + EPROSIMA_LOG_ERROR(XMLPARSER, "Cannot get text from tag: '" << tag << "'"); + ret = XMLP_ret::XML_ERROR; } - if (!use_default) + + if (ret == XMLP_ret::XML_OK) { - eprosima::fastdds::dds::Log::ClearConsumers(); + bool use_default = true; + std::string auxBool = p_element->GetText(); + if (std::strcmp(auxBool.c_str(), "FALSE") == 0) + { + use_default = false; + } + if (!use_default) + { + eprosima::fastdds::dds::Log::ClearConsumers(); + } } } else if (strcmp(tag, CONSUMER) == 0) { ret = parseXMLConsumer(*p_element); - if (ret == XMLP_ret::XML_ERROR) - { - return ret; - } } else { @@ -1446,8 +1457,13 @@ XMLP_ret XMLParser::parseLogConfig( ret = XMLP_ret::XML_ERROR; } } - p_element = p_element->NextSiblingElement(CONSUMER); + + if (ret == XMLP_ret::XML_OK) + { + p_element = p_element->NextSiblingElement(CONSUMER); + } } + return ret; } diff --git a/test/unittest/xmlparser/XMLParserTests.cpp b/test/unittest/xmlparser/XMLParserTests.cpp index a8c3a1d11e0..2c58d8fb315 100644 --- a/test/unittest/xmlparser/XMLParserTests.cpp +++ b/test/unittest/xmlparser/XMLParserTests.cpp @@ -60,6 +60,7 @@ TEST_F(XMLParserTests, regressions) EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/13513_profile_bin.xml", root)); 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)); } TEST_F(XMLParserTests, NoFile) diff --git a/test/unittest/xmlparser/regressions/18395_profile_bin.xml b/test/unittest/xmlparser/regressions/18395_profile_bin.xml new file mode 100644 index 00000000000..9ff3a6a8fda --- /dev/null +++ b/test/unittest/xmlparser/regressions/18395_profile_bin.xml @@ -0,0 +1,2 @@ +v�< log>suu/use_de�ta-g/>��< /log>��ypes/������ +