diff --git a/crates/rattler_networking/Cargo.toml b/crates/rattler_networking/Cargo.toml index d900d4bdb..5c85dfca2 100644 --- a/crates/rattler_networking/Cargo.toml +++ b/crates/rattler_networking/Cargo.toml @@ -19,9 +19,12 @@ blocking = ['reqwest/blocking'] [dependencies] anyhow = "1.0.75" dirs = "5.0.1" +fslock = "0.2.1" +itertools = "0.11.0" keyring = "2.0.5" lazy_static = "1.4.0" libc = "0.2.148" +once_cell = "1.18.0" reqwest = { version = "0.11.22", default-features = false } retry-policies = { version = "0.2.0", default-features = false } serde = "1.0.188" @@ -29,7 +32,6 @@ serde_json = "1.0.107" thiserror = "1.0.49" tracing = "0.1.37" url = "2.4.1" -itertools = "0.11.0" [target.'cfg( target_arch = "wasm32" )'.dependencies] getrandom = { version = "0.2.10", features = ["js"] } diff --git a/crates/rattler_networking/src/authentication_storage/fallback_storage.rs b/crates/rattler_networking/src/authentication_storage/fallback_storage.rs index c6f6e1f44..e792a9734 100644 --- a/crates/rattler_networking/src/authentication_storage/fallback_storage.rs +++ b/crates/rattler_networking/src/authentication_storage/fallback_storage.rs @@ -1,8 +1,8 @@ //! Fallback storage for passwords. -use std::{ - path::PathBuf, - sync::{Arc, Mutex}, -}; +use fslock::LockFile; +use once_cell::sync::Lazy; +use std::collections::{HashMap, HashSet}; +use std::{path::PathBuf, sync::Mutex}; /// A struct that implements storage and access of authentication /// information backed by a on-disk JSON file @@ -10,9 +10,6 @@ use std::{ pub struct FallbackStorage { /// The path to the JSON file pub path: PathBuf, - - /// A mutex to ensure that only one thread accesses the file at a time - mutex: Arc>, } /// An error that can occur when accessing the fallback storage @@ -22,6 +19,10 @@ pub enum FallbackStorageError { #[error("IO error: {0}")] IOError(#[from] std::io::Error), + /// Failed to lock the fallback storage file + #[error("failed to lock fallback storage file {0}.")] + FailedToLock(String, #[source] std::io::Error), + /// An error occurred when (de)serializing the credentials #[error("JSON error: {0}")] JSONError(#[from] serde_json::Error), @@ -30,15 +31,34 @@ pub enum FallbackStorageError { impl FallbackStorage { /// Create a new fallback storage with the given path pub fn new(path: PathBuf) -> Self { - Self { - path, - mutex: Arc::new(Mutex::new(())), + Self { path } + } + + /// Lock the fallback storage file for reading and writing. This will block until the lock is + /// acquired. + fn lock(&self) -> Result { + std::fs::create_dir_all(self.path.parent().unwrap())?; + let path = self.path.with_extension("lock"); + let mut lock = fslock::LockFile::open(&path).map_err(|e| { + FallbackStorageError::FailedToLock(path.to_string_lossy().into_owned(), e) + })?; + + // First try to lock the file without block. If we can't immediately get the lock we block and issue a debug message. + if !lock.try_lock_with_pid().map_err(|e| { + FallbackStorageError::FailedToLock(path.to_string_lossy().into_owned(), e) + })? { + tracing::debug!("waiting for lock on {}", path.to_string_lossy()); + lock.lock_with_pid().map_err(|e| { + FallbackStorageError::FailedToLock(path.to_string_lossy().into_owned(), e) + })?; } + + Ok(lock) } /// Store the given authentication information for the given host pub fn set_password(&self, host: &str, password: &str) -> Result<(), FallbackStorageError> { - let _lock = self.mutex.lock().unwrap(); + let _lock = self.lock()?; let mut dict = self.read_json()?; dict.insert(host.to_string(), password.to_string()); self.write_json(&dict) @@ -46,14 +66,14 @@ impl FallbackStorage { /// Retrieve the authentication information for the given host pub fn get_password(&self, host: &str) -> Result, FallbackStorageError> { - let _lock = self.mutex.lock().unwrap(); + let _lock = self.lock()?; let dict = self.read_json()?; Ok(dict.get(host).cloned()) } /// Delete the authentication information for the given host pub fn delete_password(&self, host: &str) -> Result<(), FallbackStorageError> { - let _lock = self.mutex.lock().unwrap(); + let _lock = self.lock()?; let mut dict = self.read_json()?; dict.remove(host); self.write_json(&dict) @@ -61,13 +81,18 @@ impl FallbackStorage { /// Read the JSON file and deserialize it into a HashMap, or return an empty HashMap if the file /// does not exist - fn read_json(&self) -> Result, FallbackStorageError> { + fn read_json(&self) -> Result, FallbackStorageError> { if !self.path.exists() { - tracing::warn!( - "Can't find path for fallback storage on {}", - self.path.to_string_lossy() - ); - return Ok(std::collections::HashMap::new()); + static WARN_GUARD: Lazy>> = + Lazy::new(|| Mutex::new(HashSet::new())); + let mut guard = WARN_GUARD.lock().unwrap(); + if !guard.insert(self.path.clone()) { + tracing::warn!( + "Can't find path for fallback storage on {}", + self.path.to_string_lossy() + ); + } + return Ok(HashMap::new()); } let file = std::fs::File::open(&self.path)?; let reader = std::io::BufReader::new(file); @@ -76,13 +101,7 @@ impl FallbackStorage { } /// Serialize the given HashMap and write it to the JSON file - fn write_json( - &self, - dict: &std::collections::HashMap, - ) -> Result<(), FallbackStorageError> { - if !self.path.exists() { - std::fs::create_dir_all(self.path.parent().unwrap())?; - } + fn write_json(&self, dict: &HashMap) -> Result<(), FallbackStorageError> { let file = std::fs::File::create(&self.path)?; let writer = std::io::BufWriter::new(file); serde_json::to_writer(writer, dict)?; diff --git a/crates/rattler_networking/src/lib.rs b/crates/rattler_networking/src/lib.rs index 526aae783..58b8e522c 100644 --- a/crates/rattler_networking/src/lib.rs +++ b/crates/rattler_networking/src/lib.rs @@ -252,6 +252,12 @@ mod tests { let tdir = tempdir()?; let storage = super::AuthenticationStorage::new("rattler_test", tdir.path()); let host = "conda.example.com"; + + // Make sure the keyring is empty + if let Ok(entry) = keyring::Entry::new("rattler_test", host) { + let _ = entry.delete_password(); + } + let retrieved = storage.get(host); if let Err(e) = retrieved.as_ref() { @@ -288,6 +294,12 @@ mod tests { let tdir = tempdir()?; let storage = super::AuthenticationStorage::new("rattler_test", tdir.path()); let host = "bearer.example.com"; + + // Make sure the keyring is empty + if let Ok(entry) = keyring::Entry::new("rattler_test", host) { + let _ = entry.delete_password(); + } + let retrieved = storage.get(host); if let Err(e) = retrieved.as_ref() { @@ -329,6 +341,12 @@ mod tests { let tdir = tempdir()?; let storage = super::AuthenticationStorage::new("rattler_test", tdir.path()); let host = "basic.example.com"; + + // Make sure the keyring is empty + if let Ok(entry) = keyring::Entry::new("rattler_test", host) { + let _ = entry.delete_password(); + } + let retrieved = storage.get(host); if let Err(e) = retrieved.as_ref() {