From ed5801370ea0e9d1d584ff7d0209335a956e5f35 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 20 Jul 2022 23:26:59 -0700 Subject: [PATCH 1/2] Move to using sqlite3_errmsg to extract a contextual error for SQLite failures. --- src/AppInstallerCLITests/SQLiteWrapper.cpp | 12 +++++++ .../SQLiteWrapper.cpp | 33 ++++++++++--------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/AppInstallerCLITests/SQLiteWrapper.cpp b/src/AppInstallerCLITests/SQLiteWrapper.cpp index d27b20fd24..ab1212db99 100644 --- a/src/AppInstallerCLITests/SQLiteWrapper.cpp +++ b/src/AppInstallerCLITests/SQLiteWrapper.cpp @@ -300,6 +300,18 @@ TEST_CASE("SQLiteWrapper_BindWithEmbeddedNull", "[sqlitewrapper]") REQUIRE_THROWS_HR(InsertIntoSimpleTestTable(connection, firstVal, secondVal), APPINSTALLER_CLI_ERROR_BIND_WITH_EMBEDDED_NULL); } +TEST_CASE("SQLiteWrapper_PrepareFailure", "[sqlitewrapper]") +{ + Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create); + + CreateSimpleTestTable(connection); + + Builder::StatementBuilder builder; + builder.Select({ s_firstColumn, s_secondColumn }).From(std::string{ s_tableName } + "2").Where(s_firstColumn).Equals(2); + + REQUIRE_THROWS_HR(builder.Prepare(connection), MAKE_HRESULT(SEVERITY_ERROR, FACILITY_SQLITE, SQLITE_ERROR)); +} + TEST_CASE("SQLBuilder_SimpleSelectBind", "[sqlbuilder]") { Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create); diff --git a/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp b/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp index a19d720734..26e8726354 100644 --- a/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp +++ b/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp @@ -15,18 +15,19 @@ using namespace std::string_view_literals; #include #endif -#define THROW_SQLITE(_error_) \ +#define THROW_SQLITE(_error_,_connection_) \ do { \ int _ts_sqliteReturnValue = _error_; \ - THROW_EXCEPTION_MSG(SQLiteException(_ts_sqliteReturnValue), sqlite3_errstr(_ts_sqliteReturnValue)); \ + sqlite3* _ts_sqliteConnection = _connection_; \ + THROW_EXCEPTION_MSG(SQLiteException(_ts_sqliteReturnValue), _ts_sqliteConnection ? sqlite3_errmsg(_ts_sqliteConnection) : sqlite3_errstr(_ts_sqliteReturnValue)); \ } while (0,0) -#define THROW_IF_SQLITE_FAILED(_statement_) \ +#define THROW_IF_SQLITE_FAILED(_statement_,_connection_) \ do { \ int _tisf_sqliteReturnValue = _statement_; \ if (_tisf_sqliteReturnValue != SQLITE_OK) \ { \ - THROW_SQLITE(_tisf_sqliteReturnValue); \ + THROW_SQLITE(_tisf_sqliteReturnValue,_connection_); \ } \ } while (0,0) @@ -47,7 +48,7 @@ namespace AppInstaller::Repository::SQLite { void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, nullptr_t) { - THROW_IF_SQLITE_FAILED(sqlite3_bind_null(stmt, index)); + THROW_IF_SQLITE_FAILED(sqlite3_bind_null(stmt, index), sqlite3_db_handle(stmt)); } void ThrowIfContainsEmbeddedNullCharacter(std::string_view v) @@ -58,7 +59,7 @@ namespace AppInstaller::Repository::SQLite void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, const std::string& v) { ThrowIfContainsEmbeddedNullCharacter(v); - THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.c_str(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8)); + THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.c_str(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8), sqlite3_db_handle(stmt)); } std::string ParameterSpecificsImpl::GetColumn(sqlite3_stmt* stmt, int column) @@ -77,13 +78,13 @@ namespace AppInstaller::Repository::SQLite else { ThrowIfContainsEmbeddedNullCharacter(v); - THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8)); + THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8), sqlite3_db_handle(stmt)); } } void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, int v) { - THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, v)); + THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, v), sqlite3_db_handle(stmt)); } int ParameterSpecificsImpl::GetColumn(sqlite3_stmt* stmt, int column) @@ -93,7 +94,7 @@ namespace AppInstaller::Repository::SQLite void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, int64_t v) { - THROW_IF_SQLITE_FAILED(sqlite3_bind_int64(stmt, index, v)); + THROW_IF_SQLITE_FAILED(sqlite3_bind_int64(stmt, index, v), sqlite3_db_handle(stmt)); } int64_t ParameterSpecificsImpl::GetColumn(sqlite3_stmt* stmt, int column) @@ -103,7 +104,7 @@ namespace AppInstaller::Repository::SQLite void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, bool v) { - THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, (v ? 1 : 0))); + THROW_IF_SQLITE_FAILED(sqlite3_bind_int(stmt, index, (v ? 1 : 0)), sqlite3_db_handle(stmt)); } bool ParameterSpecificsImpl::GetColumn(sqlite3_stmt* stmt, int column) @@ -120,7 +121,7 @@ namespace AppInstaller::Repository::SQLite void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, const blob_t& v) { - THROW_IF_SQLITE_FAILED(sqlite3_bind_blob64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT)); + THROW_IF_SQLITE_FAILED(sqlite3_bind_blob64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT), sqlite3_db_handle(stmt)); } blob_t ParameterSpecificsImpl::GetColumn(sqlite3_stmt* stmt, int column) @@ -143,14 +144,14 @@ namespace AppInstaller::Repository::SQLite AICLI_LOG(SQL, Info, << "Opening SQLite connection: '" << target << "' [" << std::hex << static_cast(disposition) << ", " << std::hex << static_cast(flags) << "]"); // Always force connection serialization until we determine that there are situations where it is not needed int resultingFlags = static_cast(disposition) | static_cast(flags) | SQLITE_OPEN_FULLMUTEX; - THROW_IF_SQLITE_FAILED(sqlite3_open_v2(target.c_str(), &m_dbconn, resultingFlags, nullptr)); + THROW_IF_SQLITE_FAILED(sqlite3_open_v2(target.c_str(), &m_dbconn, resultingFlags, nullptr), nullptr); } Connection Connection::Create(const std::string& target, OpenDisposition disposition, OpenFlags flags) { Connection result{ target, disposition, flags }; - THROW_IF_SQLITE_FAILED(sqlite3_extended_result_codes(result.m_dbconn.get(), 1)); + THROW_IF_SQLITE_FAILED(sqlite3_extended_result_codes(result.m_dbconn.get(), 1), result.m_dbconn.get()); return result; } @@ -158,7 +159,7 @@ namespace AppInstaller::Repository::SQLite void Connection::EnableICU() { AICLI_LOG(SQL, Verbose, << "Enabling ICU"); - THROW_IF_SQLITE_FAILED(sqlite3IcuInit(m_dbconn.get())); + THROW_IF_SQLITE_FAILED(sqlite3IcuInit(m_dbconn.get()), m_dbconn.get()); } rowid_t Connection::GetLastInsertRowID() @@ -177,7 +178,7 @@ namespace AppInstaller::Repository::SQLite AICLI_LOG(SQL, Verbose, << "Preparing statement #" << m_id << ": " << sql); // SQL string size should include the null terminator (https://www.sqlite.org/c3ref/prepare.html) assert(sql.data()[sql.size()] == '\0'); - THROW_IF_SQLITE_FAILED(sqlite3_prepare_v2(connection, sql.data(), static_cast(sql.size() + 1), &m_stmt, nullptr)); + THROW_IF_SQLITE_FAILED(sqlite3_prepare_v2(connection, sql.data(), static_cast(sql.size() + 1), &m_stmt, nullptr), connection); } #if WINGET_SQLITE_EXPLAIN_QUERY_PLAN_ENABLED @@ -264,7 +265,7 @@ namespace AppInstaller::Repository::SQLite } else { - THROW_SQLITE(result); + THROW_SQLITE(result, sqlite3_db_handle(m_stmt.get())); } } } From d4f77b6766388b06f769876ef6c82a86972e7ef3 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 20 Jul 2022 23:38:00 -0700 Subject: [PATCH 2/2] Spelling --- .github/actions/spelling/expect.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index a285478914..cf2f16274a 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -117,6 +117,7 @@ enr enums EQU ERANGE +errmsg errno ESRB etest