From 4103d05c7f7f3950a3cc819f43f519be1a5160e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Wed, 7 Dec 2022 18:00:45 +0100 Subject: [PATCH] improved `ErrorMessage::deserialize()` error messages (#4617) --- cli/processexecutor.cpp | 5 +---- lib/errorlogger.cpp | 24 +++++++++++------------- lib/errorlogger.h | 2 +- test/testerrorlogger.cpp | 37 ++++++++++++++++++++++++++----------- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/cli/processexecutor.cpp b/cli/processexecutor.cpp index 5cbed00a1..707a9c6d1 100644 --- a/cli/processexecutor.cpp +++ b/cli/processexecutor.cpp @@ -162,10 +162,7 @@ int ProcessExecutor::handleRead(int rpipe, unsigned int &result) } else if (type == PipeWriter::REPORT_ERROR || type == PipeWriter::REPORT_INFO) { ErrorMessage msg; try { - if (!msg.deserialize(buf)) { - std::cerr << "#### ThreadExecutor::handleRead error" << std::endl; - std::exit(EXIT_FAILURE); - } + msg.deserialize(buf); } catch (const InternalError& e) { std::cerr << "#### ThreadExecutor::handleRead error, internal error:" << e.errorMessage << std::endl; std::exit(EXIT_FAILURE); diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index a69c6f356..c5352deed 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -256,7 +256,7 @@ std::string ErrorMessage::serialize() const return oss.str(); } -bool ErrorMessage::deserialize(const std::string &data) +void ErrorMessage::deserialize(const std::string &data) { // TODO: clear all fields certainty = Certainty::normal; @@ -268,13 +268,13 @@ bool ErrorMessage::deserialize(const std::string &data) while (iss.good() && elem < 7) { unsigned int len = 0; if (!(iss >> len)) - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid length"); if (iss.get() != ' ') - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid separator"); if (!iss.good()) - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - premature end of data"); std::string temp; for (unsigned int i = 0; i < len && iss.good(); ++i) { @@ -283,7 +283,7 @@ bool ErrorMessage::deserialize(const std::string &data) } if (!iss.good()) - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - premature end of data"); if (temp == "inconclusive") { certainty = Certainty::inconclusive; @@ -294,7 +294,7 @@ bool ErrorMessage::deserialize(const std::string &data) } if (!iss.good()) - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - premature end of data"); if (elem != 7) throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - insufficient elements"); @@ -311,18 +311,18 @@ bool ErrorMessage::deserialize(const std::string &data) unsigned int stackSize = 0; if (!(iss >> stackSize)) - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid stack size"); if (iss.get() != ' ') - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid separator"); if (stackSize == 0) - return true; + return; while (iss.good()) { unsigned int len = 0; if (!(iss >> len)) - return false; + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid length (stack)"); iss.get(); std::string temp; @@ -346,7 +346,7 @@ bool ErrorMessage::deserialize(const std::string &data) substrings.push_back(temp.substr(start, pos - start)); } if (substrings.size() < 4) - throw InternalError(nullptr, "Internal Error: serializing/deserializing of error message failed!"); + throw InternalError(nullptr, "Internal Error: Deserializing of error message failed"); // (*loc).line << '\t' << (*loc).column << '\t' << (*loc).getfile(false) << '\t' << loc->getOrigFile(false) << '\t' << loc->getinfo(); @@ -360,8 +360,6 @@ bool ErrorMessage::deserialize(const std::string &data) if (callStack.size() >= stackSize) break; } - - return true; } std::string ErrorMessage::getXMLHeader(const std::string& productName) diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 7a2e0a566..d8ae116d3 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -165,7 +165,7 @@ public: const std::string &templateLocation = emptyString) const; std::string serialize() const; - bool deserialize(const std::string &data); + void deserialize(const std::string &data); std::list callStack; std::string id; diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 10578d5f7..64746f645 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -286,6 +286,8 @@ private: std::list locs; ErrorMessage msg(locs, emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); msg.file0 = "test.cpp"; + + const std::string msg_str = msg.serialize(); ASSERT_EQUALS("7 errorId" "5 error" "1 0" @@ -294,10 +296,10 @@ private: "12 inconclusive" "17 Programming error" "17 Programming error" - "0 ", msg.serialize()); + "0 ", msg_str); ErrorMessage msg2; - ASSERT(msg2.deserialize(msg.serialize())); + ASSERT_NO_THROW(msg2.deserialize(msg_str)); ASSERT_EQUALS("errorId", msg2.id); ASSERT_EQUALS(Severity::error, msg2.severity); ASSERT_EQUALS("test.cpp", msg2.file0); @@ -311,22 +313,22 @@ private: // missing/invalid length // missing separator ErrorMessage msg; - ASSERT(!msg.deserialize("500foobar")); + ASSERT_THROW_EQUALS(msg.deserialize("500foobar"), InternalError, "Internal Error: Deserialization of error message failed - invalid separator"); } { // invalid length ErrorMessage msg; - ASSERT(!msg.deserialize("foo foobar")); + ASSERT_THROW_EQUALS(msg.deserialize("foo foobar"), InternalError, "Internal Error: Deserialization of error message failed - invalid length"); } { // mismatching length ErrorMessage msg; - ASSERT(!msg.deserialize("8 errorId")); + ASSERT_THROW_EQUALS(msg.deserialize("8 errorId"), InternalError, "Internal Error: Deserialization of error message failed - premature end of data"); } { // incomplete message ErrorMessage msg; - ASSERT(!msg.deserialize("7 errorId")); + ASSERT_THROW_EQUALS(msg.deserialize("7 errorId"), InternalError, "Internal Error: Deserialization of error message failed - invalid length"); } { // invalid CWE ID @@ -339,7 +341,7 @@ private: "17 Programming error" "0 "; ErrorMessage msg; - ASSERT_THROW(msg.deserialize(str), InternalError); + ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid CWE ID"); } { // invalid hash @@ -352,7 +354,7 @@ private: "17 Programming error" "0 "; ErrorMessage msg; - ASSERT_THROW(msg.deserialize(str), InternalError); + ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid hash"); } } @@ -361,6 +363,7 @@ private: ErrorMessage msg(locs, emptyString, Severity::error, std::string("Illegal character in \"foo\001bar\""), "errorId", Certainty::normal); msg.file0 = "1.c"; + const std::string msg_str = msg.serialize(); ASSERT_EQUALS("7 errorId" "5 error" "1 0" @@ -368,10 +371,10 @@ private: "3 1.c" "33 Illegal character in \"foo\\001bar\"" "33 Illegal character in \"foo\\001bar\"" - "0 ", msg.serialize()); + "0 ", msg_str); ErrorMessage msg2; - ASSERT(msg2.deserialize(msg.serialize())); + ASSERT_NO_THROW(msg2.deserialize(msg_str)); ASSERT_EQUALS("errorId", msg2.id); ASSERT_EQUALS(Severity::error, msg2.severity); ASSERT_EQUALS("1.c", msg2.file0); @@ -388,8 +391,20 @@ private: ErrorMessage msg(locs, emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); + const std::string msg_str = msg.serialize(); + ASSERT_EQUALS("7 errorId" + "5 error" + "1 0" + "1 0" + "0 " + "12 inconclusive" + "17 Programming error" + "17 Programming error" + "1 " + "27 654\t33\t[]:;,()\t:/,;\tabcd:/,", msg_str); + ErrorMessage msg2; - ASSERT(msg2.deserialize(msg.serialize())); + ASSERT_NO_THROW(msg2.deserialize(msg_str)); ASSERT_EQUALS("[]:;,()", msg2.callStack.front().getfile(false)); ASSERT_EQUALS(":/,;", msg2.callStack.front().getOrigFile(false)); ASSERT_EQUALS(654, msg2.callStack.front().line);