From e01748586d266657178957f5a18d409cedb5ad74 Mon Sep 17 00:00:00 2001 From: Victor Costan Date: Thu, 17 Mar 2022 19:33:38 +0000 Subject: [PATCH] M100: quota: Log SQLite errors. Merge conflict resolution: 1) In order to get the code building in M100, this CL brings in test helpers from the following CLs: * https://crrev.com/c/3508287 - QuotaDatabase::CorruptForTesting() * https://crrev.com/c/3490088 - sql::test::CorruptIndexRootPage() and its dependencies ReadPageSize() and GetRootPage() in //sql/test/test_helpers.cc's anonymous namespace 2) https://crrev.com/c/3526707 fixed the histogram name mismatch in histograms.xml. This merge CL uses the corrected name. This CL adds //sql helpers for UMA logging of SQLite error codes, and uses the helpers to log quota database errors. (cherry picked from commit b28cb1d2fb74217c113de97b08852393ca81d120) Bug: 1306332 Change-Id: Ia174596603042d1a8d3f7a29137598dbf3482d14 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3524101 Reviewed-by: Ayu Ishii Reviewed-by: Marijn Kruisselbrink Commit-Queue: Victor Costan Cr-Original-Commit-Position: refs/heads/main@{#980950} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3531247 Auto-Submit: Victor Costan Cr-Commit-Position: refs/branch-heads/4896@{#642} Cr-Branched-From: 1f63ff4bc27570761b35ffbc7f938f6586f7bee8-refs/heads/main@{#972766} --- sql/BUILD.gn | 3 + sql/error_metrics.cc | 259 ++++++++++++++++++ sql/error_metrics.h | 223 +++++++++++++++ sql/error_metrics_unittest.cc | 102 +++++++ sql/test/test_helpers.cc | 74 +++++ sql/test/test_helpers.h | 9 + storage/browser/quota/quota_database.cc | 31 +++ storage/browser/quota/quota_database.h | 5 + .../browser/quota/quota_database_unittest.cc | 40 +++ tools/metrics/histograms/enums.xml | 73 +++++ .../histograms/metadata/quota/histograms.xml | 7 + 11 files changed, 826 insertions(+) create mode 100644 sql/error_metrics.cc create mode 100644 sql/error_metrics.h create mode 100644 sql/error_metrics_unittest.cc diff --git a/sql/BUILD.gn b/sql/BUILD.gn index e589f9a099436..35b556dc36530 100644 --- a/sql/BUILD.gn +++ b/sql/BUILD.gn @@ -12,6 +12,8 @@ component("sql") { "database_memory_dump_provider.h", "error_delegate_util.cc", "error_delegate_util.h", + "error_metrics.cc", + "error_metrics.h", "init_status.h", "initialization.cc", "initialization.h", @@ -109,6 +111,7 @@ bundle_data("sql_unittests_bundle_data") { test("sql_unittests") { sources = [ "database_unittest.cc", + "error_metrics_unittest.cc", "meta_table_unittest.cc", "recover_module/module_unittest.cc", "recovery_unittest.cc", diff --git a/sql/error_metrics.cc b/sql/error_metrics.cc new file mode 100644 index 0000000000000..172458235e398 --- /dev/null +++ b/sql/error_metrics.cc @@ -0,0 +1,259 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sql/error_metrics.h" + +#include // Needed to compile NOTREACHED() with operator <<. +#include + +#include "base/check_op.h" +#include "base/metrics/histogram_functions.h" +#include "base/notreached.h" +#include "base/ranges/algorithm.h" +#include "third_party/sqlite/sqlite3.h" + +namespace sql { + +namespace { + +constexpr std::pair kResultCodeMapping[] = { + // Entries are ordered by SQLite result code value. This should match the + // ordering in https://www.sqlite.org/rescode.html. + + {SQLITE_OK, SqliteLoggedResultCode::kNoError}, + {SQLITE_ERROR, SqliteLoggedResultCode::kGeneric}, + {SQLITE_INTERNAL, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_PERM, SqliteLoggedResultCode::kPermission}, + {SQLITE_ABORT, SqliteLoggedResultCode::kAbort}, + + // Chrome features shouldn't execute conflicting statements concurrently. + {SQLITE_LOCKED, SqliteLoggedResultCode::kUnusedChrome}, + + // Chrome should crash on OOM. + {SQLITE_NOMEM, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_READONLY, SqliteLoggedResultCode::kReadOnly}, + + // Chrome doesn't use sqlite3_interrupt(). + {SQLITE_INTERRUPT, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR, SqliteLoggedResultCode::kIo}, + {SQLITE_CORRUPT, SqliteLoggedResultCode::kCorrupt}, + + // Chrome only passes a few known-good opcodes to sqlite3_file_control(). + {SQLITE_NOTFOUND, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_FULL, SqliteLoggedResultCode::kFullDisk}, + {SQLITE_CANTOPEN, SqliteLoggedResultCode::kCantOpen}, + {SQLITE_PROTOCOL, SqliteLoggedResultCode::kLockingProtocol}, + {SQLITE_EMPTY, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_SCHEMA, SqliteLoggedResultCode::kSchemaChanged}, + {SQLITE_TOOBIG, SqliteLoggedResultCode::kTooBig}, + {SQLITE_CONSTRAINT, SqliteLoggedResultCode::kConstraint}, + {SQLITE_MISMATCH, SqliteLoggedResultCode::kTypeMismatch}, + + // Chrome should not misuse SQLite's API. + {SQLITE_MISUSE, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_NOLFS, SqliteLoggedResultCode::kNoLargeFileSupport}, + + // Chrome does not set an authorizer callback. + {SQLITE_AUTH, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_FORMAT, SqliteLoggedResultCode::kUnusedSqlite}, + + // Chrome should not use invalid column indexes in sqlite3_{bind,column}*(). + {SQLITE_RANGE, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_NOTADB, SqliteLoggedResultCode::kNotADatabase}, + {SQLITE_NOTICE, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_WARNING, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_ROW, SqliteLoggedResultCode::kNoError}, + {SQLITE_DONE, SqliteLoggedResultCode::kNoError}, + {SQLITE_OK_LOAD_PERMANENTLY, SqliteLoggedResultCode::kUnusedSqlite}, + + // Chrome should not use collating sequence names in SQL statements. + {SQLITE_ERROR_MISSING_COLLSEQ, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_BUSY_RECOVERY, SqliteLoggedResultCode::kBusyRecovery}, + + // Chrome does not use a shared page cache. + {SQLITE_LOCKED_SHAREDCACHE, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_READONLY_RECOVERY, SqliteLoggedResultCode::kReadOnlyRecovery}, + {SQLITE_IOERR_READ, SqliteLoggedResultCode::kIoRead}, + + // Chrome does not use a virtual table that signals corruption. We only use + // a + // virtual table code for recovery. That code does not use this error. + {SQLITE_CORRUPT_VTAB, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_CANTOPEN_NOTEMPDIR, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_CONSTRAINT_CHECK, SqliteLoggedResultCode::kConstraintCheck}, + + // Chrome does not set an authorizer callback. + {SQLITE_AUTH_USER, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_NOTICE_RECOVER_WAL, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_WARNING_AUTOINDEX, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_ERROR_RETRY, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_ABORT_ROLLBACK, SqliteLoggedResultCode::kAbortRollback}, + {SQLITE_BUSY_SNAPSHOT, SqliteLoggedResultCode::kBusySnapshot}, + + // Chrome does not use a virtual table that signals conflicts. We only use a + // virtual table code for recovery. That code does not use this error. + {SQLITE_LOCKED_VTAB, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_READONLY_CANTLOCK, SqliteLoggedResultCode::kReadOnlyCantLock}, + {SQLITE_IOERR_SHORT_READ, SqliteLoggedResultCode::kIoShortRead}, + {SQLITE_CORRUPT_SEQUENCE, SqliteLoggedResultCode::kCorruptSequence}, + {SQLITE_CANTOPEN_ISDIR, SqliteLoggedResultCode::kCantOpenIsDir}, + + // Chrome does not use commit hook callbacks. + {SQLITE_CONSTRAINT_COMMITHOOK, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_NOTICE_RECOVER_ROLLBACK, SqliteLoggedResultCode::kUnusedSqlite}, + + // Chrome does not use sqlite3_snapshot_open(). + {SQLITE_ERROR_SNAPSHOT, SqliteLoggedResultCode::kUnusedChrome}, +#ifdef SQLITE_ENABLE_SNAPSHOT +#error "This code assumes that Chrome does not use sqlite3_snapshot_open()" +#endif + + // Chrome does not use blocking Posix advisory file lock requests. + {SQLITE_ERROR_SNAPSHOT, SqliteLoggedResultCode::kUnusedChrome}, +#ifdef SQLITE_ENABLE_SETLK_TIMEOUT +#error "This code assumes that Chrome does not use +#endif + + {SQLITE_READONLY_ROLLBACK, SqliteLoggedResultCode::kReadOnlyRollback}, + {SQLITE_IOERR_WRITE, SqliteLoggedResultCode::kIoWrite}, + {SQLITE_CORRUPT_INDEX, SqliteLoggedResultCode::kCorruptIndex}, + + // Chrome should always pass full paths to SQLite. + {SQLITE_CANTOPEN_FULLPATH, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_CONSTRAINT_FOREIGNKEY, + SqliteLoggedResultCode::kConstraintForeignKey}, + {SQLITE_READONLY_DBMOVED, SqliteLoggedResultCode::kReadOnlyDbMoved}, + {SQLITE_IOERR_FSYNC, SqliteLoggedResultCode::kIoFsync}, + + // Chrome does not support Cygwin and does not use its VFS. + {SQLITE_CANTOPEN_CONVPATH, SqliteLoggedResultCode::kUnusedChrome}, + + // Chrome does not use extension functions. + {SQLITE_CONSTRAINT_FUNCTION, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_READONLY_CANTINIT, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_IOERR_DIR_FSYNC, SqliteLoggedResultCode::kIoDirFsync}, + {SQLITE_CANTOPEN_DIRTYWAL, SqliteLoggedResultCode::kUnusedSqlite}, + {SQLITE_CONSTRAINT_NOTNULL, SqliteLoggedResultCode::kConstraintNotNull}, + {SQLITE_READONLY_DIRECTORY, SqliteLoggedResultCode::kReadOnlyDirectory}, + {SQLITE_IOERR_TRUNCATE, SqliteLoggedResultCode::kIoTruncate}, + + // Chrome does not use the SQLITE_OPEN_NOFOLLOW flag. + {SQLITE_CANTOPEN_SYMLINK, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_CONSTRAINT_PRIMARYKEY, + SqliteLoggedResultCode::kConstraintPrimaryKey}, + {SQLITE_IOERR_FSTAT, SqliteLoggedResultCode::kIoFstat}, + + // Chrome unconditionally disables database triggers via + // sqlite3_db_config(SQLITE_DBCONFIG_ENABLE_TRIGGER). + {SQLITE_CONSTRAINT_TRIGGER, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR_UNLOCK, SqliteLoggedResultCode::kIoUnlock}, + {SQLITE_CONSTRAINT_UNIQUE, SqliteLoggedResultCode::kConstraintUnique}, + {SQLITE_IOERR_RDLOCK, SqliteLoggedResultCode::kIoReadLock}, + + // Chrome does not use a virtual table that signals constraints. We only use + // a virtual table code for recovery. That code does not use this error. + {SQLITE_CONSTRAINT_VTAB, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR_DELETE, SqliteLoggedResultCode::kIoDelete}, + {SQLITE_CONSTRAINT_ROWID, SqliteLoggedResultCode::kConstraintRowId}, + {SQLITE_IOERR_BLOCKED, SqliteLoggedResultCode::kUnusedSqlite}, + + // Chrome unconditionally disables database triggers via + // sqlite3_db_config(SQLITE_DBCONFIG_ENABLE_TRIGGER). + {SQLITE_CONSTRAINT_PINNED, SqliteLoggedResultCode::kUnusedChrome}, + + // The SQLite docus claim that this error code is "normally" converted to + // SQLITE_NOMEM. This doesn't seem 100% categorical, so we're flagging this + // as "unused in Chrome" per the same rationale as SQLITE_NOMEM. + {SQLITE_IOERR_NOMEM, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_CONSTRAINT_DATATYPE, SqliteLoggedResultCode::kConstraintDataType}, + {SQLITE_IOERR_ACCESS, SqliteLoggedResultCode::kIoAccess}, + {SQLITE_IOERR_CHECKRESERVEDLOCK, + SqliteLoggedResultCode::kIoCheckReservedLock}, + {SQLITE_IOERR_LOCK, SqliteLoggedResultCode::kIoLock}, + {SQLITE_IOERR_CLOSE, SqliteLoggedResultCode::kIoClose}, + {SQLITE_IOERR_DIR_CLOSE, SqliteLoggedResultCode::kUnusedSqlite}, + + // Chrome will only allow enabling WAL on databases with exclusive locking. + {SQLITE_IOERR_SHMOPEN, SqliteLoggedResultCode::kUnusedChrome}, + + // Chrome will only allow enabling WAL on databases with exclusive locking. + {SQLITE_IOERR_SHMSIZE, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR_SHMLOCK, SqliteLoggedResultCode::kUnusedSqlite}, + + // Chrome will only allow enabling WAL on databases with exclusive locking. + {SQLITE_IOERR_SHMMAP, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR_SEEK, SqliteLoggedResultCode::kIoSeek}, + {SQLITE_IOERR_DELETE_NOENT, SqliteLoggedResultCode::kIoDeleteNoEntry}, + {SQLITE_IOERR_MMAP, SqliteLoggedResultCode::kIoMemoryMapping}, + {SQLITE_IOERR_GETTEMPPATH, SqliteLoggedResultCode::kIoGetTemporaryPath}, + + // Chrome does not support Cygwin and does not use its VFS. + {SQLITE_IOERR_CONVPATH, SqliteLoggedResultCode::kUnusedChrome}, + + // Chrome does not use SQLite extensions. + {SQLITE_IOERR_VNODE, SqliteLoggedResultCode::kUnusedChrome}, + + // Chrome does not use SQLite extensions. + {SQLITE_IOERR_AUTH, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR_BEGIN_ATOMIC, SqliteLoggedResultCode::kIoBeginAtomic}, + {SQLITE_IOERR_COMMIT_ATOMIC, SqliteLoggedResultCode::kIoCommitAtomic}, + {SQLITE_IOERR_ROLLBACK_ATOMIC, SqliteLoggedResultCode::kIoRollbackAtomic}, + + // Chrome does not use the checksum VFS shim. + {SQLITE_IOERR_DATA, SqliteLoggedResultCode::kUnusedChrome}, + + {SQLITE_IOERR_CORRUPTFS, SqliteLoggedResultCode::kIoCorruptFileSystem}, +}; + +} // namespace + +SqliteLoggedResultCode CreateSqliteLoggedResultCode(int sqlite_result_code) { + const auto* mapping_it = base::ranges::find_if( + kResultCodeMapping, + [&sqlite_result_code](const std::pair& rhs) { + return sqlite_result_code == rhs.first; + }); + + if (mapping_it == base::ranges::end(kResultCodeMapping)) { + NOTREACHED() << "Unsupported SQLite result code: " << sqlite_result_code; + return SqliteLoggedResultCode::kUnusedChrome; + } + SqliteLoggedResultCode logged_code = mapping_it->second; + + DCHECK_NE(logged_code, SqliteLoggedResultCode::kUnusedSqlite) + << "SQLite reported code marked for internal use: " << sqlite_result_code; + DCHECK_NE(logged_code, SqliteLoggedResultCode::kUnusedChrome) + << "SQLite reported code that should never show up in Chrome: " + << sqlite_result_code; + return logged_code; +} + +void UmaHistogramSqliteResult(const char* histogram_name, + int sqlite_result_code) { + auto logged_code = CreateSqliteLoggedResultCode(sqlite_result_code); + base::UmaHistogramEnumeration(histogram_name, logged_code); +} + +} // namespace sql diff --git a/sql/error_metrics.h b/sql/error_metrics.h new file mode 100644 index 0000000000000..300c262ee33f3 --- /dev/null +++ b/sql/error_metrics.h @@ -0,0 +1,223 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SQL_ERROR_METRICS_H_ +#define SQL_ERROR_METRICS_H_ + +#include "base/component_export.h" + +namespace sql { + +// Helper for logging a SQLite result code to a UMA histogram. +// +// The histogram should be declared as enum="SqliteLoggedResultCode". +// +// Works for all result codes, including success codes and extended error codes. +// DCHECKs if provided result code should not occur in Chrome's usage of SQLite. +COMPONENT_EXPORT(SQL) +void UmaHistogramSqliteResult(const char* histogram_name, + int sqlite_result_code); + +// SQLite result codes, mapped into a more compact form for UMA logging. +// +// SQLite's (extended) result codes cover a wide range of integer values, and +// are not suitable for direct use with our UMA logging infrastructure. This +// enum compresses the range by removing gaps and by mapping multiple SQLite +// result codes to the same value where appropriate. +// +// These values are persisted to logs. Entries should not be renumbered and +// numeric values should never be reused. +enum class SqliteLoggedResultCode { + // A success code: OK, DONE, ROW. + kNoError = 0, + + // Codes that SQLite APIs should never return, such as SQLITE_INTERNAL. + kUnusedSqlite = 1, + + // Codes that SQLite APIs should never return, given Chrome's usage pattern. + kUnusedChrome = 2, + + // SQLITE_ERROR + kGeneric = 3, + + // SQLITE_PERM + kPermission = 4, + + // SQLITE_ABORT + kAbort = 5, + + // SQLITE_BUSY + kBusy = 6, + + // SQLITE_READONLY + kReadOnly = 7, + + // SQLITE_IOERR + kIo = 8, + + // SQLITE_CORRUPT + kCorrupt = 9, + + // SQLITE_FULL + kFullDisk = 10, + + // SQLITE_CANTOPEN + kCantOpen = 11, + + // SQLITE_PROTOCOL + kLockingProtocol = 12, + + // SQLITE_SCHEMA + kSchemaChanged = 13, + + // SQLITE_TOOBIG + kTooBig = 14, + + // SQLITE_CONSTRAINT + kConstraint = 15, + + // SQLITE_MISMATCH + kTypeMismatch = 16, + + // SQLITE_NOLFS + kNoLargeFileSupport = 17, + + // SQLITE_NOTADB + kNotADatabase = 18, + + // SQLITE_BUSY_RECOVERY + kBusyRecovery = 19, + + // SQLITE_READONLY_RECOVERY + kReadOnlyRecovery = 20, + + // SQLITE_IOERR_READ + kIoRead = 21, + + // SQLITE_CONSTRAINT_CHECK + kConstraintCheck = 22, + + // SQLITE_ABORT_ROLLBACK + kAbortRollback = 23, + + // SQLITE_BUSY_SNAPSHOT + kBusySnapshot = 24, + + // SQLITE_READONLY_CANTLOCK + kReadOnlyCantLock = 25, + + // SQLITE_IOERR_SHORT_READ + kIoShortRead = 26, + + // SQLITE_CORRUPT_SEQUENCE + kCorruptSequence = 27, + + // SQLITE_CANTOPEN_ISDIR + kCantOpenIsDir = 28, + + // SQLITE_READONLY_ROLLBACK + kReadOnlyRollback = 29, + + // SQLITE_IOERR_WRITE + kIoWrite = 30, + + // SQLITE_CORRUPT_INDEX + kCorruptIndex = 31, + + // SQLITE_CONSTRAINT_FOREIGN_KEY + kConstraintForeignKey = 32, + + // SQLITE_READONLY_DBMOVED + kReadOnlyDbMoved = 33, + + // SQLITE_IOERR_FSYNC + kIoFsync = 34, + + // SQLITE_IOERR_DIR_FSYNC + kIoDirFsync = 35, + + // SQLITE_CONSTRAINT_NOTNULL + kConstraintNotNull = 36, + + // SQLITE_READONLY_DIRECTORY + kReadOnlyDirectory = 37, + + // SQLITE_IOERR_TRUNCATE + kIoTruncate = 38, + + // SQLITE_CONSTRAINT_PRIMARYKEY + kConstraintPrimaryKey = 39, + + // SQLITE_IOERR_FSTAT + kIoFstat = 40, + + // SQLITE_IOERR_UNLOCK + kIoUnlock = 41, + + // SQLITE_CONSTRAINT_UNIQUE + kConstraintUnique = 42, + + // SQLITE_IOERR_RDLOCK + kIoReadLock = 43, + + // SQLITE_IOERR_DELETE + kIoDelete = 44, + + // SQLITE_CONSTRAINT_ROWID + kConstraintRowId = 45, + + // SQLITE_CONSTRAINT_DATATYPE + kConstraintDataType = 46, + + // SQLITE_IOERR_ACCESS + kIoAccess = 47, + + // SQLITE_IOERR_CHECKRESERVEDLOCK + kIoCheckReservedLock = 48, + + // SQLITE_IOERR_LOCK + kIoLock = 49, + + // SQLITE_IOERR_CLOSE + kIoClose = 50, + + // SQLITE_IOERR_SEEK + kIoSeek = 51, + + // SQLITE_IOERR_DELETE_NOENT + kIoDeleteNoEntry = 52, + + // SQLITE_IOERR_MMAP + kIoMemoryMapping = 53, + + // SQLITE_IOERR_GETTEMPPATH + kIoGetTemporaryPath = 54, + + // SQLITE_IOERR_BEGIN_ATOMIC + kIoBeginAtomic = 55, + + // SQLITE_IOERR_COMMIT_ATOMIC + kIoCommitAtomic = 56, + + // SQLITE_IOERR_ROLLBACK_ATOMIC + kIoRollbackAtomic = 57, + + // SQLITE_IOERR_CORRUPTFS + kIoCorruptFileSystem = 58, + + kMaxValue = kIoCorruptFileSystem, +}; + +// Converts a SQLite result code into a UMA logging-friendly form. +// +// Works for all result codes, including success codes and extended error codes. +// DCHECKs if provided result code should not occur in Chrome's usage of SQLite. +// +// UmaHistogramSqliteResult() should be preferred for logging results to UMA. +COMPONENT_EXPORT(SQL) +SqliteLoggedResultCode CreateSqliteLoggedResultCode(int sqlite_result_code); + +} // namespace sql + +#endif // SQL_ERROR_METRICS_H_ diff --git a/sql/error_metrics_unittest.cc b/sql/error_metrics_unittest.cc new file mode 100644 index 0000000000000..74a4123ec0a0a --- /dev/null +++ b/sql/error_metrics_unittest.cc @@ -0,0 +1,102 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sql/error_metrics.h" + +#include "base/test/gtest_util.h" +#include "base/test/metrics/histogram_tester.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/sqlite/sqlite3.h" + +namespace sql { + +namespace { + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_Success) { + EXPECT_EQ(SqliteLoggedResultCode::kNoError, + CreateSqliteLoggedResultCode(SQLITE_OK)); + EXPECT_EQ(SqliteLoggedResultCode::kNoError, + CreateSqliteLoggedResultCode(SQLITE_DONE)); + EXPECT_EQ(SqliteLoggedResultCode::kNoError, + CreateSqliteLoggedResultCode(SQLITE_ROW)); +} + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_PrimaryErrorCodes) { + EXPECT_EQ(SqliteLoggedResultCode::kIo, + CreateSqliteLoggedResultCode(SQLITE_IOERR)); + EXPECT_EQ(SqliteLoggedResultCode::kCorrupt, + CreateSqliteLoggedResultCode(SQLITE_CORRUPT)); + EXPECT_EQ(SqliteLoggedResultCode::kConstraint, + CreateSqliteLoggedResultCode(SQLITE_CONSTRAINT)); +} + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_ExtendedErrorCodes) { + EXPECT_EQ(SqliteLoggedResultCode::kIoRead, + CreateSqliteLoggedResultCode(SQLITE_IOERR_READ)); + EXPECT_EQ(SqliteLoggedResultCode::kIoWrite, + CreateSqliteLoggedResultCode(SQLITE_IOERR_WRITE)); + EXPECT_EQ(SqliteLoggedResultCode::kCorruptIndex, + CreateSqliteLoggedResultCode(SQLITE_CORRUPT_INDEX)); + EXPECT_EQ(SqliteLoggedResultCode::kConstraintUnique, + CreateSqliteLoggedResultCode(SQLITE_CONSTRAINT_UNIQUE)); +} + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_MissingLowValue) { + EXPECT_DCHECK_DEATH_WITH(CreateSqliteLoggedResultCode(-65536), + "Unsupported SQLite result code: -65536"); +} + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_MissingHighValue) { + EXPECT_DCHECK_DEATH_WITH(CreateSqliteLoggedResultCode(65536), + "Unsupported SQLite result code: 65536"); +} + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_SqliteInternalError) { +#if DCHECK_IS_ON() + EXPECT_DCHECK_DEATH_WITH(CreateSqliteLoggedResultCode(SQLITE_INTERNAL), + "SQLite reported code marked for internal use: 2"); +#else + EXPECT_EQ(SqliteLoggedResultCode::kUnusedSqlite, + CreateSqliteLoggedResultCode(SQLITE_INTERNAL)); +#endif +} + +TEST(ErrorMetricsTest, CreateSqliteLoggedResultCode_ChromeBugError) { +#if DCHECK_IS_ON() + EXPECT_DCHECK_DEATH_WITH( + CreateSqliteLoggedResultCode(SQLITE_NOTFOUND), + "SQLite reported code that should never show up in Chrome: 12"); +#else + EXPECT_EQ(SqliteLoggedResultCode::kUnusedSqlite, + CreateSqliteLoggedResultCode(SQLITE_NOTFOUND)); +#endif +} + +TEST(ErrorMetricsTest, UmaHistogramSqliteResult_Success) { + base::HistogramTester histogram_tester; + UmaHistogramSqliteResult("Sql.ResultTest", SQLITE_OK); + histogram_tester.ExpectTotalCount("Sql.ResultTest", 1); + histogram_tester.ExpectBucketCount("Sql.ResultTest", + SqliteLoggedResultCode::kNoError, 1); +} + +TEST(ErrorMetricsTest, UmaHistogramSqliteResult_PrimaryErrorCode) { + base::HistogramTester histogram_tester; + UmaHistogramSqliteResult("Sql.ResultTest", SQLITE_CORRUPT); + histogram_tester.ExpectTotalCount("Sql.ResultTest", 1); + histogram_tester.ExpectBucketCount("Sql.ResultTest", + SqliteLoggedResultCode::kCorrupt, 1); +} + +TEST(ErrorMetricsTest, UmaHistogramSqliteResult_ExtendedErrorCode) { + base::HistogramTester histogram_tester; + UmaHistogramSqliteResult("Sql.ResultTest", SQLITE_CORRUPT_INDEX); + histogram_tester.ExpectTotalCount("Sql.ResultTest", 1); + histogram_tester.ExpectBucketCount("Sql.ResultTest", + SqliteLoggedResultCode::kCorruptIndex, 1); +} + +} // namespace + +} // namespace sql diff --git a/sql/test/test_helpers.cc b/sql/test/test_helpers.cc index daed1291fbc74..375eeaa3f8b23 100644 --- a/sql/test/test_helpers.cc +++ b/sql/test/test_helpers.cc @@ -11,6 +11,7 @@ #include #include +#include "base/big_endian.h" #include "base/check.h" #include "base/check_op.h" #include "base/files/file_util.h" @@ -41,6 +42,37 @@ bool GetPageSize(sql::Database* db, int* page_size) { return true; } +// Read a database's page size. Returns nullopt in case of error. +absl::optional ReadPageSize(const base::FilePath& db_path) { + // See https://www.sqlite.org/fileformat2.html#page_size + constexpr size_t kPageSizeOffset = 16; + uint8_t raw_page_size_bytes[2]; + base::File file(db_path, base::File::FLAG_OPEN | base::File::FLAG_READ); + if (!file.IsValid()) + return absl::nullopt; + if (!file.ReadAndCheck(kPageSizeOffset, raw_page_size_bytes)) + return absl::nullopt; + + uint16_t raw_page_size; + base::ReadBigEndian(raw_page_size_bytes, &raw_page_size); + // The SQLite database format initially allocated a 16 bits for storing the + // page size. This worked out until SQLite wanted to support 64kb pages, + // because 65536 (64kb) doesn't fit in a 16-bit unsigned integer. + // + // Currently, the page_size field value of 1 is a special case for 64kb pages. + // The documentation hints at the path for future expansion -- the page_size + // field may become a litte-endian number that indicates the database page + // size divided by 256. This happens to work out because the smallest + // supported page size is 512. + const int page_size = (raw_page_size == 1) ? 65536 : raw_page_size; + // Sanity-check that the page size is valid. + constexpr uint16_t kMinPageSize = 512; + if (page_size < kMinPageSize || (page_size & (page_size - 1)) != 0) + return absl::nullopt; + + return page_size; +} + // Get |name|'s root page number in the database. bool GetRootPage(sql::Database* db, const char* name, int* page_number) { static const char kPageSql[] = @@ -53,6 +85,24 @@ bool GetRootPage(sql::Database* db, const char* name, int* page_number) { return true; } +// Read the number of the root page of a B-tree (index/table). +// +// Returns a 0-indexed page number, not the raw SQLite page number. +absl::optional GetRootPage(sql::Database& db, + base::StringPiece tree_name) { + sql::Statement select( + db.GetUniqueStatement("SELECT rootpage FROM sqlite_schema WHERE name=?")); + select.BindString(0, tree_name); + if (!select.Step()) + return absl::nullopt; + + int sqlite_page_number = select.ColumnInt(0); + if (!sqlite_page_number) + return absl::nullopt; + + return sqlite_page_number - 1; +} + // Helper for reading a number from the SQLite header. // See base/big_endian.h. unsigned ReadBigEndian(unsigned char* buf, size_t bytes) { @@ -245,6 +295,30 @@ bool CorruptTableOrIndex(const base::FilePath& db_path, return true; } +bool CorruptIndexRootPage(const base::FilePath& db_path, + base::StringPiece index_name) { + absl::optional page_size = ReadPageSize(db_path); + if (!page_size.has_value()) + return false; + + sql::Database db; + if (!db.Open(db_path)) + return false; + + absl::optional page_number = GetRootPage(db, index_name); + db.Close(); + if (!page_number.has_value()) + return false; + + std::vector page_buffer(page_size.value()); + const int64_t page_offset = int64_t{page_number.value()} * page_size.value(); + + base::File file(db_path, base::File::FLAG_OPEN | base::File::FLAG_READ | + base::File::FLAG_WRITE); + if (!file.IsValid()) + return false; + return file.WriteAndCheck(page_offset, page_buffer); +} size_t CountSQLTables(sql::Database* db) { return CountSQLItemsOfType(db, "table"); } diff --git a/sql/test/test_helpers.h b/sql/test/test_helpers.h index 82e99e2dd385f..3dce4d7dd71b2 100644 --- a/sql/test/test_helpers.h +++ b/sql/test/test_helpers.h @@ -69,6 +69,15 @@ void CorruptSizeInHeaderMemory(unsigned char* header, int64_t db_size); const char* tree_name, const char* update_sql); +// Simulates total index corruption by zeroing the root page of an index B-tree. +// +// The corrupted database will still open successfully. SELECTs on the table +// associated with the index will work, as long as they don't access the index. +// However, any query that accesses the index will fail with SQLITE_CORRUPT. +// DROPping the table or the index will fail. +[[nodiscard]] bool CorruptIndexRootPage(const base::FilePath& db_path, + base::StringPiece index_name); + // Return the number of tables in sqlite_schema. [[nodiscard]] size_t CountSQLTables(sql::Database* db); diff --git a/storage/browser/quota/quota_database.cc b/storage/browser/quota/quota_database.cc index ef8f63896b7cc..dc62026cc97ec 100644 --- a/storage/browser/quota/quota_database.cc +++ b/storage/browser/quota/quota_database.cc @@ -19,6 +19,7 @@ #include "base/metrics/histogram_functions.h" #include "components/services/storage/public/cpp/buckets/constants.h" #include "sql/database.h" +#include "sql/error_metrics.h" #include "sql/meta_table.h" #include "sql/statement.h" #include "sql/transaction.h" @@ -686,6 +687,29 @@ QuotaError QuotaDatabase::SetIsBootstrapped(bool bootstrap_flag) { : QuotaError::kDatabaseError; } +QuotaError QuotaDatabase::CorruptForTesting( + base::OnceCallback corrupter) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + + if (db_) { + // Commit the long-running transaction. + db_->CommitTransaction(); + db_->Close(); + } + + std::move(corrupter).Run(db_file_path_); + + if (!db_) + return QuotaError::kDatabaseError; + if (!OpenDatabase()) + return QuotaError::kDatabaseError; + + // Begin a long-running transaction. This matches EnsureOpen(). + if (!db_->BeginTransaction()) + return QuotaError::kDatabaseError; + return QuotaError::kNone; +} + void QuotaDatabase::Commit() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (!db_) @@ -734,6 +758,13 @@ QuotaError QuotaDatabase::EnsureOpened(EnsureOpenedMode mode) { db_->set_histogram_tag("Quota"); + // UMA logging and don't crash on database errors in DCHECK builds. + db_->set_error_callback( + base::BindRepeating([](int sqlite_error_code, sql::Statement* statement) { + sql::UmaHistogramSqliteResult("Quota.QuotaDatabaseError", + sqlite_error_code); + })); + if (!OpenDatabase() || !EnsureDatabaseVersion()) { LOG(ERROR) << "Could not open the quota database, resetting."; if (!ResetSchema()) { diff --git a/storage/browser/quota/quota_database.h b/storage/browser/quota/quota_database.h index 96915f6c85271..75de1871c8295 100644 --- a/storage/browser/quota/quota_database.h +++ b/storage/browser/quota/quota_database.h @@ -210,6 +210,11 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaDatabase { bool IsBootstrapped(); QuotaError SetIsBootstrapped(bool bootstrap_flag); + // Returns QuotaError::kNone if the database was successfully reopened after + // `corrupter` was run, or QuotaError::kDatabaseError otherwise. + QuotaError CorruptForTesting( + base::OnceCallback corrupter); + // Manually disable database to test database error scenarios for testing. void SetDisabledForTesting(bool disable) { is_disabled_ = disable; } diff --git a/storage/browser/quota/quota_database_unittest.cc b/storage/browser/quota/quota_database_unittest.cc index 47b59b87bd63a..18f6f282e6dbb 100644 --- a/storage/browser/quota/quota_database_unittest.cc +++ b/storage/browser/quota/quota_database_unittest.cc @@ -21,6 +21,7 @@ #include "components/services/storage/public/cpp/buckets/bucket_locator.h" #include "components/services/storage/public/cpp/buckets/constants.h" #include "sql/database.h" +#include "sql/error_metrics.h" #include "sql/meta_table.h" #include "sql/statement.h" #include "sql/test/scoped_error_expecter.h" @@ -889,6 +890,45 @@ TEST_F(QuotaDatabaseTest, OpenCorruptedDatabase) { histograms.ExpectTotalCount("Quota.QuotaDatabaseReset", 1); histograms.ExpectBucketCount("Quota.QuotaDatabaseReset", DatabaseResetReason::kCreateSchema, 1); + + EXPECT_GE(histograms.GetTotalSum("Quota.QuotaDatabaseError"), 1); + EXPECT_GE(histograms.GetBucketCount("Quota.QuotaDatabaseError", + sql::SqliteLoggedResultCode::kCorrupt), + 1); +} + +TEST_F(QuotaDatabaseTest, GetOrCreateBucket_CorruptedDatabase) { + QuotaDatabase db(DbPath()); + StorageKey storage_key = + StorageKey::CreateFromStringForTesting("http://google/"); + std::string bucket_name = "google_bucket"; + + { + QuotaErrorOr result = + db.GetOrCreateBucket(storage_key, bucket_name); + ASSERT_TRUE(result.ok()) << "Failed to create bucket to be used in test"; + } + + // Bucket lookup uses the `buckets_by_storage_key` index. + QuotaError corruption_error = + db.CorruptForTesting(base::BindOnce([](const base::FilePath& db_path) { + ASSERT_TRUE( + sql::test::CorruptIndexRootPage(db_path, "buckets_by_storage_key")); + })); + ASSERT_EQ(QuotaError::kNone, corruption_error) + << "Failed to corrupt the database"; + + { + base::HistogramTester histograms; + + QuotaErrorOr result = + db.GetOrCreateBucket(storage_key, bucket_name); + EXPECT_FALSE(result.ok()); + + histograms.ExpectTotalCount("Quota.QuotaDatabaseError", 1); + histograms.ExpectBucketCount("Quota.QuotaDatabaseError", + sql::SqliteLoggedResultCode::kCorrupt, 1); + } } INSTANTIATE_TEST_SUITE_P(All, diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index 4c5f292720798..8cb77e91f80d8 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -84364,6 +84364,79 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf Unused + + SQLite result codes mapped to logging-friendly values. + Success result codes: OK, DONE, ROW + + Codes that SQLite should never return, like SQLITE_INTERNAL + + + Codes that SQLite should never return, given Chrome's usage pattern + + SQLITE_ERROR + SQLITE_PERM + SQLITE_ABORT + SQLITE_BUSY + SQLITE_READONLY + SQLITE_IOERR + SQLITE_CORRUPT + SQLITE_FULL + SQLITE_CANTOPEN + SQLITE_PROTOCOL + SQLITE_SCHEMA + SQLITE_TOOBIG + SQLITE_CONSTRAINT + SQLITE_MISMATCH + SQLITE_NOLFS + SQLITE_NOTADB + SQLITE_BUSY_RECOVERY + SQLITE_READONLY_RECOVERY + SQLITE_IOERR_READ + SQLITE_CONSTRAINT_CHECK + SQLITE_ABORT_ROLLBACK + SQLITE_BUSY_SNAPSHOT + SQLITE_READONLY_CANTLOCK + SQLITE_IOERR_SHORT_READ + SQLITE_CORRUPT_SEQUENCE + SQLITE_CANTOPEN_ISDIR + SQLITE_READONLY_ROLLBACK + SQLITE_IOERR_WRITE + SQLITE_CORRUPT_INDEX + + SQLITE_CONSTRAINT_FOREIGN_KEY + + SQLITE_READONLY_DBMOVED + SQLITE_IOERR_FSYNC + SQLITE_IOERR_DIR_FSYNC + SQLITE_CONSTRAINT_NOTNULL + SQLITE_READONLY_DIRECTORY + SQLITE_IOERR_TRUNCATE + + SQLITE_CONSTRAINT_PRIMARYKEY + + SQLITE_IOERR_FSTAT + SQLITE_IOERR_UNLOCK + SQLITE_CONSTRAINT_UNIQUE + SQLITE_IOERR_RDLOCK + SQLITE_IOERR_DELETE + SQLITE_CONSTRAINT_ROWID + SQLITE_CONSTRAINT_DATATYPE + SQLITE_IOERR_ACCESS + + SQLITE_IOERR_CHECKRESERVEDLOCK + + SQLITE_IOERR_LOCK + SQLITE_IOERR_CLOSE + SQLITE_IOERR_SEEK + SQLITE_IOERR_DELETE_NOENT + SQLITE_IOERR_MMAP + SQLITE_IOERR_GETTEMPPATH + SQLITE_IOERR_BEGIN_ATOMIC + SQLITE_IOERR_COMMIT_ATOMIC + SQLITE_IOERR_ROLLBACK_ATOMIC + SQLITE_IOERR_CORRUPTFS + + Track successful completion or failure of sql::Recovery implementation. diff --git a/tools/metrics/histograms/metadata/quota/histograms.xml b/tools/metrics/histograms/metadata/quota/histograms.xml index 1f7f7003bd9e3..296bd691f6452 100644 --- a/tools/metrics/histograms/metadata/quota/histograms.xml +++ b/tools/metrics/histograms/metadata/quota/histograms.xml @@ -188,6 +188,13 @@ chromium-metrics-reviews@google.com. + + ayui@chromium.org + chrome-owp-storage@google.com + Errors reported by SQLite while using the quota database. + + ayui@chromium.org