From f596b3ce0519331224cd938e666f0a6f15410a7a Mon Sep 17 00:00:00 2001 From: Christien Rioux Date: Mon, 18 Sep 2023 15:22:40 -0400 Subject: [PATCH] more clippy --- veilid-core/src/api_tracing_layer.rs | 4 +- veilid-core/src/attachment_manager.rs | 16 +- veilid-core/src/crypto/tests/test_types.rs | 18 +- veilid-core/src/network_manager/stats.rs | 10 +- veilid-core/src/receipt_manager.rs | 29 +- .../src/routing_table/route_spec_store/mod.rs | 4 +- veilid-core/src/rpc_processor/rpc_app_call.rs | 4 +- .../src/rpc_processor/rpc_app_message.rs | 4 +- veilid-core/src/table_store/mod.rs | 581 ++++++++++++++++- veilid-core/src/table_store/native.rs | 4 +- veilid-core/src/table_store/table_store.rs | 589 ------------------ .../src/table_store/tests/test_table_store.rs | 2 +- .../src/tests/common/test_protected_store.rs | 40 +- .../src/tests/common/test_veilid_config.rs | 38 +- veilid-core/src/veilid_api/api.rs | 4 +- veilid-core/src/veilid_api/debug.rs | 73 ++- veilid-core/src/veilid_api/json_api/mod.rs | 4 +- .../src/veilid_api/json_api/process.rs | 48 +- .../veilid_api/json_api/routing_context.rs | 4 +- .../serialize_helpers/compression.rs | 2 +- .../serialize_helpers/serialize_json.rs | 6 +- .../src/veilid_api/tests/test_types.rs | 24 +- .../src/veilid_api/types/dht/schema/dflt.rs | 2 +- .../src/veilid_api/types/dht/schema/smpl.rs | 2 +- .../types/dht/value_subkey_range_set.rs | 7 +- .../src/veilid_api/types/veilid_state.rs | 22 +- veilid-core/src/veilid_config.rs | 14 +- veilid-server/src/client_api.rs | 10 +- veilid-server/src/main.rs | 26 +- veilid-server/src/server.rs | 21 +- veilid-server/src/settings.rs | 76 ++- veilid-server/src/unix.rs | 4 +- 32 files changed, 839 insertions(+), 853 deletions(-) delete mode 100644 veilid-core/src/table_store/table_store.rs diff --git a/veilid-core/src/api_tracing_layer.rs b/veilid-core/src/api_tracing_layer.rs index 8de371ec..44b9b67a 100644 --- a/veilid-core/src/api_tracing_layer.rs +++ b/veilid-core/src/api_tracing_layer.rs @@ -103,11 +103,11 @@ impl registry::LookupSpan<'a>> Layer for ApiTracingLa None }; - (inner.update_callback)(VeilidUpdate::Log(VeilidLog { + (inner.update_callback)(VeilidUpdate::Log(Box::new(VeilidLog { log_level, message, backtrace, - })) + }))) } } } diff --git a/veilid-core/src/attachment_manager.rs b/veilid-core/src/attachment_manager.rs index 14b7856d..40fa9b78 100644 --- a/veilid-core/src/attachment_manager.rs +++ b/veilid-core/src/attachment_manager.rs @@ -168,7 +168,7 @@ impl AttachmentManager { }) .unwrap_or(true); if send_update { - Some((update_callback, Self::get_veilid_state_inner(&*inner))) + Some((update_callback, Self::get_veilid_state_inner(&inner))) } else { None } @@ -197,11 +197,11 @@ impl AttachmentManager { }; if let Some(update_callback) = update_callback { - update_callback(VeilidUpdate::Attachment(VeilidStateAttachment { + update_callback(VeilidUpdate::Attachment(Box::new(VeilidStateAttachment { state, public_internet_ready: false, local_network_ready: false, - })) + }))) } } @@ -325,8 +325,8 @@ impl AttachmentManager { // self.inner.lock().last_attachment_state // } - fn get_veilid_state_inner(inner: &AttachmentManagerInner) -> VeilidStateAttachment { - VeilidStateAttachment { + fn get_veilid_state_inner(inner: &AttachmentManagerInner) -> Box { + Box::new(VeilidStateAttachment { state: inner.last_attachment_state, public_internet_ready: inner .last_routing_table_health @@ -338,11 +338,11 @@ impl AttachmentManager { .as_ref() .map(|x| x.local_network_ready) .unwrap_or(false), - } + }) } - pub fn get_veilid_state(&self) -> VeilidStateAttachment { + pub fn get_veilid_state(&self) -> Box { let inner = self.inner.lock(); - Self::get_veilid_state_inner(&*inner) + Self::get_veilid_state_inner(&inner) } } diff --git a/veilid-core/src/crypto/tests/test_types.rs b/veilid-core/src/crypto/tests/test_types.rs index b2f7a3bc..6d08d34b 100644 --- a/veilid-core/src/crypto/tests/test_types.rs +++ b/veilid-core/src/crypto/tests/test_types.rs @@ -1,5 +1,3 @@ -#![allow(clippy::bool_assert_comparison)] - use super::*; use core::convert::TryFrom; @@ -338,14 +336,14 @@ async fn test_operations(vcrypto: CryptoSystemVersion) { assert_eq!(d4.first_nonzero_nibble(), Some((0, 0x9u8))); // Verify bits - assert_eq!(d1.bit(0), true); - assert_eq!(d1.bit(1), false); - assert_eq!(d1.bit(7), false); - assert_eq!(d1.bit(8), false); - assert_eq!(d1.bit(14), true); - assert_eq!(d1.bit(15), false); - assert_eq!(d1.bit(254), true); - assert_eq!(d1.bit(255), false); + assert!(d1.bit(0)); + assert!(!d1.bit(1)); + assert!(!d1.bit(7)); + assert!(!d1.bit(8)); + assert!(d1.bit(14)); + assert!(!d1.bit(15)); + assert!(d1.bit(254)); + assert!(!d1.bit(255)); assert_eq!(d1.first_nonzero_bit(), Some(0)); assert_eq!(d2.first_nonzero_bit(), Some(0)); diff --git a/veilid-core/src/network_manager/stats.rs b/veilid-core/src/network_manager/stats.rs index 24836707..f96da01d 100644 --- a/veilid-core/src/network_manager/stats.rs +++ b/veilid-core/src/network_manager/stats.rs @@ -73,7 +73,7 @@ impl NetworkManager { inner.stats.clone() } - pub fn get_veilid_state(&self) -> VeilidStateNetwork { + pub fn get_veilid_state(&self) -> Box { let has_state = self .unlocked_inner .components @@ -83,12 +83,12 @@ impl NetworkManager { .unwrap_or(false); if !has_state { - return VeilidStateNetwork { + return Box::new(VeilidStateNetwork { started: false, bps_down: 0.into(), bps_up: 0.into(), peers: Vec::new(), - }; + }); } let routing_table = self.routing_table(); @@ -100,7 +100,7 @@ impl NetworkManager { ) }; - VeilidStateNetwork { + Box::new(VeilidStateNetwork { started: true, bps_down, bps_up, @@ -119,7 +119,7 @@ impl NetworkManager { } out }, - } + }) } pub(super) fn send_network_update(&self) { diff --git a/veilid-core/src/receipt_manager.rs b/veilid-core/src/receipt_manager.rs index 0df15aa1..416e5573 100644 --- a/veilid-core/src/receipt_manager.rs +++ b/veilid-core/src/receipt_manager.rs @@ -261,9 +261,8 @@ impl ReceiptManager { // Wait on all the multi-call callbacks loop { - match callbacks.next().timeout_at(stop_token.clone()).await { - Ok(Some(_)) => {} - Ok(None) | Err(_) => break, + if let Ok(None) | Err(_) = callbacks.next().timeout_at(stop_token.clone()).await { + break; } } } @@ -307,7 +306,7 @@ impl ReceiptManager { // Wait for everything to stop debug!("waiting for timeout task to stop"); - if !timeout_task.join().await.is_ok() { + if timeout_task.join().await.is_err() { panic!("joining timeout task failed"); } @@ -333,7 +332,7 @@ impl ReceiptManager { let mut inner = self.inner.lock(); inner.records_by_nonce.insert(receipt_nonce, record); - Self::update_next_oldest_timestamp(&mut *inner); + Self::update_next_oldest_timestamp(&mut inner); } pub fn record_single_shot_receipt( @@ -351,7 +350,7 @@ impl ReceiptManager { let mut inner = self.inner.lock(); inner.records_by_nonce.insert(receipt_nonce, record); - Self::update_next_oldest_timestamp(&mut *inner); + Self::update_next_oldest_timestamp(&mut inner); } fn update_next_oldest_timestamp(inner: &mut ReceiptManagerInner) { @@ -382,7 +381,7 @@ impl ReceiptManager { bail!("receipt not recorded"); } }; - Self::update_next_oldest_timestamp(&mut *inner); + Self::update_next_oldest_timestamp(&mut inner); record }; @@ -448,14 +447,12 @@ impl ReceiptManager { let receipt_event = match receipt_returned { ReceiptReturned::OutOfBand => ReceiptEvent::ReturnedOutOfBand, ReceiptReturned::Safety => ReceiptEvent::ReturnedSafety, - ReceiptReturned::InBand { - ref inbound_noderef, - } => ReceiptEvent::ReturnedInBand { - inbound_noderef: inbound_noderef.clone(), - }, - ReceiptReturned::Private { ref private_route } => ReceiptEvent::ReturnedPrivate { - private_route: private_route.clone(), - }, + ReceiptReturned::InBand { inbound_noderef } => { + ReceiptEvent::ReturnedInBand { inbound_noderef } + } + ReceiptReturned::Private { private_route } => { + ReceiptEvent::ReturnedPrivate { private_route } + } }; let callback_future = Self::perform_callback(receipt_event, &mut record_mut); @@ -464,7 +461,7 @@ impl ReceiptManager { if record_mut.returns_so_far == record_mut.expected_returns { inner.records_by_nonce.remove(&receipt_nonce); - Self::update_next_oldest_timestamp(&mut *inner); + Self::update_next_oldest_timestamp(&mut inner); } (callback_future, stop_token) }; diff --git a/veilid-core/src/routing_table/route_spec_store/mod.rs b/veilid-core/src/routing_table/route_spec_store/mod.rs index 3f1f19fb..9cee1340 100644 --- a/veilid-core/src/routing_table/route_spec_store/mod.rs +++ b/veilid-core/src/routing_table/route_spec_store/mod.rs @@ -141,10 +141,10 @@ impl RouteSpecStore { dr }; - let update = VeilidUpdate::RouteChange(VeilidRouteChange { + let update = VeilidUpdate::RouteChange(Box::new(VeilidRouteChange { dead_routes, dead_remote_routes, - }); + })); let update_callback = self.unlocked_inner.routing_table.update_callback(); update_callback(update); diff --git a/veilid-core/src/rpc_processor/rpc_app_call.rs b/veilid-core/src/rpc_processor/rpc_app_call.rs index 4a34c796..37c21deb 100644 --- a/veilid-core/src/rpc_processor/rpc_app_call.rs +++ b/veilid-core/src/rpc_processor/rpc_app_call.rs @@ -94,9 +94,9 @@ impl RPCProcessor { // Pass the call up through the update callback let message_q = app_call_q.destructure(); - (self.unlocked_inner.update_callback)(VeilidUpdate::AppCall(VeilidAppCall::new( + (self.unlocked_inner.update_callback)(VeilidUpdate::AppCall(Box::new(VeilidAppCall::new( sender, message_q, op_id, - ))); + )))); // Wait for an app call answer to come back from the app let res = self diff --git a/veilid-core/src/rpc_processor/rpc_app_message.rs b/veilid-core/src/rpc_processor/rpc_app_message.rs index 69ac7046..9f6a3d89 100644 --- a/veilid-core/src/rpc_processor/rpc_app_message.rs +++ b/veilid-core/src/rpc_processor/rpc_app_message.rs @@ -58,8 +58,8 @@ impl RPCProcessor { // Pass the message up through the update callback let message = app_message.destructure(); - (self.unlocked_inner.update_callback)(VeilidUpdate::AppMessage(VeilidAppMessage::new( - sender, message, + (self.unlocked_inner.update_callback)(VeilidUpdate::AppMessage(Box::new( + VeilidAppMessage::new(sender, message), ))); Ok(NetworkResult::value(())) diff --git a/veilid-core/src/table_store/mod.rs b/veilid-core/src/table_store/mod.rs index 1e2f27e8..8d623ea8 100644 --- a/veilid-core/src/table_store/mod.rs +++ b/veilid-core/src/table_store/mod.rs @@ -1,9 +1,7 @@ use super::*; mod table_db; -mod table_store; pub use table_db::*; -pub use table_store::*; pub mod tests; @@ -15,3 +13,582 @@ use wasm::*; mod native; #[cfg(not(target_arch = "wasm32"))] use native::*; + +use keyvaluedb::*; + +const ALL_TABLE_NAMES: &[u8] = b"all_table_names"; + +struct TableStoreInner { + opened: BTreeMap>, + encryption_key: Option, + all_table_names: HashMap, + all_tables_db: Option, + crypto: Option, +} + +/// Veilid Table Storage +/// Database for storing key value pairs persistently and securely across runs +#[derive(Clone)] +pub struct TableStore { + config: VeilidConfig, + protected_store: ProtectedStore, + table_store_driver: TableStoreDriver, + inner: Arc>, // Sync mutex here because TableDB drops can happen at any time + async_lock: Arc>, // Async mutex for operations +} + +impl TableStore { + fn new_inner() -> TableStoreInner { + TableStoreInner { + opened: BTreeMap::new(), + encryption_key: None, + all_table_names: HashMap::new(), + all_tables_db: None, + crypto: None, + } + } + pub(crate) fn new(config: VeilidConfig, protected_store: ProtectedStore) -> Self { + let inner = Self::new_inner(); + let table_store_driver = TableStoreDriver::new(config.clone()); + + Self { + config, + protected_store, + inner: Arc::new(Mutex::new(inner)), + table_store_driver, + async_lock: Arc::new(AsyncMutex::new(())), + } + } + + pub(crate) fn set_crypto(&self, crypto: Crypto) { + let mut inner = self.inner.lock(); + inner.crypto = Some(crypto); + } + + // Flush internal control state (must not use crypto) + async fn flush(&self) { + let (all_table_names_value, all_tables_db) = { + let inner = self.inner.lock(); + let all_table_names_value = serialize_json_bytes(&inner.all_table_names); + (all_table_names_value, inner.all_tables_db.clone().unwrap()) + }; + let mut dbt = DBTransaction::new(); + dbt.put(0, ALL_TABLE_NAMES, &all_table_names_value); + if let Err(e) = all_tables_db.write(dbt).await { + error!("failed to write all tables db: {}", e); + } + } + + // Internal naming support + // Adds rename capability and ensures names of tables are totally unique and valid + + fn namespaced_name(&self, table: &str) -> VeilidAPIResult { + if !table + .chars() + .all(|c| char::is_alphanumeric(c) || c == '_' || c == '-') + { + apibail_invalid_argument!("table name is invalid", "table", table); + } + let c = self.config.get(); + let namespace = c.namespace.clone(); + Ok(if namespace.is_empty() { + table.to_string() + } else { + format!("_ns_{}_{}", namespace, table) + }) + } + + async fn name_get_or_create(&self, table: &str) -> VeilidAPIResult { + let name = self.namespaced_name(table)?; + + let mut inner = self.inner.lock(); + // Do we have this name yet? + if let Some(real_name) = inner.all_table_names.get(&name) { + return Ok(real_name.clone()); + } + + // If not, make a new low level name mapping + let mut real_name_bytes = [0u8; 32]; + random_bytes(&mut real_name_bytes); + let real_name = data_encoding::BASE64URL_NOPAD.encode(&real_name_bytes); + + if inner + .all_table_names + .insert(name.to_owned(), real_name.clone()) + .is_some() + { + panic!("should not have had some value"); + }; + + Ok(real_name) + } + + async fn name_delete(&self, table: &str) -> VeilidAPIResult> { + let name = self.namespaced_name(table)?; + let mut inner = self.inner.lock(); + let real_name = inner.all_table_names.remove(&name); + Ok(real_name) + } + + async fn name_get(&self, table: &str) -> VeilidAPIResult> { + let name = self.namespaced_name(table)?; + let inner = self.inner.lock(); + let real_name = inner.all_table_names.get(&name).cloned(); + Ok(real_name) + } + + async fn name_rename(&self, old_table: &str, new_table: &str) -> VeilidAPIResult<()> { + let old_name = self.namespaced_name(old_table)?; + let new_name = self.namespaced_name(new_table)?; + + let mut inner = self.inner.lock(); + // Ensure new name doesn't exist + if inner.all_table_names.contains_key(&new_name) { + return Err(VeilidAPIError::generic("new table already exists")); + } + // Do we have this name yet? + let Some(real_name) = inner.all_table_names.remove(&old_name) else { + return Err(VeilidAPIError::generic("table does not exist")); + }; + // Insert with new name + inner.all_table_names.insert(new_name.to_owned(), real_name); + + Ok(()) + } + + /// Delete all known tables + async fn delete_all(&self) { + // Get all tables + let real_names = { + let mut inner = self.inner.lock(); + let real_names = inner + .all_table_names + .values() + .cloned() + .collect::>(); + inner.all_table_names.clear(); + real_names + }; + + // Delete all tables + for table_name in real_names { + if let Err(e) = self.table_store_driver.delete(&table_name).await { + error!("error deleting table: {}", e); + } + } + self.flush().await; + } + + pub(crate) fn maybe_unprotect_device_encryption_key( + &self, + dek_bytes: &[u8], + device_encryption_key_password: &str, + ) -> EyreResult { + // Ensure the key is at least as long as necessary if unencrypted + if dek_bytes.len() < (4 + SHARED_SECRET_LENGTH) { + bail!("device encryption key is not valid"); + } + + // Get cryptosystem + let kind = FourCC::try_from(&dek_bytes[0..4]).unwrap(); + let crypto = self.inner.lock().crypto.as_ref().unwrap().clone(); + let Some(vcrypto) = crypto.get(kind) else { + bail!("unsupported cryptosystem"); + }; + + if !device_encryption_key_password.is_empty() { + if dek_bytes.len() + != (4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead() + NONCE_LENGTH) + { + bail!("password protected device encryption key is not valid"); + } + let protected_key = &dek_bytes[4..(4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead())]; + let nonce = + Nonce::try_from(&dek_bytes[(4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead())..]) + .wrap_err("invalid nonce")?; + + let shared_secret = vcrypto + .derive_shared_secret(device_encryption_key_password.as_bytes(), &nonce.bytes) + .wrap_err("failed to derive shared secret")?; + let unprotected_key = vcrypto + .decrypt_aead(protected_key, &nonce, &shared_secret, None) + .wrap_err("failed to decrypt device encryption key")?; + return Ok(TypedSharedSecret::new( + kind, + SharedSecret::try_from(unprotected_key.as_slice()) + .wrap_err("invalid shared secret")?, + )); + } + + if dek_bytes.len() != (4 + SHARED_SECRET_LENGTH) { + bail!("password protected device encryption key is not valid"); + } + + Ok(TypedSharedSecret::new( + kind, + SharedSecret::try_from(&dek_bytes[4..])?, + )) + } + + pub(crate) fn maybe_protect_device_encryption_key( + &self, + dek: TypedSharedSecret, + device_encryption_key_password: &str, + ) -> EyreResult> { + // Check if we are to protect the key + if device_encryption_key_password.is_empty() { + debug!("no dek password"); + // Return the unprotected key bytes + let mut out = Vec::with_capacity(4 + SHARED_SECRET_LENGTH); + out.extend_from_slice(&dek.kind.0); + out.extend_from_slice(&dek.value.bytes); + return Ok(out); + } + + // Get cryptosystem + let crypto = self.inner.lock().crypto.as_ref().unwrap().clone(); + let Some(vcrypto) = crypto.get(dek.kind) else { + bail!("unsupported cryptosystem"); + }; + + let nonce = vcrypto.random_nonce(); + let shared_secret = vcrypto + .derive_shared_secret(device_encryption_key_password.as_bytes(), &nonce.bytes) + .wrap_err("failed to derive shared secret")?; + let mut protected_key = vcrypto + .encrypt_aead(&dek.value.bytes, &nonce, &shared_secret, None) + .wrap_err("failed to decrypt device encryption key")?; + let mut out = + Vec::with_capacity(4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead() + NONCE_LENGTH); + out.extend_from_slice(&dek.kind.0); + out.append(&mut protected_key); + out.extend_from_slice(&nonce.bytes); + assert!(out.len() == 4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead() + NONCE_LENGTH); + Ok(out) + } + + async fn load_device_encryption_key(&self) -> EyreResult> { + let dek_bytes: Option> = self + .protected_store + .load_user_secret("device_encryption_key") + .await?; + let Some(dek_bytes) = dek_bytes else { + debug!("no device encryption key"); + return Ok(None); + }; + + // Get device encryption key protection password if we have it + let device_encryption_key_password = { + let c = self.config.get(); + c.protected_store.device_encryption_key_password.clone() + }; + + Ok(Some(self.maybe_unprotect_device_encryption_key( + &dek_bytes, + &device_encryption_key_password, + )?)) + } + async fn save_device_encryption_key( + &self, + device_encryption_key: Option, + ) -> EyreResult<()> { + let Some(device_encryption_key) = device_encryption_key else { + // Remove the device encryption key + let existed = self + .protected_store + .remove_user_secret("device_encryption_key") + .await?; + debug!("removed device encryption key. existed: {}", existed); + return Ok(()); + }; + + // Get new device encryption key protection password if we are changing it + let new_device_encryption_key_password = { + let c = self.config.get(); + c.protected_store.new_device_encryption_key_password.clone() + }; + let device_encryption_key_password = + if let Some(new_device_encryption_key_password) = new_device_encryption_key_password { + // Change password + debug!("changing dek password"); + self.config + .with_mut(|c| { + c.protected_store.device_encryption_key_password = + new_device_encryption_key_password.clone(); + Ok(new_device_encryption_key_password) + }) + .unwrap() + } else { + // Get device encryption key protection password if we have it + debug!("saving with existing dek password"); + let c = self.config.get(); + c.protected_store.device_encryption_key_password.clone() + }; + + let dek_bytes = self.maybe_protect_device_encryption_key( + device_encryption_key, + &device_encryption_key_password, + )?; + + // Save the new device encryption key + let existed = self + .protected_store + .save_user_secret("device_encryption_key", &dek_bytes) + .await?; + debug!("saving device encryption key. existed: {}", existed); + Ok(()) + } + + pub(crate) async fn init(&self) -> EyreResult<()> { + let _async_guard = self.async_lock.lock().await; + + // Get device encryption key from protected store + let mut device_encryption_key = self.load_device_encryption_key().await?; + let mut device_encryption_key_changed = false; + if let Some(device_encryption_key) = device_encryption_key { + // If encryption in current use is not the best encryption, then run table migration + let best_kind = best_crypto_kind(); + if device_encryption_key.kind != best_kind { + // XXX: Run migration. See issue #209 + } + } else { + // If we don't have an encryption key yet, then make one with the best cryptography and save it + let best_kind = best_crypto_kind(); + let mut shared_secret = SharedSecret::default(); + random_bytes(&mut shared_secret.bytes); + + device_encryption_key = Some(TypedSharedSecret::new(best_kind, shared_secret)); + device_encryption_key_changed = true; + } + + // Check for password change + let changing_password = self + .config + .get() + .protected_store + .new_device_encryption_key_password + .is_some(); + + // Save encryption key if it has changed or if the protecting password wants to change + if device_encryption_key_changed || changing_password { + self.save_device_encryption_key(device_encryption_key) + .await?; + } + + // Deserialize all table names + let all_tables_db = self + .table_store_driver + .open("__veilid_all_tables", 1) + .await + .wrap_err("failed to create all tables table")?; + match all_tables_db.get(0, ALL_TABLE_NAMES).await { + Ok(Some(v)) => match deserialize_json_bytes::>(&v) { + Ok(all_table_names) => { + let mut inner = self.inner.lock(); + inner.all_table_names = all_table_names; + } + Err(e) => { + error!("could not deserialize __veilid_all_tables: {}", e); + } + }, + Ok(None) => { + // No table names yet, that's okay + trace!("__veilid_all_tables is empty"); + } + Err(e) => { + error!("could not get __veilid_all_tables: {}", e); + } + }; + + { + let mut inner = self.inner.lock(); + inner.encryption_key = device_encryption_key; + inner.all_tables_db = Some(all_tables_db); + } + + let do_delete = { + let c = self.config.get(); + c.table_store.delete + }; + + if do_delete { + self.delete_all().await; + } + + Ok(()) + } + + pub(crate) async fn terminate(&self) { + let _async_guard = self.async_lock.lock().await; + + self.flush().await; + + let mut inner = self.inner.lock(); + if !inner.opened.is_empty() { + panic!( + "all open databases should have been closed: {:?}", + inner.opened + ); + } + inner.all_tables_db = None; + inner.all_table_names.clear(); + inner.encryption_key = None; + } + + pub(crate) fn on_table_db_drop(&self, table: String) { + log_rtab!("dropping table db: {}", table); + let mut inner = self.inner.lock(); + if inner.opened.remove(&table).is_none() { + unreachable!("should have removed an item"); + } + } + + /// Get or create a TableDB database table. If the column count is greater than an + /// existing TableDB's column count, the database will be upgraded to add the missing columns + pub async fn open(&self, name: &str, column_count: u32) -> VeilidAPIResult { + let _async_guard = self.async_lock.lock().await; + + // If we aren't initialized yet, bail + { + let inner = self.inner.lock(); + if inner.all_tables_db.is_none() { + apibail_not_initialized!(); + } + } + + let table_name = self.name_get_or_create(name).await?; + + // See if this table is already opened, if so the column count must be the same + { + let mut inner = self.inner.lock(); + if let Some(table_db_weak_inner) = inner.opened.get(&table_name) { + match TableDB::try_new_from_weak_inner(table_db_weak_inner.clone(), column_count) { + Some(tdb) => { + // Ensure column count isnt bigger + let existing_col_count = tdb.get_column_count()?; + if column_count > existing_col_count { + return Err(VeilidAPIError::generic(format!( + "database must be closed before increasing column count {} -> {}", + existing_col_count, column_count, + ))); + } + + return Ok(tdb); + } + None => { + inner.opened.remove(&table_name); + } + }; + } + } + + // Open table db using platform-specific driver + let mut db = match self + .table_store_driver + .open(&table_name, column_count) + .await + { + Ok(db) => db, + Err(e) => { + self.name_delete(name).await.expect("cleanup failed"); + self.flush().await; + return Err(e); + } + }; + + // Flush table names to disk + self.flush().await; + + // If more columns are available, open the low level db with the max column count but restrict the tabledb object to the number requested + let existing_col_count = db.num_columns().map_err(VeilidAPIError::from)?; + if existing_col_count > column_count { + drop(db); + db = match self + .table_store_driver + .open(&table_name, existing_col_count) + .await + { + Ok(db) => db, + Err(e) => { + self.name_delete(name).await.expect("cleanup failed"); + self.flush().await; + return Err(e); + } + }; + } + + // Wrap low-level Database in TableDB object + let mut inner = self.inner.lock(); + let table_db = TableDB::new( + table_name.clone(), + self.clone(), + inner.crypto.as_ref().unwrap().clone(), + db, + inner.encryption_key, + inner.encryption_key, + column_count, + ); + + // Keep track of opened DBs + inner + .opened + .insert(table_name.clone(), table_db.weak_inner()); + + Ok(table_db) + } + + /// Delete a TableDB table by name + pub async fn delete(&self, name: &str) -> VeilidAPIResult { + let _async_guard = self.async_lock.lock().await; + // If we aren't initialized yet, bail + { + let inner = self.inner.lock(); + if inner.all_tables_db.is_none() { + apibail_not_initialized!(); + } + } + + let Some(table_name) = self.name_get(name).await? else { + // Did not exist in name table + return Ok(false); + }; + + // See if this table is opened + { + let inner = self.inner.lock(); + if inner.opened.contains_key(&table_name) { + apibail_generic!("Not deleting table that is still opened"); + } + } + + // Delete table db using platform-specific driver + let deleted = self.table_store_driver.delete(&table_name).await?; + if !deleted { + // Table missing? Just remove name + warn!( + "table existed in name table but not in storage: {} : {}", + name, table_name + ); + } + self.name_delete(name).await.expect("failed to delete name"); + self.flush().await; + + Ok(true) + } + + /// Rename a TableDB table + pub async fn rename(&self, old_name: &str, new_name: &str) -> VeilidAPIResult<()> { + let _async_guard = self.async_lock.lock().await; + // If we aren't initialized yet, bail + { + let inner = self.inner.lock(); + if inner.all_tables_db.is_none() { + apibail_not_initialized!(); + } + } + trace!("TableStore::rename {} -> {}", old_name, new_name); + self.name_rename(old_name, new_name).await?; + self.flush().await; + Ok(()) + } +} diff --git a/veilid-core/src/table_store/native.rs b/veilid-core/src/table_store/native.rs index 9e1186d5..fadbc4ef 100644 --- a/veilid-core/src/table_store/native.rs +++ b/veilid-core/src/table_store/native.rs @@ -22,7 +22,7 @@ impl TableStoreDriver { } pub async fn open(&self, table_name: &str, column_count: u32) -> VeilidAPIResult { - let dbpath = self.get_dbpath(&table_name)?; + let dbpath = self.get_dbpath(table_name)?; // Ensure permissions are correct ensure_file_private_owner(&dbpath).map_err(VeilidAPIError::internal)?; @@ -43,7 +43,7 @@ impl TableStoreDriver { } pub async fn delete(&self, table_name: &str) -> VeilidAPIResult { - let dbpath = self.get_dbpath(&table_name)?; + let dbpath = self.get_dbpath(table_name)?; if !dbpath.exists() { return Ok(false); } diff --git a/veilid-core/src/table_store/table_store.rs b/veilid-core/src/table_store/table_store.rs deleted file mode 100644 index 3f5e9821..00000000 --- a/veilid-core/src/table_store/table_store.rs +++ /dev/null @@ -1,589 +0,0 @@ -use super::*; -use keyvaluedb::*; - -const ALL_TABLE_NAMES: &[u8] = b"all_table_names"; - -struct TableStoreInner { - opened: BTreeMap>, - encryption_key: Option, - all_table_names: HashMap, - all_tables_db: Option, - crypto: Option, -} - -/// Veilid Table Storage -/// Database for storing key value pairs persistently and securely across runs -#[derive(Clone)] -pub struct TableStore { - config: VeilidConfig, - protected_store: ProtectedStore, - table_store_driver: TableStoreDriver, - inner: Arc>, // Sync mutex here because TableDB drops can happen at any time - async_lock: Arc>, // Async mutex for operations -} - -impl TableStore { - fn new_inner() -> TableStoreInner { - TableStoreInner { - opened: BTreeMap::new(), - encryption_key: None, - all_table_names: HashMap::new(), - all_tables_db: None, - crypto: None, - } - } - pub(crate) fn new(config: VeilidConfig, protected_store: ProtectedStore) -> Self { - let inner = Self::new_inner(); - let table_store_driver = TableStoreDriver::new(config.clone()); - - Self { - config, - protected_store, - inner: Arc::new(Mutex::new(inner)), - table_store_driver, - async_lock: Arc::new(AsyncMutex::new(())), - } - } - - pub(crate) fn set_crypto(&self, crypto: Crypto) { - let mut inner = self.inner.lock(); - inner.crypto = Some(crypto); - } - - // Flush internal control state (must not use crypto) - async fn flush(&self) { - let (all_table_names_value, all_tables_db) = { - let inner = self.inner.lock(); - let all_table_names_value = serialize_json_bytes(&inner.all_table_names); - (all_table_names_value, inner.all_tables_db.clone().unwrap()) - }; - let mut dbt = DBTransaction::new(); - dbt.put(0, ALL_TABLE_NAMES, &all_table_names_value); - if let Err(e) = all_tables_db.write(dbt).await { - error!("failed to write all tables db: {}", e); - } - } - - // Internal naming support - // Adds rename capability and ensures names of tables are totally unique and valid - - fn namespaced_name(&self, table: &str) -> VeilidAPIResult { - if !table - .chars() - .all(|c| char::is_alphanumeric(c) || c == '_' || c == '-') - { - apibail_invalid_argument!("table name is invalid", "table", table); - } - let c = self.config.get(); - let namespace = c.namespace.clone(); - Ok(if namespace.is_empty() { - table.to_string() - } else { - format!("_ns_{}_{}", namespace, table) - }) - } - - async fn name_get_or_create(&self, table: &str) -> VeilidAPIResult { - let name = self.namespaced_name(table)?; - - let mut inner = self.inner.lock(); - // Do we have this name yet? - if let Some(real_name) = inner.all_table_names.get(&name) { - return Ok(real_name.clone()); - } - - // If not, make a new low level name mapping - let mut real_name_bytes = [0u8; 32]; - random_bytes(&mut real_name_bytes); - let real_name = data_encoding::BASE64URL_NOPAD.encode(&real_name_bytes); - - if inner - .all_table_names - .insert(name.to_owned(), real_name.clone()) - .is_some() - { - panic!("should not have had some value"); - }; - - Ok(real_name) - } - - async fn name_delete(&self, table: &str) -> VeilidAPIResult> { - let name = self.namespaced_name(table)?; - let mut inner = self.inner.lock(); - let real_name = inner.all_table_names.remove(&name); - Ok(real_name) - } - - async fn name_get(&self, table: &str) -> VeilidAPIResult> { - let name = self.namespaced_name(table)?; - let inner = self.inner.lock(); - let real_name = inner.all_table_names.get(&name).cloned(); - Ok(real_name) - } - - async fn name_rename(&self, old_table: &str, new_table: &str) -> VeilidAPIResult<()> { - let old_name = self.namespaced_name(old_table)?; - let new_name = self.namespaced_name(new_table)?; - - let mut inner = self.inner.lock(); - // Ensure new name doesn't exist - if inner.all_table_names.contains_key(&new_name) { - return Err(VeilidAPIError::generic("new table already exists")); - } - // Do we have this name yet? - let Some(real_name) = inner.all_table_names.remove(&old_name) else { - return Err(VeilidAPIError::generic("table does not exist")); - }; - // Insert with new name - inner.all_table_names.insert(new_name.to_owned(), real_name); - - Ok(()) - } - - /// Delete all known tables - async fn delete_all(&self) { - // Get all tables - let real_names = { - let mut inner = self.inner.lock(); - let real_names = inner - .all_table_names - .values() - .cloned() - .collect::>(); - inner.all_table_names.clear(); - real_names - }; - - // Delete all tables - for table_name in real_names { - if let Err(e) = self.table_store_driver.delete(&table_name).await { - error!("error deleting table: {}", e); - } - } - self.flush().await; - } - - pub(crate) fn maybe_unprotect_device_encryption_key( - &self, - dek_bytes: &[u8], - device_encryption_key_password: &str, - ) -> EyreResult { - // Ensure the key is at least as long as necessary if unencrypted - if dek_bytes.len() < (4 + SHARED_SECRET_LENGTH) { - bail!("device encryption key is not valid"); - } - - // Get cryptosystem - let kind = FourCC::try_from(&dek_bytes[0..4]).unwrap(); - let crypto = self.inner.lock().crypto.as_ref().unwrap().clone(); - let Some(vcrypto) = crypto.get(kind) else { - bail!("unsupported cryptosystem"); - }; - - if !device_encryption_key_password.is_empty() { - if dek_bytes.len() - != (4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead() + NONCE_LENGTH) - { - bail!("password protected device encryption key is not valid"); - } - let protected_key = &dek_bytes[4..(4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead())]; - let nonce = &dek_bytes[(4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead())..]; - - let shared_secret = vcrypto - .derive_shared_secret(device_encryption_key_password.as_bytes(), &nonce) - .wrap_err("failed to derive shared secret")?; - let unprotected_key = vcrypto - .decrypt_aead( - &protected_key, - &Nonce::try_from(nonce).wrap_err("invalid nonce")?, - &shared_secret, - None, - ) - .wrap_err("failed to decrypt device encryption key")?; - return Ok(TypedSharedSecret::new( - kind, - SharedSecret::try_from(unprotected_key.as_slice()) - .wrap_err("invalid shared secret")?, - )); - } - - if dek_bytes.len() != (4 + SHARED_SECRET_LENGTH) { - bail!("password protected device encryption key is not valid"); - } - - Ok(TypedSharedSecret::new( - kind, - SharedSecret::try_from(&dek_bytes[4..])?, - )) - } - - pub(crate) fn maybe_protect_device_encryption_key( - &self, - dek: TypedSharedSecret, - device_encryption_key_password: &str, - ) -> EyreResult> { - // Check if we are to protect the key - if device_encryption_key_password.is_empty() { - debug!("no dek password"); - // Return the unprotected key bytes - let mut out = Vec::with_capacity(4 + SHARED_SECRET_LENGTH); - out.extend_from_slice(&dek.kind.0); - out.extend_from_slice(&dek.value.bytes); - return Ok(out); - } - - // Get cryptosystem - let crypto = self.inner.lock().crypto.as_ref().unwrap().clone(); - let Some(vcrypto) = crypto.get(dek.kind) else { - bail!("unsupported cryptosystem"); - }; - - let nonce = vcrypto.random_nonce(); - let shared_secret = vcrypto - .derive_shared_secret(device_encryption_key_password.as_bytes(), &nonce.bytes) - .wrap_err("failed to derive shared secret")?; - let mut protected_key = vcrypto - .encrypt_aead( - &dek.value.bytes, - &Nonce::try_from(nonce).wrap_err("invalid nonce")?, - &shared_secret, - None, - ) - .wrap_err("failed to decrypt device encryption key")?; - let mut out = - Vec::with_capacity(4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead() + NONCE_LENGTH); - out.extend_from_slice(&dek.kind.0); - out.append(&mut protected_key); - out.extend_from_slice(&nonce.bytes); - assert!(out.len() == 4 + SHARED_SECRET_LENGTH + vcrypto.aead_overhead() + NONCE_LENGTH); - Ok(out) - } - - async fn load_device_encryption_key(&self) -> EyreResult> { - let dek_bytes: Option> = self - .protected_store - .load_user_secret("device_encryption_key") - .await?; - let Some(dek_bytes) = dek_bytes else { - debug!("no device encryption key"); - return Ok(None); - }; - - // Get device encryption key protection password if we have it - let device_encryption_key_password = { - let c = self.config.get(); - c.protected_store.device_encryption_key_password.clone() - }; - - Ok(Some(self.maybe_unprotect_device_encryption_key( - &dek_bytes, - &device_encryption_key_password, - )?)) - } - async fn save_device_encryption_key( - &self, - device_encryption_key: Option, - ) -> EyreResult<()> { - let Some(device_encryption_key) = device_encryption_key else { - // Remove the device encryption key - let existed = self - .protected_store - .remove_user_secret("device_encryption_key") - .await?; - debug!("removed device encryption key. existed: {}", existed); - return Ok(()); - }; - - // Get new device encryption key protection password if we are changing it - let new_device_encryption_key_password = { - let c = self.config.get(); - c.protected_store.new_device_encryption_key_password.clone() - }; - let device_encryption_key_password = - if let Some(new_device_encryption_key_password) = new_device_encryption_key_password { - // Change password - debug!("changing dek password"); - self.config - .with_mut(|c| { - c.protected_store.device_encryption_key_password = - new_device_encryption_key_password.clone(); - Ok(new_device_encryption_key_password) - }) - .unwrap() - } else { - // Get device encryption key protection password if we have it - debug!("saving with existing dek password"); - let c = self.config.get(); - c.protected_store.device_encryption_key_password.clone() - }; - - let dek_bytes = self.maybe_protect_device_encryption_key( - device_encryption_key, - &device_encryption_key_password, - )?; - - // Save the new device encryption key - let existed = self - .protected_store - .save_user_secret("device_encryption_key", &dek_bytes) - .await?; - debug!("saving device encryption key. existed: {}", existed); - Ok(()) - } - - pub(crate) async fn init(&self) -> EyreResult<()> { - let _async_guard = self.async_lock.lock().await; - - // Get device encryption key from protected store - let mut device_encryption_key = self.load_device_encryption_key().await?; - let mut device_encryption_key_changed = false; - if let Some(device_encryption_key) = device_encryption_key { - // If encryption in current use is not the best encryption, then run table migration - let best_kind = best_crypto_kind(); - if device_encryption_key.kind != best_kind { - // XXX: Run migration. See issue #209 - } - } else { - // If we don't have an encryption key yet, then make one with the best cryptography and save it - let best_kind = best_crypto_kind(); - let mut shared_secret = SharedSecret::default(); - random_bytes(&mut shared_secret.bytes); - - device_encryption_key = Some(TypedSharedSecret::new(best_kind, shared_secret)); - device_encryption_key_changed = true; - } - - // Check for password change - let changing_password = self - .config - .get() - .protected_store - .new_device_encryption_key_password - .is_some(); - - // Save encryption key if it has changed or if the protecting password wants to change - if device_encryption_key_changed || changing_password { - self.save_device_encryption_key(device_encryption_key) - .await?; - } - - // Deserialize all table names - let all_tables_db = self - .table_store_driver - .open("__veilid_all_tables", 1) - .await - .wrap_err("failed to create all tables table")?; - match all_tables_db.get(0, ALL_TABLE_NAMES).await { - Ok(Some(v)) => match deserialize_json_bytes::>(&v) { - Ok(all_table_names) => { - let mut inner = self.inner.lock(); - inner.all_table_names = all_table_names; - } - Err(e) => { - error!("could not deserialize __veilid_all_tables: {}", e); - } - }, - Ok(None) => { - // No table names yet, that's okay - trace!("__veilid_all_tables is empty"); - } - Err(e) => { - error!("could not get __veilid_all_tables: {}", e); - } - }; - - { - let mut inner = self.inner.lock(); - inner.encryption_key = device_encryption_key; - inner.all_tables_db = Some(all_tables_db); - } - - let do_delete = { - let c = self.config.get(); - c.table_store.delete - }; - - if do_delete { - self.delete_all().await; - } - - Ok(()) - } - - pub(crate) async fn terminate(&self) { - let _async_guard = self.async_lock.lock().await; - - self.flush().await; - - let mut inner = self.inner.lock(); - if !inner.opened.is_empty() { - panic!( - "all open databases should have been closed: {:?}", - inner.opened - ); - } - inner.all_tables_db = None; - inner.all_table_names.clear(); - inner.encryption_key = None; - } - - pub(crate) fn on_table_db_drop(&self, table: String) { - log_rtab!("dropping table db: {}", table); - let mut inner = self.inner.lock(); - if inner.opened.remove(&table).is_none() { - unreachable!("should have removed an item"); - } - } - - /// Get or create a TableDB database table. If the column count is greater than an - /// existing TableDB's column count, the database will be upgraded to add the missing columns - pub async fn open(&self, name: &str, column_count: u32) -> VeilidAPIResult { - let _async_guard = self.async_lock.lock().await; - - // If we aren't initialized yet, bail - { - let inner = self.inner.lock(); - if inner.all_tables_db.is_none() { - apibail_not_initialized!(); - } - } - - let table_name = self.name_get_or_create(name).await?; - - // See if this table is already opened, if so the column count must be the same - { - let mut inner = self.inner.lock(); - if let Some(table_db_weak_inner) = inner.opened.get(&table_name) { - match TableDB::try_new_from_weak_inner(table_db_weak_inner.clone(), column_count) { - Some(tdb) => { - // Ensure column count isnt bigger - let existing_col_count = tdb.get_column_count()?; - if column_count > existing_col_count { - return Err(VeilidAPIError::generic(format!( - "database must be closed before increasing column count {} -> {}", - existing_col_count, column_count, - ))); - } - - return Ok(tdb); - } - None => { - inner.opened.remove(&table_name); - } - }; - } - } - - // Open table db using platform-specific driver - let mut db = match self - .table_store_driver - .open(&table_name, column_count) - .await - { - Ok(db) => db, - Err(e) => { - self.name_delete(name).await.expect("cleanup failed"); - self.flush().await; - return Err(e); - } - }; - - // Flush table names to disk - self.flush().await; - - // If more columns are available, open the low level db with the max column count but restrict the tabledb object to the number requested - let existing_col_count = db.num_columns().map_err(VeilidAPIError::from)?; - if existing_col_count > column_count { - drop(db); - db = match self - .table_store_driver - .open(&table_name, existing_col_count) - .await - { - Ok(db) => db, - Err(e) => { - self.name_delete(name).await.expect("cleanup failed"); - self.flush().await; - return Err(e); - } - }; - } - - // Wrap low-level Database in TableDB object - let mut inner = self.inner.lock(); - let table_db = TableDB::new( - table_name.clone(), - self.clone(), - inner.crypto.as_ref().unwrap().clone(), - db, - inner.encryption_key.clone(), - inner.encryption_key.clone(), - column_count, - ); - - // Keep track of opened DBs - inner - .opened - .insert(table_name.clone(), table_db.weak_inner()); - - Ok(table_db) - } - - /// Delete a TableDB table by name - pub async fn delete(&self, name: &str) -> VeilidAPIResult { - let _async_guard = self.async_lock.lock().await; - // If we aren't initialized yet, bail - { - let inner = self.inner.lock(); - if inner.all_tables_db.is_none() { - apibail_not_initialized!(); - } - } - - let Some(table_name) = self.name_get(name).await? else { - // Did not exist in name table - return Ok(false); - }; - - // See if this table is opened - { - let inner = self.inner.lock(); - if inner.opened.contains_key(&table_name) { - apibail_generic!("Not deleting table that is still opened"); - } - } - - // Delete table db using platform-specific driver - let deleted = self.table_store_driver.delete(&table_name).await?; - if !deleted { - // Table missing? Just remove name - warn!( - "table existed in name table but not in storage: {} : {}", - name, table_name - ); - } - self.name_delete(&name) - .await - .expect("failed to delete name"); - self.flush().await; - - Ok(true) - } - - /// Rename a TableDB table - pub async fn rename(&self, old_name: &str, new_name: &str) -> VeilidAPIResult<()> { - let _async_guard = self.async_lock.lock().await; - // If we aren't initialized yet, bail - { - let inner = self.inner.lock(); - if inner.all_tables_db.is_none() { - apibail_not_initialized!(); - } - } - trace!("TableStore::rename {} -> {}", old_name, new_name); - self.name_rename(old_name, new_name).await?; - self.flush().await; - Ok(()) - } -} diff --git a/veilid-core/src/table_store/tests/test_table_store.rs b/veilid-core/src/table_store/tests/test_table_store.rs index c0056c54..85821339 100644 --- a/veilid-core/src/table_store/tests/test_table_store.rs +++ b/veilid-core/src/table_store/tests/test_table_store.rs @@ -50,7 +50,7 @@ pub async fn test_delete_open_delete(ts: TableStore) { pub async fn test_store_delete_load(ts: TableStore) { trace!("test_store_delete_load"); - ts.delete("test").await; + let _ = ts.delete("test").await; let db = ts.open("test", 3).await.expect("should have opened"); assert!( ts.delete("test").await.is_err(), diff --git a/veilid-core/src/tests/common/test_protected_store.rs b/veilid-core/src/tests/common/test_protected_store.rs index 49d56d4e..2b242c0f 100644 --- a/veilid-core/src/tests/common/test_protected_store.rs +++ b/veilid-core/src/tests/common/test_protected_store.rs @@ -23,14 +23,12 @@ pub async fn test_protected_store(ps: ProtectedStore) { let d1: [u8; 0] = []; - assert_eq!( - ps.save_user_secret("_test_key", &[2u8, 3u8, 4u8]) - .await - .unwrap(), - false - ); + assert!(!ps + .save_user_secret("_test_key", &[2u8, 3u8, 4u8]) + .await + .unwrap()); info!("testing saving user secret"); - assert_eq!(ps.save_user_secret("_test_key", &d1).await.unwrap(), true); + assert!(ps.save_user_secret("_test_key", &d1).await.unwrap()); info!("testing loading user secret"); assert_eq!( ps.load_user_secret("_test_key").await.unwrap(), @@ -46,23 +44,21 @@ pub async fn test_protected_store(ps: ProtectedStore) { info!("testing loading broken user secret again"); assert_eq!(ps.load_user_secret("_test_broken").await.unwrap(), None); info!("testing remove user secret"); - assert_eq!(ps.remove_user_secret("_test_key").await.unwrap(), true); + assert!(ps.remove_user_secret("_test_key").await.unwrap()); info!("testing remove user secret again"); - assert_eq!(ps.remove_user_secret("_test_key").await.unwrap(), false); + assert!(!ps.remove_user_secret("_test_key").await.unwrap()); info!("testing remove broken user secret"); - assert_eq!(ps.remove_user_secret("_test_broken").await.unwrap(), false); + assert!(!ps.remove_user_secret("_test_broken").await.unwrap()); info!("testing remove broken user secret again"); - assert_eq!(ps.remove_user_secret("_test_broken").await.unwrap(), false); + assert!(!ps.remove_user_secret("_test_broken").await.unwrap()); let d2: [u8; 10] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; - assert_eq!( - ps.save_user_secret("_test_key", &[2u8, 3u8, 4u8]) - .await - .unwrap(), - false - ); - assert_eq!(ps.save_user_secret("_test_key", &d2).await.unwrap(), true); + assert!(!ps + .save_user_secret("_test_key", &[2u8, 3u8, 4u8]) + .await + .unwrap()); + assert!(ps.save_user_secret("_test_key", &d2).await.unwrap()); assert_eq!( ps.load_user_secret("_test_key").await.unwrap(), Some(d2.to_vec()) @@ -73,10 +69,10 @@ pub async fn test_protected_store(ps: ProtectedStore) { ); assert_eq!(ps.load_user_secret("_test_broken").await.unwrap(), None); assert_eq!(ps.load_user_secret("_test_broken").await.unwrap(), None); - assert_eq!(ps.remove_user_secret("_test_key").await.unwrap(), true); - assert_eq!(ps.remove_user_secret("_test_key").await.unwrap(), false); - assert_eq!(ps.remove_user_secret("_test_broken").await.unwrap(), false); - assert_eq!(ps.remove_user_secret("_test_broken").await.unwrap(), false); + assert!(ps.remove_user_secret("_test_key").await.unwrap()); + assert!(ps.remove_user_secret("_test_key").await.unwrap()); + assert!(!ps.remove_user_secret("_test_key").await.unwrap()); + assert!(!ps.remove_user_secret("_test_broken").await.unwrap()); let _ = ps.remove_user_secret("_test_key").await; let _ = ps.remove_user_secret("_test_broken").await; diff --git a/veilid-core/src/tests/common/test_veilid_config.rs b/veilid-core/src/tests/common/test_veilid_config.rs index f6b2f844..77484ad7 100644 --- a/veilid-core/src/tests/common/test_veilid_config.rs +++ b/veilid-core/src/tests/common/test_veilid_config.rs @@ -1,5 +1,3 @@ -#![allow(clippy::bool_assert_comparison)] - use crate::*; cfg_if! { @@ -168,7 +166,7 @@ fn config_callback(key: String) -> ConfigCallbackReturn { match key.as_str() { "program_name" => Ok(Box::new(String::from("VeilidCoreTests"))), "namespace" => Ok(Box::::default()), - "capabilities.disable" => Ok(Box::>::default()), + "capabilities.disable" => Ok(Box::>::default()), "table_store.directory" => Ok(Box::new(get_table_store_path())), "table_store.delete" => Ok(Box::new(true)), "block_store.directory" => Ok(Box::new(get_block_store_path())), @@ -193,7 +191,7 @@ fn config_callback(key: String) -> ConfigCallbackReturn { "network.network_key_password" => Ok(Box::new(Option::::None)), "network.routing_table.node_id" => Ok(Box::new(TypedKeyGroup::new())), "network.routing_table.node_id_secret" => Ok(Box::new(TypedSecretGroup::new())), - "network.routing_table.bootstrap" => Ok(Box::>::default()), + "network.routing_table.bootstrap" => Ok(Box::>::default()), "network.routing_table.limit_over_attached" => Ok(Box::new(64u32)), "network.routing_table.limit_fully_attached" => Ok(Box::new(32u32)), "network.routing_table.limit_attached_strong" => Ok(Box::new(16u32)), @@ -295,13 +293,13 @@ pub async fn test_config() { assert_eq!(inner.namespace, String::from("")); assert_eq!(inner.capabilities.disable, Vec::::new()); assert_eq!(inner.table_store.directory, get_table_store_path()); - assert_eq!(inner.table_store.delete, true); + assert!(inner.table_store.delete); assert_eq!(inner.block_store.directory, get_block_store_path()); - assert_eq!(inner.block_store.delete, true); - assert_eq!(inner.protected_store.allow_insecure_fallback, true); - assert_eq!(inner.protected_store.always_use_insecure_storage, false); + assert!(inner.block_store.delete); + assert!(inner.protected_store.allow_insecure_fallback); + assert!(!inner.protected_store.always_use_insecure_storage); assert_eq!(inner.protected_store.directory, get_protected_store_path()); - assert_eq!(inner.protected_store.delete, true); + assert!(inner.protected_store.delete); assert_eq!( inner.protected_store.device_encryption_key_password, "".to_owned() @@ -351,38 +349,38 @@ pub async fn test_config() { 5_000u32 ); - assert_eq!(inner.network.upnp, false); - assert_eq!(inner.network.detect_address_changes, true); + assert!(!inner.network.upnp); + assert!(inner.network.detect_address_changes); assert_eq!(inner.network.restricted_nat_retries, 3u32); assert_eq!(inner.network.tls.certificate_path, get_certfile_path()); assert_eq!(inner.network.tls.private_key_path, get_keyfile_path()); assert_eq!(inner.network.tls.connection_initial_timeout_ms, 2_000u32); - assert_eq!(inner.network.application.https.enabled, false); + assert!(!inner.network.application.https.enabled); assert_eq!(inner.network.application.https.listen_address, ""); assert_eq!(inner.network.application.https.path, "app"); assert_eq!(inner.network.application.https.url, None); - assert_eq!(inner.network.application.http.enabled, false); + assert!(!inner.network.application.http.enabled); assert_eq!(inner.network.application.http.listen_address, ""); assert_eq!(inner.network.application.http.path, "app"); assert_eq!(inner.network.application.http.url, None); - assert_eq!(inner.network.protocol.udp.enabled, true); + assert!(inner.network.protocol.udp.enabled); assert_eq!(inner.network.protocol.udp.socket_pool_size, 16u32); assert_eq!(inner.network.protocol.udp.listen_address, ""); assert_eq!(inner.network.protocol.udp.public_address, None); - assert_eq!(inner.network.protocol.tcp.connect, true); - assert_eq!(inner.network.protocol.tcp.listen, true); + assert!(inner.network.protocol.tcp.connect); + assert!(inner.network.protocol.tcp.listen); assert_eq!(inner.network.protocol.tcp.max_connections, 32u32); assert_eq!(inner.network.protocol.tcp.listen_address, ""); assert_eq!(inner.network.protocol.tcp.public_address, None); - assert_eq!(inner.network.protocol.ws.connect, false); - assert_eq!(inner.network.protocol.ws.listen, false); + assert!(!inner.network.protocol.ws.connect); + assert!(!inner.network.protocol.ws.listen); assert_eq!(inner.network.protocol.ws.max_connections, 16u32); assert_eq!(inner.network.protocol.ws.listen_address, ""); assert_eq!(inner.network.protocol.ws.path, "ws"); assert_eq!(inner.network.protocol.ws.url, None); - assert_eq!(inner.network.protocol.wss.connect, false); - assert_eq!(inner.network.protocol.wss.listen, false); + assert!(!inner.network.protocol.wss.connect); + assert!(!inner.network.protocol.wss.listen); assert_eq!(inner.network.protocol.wss.max_connections, 16u32); assert_eq!(inner.network.protocol.wss.listen_address, ""); assert_eq!(inner.network.protocol.wss.path, "ws"); diff --git a/veilid-core/src/veilid_api/api.rs b/veilid-core/src/veilid_api/api.rs index 6fcc5b6b..e2139e49 100644 --- a/veilid-core/src/veilid_api/api.rs +++ b/veilid-core/src/veilid_api/api.rs @@ -263,7 +263,7 @@ impl VeilidAPI { let rss = self.routing_table()?.route_spec_store(); let r = rss .allocate_route( - &crypto_kinds, + crypto_kinds, stability, sequencing, default_route_hop_count, @@ -275,7 +275,7 @@ impl VeilidAPI { apibail_generic!("unable to allocate route"); }; if !rss - .test_route(route_id.clone()) + .test_route(route_id) .await .map_err(VeilidAPIError::no_connection)? { diff --git a/veilid-core/src/veilid_api/debug.rs b/veilid-core/src/veilid_api/debug.rs index 3801e4cd..7cbd3ce6 100644 --- a/veilid-core/src/veilid_api/debug.rs +++ b/veilid-core/src/veilid_api/debug.rs @@ -61,9 +61,9 @@ fn get_string(text: &str) -> Option { } fn get_data(text: &str) -> Option> { - if text.starts_with("#") { - hex::decode(&text[1..]).ok() - } else if text.starts_with("\"") || text.starts_with("'") { + if let Some(stripped_text) = text.strip_prefix('#') { + hex::decode(stripped_text).ok() + } else if text.starts_with('"') || text.starts_with('\'') { json::parse(text) .ok()? .as_str() @@ -86,7 +86,7 @@ fn get_route_id( allow_allocated: bool, allow_remote: bool, ) -> impl Fn(&str) -> Option { - return move |text: &str| { + move |text: &str| { if text.is_empty() { return None; } @@ -127,7 +127,7 @@ fn get_route_id( } } None - }; + } } fn get_dht_schema(text: &str) -> Option { @@ -140,7 +140,7 @@ fn get_safety_selection(routing_table: RoutingTable) -> impl Fn(&str) -> Option< let default_route_hop_count = routing_table.with_config(|c| c.network.rpc.default_route_hop_count as usize); - if text.len() != 0 && &text[0..1] == "-" { + if !text.is_empty() && &text[0..1] == "-" { // Unsafe let text = &text[1..]; let seq = get_sequencing(text).unwrap_or_default(); @@ -151,7 +151,7 @@ fn get_safety_selection(routing_table: RoutingTable) -> impl Fn(&str) -> Option< let mut hop_count = default_route_hop_count; let mut stability = Stability::default(); let mut sequencing = Sequencing::default(); - for x in text.split(",") { + for x in text.split(',') { let x = x.trim(); if let Some(pr) = get_route_id(rss.clone(), true, false)(x) { preferred_route = Some(pr) @@ -179,7 +179,7 @@ fn get_safety_selection(routing_table: RoutingTable) -> impl Fn(&str) -> Option< fn get_node_ref_modifiers(mut node_ref: NodeRef) -> impl FnOnce(&str) -> Option { move |text| { - for m in text.split("/") { + for m in text.split('/') { if let Some(pt) = get_protocol_type(m) { node_ref.merge_filter(NodeRefFilter::new().with_protocol_type(pt)); } else if let Some(at) = get_address_type(m) { @@ -207,7 +207,7 @@ fn get_destination( } else { (text.as_str(), None) }; - if text.len() == 0 { + if text.is_empty() { return None; } if &text[0..1] == "#" { @@ -225,7 +225,7 @@ fn get_destination( } else { let mut dc = DEBUG_CACHE.lock(); let n = get_number(text)?; - let prid = dc.imported_routes.get(n)?.clone(); + let prid = *dc.imported_routes.get(n)?; let Some(private_route) = rss.best_remote_private_route(&prid) else { // Remove imported route dc.imported_routes.remove(n); @@ -312,7 +312,7 @@ fn get_dht_key( } else { (text, None) }; - if text.len() == 0 { + if text.is_empty() { return None; } @@ -528,13 +528,13 @@ pub fn print_data(data: &[u8], truncate_len: Option) -> String { let (data, truncated) = if truncate_len.is_some() && data.len() > truncate_len.unwrap() { (&data[0..64], true) } else { - (&data[..], false) + (data, false) }; let strdata = if printable { - format!("{}", String::from_utf8_lossy(&data).to_string()) + String::from_utf8_lossy(data).to_string() } else { - let sw = shell_words::quote(&String::from_utf8_lossy(&data).to_string()).to_string(); + let sw = shell_words::quote(String::from_utf8_lossy(data).as_ref()).to_string(); let h = hex::encode(data); if h.len() < sw.len() { h @@ -1019,8 +1019,8 @@ impl VeilidAPI { let netman = self.network_manager()?; let rpc = netman.rpc_processor(); - let (call_id, data) = if args.starts_with("#") { - let (arg, rest) = args[1..].split_once(' ').unwrap_or((&args, "")); + let (call_id, data) = if let Some(stripped_args) = args.strip_prefix('#') { + let (arg, rest) = stripped_args.split_once(' ').unwrap_or((&args, "")); let call_id = OperationId::new(u64::from_str_radix(arg, 16).map_err(VeilidAPIError::generic)?); let rest = rest.trim_start().to_owned(); @@ -1097,7 +1097,7 @@ impl VeilidAPI { &[], ) { Ok(Some(v)) => format!("{}", v), - Ok(None) => format!(""), + Ok(None) => "".to_string(), Err(e) => { format!("Route allocation failed: {}", e) } @@ -1270,7 +1270,7 @@ impl VeilidAPI { let out = format!("Private route #{} imported: {}", n, route_id); dc.imported_routes.push(route_id); - return Ok(out); + Ok(out) } async fn debug_route_test(&self, args: Vec) -> VeilidAPIResult { @@ -1298,7 +1298,7 @@ impl VeilidAPI { "FAILED".to_owned() }; - return Ok(out); + Ok(out) } async fn debug_route(&self, args: String) -> VeilidAPIResult { @@ -1334,18 +1334,18 @@ impl VeilidAPI { let scope = get_debug_argument_at(&args, 1, "debug_record_list", "scope", get_string)?; let out = match scope.as_str() { "local" => { - let mut out = format!("Local Records:\n"); + let mut out = "Local Records:\n".to_string(); out += &storage_manager.debug_local_records().await; out } "remote" => { - let mut out = format!("Remote Records:\n"); + let mut out = "Remote Records:\n".to_string(); out += &storage_manager.debug_remote_records().await; out } _ => "Invalid scope\n".to_owned(), }; - return Ok(out); + Ok(out) } async fn debug_record_purge(&self, args: Vec) -> VeilidAPIResult { @@ -1359,7 +1359,7 @@ impl VeilidAPI { "remote" => storage_manager.purge_remote_records(bytes).await, _ => "Invalid scope\n".to_owned(), }; - return Ok(out); + Ok(out) } async fn debug_record_create(&self, args: Vec) -> VeilidAPIResult { @@ -1397,11 +1397,10 @@ impl VeilidAPI { // Get routing context with optional privacy let rc = self.routing_context(); let rc = if let Some(ss) = ss { - let rcp = match rc.with_custom_privacy(ss) { + match rc.with_custom_privacy(ss) { Err(e) => return Ok(format!("Can't use safety selection: {}", e)), Ok(v) => v, - }; - rcp + } } else { rc }; @@ -1416,7 +1415,7 @@ impl VeilidAPI { Ok(v) => v, }; debug!("DHT Record Created:\n{:#?}", record); - return Ok(format!("{:?}", record)); + Ok(format!("{:?}", record)) } async fn debug_record_get(&self, args: Vec) -> VeilidAPIResult { @@ -1456,11 +1455,10 @@ impl VeilidAPI { // Get routing context with optional privacy let rc = self.routing_context(); let rc = if let Some(ss) = ss { - let rcp = match rc.with_custom_privacy(ss) { + match rc.with_custom_privacy(ss) { Err(e) => return Ok(format!("Can't use safety selection: {}", e)), Ok(v) => v, - }; - rcp + } } else { rc }; @@ -1497,7 +1495,7 @@ impl VeilidAPI { Err(e) => return Ok(format!("Can't close DHT record: {}", e)), Ok(v) => v, }; - return Ok(out); + Ok(out) } async fn debug_record_set(&self, args: Vec) -> VeilidAPIResult { @@ -1518,11 +1516,10 @@ impl VeilidAPI { // Get routing context with optional privacy let rc = self.routing_context(); let rc = if let Some(ss) = ss { - let rcp = match rc.with_custom_privacy(ss) { + match rc.with_custom_privacy(ss) { Err(e) => return Ok(format!("Can't use safety selection: {}", e)), Ok(v) => v, - }; - rcp + } } else { rc }; @@ -1556,7 +1553,7 @@ impl VeilidAPI { Err(e) => return Ok(format!("Can't close DHT record: {}", e)), Ok(v) => v, }; - return Ok(out); + Ok(out) } async fn debug_record_delete(&self, args: Vec) -> VeilidAPIResult { @@ -1568,7 +1565,7 @@ impl VeilidAPI { Err(e) => return Ok(format!("Can't delete DHT record: {}", e)), Ok(v) => v, }; - Ok(format!("DHT record deleted")) + Ok("DHT record deleted".to_string()) } async fn debug_record_info(&self, args: Vec) -> VeilidAPIResult { @@ -1595,7 +1592,7 @@ impl VeilidAPI { let ri = storage_manager.debug_remote_record_info(key).await; format!("Local Info:\n{}\n\nRemote Info:\n{}\n", li, ri) }; - return Ok(out); + Ok(out) } async fn debug_record(&self, args: String) -> VeilidAPIResult { @@ -1629,7 +1626,7 @@ impl VeilidAPI { let address_filter = network_manager.address_filter(); let out = format!("Address Filter Punishments:\n{:#?}", address_filter); - return Ok(out); + Ok(out) } async fn debug_punish(&self, args: String) -> VeilidAPIResult { diff --git a/veilid-core/src/veilid_api/json_api/mod.rs b/veilid-core/src/veilid_api/json_api/mod.rs index 5b04a4bd..b35e1ea2 100644 --- a/veilid-core/src/veilid_api/json_api/mod.rs +++ b/veilid-core/src/veilid_api/json_api/mod.rs @@ -140,7 +140,7 @@ pub enum ResponseOp { }, GetState { #[serde(flatten)] - result: ApiResult, + result: ApiResult>, }, Attach { #[serde(flatten)] @@ -175,7 +175,7 @@ pub enum ResponseOp { NewRoutingContext { value: u32, }, - RoutingContext(RoutingContextResponse), + RoutingContext(Box), // TableDb OpenTableDb { #[serde(flatten)] diff --git a/veilid-core/src/veilid_api/json_api/process.rs b/veilid-core/src/veilid_api/json_api/process.rs index 8e1264df..a84be568 100644 --- a/veilid-core/src/veilid_api/json_api/process.rs +++ b/veilid-core/src/veilid_api/json_api/process.rs @@ -87,10 +87,10 @@ impl JsonRequestProcessor { let Some(routing_context) = inner.routing_contexts.get(&rc_id).cloned() else { return Err(Response { id, - op: ResponseOp::RoutingContext(RoutingContextResponse { + op: ResponseOp::RoutingContext(Box::new(RoutingContextResponse { rc_id, - rc_op: RoutingContextResponseOp::InvalidId - }) + rc_op: RoutingContextResponseOp::InvalidId, + })), }); }; Ok(routing_context) @@ -100,7 +100,7 @@ impl JsonRequestProcessor { if inner.routing_contexts.remove(&id).is_none() { return 0; } - return 1; + 1 } // TableDB @@ -120,8 +120,8 @@ impl JsonRequestProcessor { id, op: ResponseOp::TableDb(TableDbResponse { db_id, - db_op: TableDbResponseOp::InvalidId - }) + db_op: TableDbResponseOp::InvalidId, + }), }); }; Ok(table_db) @@ -131,7 +131,7 @@ impl JsonRequestProcessor { if inner.table_dbs.remove(&id).is_none() { return 0; } - return 1; + 1 } // TableDBTransaction @@ -155,8 +155,8 @@ impl JsonRequestProcessor { id, op: ResponseOp::TableDbTransaction(TableDbTransactionResponse { tx_id, - tx_op: TableDbTransactionResponseOp::InvalidId - }) + tx_op: TableDbTransactionResponseOp::InvalidId, + }), }); }; Ok(table_db_transaction) @@ -166,7 +166,7 @@ impl JsonRequestProcessor { if inner.table_db_transactions.remove(&id).is_none() { return 0; } - return 1; + 1 } // CryptoSystem @@ -186,8 +186,8 @@ impl JsonRequestProcessor { id, op: ResponseOp::CryptoSystem(CryptoSystemResponse { cs_id, - cs_op: CryptoSystemResponseOp::InvalidId - }) + cs_op: CryptoSystemResponseOp::InvalidId, + }), }); }; Ok(crypto_system) @@ -197,7 +197,7 @@ impl JsonRequestProcessor { if inner.crypto_systems.remove(&id).is_none() { return 0; } - return 1; + 1 } // Target @@ -280,13 +280,21 @@ impl JsonRequestProcessor { RoutingContextRequestOp::CreateDhtRecord { schema, kind } => { RoutingContextResponseOp::CreateDhtRecord { result: to_json_api_result( - routing_context.create_dht_record(schema, kind).await, + routing_context + .create_dht_record(schema, kind) + .await + .map(Box::new), ), } } RoutingContextRequestOp::OpenDhtRecord { key, writer } => { RoutingContextResponseOp::OpenDhtRecord { - result: to_json_api_result(routing_context.open_dht_record(key, writer).await), + result: to_json_api_result( + routing_context + .open_dht_record(key, writer) + .await + .map(Box::new), + ), } } RoutingContextRequestOp::CloseDhtRecord { key } => { @@ -508,7 +516,7 @@ impl JsonRequestProcessor { &body, &nonce, &shared_secret, - associated_data.as_ref().map(|ad| ad.as_slice()), + associated_data.as_deref(), )), }, CryptoSystemRequestOp::EncryptAead { @@ -521,7 +529,7 @@ impl JsonRequestProcessor { &body, &nonce, &shared_secret, - associated_data.as_ref().map(|ad| ad.as_slice()), + associated_data.as_deref(), )), }, CryptoSystemRequestOp::CryptNoAuth { @@ -548,7 +556,7 @@ impl JsonRequestProcessor { ))), }, RequestOp::GetState => ResponseOp::GetState { - result: to_json_api_result(self.api.get_state().await), + result: to_json_api_result(self.api.get_state().await.map(Box::new)), }, RequestOp::Attach => ResponseOp::Attach { result: to_json_api_result(self.api.attach().await), @@ -596,10 +604,10 @@ impl JsonRequestProcessor { Ok(v) => v, Err(e) => return e, }; - ResponseOp::RoutingContext( + ResponseOp::RoutingContext(Box::new( self.process_routing_context_request(routing_context, rcr) .await, - ) + )) } RequestOp::OpenTableDb { name, column_count } => { let table_store = match self.api.table_store() { diff --git a/veilid-core/src/veilid_api/json_api/routing_context.rs b/veilid-core/src/veilid_api/json_api/routing_context.rs index dc4ac323..18b1e329 100644 --- a/veilid-core/src/veilid_api/json_api/routing_context.rs +++ b/veilid-core/src/veilid_api/json_api/routing_context.rs @@ -110,11 +110,11 @@ pub enum RoutingContextResponseOp { }, CreateDhtRecord { #[serde(flatten)] - result: ApiResult, + result: ApiResult>, }, OpenDhtRecord { #[serde(flatten)] - result: ApiResult, + result: ApiResult>, }, CloseDhtRecord { #[serde(flatten)] diff --git a/veilid-core/src/veilid_api/serialize_helpers/compression.rs b/veilid-core/src/veilid_api/serialize_helpers/compression.rs index cb45ed30..627b21fe 100644 --- a/veilid-core/src/veilid_api/serialize_helpers/compression.rs +++ b/veilid-core/src/veilid_api/serialize_helpers/compression.rs @@ -19,5 +19,5 @@ pub fn decompress_size_prepended( )); } } - Ok(block::decompress(input, uncompressed_size).map_err(VeilidAPIError::generic)?) + block::decompress(input, uncompressed_size).map_err(VeilidAPIError::generic) } diff --git a/veilid-core/src/veilid_api/serialize_helpers/serialize_json.rs b/veilid-core/src/veilid_api/serialize_helpers/serialize_json.rs index 1f41ebf4..f30e6deb 100644 --- a/veilid-core/src/veilid_api/serialize_helpers/serialize_json.rs +++ b/veilid-core/src/veilid_api/serialize_helpers/serialize_json.rs @@ -93,7 +93,7 @@ pub mod as_human_base64 { let base64 = String::deserialize(d)?; BASE64URL_NOPAD .decode(base64.as_bytes()) - .map_err(|e| serde::de::Error::custom(e)) + .map_err(serde::de::Error::custom) } else { Vec::::deserialize(d) } @@ -106,7 +106,7 @@ pub mod as_human_opt_base64 { pub fn serialize(v: &Option>, s: S) -> Result { if s.is_human_readable() { - let base64 = v.as_ref().map(|x| BASE64URL_NOPAD.encode(&x)); + let base64 = v.as_ref().map(|x| BASE64URL_NOPAD.encode(x)); Option::::serialize(&base64, s) } else { Option::>::serialize(v, s) @@ -120,7 +120,7 @@ pub mod as_human_opt_base64 { .map(|x| { BASE64URL_NOPAD .decode(x.as_bytes()) - .map_err(|e| serde::de::Error::custom(e)) + .map_err(serde::de::Error::custom) }) .transpose() } else { diff --git a/veilid-core/src/veilid_api/tests/test_types.rs b/veilid-core/src/veilid_api/tests/test_types.rs index 7cbe27bb..3a9b38bf 100644 --- a/veilid-core/src/veilid_api/tests/test_types.rs +++ b/veilid-core/src/veilid_api/tests/test_types.rs @@ -43,21 +43,21 @@ pub async fn test_fourcc() { pub async fn test_sequencing() { let orig = Sequencing::PreferOrdered; - let copy = deserialize_json(&serialize_json(&orig)).unwrap(); + let copy = deserialize_json(&serialize_json(orig)).unwrap(); assert_eq!(orig, copy); } pub async fn test_stability() { let orig = Stability::Reliable; - let copy = deserialize_json(&serialize_json(&orig)).unwrap(); + let copy = deserialize_json(&serialize_json(orig)).unwrap(); assert_eq!(orig, copy); } pub async fn test_safetyselection() { let orig = SafetySelection::Unsafe(Sequencing::EnsureOrdered); - let copy = deserialize_json(&serialize_json(&orig)).unwrap(); + let copy = deserialize_json(&serialize_json(orig)).unwrap(); assert_eq!(orig, copy); } @@ -178,7 +178,7 @@ pub async fn test_partialtunnel() { pub async fn test_veilidloglevel() { let orig = VeilidLogLevel::Info; - let copy = deserialize_json(&serialize_json(&orig)).unwrap(); + let copy = deserialize_json(&serialize_json(orig)).unwrap(); assert_eq!(orig, copy); } @@ -198,7 +198,7 @@ pub async fn test_veilidlog() { pub async fn test_attachmentstate() { let orig = AttachmentState::FullyAttached; - let copy = deserialize_json(&serialize_json(&orig)).unwrap(); + let copy = deserialize_json(&serialize_json(orig)).unwrap(); assert_eq!(orig, copy); } @@ -260,7 +260,7 @@ pub async fn test_veilidvaluechange() { } pub async fn test_veilidupdate() { - let orig = VeilidUpdate::ValueChange(fix_veilidvaluechange()); + let orig = VeilidUpdate::ValueChange(Box::new(fix_veilidvaluechange())); let copy = deserialize_json(&serialize_json(&orig)).unwrap(); assert_eq!(orig, copy); @@ -268,20 +268,20 @@ pub async fn test_veilidupdate() { pub async fn test_veilidstate() { let orig = VeilidState { - attachment: VeilidStateAttachment { + attachment: Box::new(VeilidStateAttachment { state: AttachmentState::OverAttached, public_internet_ready: true, local_network_ready: false, - }, - network: VeilidStateNetwork { + }), + network: Box::new(VeilidStateNetwork { started: true, bps_down: AlignedU64::from(14_400), bps_up: AlignedU64::from(1200), peers: vec![fix_peertabledata()], - }, - config: VeilidStateConfig { + }), + config: Box::new(VeilidStateConfig { config: fix_veilidconfiginner(), - }, + }), }; let copy = deserialize_json(&serialize_json(&orig)).unwrap(); diff --git a/veilid-core/src/veilid_api/types/dht/schema/dflt.rs b/veilid-core/src/veilid_api/types/dht/schema/dflt.rs index bb4e7c6e..144d6075 100644 --- a/veilid-core/src/veilid_api/types/dht/schema/dflt.rs +++ b/veilid-core/src/veilid_api/types/dht/schema/dflt.rs @@ -61,7 +61,7 @@ impl TryFrom<&[u8]> for DHTSchemaDFLT { if b.len() != Self::FIXED_SIZE { apibail_generic!("invalid size"); } - if &b[0..4] != &Self::FCC { + if b[0..4] != Self::FCC { apibail_generic!("wrong fourcc"); } diff --git a/veilid-core/src/veilid_api/types/dht/schema/smpl.rs b/veilid-core/src/veilid_api/types/dht/schema/smpl.rs index b14ed67c..c1511a4b 100644 --- a/veilid-core/src/veilid_api/types/dht/schema/smpl.rs +++ b/veilid-core/src/veilid_api/types/dht/schema/smpl.rs @@ -101,7 +101,7 @@ impl TryFrom<&[u8]> for DHTSchemaSMPL { if b.len() < Self::FIXED_SIZE { apibail_generic!("invalid size"); } - if &b[0..4] != &Self::FCC { + if b[0..4] != Self::FCC { apibail_generic!("wrong fourcc"); } if (b.len() - Self::FIXED_SIZE) % (PUBLIC_KEY_LENGTH + 2) != 0 { diff --git a/veilid-core/src/veilid_api/types/dht/value_subkey_range_set.rs b/veilid-core/src/veilid_api/types/dht/value_subkey_range_set.rs index 9e7eb970..99ba294b 100644 --- a/veilid-core/src/veilid_api/types/dht/value_subkey_range_set.rs +++ b/veilid-core/src/veilid_api/types/dht/value_subkey_range_set.rs @@ -32,10 +32,13 @@ impl FromStr for ValueSubkeyRangeSet { fn from_str(value: &str) -> Result { let mut data = RangeSetBlaze::::new(); - for r in value.split(",") { + for r in value.split(',') { let r = r.trim(); let Some((ss, es)) = r.split_once("..=") else { - return Err(VeilidAPIError::parse_error("can not parse ValueSubkeyRangeSet", r)); + return Err(VeilidAPIError::parse_error( + "can not parse ValueSubkeyRangeSet", + r, + )); }; let sn = ValueSubkey::from_str(ss) .map_err(|e| VeilidAPIError::parse_error("could not parse ValueSubkey", e))?; diff --git a/veilid-core/src/veilid_api/types/veilid_state.rs b/veilid-core/src/veilid_api/types/veilid_state.rs index 49b180c9..4f2d441d 100644 --- a/veilid-core/src/veilid_api/types/veilid_state.rs +++ b/veilid-core/src/veilid_api/types/veilid_state.rs @@ -108,14 +108,14 @@ pub struct VeilidValueChange { #[cfg_attr(target_arch = "wasm32", derive(Tsify), tsify(into_wasm_abi))] #[serde(tag = "kind")] pub enum VeilidUpdate { - Log(VeilidLog), - AppMessage(VeilidAppMessage), - AppCall(VeilidAppCall), - Attachment(VeilidStateAttachment), - Network(VeilidStateNetwork), - Config(VeilidStateConfig), - RouteChange(VeilidRouteChange), - ValueChange(VeilidValueChange), + Log(Box), + AppMessage(Box), + AppCall(Box), + Attachment(Box), + Network(Box), + Config(Box), + RouteChange(Box), + ValueChange(Box), Shutdown, } from_impl_to_jsvalue!(VeilidUpdate); @@ -123,8 +123,8 @@ from_impl_to_jsvalue!(VeilidUpdate); #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] #[cfg_attr(target_arch = "wasm32", derive(Tsify), tsify(into_wasm_abi))] pub struct VeilidState { - pub attachment: VeilidStateAttachment, - pub network: VeilidStateNetwork, - pub config: VeilidStateConfig, + pub attachment: Box, + pub network: Box, + pub config: Box, } from_impl_to_jsvalue!(VeilidState); diff --git a/veilid-core/src/veilid_config.rs b/veilid-core/src/veilid_config.rs index f8a5cf88..2877eea6 100644 --- a/veilid-core/src/veilid_config.rs +++ b/veilid-core/src/veilid_config.rs @@ -565,11 +565,11 @@ impl VeilidConfig { }) } - pub fn get_veilid_state(&self) -> VeilidStateConfig { + pub fn get_veilid_state(&self) -> Box { let inner = self.inner.read(); - VeilidStateConfig { + Box::new(VeilidStateConfig { config: inner.clone(), - } + }) } pub fn get(&self) -> RwLockReadGuard { @@ -612,7 +612,7 @@ impl VeilidConfig { // Make changes let out = f(&mut editedinner)?; // Validate - Self::validate(&mut editedinner)?; + Self::validate(&editedinner)?; // See if things have changed if *inner == editedinner { // No changes, return early @@ -626,7 +626,9 @@ impl VeilidConfig { // Send configuration update to clients if let Some(update_cb) = &self.update_cb { let safe_cfg = self.safe_config_inner(); - update_cb(VeilidUpdate::Config(VeilidStateConfig { config: safe_cfg })); + update_cb(VeilidUpdate::Config(Box::new(VeilidStateConfig { + config: safe_cfg, + }))); } Ok(out) @@ -680,7 +682,7 @@ impl VeilidConfig { // Replace subkey let mut out = &mut jvc; for k in objkeypath { - if !out.has_key(*k) { + if !out.has_key(k) { apibail_parse_error!(format!("invalid subkey in key '{}'", key), k); } out = &mut out[*k]; diff --git a/veilid-server/src/client_api.rs b/veilid-server/src/client_api.rs index c12ad8c7..68ab0cb7 100644 --- a/veilid-server/src/client_api.rs +++ b/veilid-server/src/client_api.rs @@ -150,7 +150,7 @@ impl ClientApi { // Process control messages for the server async fn process_control(self, args: Vec) -> VeilidAPIResult { - if args.len() == 0 { + if args.is_empty() { apibail_generic!("no control request specified"); } if args[0] == "Shutdown" { @@ -207,7 +207,7 @@ impl ClientApi { // Avoid logging failed deserialization of large adversarial payloads from // http://127.0.0.1:5959 by using an initial colon to force a parse error. - let sanitized_line = if line.len() > MAX_NON_JSON_LOGGING && !line.starts_with("{") { + let sanitized_line = if line.len() > MAX_NON_JSON_LOGGING && !line.starts_with('{') { ":skipped long input that's not a JSON object".to_string() } else { line.to_string() @@ -265,7 +265,7 @@ impl ClientApi { linebuf.clear(); // Ignore newlines - if line.len() == 0 { + if line.is_empty() { continue; } @@ -289,7 +289,7 @@ impl ClientApi { mut writer: W, ) -> VeilidAPIResult> { while let Ok(resp) = responses_rx.recv_async().await { - if let Err(_) = writer.write_all(resp.as_bytes()).await { + if (writer.write_all(resp.as_bytes()).await).is_err() { break; } } @@ -420,7 +420,7 @@ impl ClientApi { // Pass other updates to clients let inner = self.inner.lock(); for ch in inner.update_channels.values() { - if let Err(_) = ch.send(veilid_update.clone()) { + if ch.send(veilid_update.clone()).is_err() { // eprintln!("failed to send update: {}", e); } } diff --git a/veilid-server/src/main.rs b/veilid-server/src/main.rs index a555f2ee..252ba2d4 100644 --- a/veilid-server/src/main.rs +++ b/veilid-server/src/main.rs @@ -20,7 +20,7 @@ use clap::{Args, Parser}; use server::*; use settings::LogLevel; use std::collections::HashMap; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::path::Path; use std::str::FromStr; use tools::*; @@ -162,17 +162,11 @@ fn main() -> EyreResult<()> { } // Attempt to load configuration - let settings_path: Option<&OsStr> = if let Some(config_file) = &args.config_file { - if Path::new(&config_file).exists() { - Some(config_file) - } else { - None - } - } else { - None - }; + let settings_path: Option = args + .config_file + .filter(|config_file| Path::new(&config_file).exists()); - let settings = Settings::new(settings_path).wrap_err("configuration is invalid")?; + let settings = Settings::new(settings_path.as_deref()).wrap_err("configuration is invalid")?; // write lock the settings let mut settingsrw = settings.write(); @@ -303,7 +297,7 @@ fn main() -> EyreResult<()> { // --- Generate DHT Key --- if let Some(ckstr) = args.generate_key_pair { - if ckstr == "" { + if ckstr.is_empty() { let mut tks = veilid_core::TypedKeyGroup::new(); let mut tss = veilid_core::TypedSecretGroup::new(); for ck in veilid_core::VALID_CRYPTO_KINDS { @@ -312,16 +306,12 @@ fn main() -> EyreResult<()> { tks.add(veilid_core::TypedKey::new(tkp.kind, tkp.value.key)); tss.add(veilid_core::TypedSecret::new(tkp.kind, tkp.value.secret)); } - println!( - "Public Keys:\n{}\nSecret Keys:\n{}\n", - tks.to_string(), - tss.to_string() - ); + println!("Public Keys:\n{}\nSecret Keys:\n{}\n", tks, tss); } else { let ck: veilid_core::CryptoKind = veilid_core::FourCC::from_str(&ckstr).wrap_err("couldn't parse crypto kind")?; let tkp = veilid_core::Crypto::generate_keypair(ck).wrap_err("invalid crypto kind")?; - println!("{}", tkp.to_string()); + println!("{}", tkp); } return Ok(()); } diff --git a/veilid-server/src/server.rs b/veilid-server/src/server.rs index e918fc2f..8cfbe032 100644 --- a/veilid-server/src/server.rs +++ b/veilid-server/src/server.rs @@ -48,7 +48,19 @@ pub async fn run_veilid_server_internal( ) -> EyreResult<()> { trace!(?settings, ?server_mode); - let settingsr = settings.read(); + let ( + settings_auto_attach, + settings_client_api_enabled, + settings_client_api_listen_address_addrs, + ) = { + let settingsr = settings.read(); + + ( + settingsr.auto_attach, + settingsr.client_api.enabled, + settingsr.client_api.listen_address.addrs.clone(), + ) + }; // Create client api state change pipe let (sender, receiver): ( @@ -72,20 +84,19 @@ pub async fn run_veilid_server_internal( .wrap_err("VeilidCore startup failed")?; // Start client api if one is requested - let mut capi = if settingsr.client_api.enabled && matches!(server_mode, ServerMode::Normal) { + let mut capi = if settings_client_api_enabled && matches!(server_mode, ServerMode::Normal) { let some_capi = client_api::ClientApi::new(veilid_api.clone(), veilid_logs.clone(), settings.clone()); some_capi .clone() - .run(settingsr.client_api.listen_address.addrs.clone()); + .run(settings_client_api_listen_address_addrs); Some(some_capi) } else { None }; // Drop rwlock on settings - let auto_attach = settingsr.auto_attach || !matches!(server_mode, ServerMode::Normal); - drop(settingsr); + let auto_attach = settings_auto_attach || !matches!(server_mode, ServerMode::Normal); // Process all updates let capi2 = capi.clone(); diff --git a/veilid-server/src/settings.rs b/veilid-server/src/settings.rs index 498882fd..abc73335 100644 --- a/veilid-server/src/settings.rs +++ b/veilid-server/src/settings.rs @@ -1,5 +1,3 @@ -#![allow(clippy::bool_assert_comparison)] - use clap::ValueEnum; use directories::*; @@ -714,62 +712,62 @@ impl Settings { } // bump client api port - (*settingsrw).client_api.listen_address.offset_port(idx)?; + settingsrw.client_api.listen_address.offset_port(idx)?; // bump protocol ports - (*settingsrw) + settingsrw .core .network .protocol .udp .listen_address .offset_port(idx)?; - (*settingsrw) + settingsrw .core .network .protocol .tcp .listen_address .offset_port(idx)?; - (*settingsrw) + settingsrw .core .network .protocol .ws .listen_address .offset_port(idx)?; - if let Some(url) = &mut (*settingsrw).core.network.protocol.ws.url { + if let Some(url) = &mut settingsrw.core.network.protocol.ws.url { url.offset_port(idx)?; } - (*settingsrw) + settingsrw .core .network .protocol .wss .listen_address .offset_port(idx)?; - if let Some(url) = &mut (*settingsrw).core.network.protocol.wss.url { + if let Some(url) = &mut settingsrw.core.network.protocol.wss.url { url.offset_port(idx)?; } // bump application ports - (*settingsrw) + settingsrw .core .network .application .http .listen_address .offset_port(idx)?; - if let Some(url) = &mut (*settingsrw).core.network.application.http.url { + if let Some(url) = &mut settingsrw.core.network.application.http.url { url.offset_port(idx)?; } - (*settingsrw) + settingsrw .core .network .application .https .listen_address .offset_port(idx)?; - if let Some(url) = &mut (*settingsrw).core.network.application.https.url { + if let Some(url) = &mut settingsrw.core.network.application.https.url { url.offset_port(idx)?; } Ok(()) @@ -1531,7 +1529,7 @@ mod tests { let settings = Settings::new(None).unwrap(); let s = settings.read(); - assert_eq!(s.daemon.enabled, false); + assert!(!s.daemon.enabled); assert_eq!(s.daemon.pid_file, None); assert_eq!(s.daemon.chroot, None); assert_eq!(s.daemon.working_directory, None); @@ -1539,51 +1537,51 @@ mod tests { assert_eq!(s.daemon.group, None); assert_eq!(s.daemon.stdout_file, None); assert_eq!(s.daemon.stderr_file, None); - assert_eq!(s.client_api.enabled, true); + assert!(s.client_api.enabled); assert_eq!(s.client_api.listen_address.name, "localhost:5959"); assert_eq!( s.client_api.listen_address.addrs, listen_address_to_socket_addrs("localhost:5959").unwrap() ); - assert_eq!(s.auto_attach, true); - assert_eq!(s.logging.system.enabled, false); + assert!(s.auto_attach); + assert!(!s.logging.system.enabled); assert_eq!(s.logging.system.level, LogLevel::Info); - assert_eq!(s.logging.terminal.enabled, true); + assert!(s.logging.terminal.enabled); assert_eq!(s.logging.terminal.level, LogLevel::Info); - assert_eq!(s.logging.file.enabled, false); + assert!(!s.logging.file.enabled); assert_eq!(s.logging.file.path, ""); - assert_eq!(s.logging.file.append, true); + assert!(s.logging.file.append); assert_eq!(s.logging.file.level, LogLevel::Info); - assert_eq!(s.logging.api.enabled, true); + assert!(s.logging.api.enabled); assert_eq!(s.logging.api.level, LogLevel::Info); - assert_eq!(s.logging.otlp.enabled, false); + assert!(!s.logging.otlp.enabled); assert_eq!(s.logging.otlp.level, LogLevel::Trace); assert_eq!( s.logging.otlp.grpc_endpoint, NamedSocketAddrs::from_str("localhost:4317").unwrap() ); - assert_eq!(s.logging.console.enabled, false); + assert!(!s.logging.console.enabled); assert_eq!(s.testing.subnode_index, 0); assert_eq!( s.core.table_store.directory, Settings::get_default_table_store_path() ); - assert_eq!(s.core.table_store.delete, false); + assert!(!s.core.table_store.delete); assert_eq!( s.core.block_store.directory, Settings::get_default_block_store_path() ); - assert_eq!(s.core.block_store.delete, false); + assert!(!s.core.block_store.delete); - assert_eq!(s.core.protected_store.allow_insecure_fallback, true); - assert_eq!(s.core.protected_store.always_use_insecure_storage, true); + assert!(s.core.protected_store.allow_insecure_fallback); + assert!(s.core.protected_store.always_use_insecure_storage); assert_eq!( s.core.protected_store.directory, Settings::get_default_protected_store_directory() ); - assert_eq!(s.core.protected_store.delete, false); + assert!(!s.core.protected_store.delete); assert_eq!(s.core.protected_store.device_encryption_key_password, ""); assert_eq!( s.core.protected_store.new_device_encryption_key_password, @@ -1633,8 +1631,8 @@ mod tests { 2_000u32 ); // - assert_eq!(s.core.network.upnp, true); - assert_eq!(s.core.network.detect_address_changes, true); + assert!(s.core.network.upnp); + assert!(s.core.network.detect_address_changes); assert_eq!(s.core.network.restricted_nat_retries, 0u32); // assert_eq!( @@ -1647,7 +1645,7 @@ mod tests { ); assert_eq!(s.core.network.tls.connection_initial_timeout_ms, 2_000u32); // - assert_eq!(s.core.network.application.https.enabled, false); + assert!(!s.core.network.application.https.enabled); assert_eq!(s.core.network.application.https.listen_address.name, ":443"); assert_eq!( s.core.network.application.https.listen_address.addrs, @@ -1658,7 +1656,7 @@ mod tests { std::path::PathBuf::from("app") ); assert_eq!(s.core.network.application.https.url, None); - assert_eq!(s.core.network.application.http.enabled, false); + assert!(!s.core.network.application.http.enabled); assert_eq!(s.core.network.application.http.listen_address.name, ":80"); assert_eq!( s.core.network.application.http.listen_address.addrs, @@ -1670,23 +1668,23 @@ mod tests { ); assert_eq!(s.core.network.application.http.url, None); // - assert_eq!(s.core.network.protocol.udp.enabled, true); + assert!(s.core.network.protocol.udp.enabled); assert_eq!(s.core.network.protocol.udp.socket_pool_size, 0); assert_eq!(s.core.network.protocol.udp.listen_address.name, ""); assert_eq!(s.core.network.protocol.udp.listen_address.addrs, vec![]); assert_eq!(s.core.network.protocol.udp.public_address, None); // - assert_eq!(s.core.network.protocol.tcp.connect, true); - assert_eq!(s.core.network.protocol.tcp.listen, true); + assert!(s.core.network.protocol.tcp.connect); + assert!(s.core.network.protocol.tcp.listen); assert_eq!(s.core.network.protocol.tcp.max_connections, 32); assert_eq!(s.core.network.protocol.tcp.listen_address.name, ""); assert_eq!(s.core.network.protocol.tcp.listen_address.addrs, vec![]); assert_eq!(s.core.network.protocol.tcp.public_address, None); // - assert_eq!(s.core.network.protocol.ws.connect, true); - assert_eq!(s.core.network.protocol.ws.listen, true); + assert!(s.core.network.protocol.ws.connect); + assert!(s.core.network.protocol.ws.listen); assert_eq!(s.core.network.protocol.ws.max_connections, 32); assert_eq!(s.core.network.protocol.ws.listen_address.name, ""); assert_eq!(s.core.network.protocol.ws.listen_address.addrs, vec![]); @@ -1696,8 +1694,8 @@ mod tests { ); assert_eq!(s.core.network.protocol.ws.url, None); // - assert_eq!(s.core.network.protocol.wss.connect, true); - assert_eq!(s.core.network.protocol.wss.listen, false); + assert!(s.core.network.protocol.wss.connect); + assert!(!s.core.network.protocol.wss.listen); assert_eq!(s.core.network.protocol.wss.max_connections, 32); assert_eq!(s.core.network.protocol.wss.listen_address.name, ""); assert_eq!(s.core.network.protocol.wss.listen_address.addrs, vec![]); diff --git a/veilid-server/src/unix.rs b/veilid-server/src/unix.rs index 0738c7a6..b9ab9dba 100644 --- a/veilid-server/src/unix.rs +++ b/veilid-server/src/unix.rs @@ -1,8 +1,8 @@ -use crate::*; use crate::server::*; use crate::settings::Settings; use crate::tools::*; use crate::veilid_logs::*; +use crate::*; use futures_util::StreamExt; use signal_hook::consts::signal::*; use signal_hook_async_std::Signals; @@ -84,7 +84,7 @@ pub fn run_daemon(settings: Settings, _args: CmdlineArgs) -> EyreResult<()> { // Catch signals let signals = - Signals::new(&[SIGHUP, SIGTERM, SIGINT, SIGQUIT]).wrap_err("failed to init signals")?; + Signals::new([SIGHUP, SIGTERM, SIGINT, SIGQUIT]).wrap_err("failed to init signals")?; let handle = signals.handle(); let signals_task = spawn(handle_signals(signals));