mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse): mirror sync no longer deletes what Drive or this disk never had
Five ways a mirror pass could lose files: - A folder deleted on Drive was removed here with everything in it, including new files and edits whose uploads had just failed for want of a parent, and ignored content such as .git. Such a folder now stays and is created on Drive again (B211). - A node the SDK could not decrypt was missing from the remote walk and so read as deleted on Drive. A short listing or an unreadable size now leaves the affected paths out of the pass, and a pass in which no synced path is left on Drive is refused (B212). - A file Drive deleted was removed here even when it had been edited since the scan. It is now checked again first, and kept (B213). - A folder deleted here was trashed on Drive although another device had just put new files in it. It now stays on Drive (B214). - A decrypted name holding / or .. became a path outside the sync folder, and failed the whole listing in the mount. Such names are left out (B215).
This commit is contained in:
1 parent
3d96cdbab1
commit
3221e4aa97
4 files changed
+641
-31
No files matched your search
@@ -6743,7 +6743,27 @@ fn fuse_name(name: &OsStr) -> Result<String, Errno> {
|
||||
name.to_str().map(str::to_owned).ok_or(Errno::EILSEQ)
|
||||
}
|
||||
|
||||
/// Whether a decrypted remote name can stand as one local path component.
|
||||
///
|
||||
/// Drive checks names only when a client creates or renames a node, and only
|
||||
/// for length, so a name made by another client, or by a server that should
|
||||
/// not be trusted, can hold anything. Joined onto a local path, a `/` or a `..`
|
||||
/// in it walks out of the folder it belongs to, and the mirror engine would
|
||||
/// then write or delete outside the sync root. In the mount, the kernel fails
|
||||
/// the whole listing with EIO on a single such name (`docs/BUGS.md` B215).
|
||||
pub(crate) fn remote_name_is_safe(name: &str) -> bool {
|
||||
!name.is_empty()
|
||||
&& name != "."
|
||||
&& name != ".."
|
||||
&& name.len() <= 255
|
||||
&& !name.contains(['/', '\0'])
|
||||
}
|
||||
|
||||
fn node_visible(node: &Node, folder_uid: &NodeUid, hidden: &HashSet<NodeUid>) -> bool {
|
||||
if !remote_name_is_safe(&node.name) {
|
||||
warn!(uid = %node.uid, "a remote name cannot be a local file name; leaving it out");
|
||||
return false;
|
||||
}
|
||||
!node.trashed && node.uid != *folder_uid && !hidden.contains(&node.uid)
|
||||
}
|
||||
|
||||
@@ -8654,6 +8674,28 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_remote_name_that_is_not_one_path_component_is_left_out() {
|
||||
let folder_uid = NodeUid::new(VolumeId::from("vol"), LinkId::from("folder"));
|
||||
let none = std::collections::HashSet::new();
|
||||
for name in [
|
||||
"a/b",
|
||||
"../../.bashrc",
|
||||
"..",
|
||||
".",
|
||||
"",
|
||||
"a\0b",
|
||||
&"x".repeat(256),
|
||||
] {
|
||||
let node = node_helper("child", "folder", name, false);
|
||||
assert!(!node_visible(&node, &folder_uid, &none), "{name:?}");
|
||||
}
|
||||
for name in [".hidden", "a..b", "unicodé-文件", &"x".repeat(255)] {
|
||||
let node = node_helper("child", "folder", name, false);
|
||||
assert!(node_visible(&node, &folder_uid, &none), "{name:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_queue_guard_preserves_accepted_bytes_and_pending_count() {
|
||||
use std::cell::Cell;
|
||||
|
||||
+250
-31
@@ -59,9 +59,10 @@ pub(crate) use planner::base_name;
|
||||
#[cfg(test)]
|
||||
use planner::conflict_path;
|
||||
use planner::{
|
||||
FilePlan, Unscanned, classification_order, conflict_path_with_suffix, filter_baseline,
|
||||
guard_local_wipe, join_rel, local_deletions, parent_rel, plan_file, rel_to_path, remote_sig,
|
||||
tracked_baseline, unchanged_remote_size,
|
||||
FilePlan, RemoteGaps, Unscanned, classification_order, conflict_path_with_suffix,
|
||||
filter_baseline, guard_local_wipe, guard_remote_wipe, join_rel, local_deletions, parent_rel,
|
||||
plan_file, rel_to_path, remote_sig, remote_subtree_changed, tracked_baseline,
|
||||
unchanged_remote_size,
|
||||
};
|
||||
/// What a [`Pending`] op did, so the reconcile can update shared state on the
|
||||
/// engine thread (never inside a task).
|
||||
@@ -638,23 +639,32 @@ impl Core {
|
||||
|
||||
let mut remote: HashMap<String, RemoteItem> = HashMap::new();
|
||||
let mut remote_dirs: HashMap<String, NodeUid> = HashMap::new();
|
||||
let mut gaps = RemoteGaps::default();
|
||||
remote_dirs.insert(String::new(), remote_root.clone());
|
||||
self.walk_remote(
|
||||
folder_id,
|
||||
remote_root,
|
||||
"",
|
||||
&mut remote,
|
||||
&mut remote_dirs,
|
||||
&mut RemoteWalk {
|
||||
out: &mut remote,
|
||||
dirs: &mut remote_dirs,
|
||||
gaps: &mut gaps,
|
||||
},
|
||||
&baseline,
|
||||
)?;
|
||||
guard_remote_wipe(&tracked_baseline(&baseline, rules, &unscanned), &remote)?;
|
||||
|
||||
let mut order = classification_order(&local, &remote, &baseline, rules);
|
||||
// A path the scan could not read is no more a local deletion than an
|
||||
// ignored one, and a remote path under it has nowhere to be written.
|
||||
order.retain(|rel| !unscanned.covers(rel));
|
||||
// The same holds for a path the remote walk could not read: missing
|
||||
// from it, it would read as deleted on Drive (B212).
|
||||
order.retain(|rel| !unscanned.covers(rel) && !gaps.hides(rel, &remote, &remote_dirs));
|
||||
|
||||
let mut unscanned_report = unscanned.report();
|
||||
unscanned_report.extend(gaps.report());
|
||||
let mut outcome = Outcome {
|
||||
unscanned: unscanned.report(),
|
||||
unscanned: unscanned_report,
|
||||
..Default::default()
|
||||
};
|
||||
// Folders to delete, collected here and removed deepest-first at the end
|
||||
@@ -814,9 +824,17 @@ impl Core {
|
||||
}
|
||||
},
|
||||
FilePlan::DeleteLocal => {
|
||||
if let Err(e) =
|
||||
removed_locally(std::fs::remove_file(local_root.join(rel_to_path(rel))))
|
||||
{
|
||||
let path = local_root.join(rel_to_path(rel));
|
||||
// The plan comes from a scan taken before the remote walk,
|
||||
// which can take minutes. An edit saved since then is not
|
||||
// something Drive deleted: keep it, and the next pass
|
||||
// uploads it as a new file (B213).
|
||||
if l.is_some_and(|planned| changed_since_scan(&path, planned)) {
|
||||
info!(rel, "sync: changed since the scan; not deleting it");
|
||||
outcome.deferred += 1;
|
||||
continue;
|
||||
}
|
||||
if let Err(e) = removed_locally(std::fs::remove_file(&path)) {
|
||||
warn!(rel, error = %e, "sync: remove local file failed");
|
||||
self.log_activity(
|
||||
ActivityKind::Trash,
|
||||
@@ -874,17 +892,17 @@ impl Core {
|
||||
&mut outcome,
|
||||
)?;
|
||||
|
||||
// Deferred folder deletions, deepest first.
|
||||
delete_local_dirs.sort_by_key(|p| std::cmp::Reverse(p.matches('/').count()));
|
||||
for rel in delete_local_dirs {
|
||||
if let Err(e) =
|
||||
removed_locally(std::fs::remove_dir_all(local_root.join(rel_to_path(&rel))))
|
||||
{
|
||||
warn!(rel, error = %e, "sync: remove local folder failed");
|
||||
self.log_activity(ActivityKind::Trash, base_name(&rel), e.to_string(), false);
|
||||
outcome.errors += 1;
|
||||
continue;
|
||||
}
|
||||
// Deferred folder deletions, deepest first. A folder that holds what
|
||||
// Drive never had stays, and its baseline row goes, so the next pass
|
||||
// creates it on Drive again and uploads what it holds (B211).
|
||||
if !delete_local_dirs.is_empty() {
|
||||
let mut current = self
|
||||
.db
|
||||
.sync_entries(folder_id)
|
||||
.map_err(|e| format!("reload baseline: {e:?}"))?;
|
||||
for removal in remove_local_dirs(local_root, delete_local_dirs, &mut current) {
|
||||
match removal {
|
||||
DirRemoval::Removed(rel) => {
|
||||
let _ = self.db.sync_entry_remove(folder_id, &rel);
|
||||
self.log_activity(
|
||||
ActivityKind::Trash,
|
||||
@@ -894,8 +912,50 @@ impl Core {
|
||||
);
|
||||
outcome.deleted += 1;
|
||||
}
|
||||
DirRemoval::Kept(rel, reason) => {
|
||||
warn!(
|
||||
rel,
|
||||
reason, "sync: deleted on Drive, but holds unsynced data; keeping it"
|
||||
);
|
||||
let _ = self.db.sync_entry_remove(folder_id, &rel);
|
||||
self.log_activity(
|
||||
ActivityKind::Trash,
|
||||
base_name(&rel),
|
||||
format!("deleted on Proton Drive, but kept here: {reason}"),
|
||||
false,
|
||||
);
|
||||
}
|
||||
DirRemoval::Failed(rel, e) => {
|
||||
warn!(rel, error = %e, "sync: remove local folder failed");
|
||||
self.log_activity(
|
||||
ActivityKind::Trash,
|
||||
base_name(&rel),
|
||||
e.to_string(),
|
||||
false,
|
||||
);
|
||||
outcome.errors += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
delete_remote_dirs.sort_by_key(|(p, _)| std::cmp::Reverse(p.matches('/').count()));
|
||||
for (rel, uid) in delete_remote_dirs {
|
||||
// Deleted here, but another device has put something new in it on
|
||||
// Drive. This pass downloaded that, so the folder is back here
|
||||
// with it; trashing the folder now would take it away again (B214).
|
||||
if let Some(reason) = remote_subtree_changed(&rel, &remote, &baseline) {
|
||||
warn!(
|
||||
rel,
|
||||
reason, "sync: deleted here, but changed on Drive; not trashing it"
|
||||
);
|
||||
self.log_activity(
|
||||
ActivityKind::Trash,
|
||||
base_name(&rel),
|
||||
format!("deleted here, but kept on Proton Drive: {reason}"),
|
||||
false,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if let Err(e) = self
|
||||
.rt
|
||||
.block_on(self.drive.trash_nodes(&[uid]))
|
||||
@@ -963,7 +1023,7 @@ impl Core {
|
||||
.block_on(self.drive.enumerate_nodes_light(&uids))
|
||||
.map_err(|e| format!("resolve nodes: {e}"))?;
|
||||
for node in nodes {
|
||||
if node.trashed || !node.is_folder() {
|
||||
if node.trashed || !node.is_folder() || !crate::remote_name_is_safe(&node.name) {
|
||||
continue;
|
||||
}
|
||||
let rel = join_rel(prefix, &node.name);
|
||||
@@ -985,13 +1045,15 @@ impl Core {
|
||||
/// unchanged mtime means the recorded size still stands.
|
||||
///
|
||||
/// [`enumerate_nodes_light`]: proton_drive_rs::ProtonDriveClient::enumerate_nodes_light
|
||||
///
|
||||
/// What the walk could not read goes to `walk.gaps`, never silently out of
|
||||
/// the map: a missing path reads as deleted on Drive.
|
||||
fn walk_remote(
|
||||
&self,
|
||||
folder_id: i64,
|
||||
folder: &NodeUid,
|
||||
prefix: &str,
|
||||
out: &mut HashMap<String, RemoteItem>,
|
||||
dirs: &mut HashMap<String, NodeUid>,
|
||||
walk: &mut RemoteWalk<'_>,
|
||||
baseline: &HashMap<String, StoredSyncEntry>,
|
||||
) -> Result<(), PassAbort> {
|
||||
// Checked per folder, not per pass: this walk is the long pole on a large
|
||||
@@ -1017,6 +1079,27 @@ impl Core {
|
||||
.rt
|
||||
.block_on(self.drive.enumerate_nodes_light(&uids))
|
||||
.map_err(|e| format!("resolve nodes: {e}"))?;
|
||||
// The SDK leaves out a node it cannot decrypt, and says so only in its
|
||||
// log. Its name went with it, so all this can record is the folder.
|
||||
if nodes.len() < uids.len() {
|
||||
warn!(
|
||||
folder = prefix,
|
||||
listed = uids.len(),
|
||||
read = nodes.len(),
|
||||
"sync: some nodes on Drive cannot be read; leaving them out"
|
||||
);
|
||||
walk.gaps.short_listing(prefix);
|
||||
}
|
||||
let nodes: Vec<Node> = nodes
|
||||
.into_iter()
|
||||
.filter(|node| {
|
||||
let safe = crate::remote_name_is_safe(&node.name);
|
||||
if !safe {
|
||||
warn!(folder = prefix, uid = %node.uid, "sync: a name on Drive cannot be a local file name; leaving it out");
|
||||
}
|
||||
safe
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Files whose recorded mtime no longer matches: their size has to be
|
||||
// read for real, so these are the only node keys worth unlocking.
|
||||
@@ -1048,8 +1131,8 @@ impl Core {
|
||||
let rel = join_rel(prefix, &node.name);
|
||||
self.progress_scanned(folder_id, base_name(&rel));
|
||||
if node.is_folder() {
|
||||
dirs.insert(rel.clone(), node.uid.clone());
|
||||
out.insert(
|
||||
walk.dirs.insert(rel.clone(), node.uid.clone());
|
||||
walk.out.insert(
|
||||
rel.clone(),
|
||||
RemoteItem {
|
||||
uid: node.uid.clone(),
|
||||
@@ -1058,16 +1141,19 @@ impl Core {
|
||||
size: 0,
|
||||
},
|
||||
);
|
||||
self.walk_remote(folder_id, &node.uid, &rel, out, dirs, baseline)?;
|
||||
self.walk_remote(folder_id, &node.uid, &rel, walk, baseline)?;
|
||||
} else {
|
||||
let size = if stale.contains(&node.uid.to_string()) {
|
||||
match sized.get(&node.uid.to_string()) {
|
||||
Some(size) => *size,
|
||||
// Its size was needed but could not be read (the full
|
||||
// enumeration skipped it as undecryptable). Leave it out
|
||||
// rather than record a stale size, exactly as a full
|
||||
// walk would have.
|
||||
None => continue,
|
||||
// rather than record a stale size, and say so: missing,
|
||||
// it would read as deleted on Drive.
|
||||
None => {
|
||||
walk.gaps.unread(rel);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
match unchanged_remote_size(baseline, &rel, node.modification_time) {
|
||||
@@ -1075,7 +1161,7 @@ impl Core {
|
||||
None => continue,
|
||||
}
|
||||
};
|
||||
out.insert(
|
||||
walk.out.insert(
|
||||
rel,
|
||||
RemoteItem {
|
||||
uid: node.uid,
|
||||
@@ -1613,6 +1699,62 @@ fn keep_racing_local_edit(path: &Path, expected: Option<LocalSig>) -> std::io::R
|
||||
preserve_conflict_copy(path, now_secs()).map(|_| ())
|
||||
}
|
||||
|
||||
/// What became of one folder [`remove_local_dirs`] was asked to remove.
|
||||
enum DirRemoval {
|
||||
Removed(String),
|
||||
/// Left in place, with why: it holds something Drive does not have.
|
||||
Kept(String, String),
|
||||
Failed(String, std::io::Error),
|
||||
}
|
||||
|
||||
/// Remove the mirror folders in `dirs`, which Drive deleted, deepest first,
|
||||
/// except those that hold something Drive never had.
|
||||
///
|
||||
/// The uploads of a new file or an edit in such a folder failed earlier in the
|
||||
/// pass for want of a parent, and an ignored `.git` is never uploaded at all,
|
||||
/// so removing the folder would lose them for good (B211). `current` is the
|
||||
/// baseline as the pass has left it: a row the pass already removed with its
|
||||
/// file must not read as a deletion not carried up yet, and a subfolder removed
|
||||
/// here takes its rows with it for the same reason. A kept folder loses its own
|
||||
/// row, so its parent sees it as new and is kept too.
|
||||
fn remove_local_dirs(
|
||||
root: &Path,
|
||||
mut dirs: Vec<String>,
|
||||
current: &mut HashMap<String, StoredSyncEntry>,
|
||||
) -> Vec<DirRemoval> {
|
||||
dirs.sort_by_key(|p| std::cmp::Reverse(p.matches('/').count()));
|
||||
dirs.into_iter()
|
||||
.map(|rel| {
|
||||
if let Some(reason) = mirror_subtree_unsynced(root, &rel, current) {
|
||||
current.remove(&rel);
|
||||
return DirRemoval::Kept(rel, reason);
|
||||
}
|
||||
let prefix = format!("{rel}/");
|
||||
current.retain(|path, _| *path != rel && !path.starts_with(&prefix));
|
||||
match removed_locally(std::fs::remove_dir_all(root.join(rel_to_path(&rel)))) {
|
||||
Ok(()) => DirRemoval::Removed(rel),
|
||||
Err(e) => DirRemoval::Failed(rel, e),
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// What a remote walk fills in as it goes down the tree.
|
||||
struct RemoteWalk<'a> {
|
||||
/// Every path it read, by rel path.
|
||||
out: &'a mut HashMap<String, RemoteItem>,
|
||||
/// Every folder it read, by rel path, to its uid.
|
||||
dirs: &'a mut HashMap<String, NodeUid>,
|
||||
/// What it saw but could not read.
|
||||
gaps: &'a mut RemoteGaps,
|
||||
}
|
||||
|
||||
/// Whether the file at `path` is no longer what the scan saw as `planned`.
|
||||
/// A file that is gone has nothing left to lose, so it has not changed.
|
||||
fn changed_since_scan(path: &Path, planned: &LocalItem) -> bool {
|
||||
local_sig(path).is_some_and(|now| !now.same_content(&LocalSig::from(planned)))
|
||||
}
|
||||
|
||||
/// Read a local removal as the sync engine has to read it: the *state* it was
|
||||
/// asking for is "this path is gone", so a path that was already gone is a
|
||||
/// success, not a failure.
|
||||
@@ -2892,6 +3034,83 @@ mod tests {
|
||||
(root, baseline)
|
||||
}
|
||||
|
||||
/// Drive deleted `dir`. Everything it held that was synced goes with it;
|
||||
/// a file Drive never had keeps the folder, and the parent with it
|
||||
/// (`docs/BUGS.md` B211).
|
||||
#[test]
|
||||
fn a_folder_deleted_on_drive_keeps_what_drive_never_had() {
|
||||
let (root, mut baseline) = synced_subtree("b211-keep");
|
||||
// The pass already removed the synced files with their rows.
|
||||
std::fs::remove_file(root.join("dir/a.txt")).unwrap();
|
||||
baseline.remove("dir/a.txt");
|
||||
std::fs::remove_file(root.join("dir/sub/b.txt")).unwrap();
|
||||
baseline.remove("dir/sub/b.txt");
|
||||
std::fs::write(root.join("dir/new.txt"), b"never uploaded").unwrap();
|
||||
|
||||
let removals = remove_local_dirs(
|
||||
&root,
|
||||
vec!["dir".to_string(), "dir/sub".to_string()],
|
||||
&mut baseline,
|
||||
);
|
||||
let shape: Vec<(&str, &str)> = removals
|
||||
.iter()
|
||||
.map(|r| match r {
|
||||
DirRemoval::Removed(rel) => ("removed", rel.as_str()),
|
||||
DirRemoval::Kept(rel, _) => ("kept", rel.as_str()),
|
||||
DirRemoval::Failed(rel, _) => ("failed", rel.as_str()),
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(shape, [("removed", "dir/sub"), ("kept", "dir")]);
|
||||
assert_eq!(
|
||||
std::fs::read(root.join("dir/new.txt")).unwrap(),
|
||||
b"never uploaded"
|
||||
);
|
||||
assert!(!root.join("dir/sub").exists());
|
||||
// Its row is gone, so the next pass sees a new local folder.
|
||||
assert!(!baseline.contains_key("dir"));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_folder_deleted_on_drive_with_only_synced_content_is_removed() {
|
||||
let (root, mut baseline) = synced_subtree("b211-remove");
|
||||
for file in ["dir/a.txt", "dir/sub/b.txt"] {
|
||||
std::fs::remove_file(root.join(file)).unwrap();
|
||||
baseline.remove(file);
|
||||
}
|
||||
let removals = remove_local_dirs(
|
||||
&root,
|
||||
vec!["dir/sub".to_string(), "dir".to_string()],
|
||||
&mut baseline,
|
||||
);
|
||||
assert!(removals.iter().all(|r| matches!(r, DirRemoval::Removed(_))));
|
||||
assert!(!root.join("dir").exists());
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
/// An edit to a file saved after the scan is not a deletion on Drive
|
||||
/// (`docs/BUGS.md` B213).
|
||||
#[test]
|
||||
fn a_file_changed_since_the_scan_is_not_deleted() {
|
||||
let root = sync_test_dir("b213");
|
||||
let path = root.join("notes.txt");
|
||||
std::fs::write(&path, b"as scanned").unwrap();
|
||||
let sig = local_sig(&path).unwrap();
|
||||
let planned = LocalItem {
|
||||
is_dir: false,
|
||||
mtime: sig.mtime,
|
||||
mtime_ns: sig.mtime_ns,
|
||||
size: sig.size,
|
||||
open_for_write: false,
|
||||
};
|
||||
assert!(!changed_since_scan(&path, &planned));
|
||||
std::fs::write(&path, b"edited after the scan").unwrap();
|
||||
assert!(changed_since_scan(&path, &planned));
|
||||
std::fs::remove_file(&path).unwrap();
|
||||
assert!(!changed_since_scan(&path, &planned));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_fully_synced_mirror_subtree_may_leave() {
|
||||
let (root, baseline) = synced_subtree("move-synced");
|
||||
|
||||
@@ -199,6 +199,135 @@ pub(super) fn guard_local_wipe<B, L>(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Refuse to run a pass whose remote side has vanished in its entirety: the
|
||||
/// remote twin of [`guard_local_wipe`].
|
||||
///
|
||||
/// An empty or truncated remote walk reads to the planner as "Drive deleted
|
||||
/// everything", which a pass answers by deleting the local copies. When not
|
||||
/// one synced path is left on Drive, a failed listing is far likelier than a
|
||||
/// deliberate wipe of the whole folder, so the pass stops instead.
|
||||
pub(super) fn guard_remote_wipe<B, R>(
|
||||
baseline: &HashMap<String, B>,
|
||||
remote: &HashMap<String, R>,
|
||||
) -> Result<(), String> {
|
||||
if !baseline.is_empty() && baseline.keys().all(|rel| !remote.contains_key(rel)) {
|
||||
return Err(format!(
|
||||
"every one of the {} synced paths is missing on Proton Drive; refusing to \
|
||||
delete them here. If they were deleted on purpose, remove the folder from sync.",
|
||||
baseline.len()
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What a remote walk saw but could not read: a node the SDK left out because
|
||||
/// it would not decrypt, or a file whose size could not be read.
|
||||
///
|
||||
/// A path missing from the walk is read as deleted on Drive, so a pass must
|
||||
/// not classify one it only failed to read. Since a node that failed to decrypt
|
||||
/// has no name, all a walk can say is that a folder's listing came back short;
|
||||
/// every path under that folder that the walk did not return is then left out,
|
||||
/// as [`Unscanned`] leaves out what the local scan could not read.
|
||||
#[derive(Default)]
|
||||
pub(super) struct RemoteGaps {
|
||||
/// Folders whose listing came back with fewer nodes than it named.
|
||||
short: HashSet<String>,
|
||||
/// Paths seen, but whose entry could not be read.
|
||||
unread: HashSet<String>,
|
||||
}
|
||||
|
||||
impl RemoteGaps {
|
||||
/// The listing of `folder` named nodes that did not come back.
|
||||
pub(super) fn short_listing(&mut self, folder: &str) {
|
||||
self.short.insert(folder.to_string());
|
||||
}
|
||||
|
||||
/// `rel` is on Drive, but the walk could not read what it needs of it.
|
||||
pub(super) fn unread(&mut self, rel: String) {
|
||||
self.unread.insert(rel);
|
||||
}
|
||||
|
||||
pub(super) fn is_empty(&self) -> bool {
|
||||
self.short.is_empty() && self.unread.is_empty()
|
||||
}
|
||||
|
||||
/// Whether `rel` may be missing from `remote` only because the walk could
|
||||
/// not read it. A path the walk returned is never hidden.
|
||||
pub(super) fn hides<R>(
|
||||
&self,
|
||||
rel: &str,
|
||||
remote: &HashMap<String, R>,
|
||||
remote_dirs: &HashMap<String, NodeUid>,
|
||||
) -> bool {
|
||||
if self.is_empty() || remote.contains_key(rel) {
|
||||
return false;
|
||||
}
|
||||
if self.unread.contains(rel) {
|
||||
return true;
|
||||
}
|
||||
// The deepest folder the walk did list decides: under a short listing,
|
||||
// anything not returned may be one of the nodes it could not read.
|
||||
let mut at = parent_rel(rel);
|
||||
loop {
|
||||
if remote_dirs.contains_key(at) {
|
||||
return self.short.contains(at);
|
||||
}
|
||||
if at.is_empty() {
|
||||
return false;
|
||||
}
|
||||
at = parent_rel(at);
|
||||
}
|
||||
}
|
||||
|
||||
/// The folders to name to the user, sorted.
|
||||
pub(super) fn report(&self) -> Vec<String> {
|
||||
let mut lines: Vec<String> = self
|
||||
.short
|
||||
.iter()
|
||||
.map(|rel| {
|
||||
let shown = if rel.is_empty() { "the folder" } else { rel };
|
||||
format!("{shown} (items on Proton Drive that cannot be read)")
|
||||
})
|
||||
.chain(
|
||||
self.unread
|
||||
.iter()
|
||||
.map(|rel| format!("{rel} (cannot be read on Proton Drive)")),
|
||||
)
|
||||
.collect();
|
||||
lines.sort_unstable();
|
||||
lines
|
||||
}
|
||||
}
|
||||
|
||||
/// Why the remote folder at `rel` must not be trashed although it was deleted
|
||||
/// here: something under it is new or changed on Drive since the last sync,
|
||||
/// so it is another device's work that this computer has never had.
|
||||
///
|
||||
/// Trashing the folder would take that work with it, and the next pass would
|
||||
/// then delete the copy this pass just downloaded. `None` when everything under
|
||||
/// it is exactly what the baseline recorded.
|
||||
pub(super) fn remote_subtree_changed(
|
||||
rel: &str,
|
||||
remote: &HashMap<String, RemoteItem>,
|
||||
baseline: &HashMap<String, StoredSyncEntry>,
|
||||
) -> Option<String> {
|
||||
let prefix = format!("{rel}/");
|
||||
let mut changed: Vec<&String> = remote
|
||||
.iter()
|
||||
.filter(|(path, _)| path.starts_with(&prefix))
|
||||
.filter(|(path, item)| match baseline.get(*path) {
|
||||
None => true,
|
||||
Some(_) if item.is_dir => false,
|
||||
Some(base) => remote_sig(base) != Some((item.mtime, item.size)),
|
||||
})
|
||||
.map(|(path, _)| path)
|
||||
.collect();
|
||||
changed.sort_unstable();
|
||||
changed
|
||||
.first()
|
||||
.map(|path| format!("{path} is new or changed on Proton Drive"))
|
||||
}
|
||||
|
||||
/// The size the baseline recorded for `rel`, if its remote signature's mtime
|
||||
/// still matches `mtime`.
|
||||
pub(super) fn unchanged_remote_size(
|
||||
@@ -530,4 +659,96 @@ mod file_plan_tests {
|
||||
FilePlan::Download
|
||||
);
|
||||
}
|
||||
|
||||
fn uid(id: &str) -> NodeUid {
|
||||
NodeUid::new(VolumeId::from("v"), LinkId::from(id))
|
||||
}
|
||||
|
||||
fn folder() -> RemoteItem {
|
||||
RemoteItem {
|
||||
is_dir: true,
|
||||
mtime: 0,
|
||||
size: 0,
|
||||
..remote(0, 0)
|
||||
}
|
||||
}
|
||||
|
||||
/// An empty or failed remote listing must not read as "Drive deleted
|
||||
/// everything" and delete the local copies.
|
||||
#[test]
|
||||
fn the_remote_wipe_guard_blocks_only_a_total_disappearance() {
|
||||
let base = HashMap::from([("a".to_string(), ()), ("b".to_string(), ())]);
|
||||
let none: HashMap<String, RemoteItem> = HashMap::new();
|
||||
assert!(guard_remote_wipe(&base, &none).is_err());
|
||||
let one = HashMap::from([("b".to_string(), remote(1, 1))]);
|
||||
assert!(guard_remote_wipe(&base, &one).is_ok());
|
||||
let empty: HashMap<String, ()> = HashMap::new();
|
||||
assert!(guard_remote_wipe(&empty, &none).is_ok());
|
||||
}
|
||||
|
||||
/// A node that failed to decrypt has no name, so everything a short
|
||||
/// listing did not return is left out, and nothing else.
|
||||
#[test]
|
||||
fn a_short_remote_listing_hides_what_it_did_not_return() {
|
||||
let remote = HashMap::from([
|
||||
("docs".to_string(), folder()),
|
||||
("docs/a.txt".to_string(), remote(1, 1)),
|
||||
("pics".to_string(), folder()),
|
||||
]);
|
||||
let dirs = HashMap::from([
|
||||
(String::new(), uid("root")),
|
||||
("docs".to_string(), uid("docs")),
|
||||
("pics".to_string(), uid("pics")),
|
||||
]);
|
||||
let mut gaps = RemoteGaps::default();
|
||||
assert!(!gaps.hides("docs/b.txt", &remote, &dirs));
|
||||
gaps.short_listing("docs");
|
||||
// Returned paths stand; unreturned ones under the short folder hide,
|
||||
// and so does anything under a subfolder it failed to return.
|
||||
assert!(!gaps.hides("docs/a.txt", &remote, &dirs));
|
||||
assert!(gaps.hides("docs/b.txt", &remote, &dirs));
|
||||
assert!(gaps.hides("docs/sub/deep.txt", &remote, &dirs));
|
||||
// A complete sibling listing still reads deletions as deletions.
|
||||
assert!(!gaps.hides("pics/gone.jpg", &remote, &dirs));
|
||||
assert!(!gaps.hides("top.txt", &remote, &dirs));
|
||||
gaps.short_listing("");
|
||||
assert!(gaps.hides("top.txt", &remote, &dirs));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_file_whose_size_could_not_be_read_is_hidden() {
|
||||
let remote: HashMap<String, RemoteItem> = HashMap::new();
|
||||
let dirs = HashMap::from([(String::new(), uid("root"))]);
|
||||
let mut gaps = RemoteGaps::default();
|
||||
gaps.unread("a.txt".to_string());
|
||||
assert!(gaps.hides("a.txt", &remote, &dirs));
|
||||
assert!(!gaps.hides("b.txt", &remote, &dirs));
|
||||
assert_eq!(gaps.report(), ["a.txt (cannot be read on Proton Drive)"]);
|
||||
}
|
||||
|
||||
/// A folder deleted here holds another device's new work on Drive, so
|
||||
/// trashing it would lose that work.
|
||||
#[test]
|
||||
fn a_remote_folder_with_new_or_changed_files_is_not_trashed() {
|
||||
let mut base = HashMap::from([
|
||||
("x".to_string(), baseline(0, 0, 0, 0)),
|
||||
("x/old.txt".to_string(), baseline(1, 1, 5, 6)),
|
||||
]);
|
||||
let mut remote_tree = HashMap::from([
|
||||
("x".to_string(), folder()),
|
||||
("x/old.txt".to_string(), remote(5, 6)),
|
||||
("xy/new.txt".to_string(), remote(1, 1)),
|
||||
]);
|
||||
// Unchanged under x; a name-prefixed sibling does not count.
|
||||
assert_eq!(remote_subtree_changed("x", &remote_tree, &base), None);
|
||||
|
||||
remote_tree.insert("x/old.txt".to_string(), remote(9, 6));
|
||||
assert!(remote_subtree_changed("x", &remote_tree, &base).is_some());
|
||||
|
||||
remote_tree.insert("x/old.txt".to_string(), remote(5, 6));
|
||||
remote_tree.insert("x/sub".to_string(), folder());
|
||||
assert!(remote_subtree_changed("x", &remote_tree, &base).is_some());
|
||||
base.insert("x/sub".to_string(), baseline(0, 0, 0, 0));
|
||||
assert_eq!(remote_subtree_changed("x", &remote_tree, &base), None);
|
||||
}
|
||||
}
|
||||
+128
@@ -100,6 +100,11 @@ a live account since.
|
||||
|
||||
| Entry | Title |
|
||||
|---|---|
|
||||
| [B215](#b215--a-remote-name-holding--or--becomes-a-path-outside-the-sync-folder) | A remote name holding `/` or `..` becomes a path outside the sync folder |
|
||||
| [B214](#b214--deleting-a-mirror-folder-here-trashes-another-devices-new-files-in-it) | Deleting a mirror folder here trashes another device's new files in it |
|
||||
| [B213](#b213--a-file-edited-during-a-mirror-pass-is-deleted-because-drive-deleted-it-earlier) | A file edited during a mirror pass is deleted because Drive deleted it earlier |
|
||||
| [B212](#b212--a-node-on-drive-that-cannot-be-decrypted-is-deleted-from-the-mirror) | A node on Drive that cannot be decrypted is deleted from the mirror |
|
||||
| [B211](#b211--a-folder-deleted-on-drive-takes-unsynced-local-files-with-it) | A folder deleted on Drive takes unsynced local files with it |
|
||||
| [B210](#b210--after-a-password-reset-the-mount-retries-missing-key-forever) | After a password reset, the mount retries "missing key" forever |
|
||||
| [B209](#b209--a-blocked-sign-in-says-the-app-version-is-no-longer-supported) | A blocked sign-in says the app version is no longer supported |
|
||||
| [B208](#b208--a-file-landing-while-its-folder-is-pinned-is-kept-but-not-available-offline) | A file landing while its folder is pinned is kept but not available offline |
|
||||
@@ -313,6 +318,129 @@ a live account since.
|
||||
|
||||
Newest first.
|
||||
|
||||
### B215 — A remote name holding `/` or `..` becomes a path outside the sync folder
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.2).
|
||||
**Where:** `walk_remote` and `walk_remote_dirs` in `crates/pdfs-fuse/src/sync.rs`; `node_visible`
|
||||
in `crates/pdfs-fuse/src/lib.rs`
|
||||
|
||||
**Repro:** Needs a node whose decrypted name holds `/`, `..` or NUL. Drive checks names only when
|
||||
a client creates or renames a node, and only for length, so another client or a server that
|
||||
should not be trusted can make one. Then let a mirror pass or a listing of its folder run.
|
||||
|
||||
**Cause.** The mirror engine joined each decrypted name onto its parent's relative path and
|
||||
turned that into a local path, so `x/../../.config/autostart/a.desktop` named a file outside the
|
||||
sync root. The pass then created, replaced or removed it there. In the mount, the kernel refuses a
|
||||
whole `readdir` with EIO when one name in it is not a single path component.
|
||||
|
||||
**Fix.** `remote_name_is_safe` accepts a name only if it is one path component: not empty, not
|
||||
`.` or `..`, no `/` or NUL, at most 255 bytes. Both remote walks of the mirror engine and the
|
||||
listing filter of the mount leave out any other name, with a warning. A name left out cannot be a
|
||||
local path at all, so leaving it out cannot read as a deletion of anything local.
|
||||
|
||||
**Test:** `tests::a_remote_name_that_is_not_one_path_component_is_left_out`.
|
||||
|
||||
---
|
||||
|
||||
### B214 — Deleting a mirror folder here trashes another device's new files in it
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.5).
|
||||
**Where:** `Core::do_reconcile` in `crates/pdfs-fuse/src/sync.rs`
|
||||
|
||||
**Repro:** Delete a synced folder locally while another device adds a file to it on Drive, before
|
||||
the next pass.
|
||||
|
||||
**Cause.** The pass planned the new file as a download, which recreated the folder locally. At
|
||||
the end of the same pass it trashed the remote folder, because the folder was in the baseline and
|
||||
gone locally, and took the new file with it. The next pass then saw the downloaded copy unchanged
|
||||
and its remote gone, so it deleted that too. The other device's work survived only in the Drive
|
||||
trash, and nothing said so.
|
||||
|
||||
**Fix.** Before a pass trashes a remote folder, `remote_subtree_changed` looks for anything under
|
||||
it on Drive that is not in the baseline or whose signature moved. If there is, the folder stays on
|
||||
Drive with its row, the activity feed says why, and only its unchanged files are trashed, one by
|
||||
one, as local deletions.
|
||||
|
||||
**Test:** `planner::file_plan_tests::a_remote_folder_with_new_or_changed_files_is_not_trashed`.
|
||||
|
||||
---
|
||||
|
||||
### B213 — A file edited during a mirror pass is deleted because Drive deleted it earlier
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.6).
|
||||
**Where:** `Core::do_reconcile`, `FilePlan::DeleteLocal`, in `crates/pdfs-fuse/src/sync.rs`
|
||||
|
||||
**Repro:** Delete a synced file on another device. On this one, edit the file while a pass over a
|
||||
large folder walks Drive.
|
||||
|
||||
**Cause.** A pass takes its local signatures before the remote walk, which takes minutes on a
|
||||
large tree, and deleted a file that Drive had deleted without looking at it again. Downloads got
|
||||
that second look in B39 (`keep_racing_local_edit`); deletions did not, so the edit was lost with
|
||||
no conflict copy.
|
||||
|
||||
**Fix.** `changed_since_scan` compares the file's signature with the one the scan planned on, right
|
||||
before the deletion. A changed file stays, and the next pass uploads it as a new file.
|
||||
|
||||
**Test:** `tests::a_file_changed_since_the_scan_is_not_deleted`.
|
||||
|
||||
---
|
||||
|
||||
### B212 — A node on Drive that cannot be decrypted is deleted from the mirror
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.4). The SDK leaves out nodes it cannot
|
||||
decrypt, which B112 showed on real accounts and B97 during cancellation.
|
||||
**Where:** `Core::walk_remote` and `Core::do_reconcile` in `crates/pdfs-fuse/src/sync.rs`
|
||||
|
||||
**Repro:** Make one synced file or folder fail to decrypt, for example with a key the session
|
||||
cannot unlock, and let a pass run.
|
||||
|
||||
**Cause.** The remote walk kept what the SDK returned and nothing else. A node the SDK left out
|
||||
was then missing from the remote map, and the planner reads "in the baseline and here, but not on
|
||||
Drive" as a deletion on Drive. A file was deleted locally; a folder was removed with
|
||||
`remove_dir_all`. The local side had a guard against a scan that came back empty
|
||||
(`guard_local_wipe`), the remote side had none.
|
||||
|
||||
**Fix.**
|
||||
- A listing that returns fewer nodes than it named marks its folder as short. A file whose size
|
||||
could not be read is marked unread.
|
||||
- `RemoteGaps::hides` leaves out of the pass every path such a gap could explain, as `Unscanned`
|
||||
does for the local scan. The folder is reported with the paths that cannot be synced.
|
||||
- `guard_remote_wipe` refuses a pass in which none of the synced paths is left on Drive.
|
||||
|
||||
**Test:** `planner::file_plan_tests::a_short_remote_listing_hides_what_it_did_not_return`,
|
||||
`planner::file_plan_tests::a_file_whose_size_could_not_be_read_is_hidden`,
|
||||
`planner::file_plan_tests::the_remote_wipe_guard_blocks_only_a_total_disappearance`.
|
||||
|
||||
---
|
||||
|
||||
### B211 — A folder deleted on Drive takes unsynced local files with it
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 1.1).
|
||||
**Where:** `Core::do_reconcile` and `remove_local_dirs` in `crates/pdfs-fuse/src/sync.rs`
|
||||
|
||||
**Repro:** In a mirror folder, add a file to a synced subfolder. Before the next pass, delete the
|
||||
subfolder on another device.
|
||||
|
||||
**Cause.** The pass put the folder on its list of local deletions. The upload of the new file
|
||||
failed for want of a remote parent and was counted as an error. At the end of the pass,
|
||||
`remove_dir_all` removed the folder with everything in it: new files, edits, files open for
|
||||
writing, and ignored content such as `.git`, none of which had ever been on Drive.
|
||||
|
||||
**Fix.** `remove_local_dirs` asks `mirror_subtree_unsynced` about each folder before it removes
|
||||
it, against the baseline as the pass has left it. A folder that holds anything Drive does not
|
||||
have stays, and loses its baseline row, so the next pass creates it on Drive again and uploads
|
||||
what it holds. What was synced in it is still deleted.
|
||||
|
||||
**Test:** `tests::a_folder_deleted_on_drive_keeps_what_drive_never_had`,
|
||||
`tests::a_folder_deleted_on_drive_with_only_synced_content_is_removed`.
|
||||
|
||||
---
|
||||
|
||||
### B210 — After a password reset, the mount retries "missing key" forever
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
|
||||
Reference in new issue
Block a user