Page MenuHomePhabricator
Differential D14516

[SQLite] refactor `backedUpTablesBlocklist` to be allowlist approach
AcceptedPublic
Actions

Authored by kamil on Thu, Apr 3, 2:22 AM.

Details

Reviewers
bartek
Summary

ENG-10463.

Depends on D14515

Test Plan

Upload compaction and make sure it contains only tables included in backedUpTablesAllowlist

Diff Detail

Repository
rCOMM Comm
Branch
backup-publish
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Thu, Apr 3, 2:22 AM
kamil held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptThu, Apr 3, 2:22 AM
kamil edited the summary of this revision. (Show Details)Thu, Apr 3, 2:27 AM
kamil added a parent revision: D14515: [SQLite] add logging to `handleSQLiteError`.
kamil added a child revision: D14517: [SQLite] implement method to copy table between databases.Thu, Apr 3, 2:37 AM
Harbormaster completed remote builds in B33871: Diff 47621.Thu, Apr 3, 2:40 AM
kamil published this revision for review.Thu, Apr 3, 3:05 AM
kamil added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
43–51

This is only set for testing purposes now, it will be replaced with new tables after Stop backing up thin thread data

bartek accepted this revision.Thu, Apr 3, 3:27 AM
bartek added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
3118–3125

👍 for replacing string concat with stringstream

This revision is now accepted and ready to land.Thu, Apr 3, 3:27 AM

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
DatabaseManagers/
4 lines
71 lines

Diff 47621

View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.h

Show First 20 Lines • Show All 3,330 Lines • ▼ Show 20 Linesvoid SQLiteQueryExecutor::restoreFromMainCompaction(
} }
sqlite3 *backupDB; sqlite3 *backupDB;
if (!is_database_queryable( if (!is_database_queryable(
backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) { backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) {
throw std::runtime_error("Backup file or encryption key corrupted."); throw std::runtime_error("Backup file or encryption key corrupted.");
} }
// We don't want to run `PRAGMA key = ...;`
// on main web database. The context is here:
// https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web
#ifdef EMSCRIPTEN
kamilAuthorUnsubmitted
Done
Inline Actions

We follow web approach so this comment is not needed anymore

kamil: We follow web approach so this comment is not needed anymore
std::string plaintextBackupPath = mainCompactionPath + "_plaintext"; std::string plaintextBackupPath = mainCompactionPath + "_plaintext";
if (file_exists(plaintextBackupPath)) { if (file_exists(plaintextBackupPath)) {
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext backup file from previous backup " "Failed to delete plaintext backup file from previous backup "
"attempt."); "attempt.");
} }
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" + std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" +
mainCompactionEncryptionKey + mainCompactionEncryptionKey +
"'\";" "'\";"
"ATTACH DATABASE '" + "ATTACH DATABASE '" +
plaintextBackupPath + plaintextBackupPath +
"' AS plaintext KEY '';" "' AS plaintext KEY '';"
"SELECT sqlcipher_export('plaintext');" "SELECT sqlcipher_export('plaintext');"
"DETACH DATABASE plaintext;"; "DETACH DATABASE plaintext;";
executeQuery(backupDB, plaintextMigrationDBQuery);
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
char *plaintextMigrationErr;
sqlite3_exec(
backupDB,
plaintextMigrationDBQuery.c_str(),
nullptr,
nullptr,
&plaintextMigrationErr);
sqlite3_close(backupDB); sqlite3_close(backupDB);
if (plaintextMigrationErr) {
std::stringstream error_message;
error_message << "Failed to migrate backup SQLCipher file to plaintext "
"SQLite file. Details"
<< plaintextMigrationErr << std::endl;
std::string error_message_str = error_message.str();
sqlite3_free(plaintextMigrationErr);
throw std::runtime_error(error_message_str);
}
kamilAuthorUnsubmitted
Done
Inline Actions

executeQuery does exactly the same

kamil: `executeQuery` does exactly the same
sqlite3_open(plaintextBackupPath.c_str(), &backupDB); sqlite3_open(plaintextBackupPath.c_str(), &backupDB);
#else
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
set_encryption_key(backupDB, mainCompactionEncryptionKey);
#endif
int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1); int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1);
if (version > std::stoi(maxVersion)) { if (version > std::stoi(maxVersion)) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore a backup because it was created " error_message << "Failed to restore a backup because it was created "
<< "with version " << version << "with version " << version
<< " that is newer than the max supported version " << " that is newer than the max supported version "
<< maxVersion << std::endl; << maxVersion << std::endl;
Show All 20 Lines throw std::runtime_error(
"Programmer error. Database in transaction during restore attempt."); "Programmer error. Database in transaction during restore attempt.");
} else if (backupResult != SQLITE_DONE) { } else if (backupResult != SQLITE_DONE) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore database from backup. Details: " error_message << "Failed to restore database from backup. Details: "
<< sqlite3_errstr(backupResult); << sqlite3_errstr(backupResult);
throw std::runtime_error(error_message.str()); throw std::runtime_error(error_message.str());
} }
#ifdef EMSCRIPTEN
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext compaction file after successful restore."); "Failed to delete plaintext compaction file after successful restore.");
#endif
attempt_delete_file( attempt_delete_file(
mainCompactionPath, mainCompactionPath,
"Failed to delete main compaction file after successful restore."); "Failed to delete main compaction file after successful restore.");
} }
kamilAuthorUnsubmitted
Done
Inline Actions

Previously, there was a bug, for web mainCompactionPath wasn't deleted

kamil: Previously, there was a bug, for web `mainCompactionPath` wasn't deleted
void SQLiteQueryExecutor::restoreFromBackupLog( void SQLiteQueryExecutor::restoreFromBackupLog(
const std::vector<std::uint8_t> &backupLog) const { const std::vector<std::uint8_t> &backupLog) const {
SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog); SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog);
} }
} // namespace comm } // namespace comm
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp

Show First 20 Lines • Show All 3,330 Lines • ▼ Show 20 Linesvoid SQLiteQueryExecutor::restoreFromMainCompaction(
} }
sqlite3 *backupDB; sqlite3 *backupDB;
if (!is_database_queryable( if (!is_database_queryable(
backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) { backupDB, true, mainCompactionPath, mainCompactionEncryptionKey)) {
throw std::runtime_error("Backup file or encryption key corrupted."); throw std::runtime_error("Backup file or encryption key corrupted.");
} }
// We don't want to run `PRAGMA key = ...;`
// on main web database. The context is here:
// https://linear.app/comm/issue/ENG-6398/issues-with-sqlcipher-on-web
#ifdef EMSCRIPTEN
kamilAuthorUnsubmitted
Done
Inline Actions

We follow web approach so this comment is not needed anymore

kamil: We follow web approach so this comment is not needed anymore
std::string plaintextBackupPath = mainCompactionPath + "_plaintext"; std::string plaintextBackupPath = mainCompactionPath + "_plaintext";
if (file_exists(plaintextBackupPath)) { if (file_exists(plaintextBackupPath)) {
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext backup file from previous backup " "Failed to delete plaintext backup file from previous backup "
"attempt."); "attempt.");
} }
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" + std::string plaintextMigrationDBQuery = "PRAGMA key = \"x'" +
mainCompactionEncryptionKey + mainCompactionEncryptionKey +
"'\";" "'\";"
"ATTACH DATABASE '" + "ATTACH DATABASE '" +
plaintextBackupPath + plaintextBackupPath +
"' AS plaintext KEY '';" "' AS plaintext KEY '';"
"SELECT sqlcipher_export('plaintext');" "SELECT sqlcipher_export('plaintext');"
"DETACH DATABASE plaintext;"; "DETACH DATABASE plaintext;";
executeQuery(backupDB, plaintextMigrationDBQuery);
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
char *plaintextMigrationErr;
sqlite3_exec(
backupDB,
plaintextMigrationDBQuery.c_str(),
nullptr,
nullptr,
&plaintextMigrationErr);
sqlite3_close(backupDB); sqlite3_close(backupDB);
if (plaintextMigrationErr) {
std::stringstream error_message;
error_message << "Failed to migrate backup SQLCipher file to plaintext "
"SQLite file. Details"
<< plaintextMigrationErr << std::endl;
std::string error_message_str = error_message.str();
sqlite3_free(plaintextMigrationErr);
throw std::runtime_error(error_message_str);
}
kamilAuthorUnsubmitted
Done
Inline Actions

executeQuery does exactly the same

kamil: `executeQuery` does exactly the same
sqlite3_open(plaintextBackupPath.c_str(), &backupDB); sqlite3_open(plaintextBackupPath.c_str(), &backupDB);
#else
sqlite3_open(mainCompactionPath.c_str(), &backupDB);
set_encryption_key(backupDB, mainCompactionEncryptionKey);
#endif
int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1); int version = this->getSyncedDatabaseVersion(backupDB).value_or(-1);
if (version > std::stoi(maxVersion)) { if (version > std::stoi(maxVersion)) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore a backup because it was created " error_message << "Failed to restore a backup because it was created "
<< "with version " << version << "with version " << version
<< " that is newer than the max supported version " << " that is newer than the max supported version "
<< maxVersion << std::endl; << maxVersion << std::endl;
Show All 20 Lines throw std::runtime_error(
"Programmer error. Database in transaction during restore attempt."); "Programmer error. Database in transaction during restore attempt.");
} else if (backupResult != SQLITE_DONE) { } else if (backupResult != SQLITE_DONE) {
std::stringstream error_message; std::stringstream error_message;
error_message << "Failed to restore database from backup. Details: " error_message << "Failed to restore database from backup. Details: "
<< sqlite3_errstr(backupResult); << sqlite3_errstr(backupResult);
throw std::runtime_error(error_message.str()); throw std::runtime_error(error_message.str());
} }
#ifdef EMSCRIPTEN
attempt_delete_file( attempt_delete_file(
plaintextBackupPath, plaintextBackupPath,
"Failed to delete plaintext compaction file after successful restore."); "Failed to delete plaintext compaction file after successful restore.");
#endif
attempt_delete_file( attempt_delete_file(
mainCompactionPath, mainCompactionPath,
"Failed to delete main compaction file after successful restore."); "Failed to delete main compaction file after successful restore.");
} }
kamilAuthorUnsubmitted
Done
Inline Actions

Previously, there was a bug, for web mainCompactionPath wasn't deleted

kamil: Previously, there was a bug, for web `mainCompactionPath` wasn't deleted
void SQLiteQueryExecutor::restoreFromBackupLog( void SQLiteQueryExecutor::restoreFromBackupLog(
const std::vector<std::uint8_t> &backupLog) const { const std::vector<std::uint8_t> &backupLog) const {
SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog); SQLiteQueryExecutor::connectionManager.restoreFromBackupLog(backupLog);
} }
} // namespace comm } // namespace comm