[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

3 files changed, 229 insertions, 35 deletions

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 @@ class ScopedWritableSchema {
   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 @@ bool Connection::Raze() {
   // 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 @@ bool Connection::OpenInternal(const std::string& file_name) {
   // 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 1afd2dd..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 @@ class SQLConnectionTest : public testing::Test {
     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 @@ TEST_F(SQLConnectionTest, Raze) {
     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 @@ TEST_F(SQLConnectionTest, RazeMultiple) {
   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 @@ TEST_F(SQLConnectionTest, RazeLocked) {
   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 @@ TEST_F(SQLConnectionTest, RazeAndClose) {
   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 @@ TEST_F(SQLConnectionTest, RazeAndClose) {
   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 @@ TEST_F(SQLConnectionTest, Delete) {
   EXPECT_FALSE(file_util::PathExists(db_path()));
   EXPECT_FALSE(file_util::PathExists(journal));
 }
+
+}  // namespace
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index dd3127f..522332c 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -11875,6 +11875,18 @@ other types of suffix sets.
   <summary>Error codes returned by sqlite for the quota db.</summary>
 </histogram>
 
+<histogram name="Sqlite.RazeDatabase" enum="SqliteErrorCode">
+  <summary>Errors attempting to Raze() database.</summary>
+</histogram>
+
+<histogram name="Sqlite.RazeDatabase2" enum="SqliteErrorCode">
+  <summary>Errors on second attempt to Raze() database.</summary>
+</histogram>
+
+<histogram name="Sqlite.RazeDatabaseTruncate" enum="SqliteErrorCode">
+  <summary>Errors truncating database for Raze().</summary>
+</histogram>
+
 <histogram name="Sqlite.SizeKB" units="Kb">
   <summary>Size in kilobytes of pre-existing database at startup.</summary>
 </histogram>