From ae2f834345e8b54ae6cd3f0f13bbf98190f8aee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= <76261501+zecakeh@users.noreply.github.com> Date: Fri, 9 Jun 2023 13:02:33 +0200 Subject: [PATCH] sqlite: Return an error when DB version is invalid or missing (#2038) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- crates/matrix-sdk-sqlite/src/crypto_store.rs | 39 +++++--------------- crates/matrix-sdk-sqlite/src/error.rs | 12 ++++++ crates/matrix-sdk-sqlite/src/state_store.rs | 36 ++++-------------- crates/matrix-sdk-sqlite/src/utils.rs | 25 +++++++++++++ 4 files changed, 54 insertions(+), 58 deletions(-) diff --git a/crates/matrix-sdk-sqlite/src/crypto_store.rs b/crates/matrix-sdk-sqlite/src/crypto_store.rs index eaf2c9a71..d9c43d8f2 100644 --- a/crates/matrix-sdk-sqlite/src/crypto_store.rs +++ b/crates/matrix-sdk-sqlite/src/crypto_store.rs @@ -37,12 +37,14 @@ use ruma::{DeviceId, OwnedDeviceId, OwnedUserId, RoomId, TransactionId, UserId}; use rusqlite::OptionalExtension; use serde::{de::DeserializeOwned, Serialize}; use tokio::{fs, sync::Mutex}; -use tracing::{debug, error, instrument, warn}; +use tracing::{debug, instrument, warn}; use crate::{ error::{Error, Result}, get_or_create_store_cipher, - utils::{Key, SqliteConnectionExt as _, SqliteObjectExt, SqliteObjectStoreExt as _}, + utils::{ + load_db_version, Key, SqliteConnectionExt as _, SqliteObjectExt, SqliteObjectStoreExt as _, + }, OpenStoreError, }; @@ -98,7 +100,8 @@ impl SqliteCryptoStore { passphrase: Option<&str>, ) -> Result { let conn = pool.get().await?; - run_migrations(&conn).await.map_err(OpenStoreError::Migration)?; + let version = load_db_version(&conn).await?; + run_migrations(&conn, version).await.map_err(OpenStoreError::Migration)?; let store_cipher = match passphrase { Some(p) => Some(Arc::new(get_or_create_store_cipher(p, &conn).await?)), None => None, @@ -192,36 +195,14 @@ impl SqliteCryptoStore { const DATABASE_VERSION: u8 = 6; -async fn run_migrations(conn: &SqliteConn) -> rusqlite::Result<()> { - let kv_exists = conn - .query_row( - "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'kv'", - (), - |row| row.get::<_, u32>(0), - ) - .await? - > 0; - - let version = if kv_exists { - match conn.get_kv("version").await?.as_deref() { - Some([v]) => *v, - Some(_) => { - error!("version database field has multiple bytes"); - return Ok(()); - } - None => { - error!("version database field is missing"); - return Ok(()); - } - } - } else { - 0 - }; - +/// Run migrations for the given version of the database. +async fn run_migrations(conn: &SqliteConn, version: u8) -> rusqlite::Result<()> { if version == 0 { debug!("Creating database"); } else if version < DATABASE_VERSION { debug!(version, new_version = DATABASE_VERSION, "Upgrading database"); + } else { + return Ok(()); } if version < 1 { diff --git a/crates/matrix-sdk-sqlite/src/error.rs b/crates/matrix-sdk-sqlite/src/error.rs index cd557952e..012bd4dd1 100644 --- a/crates/matrix-sdk-sqlite/src/error.rs +++ b/crates/matrix-sdk-sqlite/src/error.rs @@ -32,6 +32,18 @@ pub enum OpenStoreError { #[error(transparent)] CreatePool(#[from] CreatePoolError), + /// Failed to load the database's version. + #[error("Failed to load database version")] + LoadVersion(#[source] rusqlite::Error), + + /// The version of the database is missing. + #[error("Missing database version")] + MissingVersion, + + /// The version of the database is invalid. + #[error("Invalid database version")] + InvalidVersion, + /// Failed to apply migrations. #[error("Failed to run migrations")] Migration(#[source] rusqlite::Error), diff --git a/crates/matrix-sdk-sqlite/src/state_store.rs b/crates/matrix-sdk-sqlite/src/state_store.rs index 604082894..acfa7419f 100644 --- a/crates/matrix-sdk-sqlite/src/state_store.rs +++ b/crates/matrix-sdk-sqlite/src/state_store.rs @@ -30,12 +30,12 @@ use ruma::{ use rusqlite::{OptionalExtension, Transaction}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use tokio::fs; -use tracing::{debug, error, warn}; +use tracing::{debug, warn}; use crate::{ error::{Error, Result}, get_or_create_store_cipher, - utils::{chain, Key, SqliteObjectExt}, + utils::{chain, load_db_version, Key, SqliteObjectExt}, OpenStoreError, SqliteObjectStoreExt, }; @@ -93,7 +93,8 @@ impl SqliteStateStore { passphrase: Option<&str>, ) -> Result { let conn = pool.get().await?; - run_migrations(&conn).await.map_err(OpenStoreError::Migration)?; + let version = load_db_version(&conn).await?; + run_migrations(&conn, version).await.map_err(OpenStoreError::Migration)?; let store_cipher = match passphrase { Some(p) => Some(Arc::new(get_or_create_store_cipher(p, &conn).await?)), None => None, @@ -197,36 +198,13 @@ impl SqliteStateStore { const DATABASE_VERSION: u8 = 1; -async fn run_migrations(conn: &SqliteConn) -> rusqlite::Result<()> { - let kv_exists = conn - .query_row( - "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'kv'", - (), - |row| row.get::<_, u32>(0), - ) - .await? - > 0; - - let version = if kv_exists { - match conn.get_kv("version").await?.as_deref() { - Some([v]) => *v, - Some(_) => { - error!("version database field has multiple bytes"); - return Ok(()); - } - None => { - error!("version database field is missing"); - return Ok(()); - } - } - } else { - 0 - }; - +async fn run_migrations(conn: &SqliteConn, version: u8) -> rusqlite::Result<()> { if version == 0 { debug!("Creating database"); } else if version < DATABASE_VERSION { debug!(version, new_version = DATABASE_VERSION, "Upgrading database"); + } else { + return Ok(()); } if version < 1 { diff --git a/crates/matrix-sdk-sqlite/src/utils.rs b/crates/matrix-sdk-sqlite/src/utils.rs index c3e2494a4..0bd6c42a5 100644 --- a/crates/matrix-sdk-sqlite/src/utils.rs +++ b/crates/matrix-sdk-sqlite/src/utils.rs @@ -17,6 +17,8 @@ use std::ops::Deref; use async_trait::async_trait; use rusqlite::{OptionalExtension, Params, Row, Statement, Transaction}; +use crate::OpenStoreError; + pub(crate) fn chain( it1: impl IntoIterator, it2: impl IntoIterator, @@ -181,3 +183,26 @@ impl SqliteObjectStoreExt for deadpool_sqlite::Object { Ok(()) } } + +/// Load the version of the database with the given connection. +pub(crate) async fn load_db_version(conn: &deadpool_sqlite::Object) -> Result { + let kv_exists = conn + .query_row( + "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'kv'", + (), + |row| row.get::<_, u32>(0), + ) + .await + .map_err(OpenStoreError::LoadVersion)? + > 0; + + if kv_exists { + match conn.get_kv("version").await.map_err(OpenStoreError::LoadVersion)?.as_deref() { + Some([v]) => Ok(*v), + Some(_) => Err(OpenStoreError::InvalidVersion), + None => Err(OpenStoreError::MissingVersion), + } + } else { + Ok(0) + } +}