[sql] Additional Raze() unit tests.
Verify that Raze() can handle:
- empty databases.
- non-database files.
- databases overwritten with garbage.
- databases which cannot be opened successfully.
BUG=none
Review URL: https://chromiumcodereview.appspot.com/17752002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210923 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/sql/connection.cc b/sql/connection.cc
index 2a971e5..f04eb3b 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -68,6 +68,32 @@
sqlite3* db_;
};
+// Helper to wrap the sqlite3_backup_*() step of Raze(). Return
+// SQLite error code from running the backup step.
+int BackupDatabase(sqlite3* src, sqlite3* dst, const char* db_name) {
+ DCHECK_NE(src, dst);
+ sqlite3_backup* backup = sqlite3_backup_init(dst, db_name, src, db_name);
+ if (!backup) {
+ // Since this call only sets things up, this indicates a gross
+ // error in SQLite.
+ DLOG(FATAL) << "Unable to start sqlite3_backup(): " << sqlite3_errmsg(dst);
+ return sqlite3_errcode(dst);
+ }
+
+ // -1 backs up the entire database.
+ int rc = sqlite3_backup_step(backup, -1);
+ int pages = sqlite3_backup_pagecount(backup);
+ sqlite3_backup_finish(backup);
+
+ // If successful, exactly one page should have been backed up. If
+ // this breaks, check this function to make sure assumptions aren't
+ // being broken.
+ if (rc == SQLITE_DONE)
+ DCHECK_EQ(pages, 1);
+
+ return rc;
+}
+
} // namespace
namespace sql {
@@ -317,33 +343,54 @@
// page_size" can be used to query such a database.
ScopedWritableSchema writable_schema(db_);
- sqlite3_backup* backup = sqlite3_backup_init(db_, "main",
- null_db.db_, "main");
- if (!backup) {
- DLOG(FATAL) << "Unable to start sqlite3_backup().";
- return false;
- }
-
- // -1 backs up the entire database.
- int rc = sqlite3_backup_step(backup, -1);
- int pages = sqlite3_backup_pagecount(backup);
- sqlite3_backup_finish(backup);
+ const char* kMain = "main";
+ int rc = BackupDatabase(null_db.db_, db_, kMain);
+ UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.RazeDatabase",rc);
// The destination database was locked.
if (rc == SQLITE_BUSY) {
return false;
}
+ // SQLITE_NOTADB can happen if page 1 of db_ exists, but is not
+ // formatted correctly. SQLITE_IOERR_SHORT_READ can happen if db_
+ // isn't even big enough for one page. Either way, reach in and
+ // truncate it before trying again.
+ // TODO(shess): Maybe it would be worthwhile to just truncate from
+ // the get-go?
+ if (rc == SQLITE_NOTADB || rc == SQLITE_IOERR_SHORT_READ) {
+ sqlite3_file* file = NULL;
+ rc = sqlite3_file_control(db_, "main", SQLITE_FCNTL_FILE_POINTER, &file);
+ if (rc != SQLITE_OK) {
+ DLOG(FATAL) << "Failure getting file handle.";
+ return false;
+ } else if (!file) {
+ DLOG(FATAL) << "File handle is empty.";
+ return false;
+ }
+
+ rc = file->pMethods->xTruncate(file, 0);
+ if (rc != SQLITE_OK) {
+ UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.RazeDatabaseTruncate",rc);
+ DLOG(FATAL) << "Failed to truncate file.";
+ return false;
+ }
+
+ rc = BackupDatabase(null_db.db_, db_, kMain);
+ UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.RazeDatabase2",rc);
+
+ if (rc != SQLITE_DONE) {
+ DLOG(FATAL) << "Failed retrying Raze().";
+ }
+ }
+
// The entire database should have been backed up.
if (rc != SQLITE_DONE) {
+ // TODO(shess): Figure out which other cases can happen.
DLOG(FATAL) << "Unable to copy entire null database.";
return false;
}
- // Exactly one page should have been backed up. If this breaks,
- // check this function to make sure assumptions aren't being broken.
- DCHECK_EQ(pages, 1);
-
return true;
}
@@ -667,6 +714,7 @@
// TODO(shess): Revise is_open() to consider poisoned_, and review
// to see if any non-testing code even depends on it.
DLOG_IF(FATAL, poisoned_) << "sql::Connection is already open.";
+ poisoned_ = false;
int err = sqlite3_open(file_name.c_str(), &db_);
if (err != SQLITE_OK) {
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index 1afd2dde..c85382f 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/bind.h"
#include "base/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "base/logging.h"
@@ -12,6 +13,16 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/sqlite/sqlite3.h"
+namespace {
+
+// Helper to return the count of items in sqlite_master. Return -1 in
+// case of error.
+int SqliteMasterCount(sql::Connection* db) {
+ const char* kMasterCount = "SELECT COUNT(*) FROM sqlite_master";
+ sql::Statement s(db->GetUniqueStatement(kMasterCount));
+ return s.Step() ? s.ColumnInt(0) : -1;
+}
+
class SQLConnectionTest : public testing::Test {
public:
SQLConnectionTest() {}
@@ -31,6 +42,12 @@
return temp_dir_.path().AppendASCII("SQLConnectionTest.db");
}
+ // Handle errors by blowing away the database.
+ void RazeErrorCallback(int expected_error, int error, sql::Statement* stmt) {
+ EXPECT_EQ(expected_error, error);
+ db_.RazeAndClose();
+ }
+
private:
base::ScopedTempDir temp_dir_;
sql::Connection db_;
@@ -193,10 +210,7 @@
EXPECT_EQ(1, s.ColumnInt(0));
}
- {
- sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
- ASSERT_FALSE(s.Step());
- }
+ ASSERT_EQ(0, SqliteMasterCount(&db()));
{
sql::Statement s(db().GetUniqueStatement("PRAGMA auto_vacuum"));
@@ -245,18 +259,12 @@
ASSERT_TRUE(other_db.Open(db_path()));
// Check that the second connection sees the table.
- const char *kTablesQuery = "SELECT COUNT(*) FROM sqlite_master";
- sql::Statement s(other_db.GetUniqueStatement(kTablesQuery));
- ASSERT_TRUE(s.Step());
- ASSERT_EQ(1, s.ColumnInt(0));
- ASSERT_FALSE(s.Step()); // Releases the shared lock.
+ ASSERT_EQ(1, SqliteMasterCount(&other_db));
ASSERT_TRUE(db().Raze());
// The second connection sees the updated database.
- s.Reset(true);
- ASSERT_TRUE(s.Step());
- ASSERT_EQ(0, s.ColumnInt(0));
+ ASSERT_EQ(0, SqliteMasterCount(&other_db));
}
TEST_F(SQLConnectionTest, RazeLocked) {
@@ -294,6 +302,136 @@
ASSERT_TRUE(db().Raze());
}
+// Verify that Raze() can handle an empty file. SQLite should treat
+// this as an empty database.
+TEST_F(SQLConnectionTest, RazeEmptyDB) {
+ const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+ ASSERT_TRUE(db().Execute(kCreateSql));
+ db().Close();
+
+ {
+ file_util::ScopedFILE file(file_util::OpenFile(db_path(), "r+"));
+ ASSERT_TRUE(file.get() != NULL);
+ ASSERT_EQ(0, fseek(file.get(), 0, SEEK_SET));
+ ASSERT_TRUE(file_util::TruncateFile(file.get()));
+ }
+
+ ASSERT_TRUE(db().Open(db_path()));
+ ASSERT_TRUE(db().Raze());
+ EXPECT_EQ(0, SqliteMasterCount(&db()));
+}
+
+// Verify that Raze() can handle a file of junk.
+TEST_F(SQLConnectionTest, RazeNOTADB) {
+ db().Close();
+ sql::Connection::Delete(db_path());
+ ASSERT_FALSE(file_util::PathExists(db_path()));
+
+ {
+ file_util::ScopedFILE file(file_util::OpenFile(db_path(), "w"));
+ ASSERT_TRUE(file.get() != NULL);
+
+ const char* kJunk = "This is the hour of our discontent.";
+ fputs(kJunk, file.get());
+ }
+ ASSERT_TRUE(file_util::PathExists(db_path()));
+
+ // SQLite will successfully open the handle, but will fail with
+ // SQLITE_IOERR_SHORT_READ on pragma statemenets which read the
+ // header.
+ {
+ sql::ScopedErrorIgnorer ignore_errors;
+ ignore_errors.IgnoreError(SQLITE_IOERR_SHORT_READ);
+ EXPECT_TRUE(db().Open(db_path()));
+ ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
+ }
+ EXPECT_TRUE(db().Raze());
+ db().Close();
+
+ // Now empty, the open should open an empty database.
+ EXPECT_TRUE(db().Open(db_path()));
+ EXPECT_EQ(0, SqliteMasterCount(&db()));
+}
+
+// Verify that Raze() can handle a database overwritten with garbage.
+TEST_F(SQLConnectionTest, RazeNOTADB2) {
+ const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+ ASSERT_TRUE(db().Execute(kCreateSql));
+ ASSERT_EQ(1, SqliteMasterCount(&db()));
+ db().Close();
+
+ {
+ file_util::ScopedFILE file(file_util::OpenFile(db_path(), "r+"));
+ ASSERT_TRUE(file.get() != NULL);
+ ASSERT_EQ(0, fseek(file.get(), 0, SEEK_SET));
+
+ const char* kJunk = "This is the hour of our discontent.";
+ fputs(kJunk, file.get());
+ }
+
+ // SQLite will successfully open the handle, but will fail with
+ // SQLITE_NOTADB on pragma statemenets which attempt to read the
+ // corrupted header.
+ {
+ sql::ScopedErrorIgnorer ignore_errors;
+ ignore_errors.IgnoreError(SQLITE_NOTADB);
+ EXPECT_TRUE(db().Open(db_path()));
+ ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
+ }
+ EXPECT_TRUE(db().Raze());
+ db().Close();
+
+ // Now empty, the open should succeed with an empty database.
+ EXPECT_TRUE(db().Open(db_path()));
+ EXPECT_EQ(0, SqliteMasterCount(&db()));
+}
+
+// Test that a callback from Open() can raze the database. This is
+// essential for cases where the Open() can fail entirely, so the
+// Raze() cannot happen later.
+//
+// Most corruptions seen in the wild seem to happen when two pages in
+// the database were not written transactionally (the transaction
+// changed both, but one wasn't successfully written for some reason).
+// A special case of that is when the header indicates that the
+// database contains more pages than are in the file. This breaks
+// things at a very basic level, verify that Raze() can handle it.
+TEST_F(SQLConnectionTest, RazeOpenCallback) {
+ const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+ ASSERT_TRUE(db().Execute(kCreateSql));
+ ASSERT_EQ(1, SqliteMasterCount(&db()));
+ int page_size = 0;
+ {
+ sql::Statement s(db().GetUniqueStatement("PRAGMA page_size"));
+ ASSERT_TRUE(s.Step());
+ page_size = s.ColumnInt(0);
+ }
+ db().Close();
+
+ // Trim a single page from the end of the file.
+ {
+ file_util::ScopedFILE file(file_util::OpenFile(db_path(), "r+"));
+ ASSERT_TRUE(file.get() != NULL);
+ ASSERT_EQ(0, fseek(file.get(), -page_size, SEEK_END));
+ ASSERT_TRUE(file_util::TruncateFile(file.get()));
+ }
+
+ db().set_error_callback(base::Bind(&SQLConnectionTest::RazeErrorCallback,
+ base::Unretained(this),
+ SQLITE_CORRUPT));
+
+ // Open() will see SQLITE_CORRUPT due to size mismatch when
+ // attempting to run a pragma, and the callback will RazeAndClose().
+ // Later statements will fail, including the final secure_delete,
+ // which will fail the Open() itself.
+ ASSERT_FALSE(db().Open(db_path()));
+ db().Close();
+
+ // Now empty, the open should succeed with an empty database.
+ EXPECT_TRUE(db().Open(db_path()));
+ EXPECT_EQ(0, SqliteMasterCount(&db()));
+}
+
// Basic test of RazeAndClose() operation.
TEST_F(SQLConnectionTest, RazeAndClose) {
const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
@@ -307,10 +445,7 @@
ASSERT_FALSE(db().is_open());
db().Close();
ASSERT_TRUE(db().Open(db_path()));
- {
- sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
- ASSERT_FALSE(s.Step());
- }
+ ASSERT_EQ(0, SqliteMasterCount(&db()));
// Test that RazeAndClose() can break transactions.
ASSERT_TRUE(db().Execute(kCreateSql));
@@ -321,10 +456,7 @@
ASSERT_FALSE(db().CommitTransaction());
db().Close();
ASSERT_TRUE(db().Open(db_path()));
- {
- sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
- ASSERT_FALSE(s.Step());
- }
+ ASSERT_EQ(0, SqliteMasterCount(&db()));
}
// Test that various operations fail without crashing after
@@ -426,3 +558,5 @@
EXPECT_FALSE(file_util::PathExists(db_path()));
EXPECT_FALSE(file_util::PathExists(journal));
}
+
+} // namespace
