chromium/src
0
Fork 0
Code Activity

sql: Explicit DCHECKs for empty SQL statements.

Browse Source 
This CL adds DCHECKs with clear messages covering the cases when
Database::Get{Cached,Unique}Statement() and Database::IsSQLValid() are
passed SQL strings that don't contain any SQL statements.

Currently, Get{Cached,Unique}Statement() DCHECK with an obscure message
in the Database::StatementRef constructor, and Database::IsSQLValid()
does not DCHECK.

Bug: 1230443
Change-Id: I129769c849a441adeb6fcd45ed128e41a1673ef0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3042773
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#904204}
This commit is contained in:
Victor Costan
2021-07-22 06:34:17 +00:00
committed by Chromium LUCI CQ
parent 289f2c8b0d
commit 18f9e7a53b
2 changed files with 29 additions and 0 deletions

5
sql/database.cc

@ -1333,6 +1333,9 @@ scoped_refptr<Database::StatementRef> Database::GetStatementImpl(
<< "Unused text: " << std::string(unused_sql) << "\n"
<< "in prepared SQL statement: " << std::string(sql);
#endif // DCHECK_IS_ON()
DCHECK(sqlite_statement) << "No SQL statement in string: " << sql;
return base::MakeRefCounted<StatementRef>(this, sqlite_statement, true);
}
@ -1386,6 +1389,8 @@ bool Database::IsSQLValid(const char* sql) {
<< "in SQL statement: " << std::string(sql);
#endif // DCHECK_IS_ON()
DCHECK(sqlite_statement) << "No SQL statement in string: " << sql;
sqlite3_finalize(sqlite_statement);
return true;
}

24
sql/database_unittest.cc

@ -638,6 +638,30 @@ TEST_P(SQLDatabaseTest, IsSQLValid_ExtraContents) {
<< "Comment separated by whitespace";
}
TEST_P(SQLDatabaseTest, GetUniqueStatement_NoContents) {
EXPECT_DCHECK_DEATH(db_->GetUniqueStatement("")) << "Empty string";
EXPECT_DCHECK_DEATH(db_->GetUniqueStatement(" ")) << "Space";
EXPECT_DCHECK_DEATH(db_->GetUniqueStatement("\n")) << "Newline";
EXPECT_DCHECK_DEATH(db_->GetUniqueStatement("-- Comment")) << "Comment";
}
TEST_P(SQLDatabaseTest, GetCachedStatement_NoContents) {
EXPECT_DCHECK_DEATH(db_->GetCachedStatement(SQL_FROM_HERE, ""))
<< "Empty string";
EXPECT_DCHECK_DEATH(db_->GetCachedStatement(SQL_FROM_HERE, " ")) << "Space";
EXPECT_DCHECK_DEATH(db_->GetCachedStatement(SQL_FROM_HERE, "\n"))
<< "Newline";
EXPECT_DCHECK_DEATH(db_->GetCachedStatement(SQL_FROM_HERE, "-- Comment"))
<< "Comment";
}
TEST_P(SQLDatabaseTest, IsSQLValid_NoContents) {
EXPECT_DCHECK_DEATH(db_->IsSQLValid("")) << "Empty string";
EXPECT_DCHECK_DEATH(db_->IsSQLValid(" ")) << "Space";
EXPECT_DCHECK_DEATH(db_->IsSQLValid("\n")) << "Newline";
EXPECT_DCHECK_DEATH(db_->IsSQLValid("-- Comment")) << "Comment";
}
// Test that Database::Raze() results in a database without the
// tables from the original database.
TEST_P(SQLDatabaseTest, Raze) {