fix: stream transfers and close gaps against the Python client
A review against the Python client, which shares this client's config and login cache, found the transfers holding whole files in memory and several commands answering differently from their Python counterparts. - Stream uploads and downloads instead of buffering the whole file, so memory no longer scales with file size times concurrency. Uploads still declare a Content-Length rather than going out chunked. - Write a download beside its destination and move it into place once complete, and refuse a destination that cannot be written, so a failed transfer leaves what was already there untouched. - Report the server's own error detail instead of the bare status. - List locked sub-folders in `ls --json --recursive` using the given password and skip only those that cannot be unlocked, instead of failing the whole listing. - Name each folder's laboratory in JSON output and sort its entries, matching the Python client's schema and order. - Ask the server who is logged in for `whoami`, rather than trusting the cached name. - Collapse repeated separators and refuse `..` in remote paths. - Fall back to the API when a laboratory is missing from the cache, so a newly added one no longer needs a fresh login. - Verify the login cache digest on read, as the Python client does. - Accept `-e` as the short form of `--exclude`. - Satisfy clippy and rustfmt across the crate.
This commit is contained in:
Vendored
+67
-6
@@ -139,14 +139,32 @@ fn write_cache_file(cache_path: &Path, cache: &Cache) -> Result<(), anyhow::Erro
|
||||
}
|
||||
|
||||
fn parse_cache(remote: &str, data: &str) -> Result<Cache, anyhow::Error> {
|
||||
serde_json::from_str::<Cache>(data).map_err(|e| {
|
||||
let cache = serde_json::from_str::<Cache>(data).map_err(|e| {
|
||||
anyhow!(
|
||||
"Cache for `{}` is invalid or outdated ({}). Run `mdrs login {}` to refresh it.",
|
||||
remote,
|
||||
e,
|
||||
remote
|
||||
)
|
||||
})
|
||||
})?;
|
||||
// The digest is what says the file is still the one this client wrote. The Python
|
||||
// client checks it on every read and the two share the file, so a cache one of them
|
||||
// would refuse must not be honoured by the other.
|
||||
let expected = compute_digest(
|
||||
cache.user.as_ref(),
|
||||
&cache.token.access,
|
||||
&cache.token.refresh,
|
||||
&cache.laboratories,
|
||||
);
|
||||
if cache.digest != expected {
|
||||
bail!(
|
||||
"Cache for `{}` has been altered or was written by an incompatible version. \
|
||||
Run `mdrs login {}` to refresh it.",
|
||||
remote,
|
||||
remote
|
||||
);
|
||||
}
|
||||
Ok(cache)
|
||||
}
|
||||
|
||||
fn load_cache_from_dir(remote: &str, config_dir: &Path) -> Result<Cache, anyhow::Error> {
|
||||
@@ -213,7 +231,11 @@ fn load_cache_if_present_from_dir(
|
||||
};
|
||||
let cache = match parse_cache(remote, &data) {
|
||||
Ok(cache) => cache,
|
||||
Err(_) => {
|
||||
Err(e) => {
|
||||
// Said out loud before the credentials go. The alternative is a command that
|
||||
// quietly carries on anonymously and fails later for a reason that looks
|
||||
// unrelated to the cache it just threw away.
|
||||
eprintln!("{e}");
|
||||
remove_cache_in_dir(remote, config_dir)?;
|
||||
return Ok(None);
|
||||
}
|
||||
@@ -290,6 +312,9 @@ pub async fn load_cache_with_token_refresh(remote: &str) -> Result<Cache, anyhow
|
||||
let lock_file = fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create(true)
|
||||
// Nothing is ever written into it: the file exists only to be flocked, and
|
||||
// truncating it would touch a file other processes are holding open.
|
||||
.truncate(false)
|
||||
.open(&lock_path)?;
|
||||
lock_file.lock_exclusive()?;
|
||||
|
||||
@@ -331,6 +356,9 @@ async fn load_cache_with_token_refresh_optional_from_dir(
|
||||
let lock_file = fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create(true)
|
||||
// Nothing is ever written into it: the file exists only to be flocked, and
|
||||
// truncating it would touch a file other processes are holding open.
|
||||
.truncate(false)
|
||||
.open(&lock_path)?;
|
||||
lock_file.lock_exclusive()?;
|
||||
|
||||
@@ -443,7 +471,7 @@ mod tests {
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn sample_cache(username: &str) -> Cache {
|
||||
Cache {
|
||||
let mut cache = Cache {
|
||||
user: Some(CacheUser {
|
||||
id: 1,
|
||||
username: username.to_string(),
|
||||
@@ -462,8 +490,15 @@ mod tests {
|
||||
full_name: "Laboratory".to_string(),
|
||||
}],
|
||||
},
|
||||
digest: format!("digest-{username}"),
|
||||
}
|
||||
digest: String::new(),
|
||||
};
|
||||
cache.digest = compute_digest(
|
||||
cache.user.as_ref(),
|
||||
&cache.token.access,
|
||||
&cache.token.refresh,
|
||||
&cache.laboratories,
|
||||
);
|
||||
cache
|
||||
}
|
||||
|
||||
fn remote_name(prefix: &str, config_dir: &Path) -> String {
|
||||
@@ -575,6 +610,25 @@ mod tests {
|
||||
assert!(loaded.is_none());
|
||||
}
|
||||
|
||||
/// A cache whose contents no longer match its digest is not a session to act on.
|
||||
#[test]
|
||||
fn load_cache_refuses_a_tampered_cache() {
|
||||
let dir = tempdir().unwrap();
|
||||
let remote = remote_name("tampered", dir.path());
|
||||
let mut cache = sample_cache("alice");
|
||||
persist_cache_in_dir(&remote, dir.path(), &cache).unwrap();
|
||||
|
||||
// Someone edits the stored token but leaves the digest as it was.
|
||||
cache.token.access = "someone-elses-access".to_string();
|
||||
let cache_path = cache_file_path_in(dir.path(), &remote);
|
||||
fs::write(&cache_path, serde_json::to_vec_pretty(&cache).unwrap()).unwrap();
|
||||
invalidate_cached_entry(dir.path(), &remote);
|
||||
|
||||
let err = load_cache_from_dir(&remote, dir.path()).unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("has been altered"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_cache_if_present_clears_invalid_cache() {
|
||||
let dir = tempdir().unwrap();
|
||||
@@ -597,6 +651,13 @@ mod tests {
|
||||
let mut cache = sample_cache("alice");
|
||||
cache.token.access = make_jwt_with_exp(0);
|
||||
cache.token.refresh = make_jwt_with_exp(0);
|
||||
// Re-stamped, so this exercises the expired session rather than the digest check.
|
||||
cache.digest = compute_digest(
|
||||
cache.user.as_ref(),
|
||||
&cache.token.access,
|
||||
&cache.token.refresh,
|
||||
&cache.laboratories,
|
||||
);
|
||||
persist_cache_in_dir(&remote, dir.path(), &cache).unwrap();
|
||||
|
||||
let loaded = load_cache_with_token_refresh_optional_from_dir(&remote, dir.path())
|
||||
|
||||
Reference in New Issue
Block a user