Skip to content

Commit 3836b2b

Browse files
firewavefirewave
authored
fixed #14638 - report errors for invalid suppression lines (danmar#8404)
1 parent 21d4c79 commit 3836b2b

File tree

3 files changed

+93
-29
lines changed

3 files changed

+93
-29
lines changed

‎lib/suppressions.cpp‎

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,8 @@ std::vector<SuppressionList::Suppression> SuppressionList::parseMultiSuppressCom
207207
return suppressions;
208208
}
209209

210-
SuppressionList::Suppression SuppressionList::parseLine(const std::string &line)
210+
SuppressionList::Suppression SuppressionList::parseLine(std::string line)
211211
{
212-
std::istringstream lineStream;
213212
SuppressionList::Suppression suppression;
214213

215214
// Strip any end of line comments
@@ -218,13 +217,18 @@ SuppressionList::Suppression SuppressionList::parseLine(const std::string &line)
218217
while (endpos > 0 && std::isspace(line[endpos-1])) {
219218
endpos--;
220219
}
221-
lineStream.str(line.substr(0, endpos));
222-
} else {
223-
lineStream.str(line);
220+
line.resize(endpos);
224221
}
225222

226-
if (std::getline(lineStream, suppression.errorId, ':')) {
227-
if (std::getline(lineStream, suppression.fileName)) {
223+
const auto parts = splitString(line, '\n');
224+
const std::string& suppr_l = parts[0];
225+
226+
const std::string::size_type first_sep = suppr_l.find(':');
227+
suppression.errorId = suppr_l.substr(0, first_sep);
228+
if (first_sep != std::string::npos) {
229+
suppression.fileName = suppr_l.substr(first_sep+1);
230+
if (!suppression.fileName.empty()) {
231+
// TODO: this only works with files which have an extension
228232
// If there is not a dot after the last colon in "file" then
229233
// the colon is a separator and the contents after the colon
230234
// is a line number..
@@ -234,39 +238,48 @@ SuppressionList::Suppression SuppressionList::parseLine(const std::string &line)
234238

235239
// if a colon is found and there is no dot after it..
236240
if (pos != std::string::npos &&
237-
suppression.fileName.find('.', pos) == std::string::npos) {
238-
// Try to parse out the line number
239-
try {
240-
std::istringstream istr1(suppression.fileName.substr(pos+1));
241-
istr1 >> suppression.lineNumber;
242-
} catch (...) {
243-
suppression.lineNumber = SuppressionList::Suppression::NO_LINE;
244-
}
241+
suppression.fileName.find('.', pos) == std::string::npos)
242+
{
243+
// parse out the line number
244+
const std::string line_s = suppression.fileName.substr(pos+1);
245+
suppression.fileName.erase(pos);
245246

246-
if (suppression.lineNumber != SuppressionList::Suppression::NO_LINE) {
247-
suppression.fileName.erase(pos);
248-
}
249-
}
247+
if (suppression.fileName.empty())
248+
throw std::runtime_error("filename is missing");
250249

251-
// when parsing string generated internally by toString() there can be newline
252-
std::string extra;
253-
while (std::getline(lineStream, extra)) {
254-
if (startsWith(extra, "symbol="))
255-
suppression.symbolName = extra.substr(7);
256-
else if (extra == "polyspace=1")
257-
suppression.isPolyspace = true;
250+
try {
251+
suppression.lineNumber = strToInt<int>(line_s);
252+
} catch (const std::runtime_error& e) {
253+
throw std::runtime_error(std::string("invalid line number (") + e.what() + ")");
254+
}
258255
}
256+
suppression.fileName = Path::simplifyPath(suppression.fileName);
257+
} else {
258+
throw std::runtime_error("filename is missing");
259259
}
260260
}
261261

262-
suppression.fileName = Path::simplifyPath(suppression.fileName);
262+
// TODO: make this optional - do we even encounter this in production code?
263+
// when parsing string generated internally by toString() there can be newline
264+
for (std::size_t i = 1; i < parts.size(); ++i) {
265+
if (startsWith(parts[i], "symbol="))
266+
suppression.symbolName = parts[i].substr(7);
267+
else if (parts[i] == "polyspace=1")
268+
suppression.isPolyspace = true;
269+
else
270+
throw std::runtime_error("unexpected extra '" + parts[i] + "'");
271+
}
263272

264273
return suppression;
265274
}
266275

267276
std::string SuppressionList::addSuppressionLine(const std::string &line)
268277
{
269-
return addSuppression(parseLine(line));
278+
try {
279+
return addSuppression(parseLine(line));
280+
} catch (const std::exception& e) {
281+
return e.what();
282+
}
270283
}
271284

272285
std::string SuppressionList::addSuppression(SuppressionList::Suppression suppression)

‎lib/suppressions.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,9 @@ class CPPCHECKLIB SuppressionList {
196196
* Create a Suppression object from a suppression line
197197
* @param line The line to parse.
198198
* @return a suppression object
199+
* @throws std::runtime_error thrown if the given suppression is invalid
199200
*/
200-
static Suppression parseLine(const std::string &line);
201+
static Suppression parseLine(std::string line);
201202

202203
/**
203204
* @brief Don't show the given error.

‎test/testsuppressions.cpp‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ class TestSuppressions : public TestFixture {
5858
void run() override {
5959
mNewTemplate = true;
6060
TEST_CASE(parseLine);
61+
TEST_CASE(parseLineInvalid);
6162
TEST_CASE(suppressionsBadId1);
6263
TEST_CASE(suppressionsDosFormat); // Ticket #1836
6364
TEST_CASE(suppressionsFileNameWithColon); // Ticket #1919 - filename includes colon
@@ -146,13 +147,62 @@ class TestSuppressions : public TestFixture {
146147

147148

148149
void parseLine() const {
150+
ASSERT_EQUALS("bad", SuppressionList::parseLine("bad").toString());
151+
ASSERT_EQUALS("bad:test.c", SuppressionList::parseLine("bad:test.c").toString());
149152
ASSERT_EQUALS("bad:test.c:1", SuppressionList::parseLine("bad:test.c:1").toString());
150153

151154
// symbol
155+
ASSERT_EQUALS("bad\nsymbol=x", SuppressionList::parseLine("bad\nsymbol=x").toString());
156+
ASSERT_EQUALS("bad:test.c\nsymbol=x", SuppressionList::parseLine("bad:test.c\nsymbol=x").toString());
152157
ASSERT_EQUALS("bad:test.c:1\nsymbol=x", SuppressionList::parseLine("bad:test.c:1\nsymbol=x").toString());
153158

159+
// empty symbol
160+
ASSERT_EQUALS("bad", SuppressionList::parseLine("bad\nsymbol=").toString());
161+
ASSERT_EQUALS("bad:test.c", SuppressionList::parseLine("bad:test.c\nsymbol=").toString());
162+
ASSERT_EQUALS("bad:test.c:1", SuppressionList::parseLine("bad:test.c:1\nsymbol=").toString());
163+
154164
// polyspace
165+
ASSERT_EQUALS("bad\npolyspace=1", SuppressionList::parseLine("bad\npolyspace=1").toString());
166+
ASSERT_EQUALS("bad:test.c\npolyspace=1", SuppressionList::parseLine("bad:test.c\npolyspace=1").toString());
155167
ASSERT_EQUALS("bad:test.c:1\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\npolyspace=1").toString());
168+
169+
// symbol + polyspace
170+
ASSERT_EQUALS("bad\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad\nsymbol=x\npolyspace=1").toString());
171+
ASSERT_EQUALS("bad:test.c\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c\nsymbol=x\npolyspace=1").toString());
172+
ASSERT_EQUALS("bad:test.c:1\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\nsymbol=x\npolyspace=1").toString());
173+
174+
// polyspace + symbol
175+
ASSERT_EQUALS("bad\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad\npolyspace=1\nsymbol=x").toString());
176+
ASSERT_EQUALS("bad:test.c\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c\npolyspace=1\nsymbol=x").toString());
177+
ASSERT_EQUALS("bad:test.c:1\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\npolyspace=1\nsymbol=x").toString());
178+
}
179+
180+
void parseLineInvalid() const {
181+
// missing filename
182+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:"), std::runtime_error, "filename is missing");
183+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:\n"), std::runtime_error, "filename is missing");
184+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:\n1.c"), std::runtime_error, "filename is missing");
185+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:#1.c"), std::runtime_error, "filename is missing"); // TODO: looks like a valid filename
186+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id://1.c"), std::runtime_error, "filename is missing");
187+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id::"), std::runtime_error, "filename is missing");
188+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id::1"), std::runtime_error, "filename is missing");
189+
190+
// missing/invalid line
191+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)");
192+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:\n"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)");
193+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:\n1"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)");
194+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:#1"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)"); // TODO: looks like a valid filename
195+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c://1"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)");
196+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:zero"), std::runtime_error, "invalid line number (converting 'zero' to integer failed - not an integer)");
197+
198+
// invalid extras
199+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\n"), std::runtime_error, "unexpected extra ''");
200+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\nsym=x"), std::runtime_error, "unexpected extra 'sym=x'");
201+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\nsymbol:x"), std::runtime_error, "unexpected extra 'symbol:x'");
202+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\npolyspace=0"), std::runtime_error, "unexpected extra 'polyspace=0'");
203+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\npolyspace:1"), std::runtime_error, "unexpected extra 'polyspace:1'");
204+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id\n:"), std::runtime_error, "unexpected extra ':'");
205+
ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c\n:"), std::runtime_error, "unexpected extra ':'");
156206
}
157207

158208
void suppressionsBadId1() const {

0 commit comments

Comments
 (0)