Skip to content

Commit

Permalink
improved handling/testing of XMLs without a a root node / added some …
Browse files Browse the repository at this point in the history 
…TODOs
  • Loading branch information
@firewave
firewave committed Mar 5, 2024
1 parent f3cccd0 commit 4cb57e6
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 7 deletions.
6 changes: 4 additions & 2 deletions cli/cmdlineparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1040,11 +1040,13 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
// Rule file
else if (std::strncmp(argv[i], "--rule-file=", 12) == 0) {
#ifdef HAVE_RULES
// TODO: improved error handling - unknown elements, wrong root node, etc.
const std::string ruleFile = argv[i] + 12;
tinyxml2::XMLDocument doc;
const tinyxml2::XMLError err = doc.LoadFile(ruleFile.c_str());
if (err == tinyxml2::XML_SUCCESS) {
tinyxml2::XMLElement *node = doc.FirstChildElement();
const tinyxml2::XMLElement *node = doc.FirstChildElement();
// TODO: this looks like legacy handling - deprecate it
if (node && strcmp(node->Value(), "rules") == 0)
node = node->FirstChildElement("rule");
for (; node && strcmp(node->Value(), "rule") == 0; node = node->NextSiblingElement()) {
Expand All @@ -1059,7 +1061,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
rule.pattern = pattern->GetText();
}

tinyxml2::XMLElement *message = node->FirstChildElement("message");
const tinyxml2::XMLElement *message = node->FirstChildElement("message");
if (message) {
const tinyxml2::XMLElement *severity = message->FirstChildElement("severity");
if (severity)
Expand Down
11 changes: 6 additions & 5 deletions lib/suppressions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,16 @@ std::string SuppressionList::parseXmlFile(const char *filename)
{
tinyxml2::XMLDocument doc;
const tinyxml2::XMLError error = doc.LoadFile(filename);
if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND)
return "File not found";
if (error != tinyxml2::XML_SUCCESS)
return "Failed to parse XML file";
return std::string("failed to load suppressions XML '") + filename + "' (" + tinyxml2::XMLDocument::ErrorIDToName(error) + ").";

const tinyxml2::XMLElement * const rootnode = doc.FirstChildElement();
if (!rootnode)
return std::string("failed to load suppressions XML '") + filename + "' (no root node found).";
// TODO: check for proper root node 'suppressions'
for (const tinyxml2::XMLElement * e = rootnode->FirstChildElement(); e; e = e->NextSiblingElement()) {
if (std::strcmp(e->Name(), "suppress") != 0)
return "Invalid suppression xml file format, expected <suppress> element but got a \"" + std::string(e->Name()) + '\"';
return std::string("invalid suppression xml file '") + filename + "', expected 'suppress' element but got a '" + e->Name() + "'.";

Suppression s;
for (const tinyxml2::XMLElement * e2 = e->FirstChildElement(); e2; e2 = e2->NextSiblingElement()) {
Expand All @@ -127,7 +128,7 @@ std::string SuppressionList::parseXmlFile(const char *filename)
else if (*text && std::strcmp(e2->Name(), "hash") == 0)
s.hash = strToInt<std::size_t>(text);
else
return "Unknown suppression element \"" + std::string(e2->Name()) + "\", expected id/fileName/lineNumber/symbolName/hash";
return std::string("unknown element '") + e2->Name() + "' in suppressions XML '" + filename + "', expected id/fileName/lineNumber/symbolName/hash.";
}

const std::string err = addSuppression(std::move(s));
Expand Down
59 changes: 59 additions & 0 deletions test/testcmdlineparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ class TestCmdlineParser : public TestFixture {
TEST_CASE(ruleFileEmpty);
TEST_CASE(ruleFileMissing);
TEST_CASE(ruleFileInvalid);
TEST_CASE(ruleFileNoRoot);
#else
TEST_CASE(ruleFileNotSupported);
#endif
Expand All @@ -348,6 +349,11 @@ class TestCmdlineParser : public TestFixture {
TEST_CASE(signedCharUnsignedChar);
TEST_CASE(library);
TEST_CASE(libraryMissing);
TEST_CASE(suppressXml);
TEST_CASE(suppressXmlEmpty);
TEST_CASE(suppressXmlMissing);
TEST_CASE(suppressXmlInvalid);
TEST_CASE(suppressXmlNoRoot);

TEST_CASE(ignorepaths1);
TEST_CASE(ignorepaths2);
Expand Down Expand Up @@ -2162,6 +2168,13 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' (XML_ERROR_EMPTY_DOCUMENT).\n", logger->str());
}

void ruleFileNoRoot() {
REDIRECT;
ScopedFile file("rule.xml", "<?xml version=\"1.0\"?>");
const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
}
#else
void ruleFileNotSupported() {
REDIRECT;
Expand All @@ -2188,6 +2201,52 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS("cppcheck: Failed to load library configuration file 'posix2'. File not found\n", logger->str());
}

void suppressXml() {
REDIRECT;
ScopedFile file("suppress.xml",
"<suppressions>\n"
"<suppress>\n"
"<id>uninitvar</id>\n"
"</suppress>\n"
"</suppressions>");
const char * const argv[] = {"cppcheck", "--suppress-xml=suppress.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
const auto& supprs = settings->supprs.nomsg.getSuppressions();
ASSERT_EQUALS(1, supprs.size());
const auto it = supprs.cbegin();
ASSERT_EQUALS("uninitvar", it->errorId);
}

void suppressXmlEmpty() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--suppress-xml=", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: failed to load suppressions XML '' (XML_ERROR_FILE_NOT_FOUND).\n", logger->str());
}

void suppressXmlMissing() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--suppress-xml=suppress.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: failed to load suppressions XML 'suppress.xml' (XML_ERROR_FILE_NOT_FOUND).\n", logger->str());
}

void suppressXmlInvalid() {
REDIRECT;
ScopedFile file("suppress.xml", "");
const char * const argv[] = {"cppcheck", "--suppress-xml=suppress.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: failed to load suppressions XML 'suppress.xml' (XML_ERROR_EMPTY_DOCUMENT).\n", logger->str());
}

void suppressXmlNoRoot() {
REDIRECT;
ScopedFile file("suppress.xml", "<?xml version=\"1.0\"?>");
const char * const argv[] = {"cppcheck", "--suppress-xml=suppress.xml", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
ASSERT_EQUALS("cppcheck: error: failed to load suppressions XML 'suppress.xml' (no root node found).\n", logger->str());
}

void ignorepaths1() {
REDIRECT;
const char * const argv[] = {"cppcheck", "-isrc", "file.cpp"};
Expand Down
2 changes: 2 additions & 0 deletions test/testerrorlogger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ class TestErrorLogger : public TestFixture {
"</error>";
tinyxml2::XMLDocument doc;
ASSERT(doc.Parse(xmldata, sizeof(xmldata)) == tinyxml2::XML_SUCCESS);
const auto * const rootnode = doc.FirstChildElement();
ASSERT(rootnode);
ErrorMessage msg(doc.FirstChildElement());
ASSERT_EQUALS("errorId", msg.id);
ASSERT_EQUALS_ENUM(Severity::error, msg.severity);
Expand Down
3 changes: 3 additions & 0 deletions test/testlibrary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,9 @@ class TestLibrary : public TestFixture {

void loadLibErrors() const {

LOADLIBERROR("<?xml version=\"1.0\"?>",
Library::ErrorCode::BAD_XML);

LOADLIBERROR("<?xml version=\"1.0\"?>\n"
"<def>\n"
" <X name=\"uint8_t,std::uint8_t\" size=\"1\"/>\n"
Expand Down
15 changes: 15 additions & 0 deletions test/testplatform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class TestPlatform : public TestFixture {
TEST_CASE(default_platform);
TEST_CASE(limitsDefines);
TEST_CASE(charMinMax);
TEST_CASE(no_root_node);
TEST_CASE(wrong_root_node);
}

static bool readPlatform(Platform& platform, const char* xmldata) {
Expand Down Expand Up @@ -417,6 +419,19 @@ class TestPlatform : public TestFixture {
ASSERT_EQUALS(127, platform.signedCharMax());
ASSERT_EQUALS(-128, platform.signedCharMin());
}

void no_root_node() const {
constexpr char xmldata[] = "<?xml version=\"1.0\"?>";
Platform platform;
ASSERT(!readPlatform(platform, xmldata));
}

void wrong_root_node() const {
constexpr char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<platforms/>";
Platform platform;
ASSERT(!readPlatform(platform, xmldata));
}
};

REGISTER_TEST(TestPlatform)
78 changes: 78 additions & 0 deletions test/testsuppressions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ class TestSuppressions : public TestFixture {
TEST_CASE(suppressLocal);

TEST_CASE(suppressUnmatchedSuppressions);

TEST_CASE(suppressionsParseXmlFile);
}

void suppressionsBadId1() const {
Expand Down Expand Up @@ -1462,6 +1464,82 @@ class TestSuppressions : public TestFixture {
ASSERT_EQUALS(true, SuppressionList::reportUnmatchedSuppressions(suppressions, *this));
ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str());
}

void suppressionsParseXmlFile() {
{
ScopedFile file("suppressparsexml.xml",
"<suppressions>\n"
"<suppress>\n"
"<id>uninitvar</id>\n"
"<fileName>file.c</fileName>\n"
"<lineNumber>10</lineNumber>\n"
"<symbolName>sym</symbolName>\n"
"</suppress>\n"
"</suppressions>");

SuppressionList supprList;
ASSERT_EQUALS("", supprList.parseXmlFile(file.path().c_str()));
const auto& supprs = supprList.getSuppressions();
ASSERT_EQUALS(1, supprs.size());
const auto& suppr = *supprs.cbegin();
ASSERT_EQUALS("uninitvar", suppr.errorId);
ASSERT_EQUALS("file.c", suppr.fileName);
ASSERT_EQUALS(10, suppr.lineNumber);
ASSERT_EQUALS("sym", suppr.symbolName);
}

// no file specified
{
SuppressionList supprList;
ASSERT_EQUALS("failed to load suppressions XML '' (XML_ERROR_FILE_NOT_FOUND).", supprList.parseXmlFile(""));
}

// missing file
{
SuppressionList supprList;
ASSERT_EQUALS("failed to load suppressions XML 'suppressparsexml.xml' (XML_ERROR_FILE_NOT_FOUND).", supprList.parseXmlFile("suppressparsexml.xml"));
}

// empty file
{
ScopedFile file("suppressparsexml.xml",
"");

SuppressionList supprList;
ASSERT_EQUALS("failed to load suppressions XML 'suppressparsexml.xml' (XML_ERROR_EMPTY_DOCUMENT).", supprList.parseXmlFile(file.path().c_str()));
}

// wrong root node
{
ScopedFile file("suppressparsexml.xml",
"<suppress/>\n");

SuppressionList supprList;
ASSERT_EQUALS("", supprList.parseXmlFile(file.path().c_str()));
}

// no root node
{
ScopedFile file("suppressparsexml.xml",
"<?xml version=\"1.0\"?>\n");

SuppressionList supprList;
ASSERT_EQUALS("failed to load suppressions XML 'suppressparsexml.xml' (no root node found).", supprList.parseXmlFile(file.path().c_str()));
}

// unknown element
{
ScopedFile file("suppressparsexml.xml",
"<suppressions>\n"
"<suppress>\n"
"<eid>uninitvar</eid>\n"
"</suppress>\n"
"</suppressions>");

SuppressionList supprList;
ASSERT_EQUALS("unknown element 'eid' in suppressions XML 'suppressparsexml.xml', expected id/fileName/lineNumber/symbolName/hash.", supprList.parseXmlFile(file.path().c_str()));
}
}
};

REGISTER_TEST(TestSuppressions)

0 comments on commit 4cb57e6

Please sign in to comment.