diff --git a/README.md b/README.md index b61d40b..8db4e86 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ A fast, unofficial Proton Drive client for Linux. This client features an advanc - **Fuzzy Search Launcher Prompt (HUD)**: A Google Drive-style search launcher (`pdfs-prompt`) for finding and opening files or folders instantly, ideal for binding to a system-wide hotkey. - **Secure Credential Storage**: Integrates with the system Secret Service (GNOME Keyring, KWallet, etc.) with smart in-memory credential caching to avoid UI thread blockages. - **Proton Photos Support**: Access your Proton Photos timeline, view thumbnails, and download backed-up media natively (available in the GUI as a navigation tab and via the CLI). +- **Human Verification (CAPTCHA) Recovery**: Detects sign-in gates (VPN/new IP challenges) and launches an embedded `WebKitWebView` dialog to safely complete the challenge, transparently retrying authentication with the earned token. ## Performance & Caching diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index 96b8ff8..1b4dc18 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/crates/pdfs-core/src/db/migrations.rs @@ -9,7 +9,7 @@ use super::Db; use crate::Result; /// Current schema version. Bump on every forward migration added below. -pub(super) const SCHEMA_VERSION: i64 = 14; +pub(super) const SCHEMA_VERSION: i64 = 15; impl Db { pub(super) fn migrate(&self) -> Result<()> { @@ -78,6 +78,9 @@ impl Db { if current < 14 { tx.execute_batch(MIGRATION_V14)?; } + if current < 15 { + tx.execute_batch(MIGRATION_V15)?; + } tx.execute( "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1) ON CONFLICT(key) DO UPDATE SET value = excluded.value", @@ -340,3 +343,29 @@ ALTER TABLE photos ADD COLUMN kind INTEGER NOT NULL DEFAULT 0; const MIGRATION_V14: &str = " CREATE INDEX cache_entries_lru ON cache_entries(kind, last_accessed); "; + +/// Schema v15: key the search index by rowid so a row can be replaced without +/// scanning the whole index (B12). +/// +/// v3 built `nodes_fts` with the node's `uid` as an `UNINDEXED` column and +/// `upsert_nodes` refreshed a row with `DELETE FROM nodes_fts WHERE uid = ?`. +/// `UNINDEXED` means exactly what it says — the column is retrievable but not +/// searchable — so that predicate could only ever be a full scan of the index +/// (`EXPLAIN QUERY PLAN`: `SCAN nodes_fts VIRTUAL TABLE`). Every node written +/// scanned every node indexed: measured at 6.5 ms per node against a 17k-node +/// index, about half the cost of a cold listing, and *growing with the size of +/// the account* rather than the size of the listing. +/// +/// FTS5 deletes by `rowid` efficiently, and `nodes` is an ordinary rowid table +/// (`uid` is its `TEXT PRIMARY KEY`, not `WITHOUT ROWID`), so its rowid is a +/// stable per-node key that `ON CONFLICT DO UPDATE` preserves. Rebuild the index +/// keyed by it and drop the `uid` column: searches now join on `rowid`, which is +/// also a cheaper join than the old one on a TEXT uid. +const MIGRATION_V15: &str = " +DROP TABLE IF EXISTS nodes_fts; +CREATE VIRTUAL TABLE nodes_fts USING fts5( + name, tokenize='trigram' +); +INSERT INTO nodes_fts (rowid, name) + SELECT rowid, name FROM nodes WHERE trashed = 0; +"; diff --git a/crates/pdfs-core/src/db/nodes.rs b/crates/pdfs-core/src/db/nodes.rs index 119a3bd..d44d2e2 100644 --- a/crates/pdfs-core/src/db/nodes.rs +++ b/crates/pdfs-core/src/db/nodes.rs @@ -70,11 +70,22 @@ impl Db { // FTS5 has no UPSERT, so refresh the row by delete-then-insert. // Trashed nodes are kept out of the index entirely so they never // surface in search results. - tx.execute("DELETE FROM nodes_fts WHERE uid = ?1", params![uid])?; + // + // Keyed by the node's rowid, never by its uid: the uid is not an + // indexed FTS column, so deleting by it scans the entire index once + // per node written (B12). The rowid is stable across the upsert + // above — `ON CONFLICT DO UPDATE` keeps the existing row — so it is + // the same key before and after. + let rowid: i64 = tx.query_row( + "SELECT rowid FROM nodes WHERE uid = ?1", + params![uid], + |row| row.get(0), + )?; + tx.execute("DELETE FROM nodes_fts WHERE rowid = ?1", params![rowid])?; if !node.trashed { tx.execute( - "INSERT INTO nodes_fts (uid, name) VALUES (?1, ?2)", - params![uid, node.name], + "INSERT INTO nodes_fts (rowid, name) VALUES (?1, ?2)", + params![rowid, node.name], )?; } } @@ -88,8 +99,21 @@ impl Db { let uid = uid.to_string(); let mut conn = self.conn.lock(); let tx = conn.transaction()?; + // Read the rowid before dropping the node: it is the search index's key + // (B12), and it is gone once the row is. A node with no row leaves + // nothing to unindex — the index is keyed off `nodes`, so it cannot + // hold an entry the table never had. + let rowid: Option = tx + .query_row( + "SELECT rowid FROM nodes WHERE uid = ?1", + params![uid], + |row| row.get(0), + ) + .optional()?; tx.execute("DELETE FROM nodes WHERE uid = ?1", params![uid])?; - tx.execute("DELETE FROM nodes_fts WHERE uid = ?1", params![uid])?; + if let Some(rowid) = rowid { + tx.execute("DELETE FROM nodes_fts WHERE rowid = ?1", params![rowid])?; + } tx.commit()?; Ok(()) } @@ -123,7 +147,7 @@ impl Db { .join(" AND "); let mut stmt = conn.prepare( "SELECT n.node_json, n.uid - FROM nodes_fts f JOIN nodes n ON n.uid = f.uid + FROM nodes_fts f JOIN nodes n ON n.rowid = f.rowid WHERE f.name MATCH ?1 AND n.trashed = 0 AND n.node_json IS NOT NULL ORDER BY f.rank LIMIT ?2", )?; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 49f7f52..ae3ef3e 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -564,6 +564,78 @@ fn search_drops_fts_row_on_delete_and_trash() { assert_eq!(db.search("unique", 10).unwrap().len(), 0); } +/// Renaming re-indexes the node, which is the case delete-by-rowid has to get +/// right: the *old* name must leave the index. A stale row here would be +/// invisible in every other test — the new name is findable either way. +#[test] +fn renaming_a_node_replaces_the_name_in_the_search_index() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + db.upsert_node(&file("f1", "root", "beforename.txt", 1)) + .unwrap(); + assert_eq!(db.search("beforename", 10).unwrap().len(), 1); + + db.upsert_node(&file("f1", "root", "aftername.txt", 1)) + .unwrap(); + + assert_eq!( + db.search("aftername", 10).unwrap().len(), + 1, + "the new name must be searchable" + ); + assert_eq!( + db.search("beforename", 10).unwrap().len(), + 0, + "the old name must have left the index, not merely been added alongside" + ); +} + +/// B12: the cost of writing a listing must not scale with how much is already +/// indexed. The old `DELETE FROM nodes_fts WHERE uid = ?` scanned the whole +/// index once per node written, so a large account paid more to write the *same* +/// folder — measured at 6.5 ms per node against a 17k-node index on the live DB. +/// +/// Written as a ratio between two index sizes rather than an absolute time, so +/// it fails on the scaling regression and not on a slow machine. +#[test] +fn writing_nodes_does_not_scale_with_the_size_of_the_index() { + use std::time::Instant; + + /// Fill `db` with `n` indexed nodes, then time re-writing a fixed batch. + fn cost_against_index_of(n: usize) -> std::time::Duration { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + let filler: Vec<_> = (0..n) + .map(|i| file(&format!("bg{i}"), "root", &format!("background{i}.txt"), 1)) + .collect(); + db.upsert_nodes(&filler).unwrap(); + + // The batch under test is identical in both runs; only the amount of + // already-indexed data around it differs. + let batch: Vec<_> = (0..200) + .map(|i| file(&format!("m{i}"), "root", &format!("measured{i}.txt"), 1)) + .collect(); + db.upsert_nodes(&batch).unwrap(); + + let t = Instant::now(); + db.upsert_nodes(&batch).unwrap(); + t.elapsed() + } + + let small = cost_against_index_of(200); + let large = cost_against_index_of(5000); + + // Calibration, measured while writing this: over the same 25× growth in + // index size, the old full-scan path cost 6.4× more (46 ms → 294 ms) while + // the rowid path costs 1.04× (39 ms → 41 ms). 3× sits well clear of both. + println!("B12: 200-node write — against 200 indexed {small:?}, against 5000 {large:?}"); + assert!( + large < small * 3, + "writing a listing got dramatically more expensive as the index grew, \ + which is the B12 full-scan regression; small={small:?} large={large:?}" + ); +} + fn activity(target: &str, kind: ActivityKind, ok: bool) -> ActivityEntry { ActivityEntry { time: 1700, diff --git a/crates/pdfs-fuse/src/devices.rs b/crates/pdfs-fuse/src/devices.rs index 33f42dc..7f02c73 100644 --- a/crates/pdfs-fuse/src/devices.rs +++ b/crates/pdfs-fuse/src/devices.rs @@ -20,6 +20,8 @@ use pdfs_core::db::{StoredDevice, StoredSyncFolder}; use pdfs_core::{CoreError, CoreResult}; use proton_drive_rs::proton_sdk::ids::{DeviceUid, NodeUid}; use proton_drive_rs::{DeviceType, Node}; +use std::collections::HashSet; +use std::sync::OnceLock; use tracing::{info, warn}; use super::sync::{self, base_name}; @@ -394,6 +396,12 @@ impl Core { next_fh: 1, db: self.db.clone(), })); + // A fresh inode space needs a fresh notification channel: this fork's + // session is the only one that knows these inodes, so it must be the one + // notified about them. Filled in by `spawn_ondemand_mount`. + fork.notifier = Arc::new(OnceLock::new()); + // Size upgrades are keyed by inode, which is per-fork too. + fork.size_upgrades = Arc::new(Mutex::new(HashSet::new())); fork } @@ -639,10 +647,15 @@ impl Core { MountOption::Subtype("protondrive".to_string()), MountOption::DefaultPermissions, ]; - let fs = ProtonFs::new(self.fork_state(), root); - Session::new(fs, local, &config) + let core = self.fork_state(); + let fs = ProtonFs::new(core.clone(), root); + let session = Session::new(fs, local, &config) .and_then(|s| s.spawn()) - .map_err(|e| CoreError::internal(format!("mount {}: {e}", local.display()))) + .map_err(|e| CoreError::internal(format!("mount {}: {e}", local.display())))?; + // Hand this fork its own session's notification channel, so a background + // size upgrade can drop the kernel's cached attrs for these inodes. + let _ = core.notifier.set(session.notifier()); + Ok(session) } /// Re-establish FUSE mounts for folders left in `ondemand` mode across a daemon diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index b5a7b36..f3f829e 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -39,6 +39,7 @@ use std::os::unix::fs::FileExt; use std::os::unix::net::UnixListener; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; +use std::sync::OnceLock; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -464,6 +465,19 @@ struct Core { /// is the validity tag — a new revision may well have a thumbnail — matching /// how [`ContentCache::read_thumbnail`] validates the positive side. no_thumbnail: Arc>>, + /// Folders whose listing was enumerated cheaply and is having its file sizes + /// filled in right now, so a burst of `stat`s over a fresh listing starts one + /// upgrade rather than one per entry. See [`Core::spawn_size_upgrade`]. + size_upgrades: Arc>>, + /// This mount's kernel notification channel, for telling the kernel to drop + /// metadata it has cached. Set once the session exists — which is *after* + /// the `Core` it is built from, hence the cell. + /// + /// Per mount, not per daemon: each on-demand fork runs its own session over + /// its own inode space, so notifying through the primary mount's channel + /// would name inodes that session has never heard of. [`Core::fork_state`] + /// gives each fork an empty cell of its own. + notifier: Arc>, /// In-flight upload/download progress, served to `GetQueueStatus`. Shared /// across the FUSE session and the control-socket task. transfers: Arc, @@ -777,6 +791,38 @@ impl Core { self.state.lock().children.contains_key(&ino) } + /// Re-apply the optimistic size of any queued write to `nodes`. + /// + /// A node that arrives from the remote (or from its DB row) carries the size + /// of the revision the *server* holds, which for a file with a write still + /// queued is the pre-write size — often 0 for a file created moments ago. + /// Interning it as-is silently reverts the optimistic size that + /// `record_pending_write` stamped, and a file that stats as 0 bytes is a file + /// the kernel will not issue a single `read` for: `cat` prints nothing and + /// the staged blob that `read_range` would have served is never asked for. + /// That reads as data loss even though nothing is lost (B11). + /// + /// [`Core::hydrate`] does the same thing for the restart case. This covers + /// every *live* re-enumeration — which is what a rename or move triggers, + /// since both invalidate the listings they touch. + /// + /// Snapshots the pending map and returns before any caller takes the state + /// lock: no site in the daemon holds `pending` and `state` at once, and this + /// is not the place to become the first. + fn stamp_pending_sizes(&self, nodes: &mut [Node]) { + let sizes: HashMap = { + let pending = self.pending.lock(); + if pending.is_empty() { + return; + } + pending + .iter() + .map(|(uid, pr)| (uid.clone(), pr.meta.len)) + .collect() + }; + apply_pending_sizes(nodes, &sizes); + } + /// Enumerate `ino`'s children from the remote and cache them. No-op if the /// directory has already been listed. Network I/O happens without the lock /// held so concurrent metadata reads aren't blocked behind a fetch. @@ -796,19 +842,36 @@ impl Core { // can be rebuilt from disk without hitting the API, even if its listing // was trimmed from the hot cache mid-run. match self.db.children_if_listed(&folder_uid) { - Ok(Some(nodes)) => { + Ok(Some(mut nodes)) => { + // Before the lock: a DB row carries the size the server last + // sealed, which a queued write is ahead of (B11). + self.stamp_pending_sizes(&mut nodes); let mut st = self.state.lock(); if st.children.contains_key(&ino) { return Ok(()); } let mut child_inos = Vec::with_capacity(nodes.len()); + let mut needs_size = Vec::new(); for node in nodes { if node.trashed || node.uid == folder_uid { continue; } + if matches!( + &node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) { + needs_size.push(node.uid.clone()); + } child_inos.push(st.intern_from_db(ino, node)); } st.children.insert(ino, child_inos); + drop(st); + // Rows persisted from a cheap enumeration whose upgrade never + // ran (a restart in between, say) still owe their real sizes. + self.spawn_size_upgrade(ino, needs_size); return Ok(()); } Ok(None) => {} @@ -822,13 +885,25 @@ impl Core { error!(%folder_uid, error = %e, "enumerate folder children failed"); Errno::EIO })?; - let nodes = self + // Cheap enumeration: `Light` skips unlocking each *file's* node key, + // which is an S2K derivation per file and was ~74% of the cost of a cold + // listing (B12 — measured with `perf`, 64% of cycles in SHA-256 alone). + // Folders are unlocked either way; their keys are what the children are + // decrypted with, so the walk cannot proceed without them. + // + // The price is that files come back without a `claimed_size`, so + // `node_size` falls back to the *ciphertext* size until + // `spawn_size_upgrade` below fills the real one in. + let mut nodes = self .rt - .block_on(self.client.enumerate_nodes(&uids)) + .block_on(self.client.enumerate_nodes_light(&uids)) .map_err(|e| { error!(%folder_uid, error = %e, "enumerate nodes failed"); Errno::EIO })?; + // Same as the DB path above: the remote's size for a file with a write + // still queued is the pre-write one (B11). + self.stamp_pending_sizes(&mut nodes); let mut st = self.state.lock(); // Lost the race? Another thread already populated it. @@ -840,6 +915,21 @@ impl Core { .into_iter() .filter(|node| !node.trashed && node.uid != folder_uid) .collect(); + // Files whose real size the cheap enumeration could not read. Collected + // before interning so the upgrade below has the uids without re-walking. + let needs_size: Vec = filtered_nodes + .iter() + .filter(|n| { + matches!( + &n.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + }) + .map(|n| n.uid.clone()) + .collect(); let inos = st.intern_batch(ino, filtered_nodes); child_inos.extend(inos); st.children.insert(ino, child_inos); @@ -848,9 +938,132 @@ impl Core { if let Err(e) = self.db.set_listed(&folder_uid, true) { warn!(%folder_uid, error = %e, "db set_listed(true) failed"); } + drop(st); + self.spawn_size_upgrade(ino, needs_size); Ok(()) } + /// Kick a size upgrade for every file in `parent`'s listing still carrying a + /// provisional size. + /// + /// Called when a `stat` lands on such a file. That covers the paths + /// [`Core::ensure_children`] cannot: a listing rebuilt from the DB, or one + /// restored by [`Core::hydrate`] on mount, whose rows were persisted before + /// an earlier upgrade had a chance to run. Gathers the whole folder rather + /// than the one file asked for, because a `stat` of one entry in a listing + /// almost always means a `stat` of all of them. + fn upgrade_sizes_for_parent(&self, parent: u64) { + let missing: Vec = { + let st = self.state.lock(); + let Some(kids) = st.children.get(&parent) else { + return; + }; + kids.iter() + .filter_map(|k| st.entries.get(k)) + .filter(|e| { + matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + }) + .map(|e| e.uid.clone()) + .collect() + }; + self.spawn_size_upgrade(parent, missing); + } + + /// Fill in the true sizes of files a `Light` enumeration returned without + /// one, on a worker, after the listing has already been served. + /// + /// This is the other half of the split in [`Core::ensure_children`]. The + /// listing itself needs only names and parentage, so it is served from the + /// cheap enumeration immediately; the S2K-per-file work that produces + /// `claimed_size` happens here, off the path the user is waiting on. + /// + /// **Sizes are provisional until this lands.** `node_size` falls back to + /// `total_size_on_storage`, the *ciphertext* size, which is slightly larger + /// than the real one. Reads are unaffected — the revision reader carries its + /// own authoritative size — so this is a cosmetic discrepancy in `stat` that + /// closes within a round trip, not a repeat of B11 (which reported **0** and + /// so suppressed reads entirely). + /// + /// Single-flight per folder: a `stat` of every entry in a fresh listing is + /// the normal case, and each one must not start its own upgrade. + fn spawn_size_upgrade(&self, folder_ino: u64, uids: Vec) { + if uids.is_empty() { + return; + } + if !self.size_upgrades.lock().insert(folder_ino) { + return; + } + let core = self.clone(); + self.workers.run(Lane::Meta, move || { + let result = core.rt.block_on(core.client.enumerate_nodes(&uids)); + core.size_upgrades.lock().remove(&folder_ino); + let mut nodes = match result { + Ok(nodes) => nodes, + Err(e) => { + // Not fatal: the listing stands, sizes just stay provisional + // until something invalidates and re-enumerates it. + warn!(folder_ino, error = %e, "size upgrade failed; sizes stay provisional"); + return; + } + }; + // A queued write is ahead of anything the server can report, so its + // optimistic size must survive this just as it survives a re-listing + // (B11). + core.stamp_pending_sizes(&mut nodes); + let mut changed: Vec = Vec::new(); + let mut st = core.state.lock(); + for node in nodes { + // Only adopt the size. Re-interning wholesale would also adopt a + // name or parent that a rename/move may have changed locally + // while this was in flight, undoing it. + let Some(&ino) = st.by_uid.get(&node.uid) else { + continue; + }; + changed.push(ino); + let NodeKind::File { claimed_size, .. } = &node.kind else { + continue; + }; + let (Some(size), Some(entry)) = (*claimed_size, st.entries.get_mut(&ino)) else { + continue; + }; + if let NodeKind::File { claimed_size, .. } = &mut entry.node.kind { + *claimed_size = Some(size); + } + } + let updated: Vec = st + .children + .get(&folder_ino) + .map(|kids| { + kids.iter() + .filter_map(|k| st.entries.get(k).map(|e| e.node.clone())) + .collect() + }) + .unwrap_or_default(); + drop(st); + if let Err(e) = core.db.upsert_nodes(&updated) { + warn!(folder_ino, error = %e, "persisting upgraded sizes failed"); + } + // Without this the corrected size is invisible for the length of the + // attr TTL: the kernel answers `stat` from the provisional attrs it + // cached while the listing was being served, so `ls -l` reports the + // ciphertext size for up to 30 s even though the daemon has had the + // real one all along. Notify *after* the DB write, so a re-`getattr` + // provoked by the invalidation cannot race the persistence. + if let Some(notifier) = core.notifier.get() { + for ino in changed { + let _ = notifier.inval_inode(INodeNo(ino), 0, 0); + } + } + debug!(folder_ino, files = updated.len(), "filled in listing sizes"); + }); + } + /// Resolve a child `name` within `parent` to its `(inode, uid)`, ensuring /// the parent's listing is cached first. fn lookup_child(&self, parent: u64, name: &str) -> Result<(u64, NodeUid), Errno> { @@ -3251,6 +3464,19 @@ fn media_type_for(name: &str) -> &'static str { } /// The plaintext size, in bytes, that a node reports. +/// Overwrite each file node's `claimed_size` with the optimistic size of its +/// queued write, where it has one. Folders and nodes with nothing queued are +/// left alone. See [`Core::stamp_pending_sizes`] for why this exists. +fn apply_pending_sizes(nodes: &mut [Node], sizes: &HashMap) { + for node in nodes { + if let Some(&len) = sizes.get(&node.uid) + && let NodeKind::File { claimed_size, .. } = &mut node.kind + { + *claimed_size = Some(len as i64); + } + } +} + fn node_size(node: &Node) -> u64 { match &node.kind { NodeKind::Folder => 0, @@ -3392,13 +3618,32 @@ impl Filesystem for ProtonFs { } fn getattr(&self, _req: &Request, ino: INodeNo, _fh: Option, reply: ReplyAttr) { - let st = self.core.state.lock(); - match st.entries.get(&ino.0) { - Some(e) => { - let attr = self.attr(ino.0, &e.node); - reply.attr(&TTL, &attr); + let (attr, provisional_in) = { + let st = self.core.state.lock(); + match st.entries.get(&ino.0) { + Some(e) => { + // A file still carrying the cheap enumeration's placeholder + // size: answer now with what we have and have the real size + // fetched in the background (B12). + let provisional = matches!( + &e.node.kind, + NodeKind::File { + claimed_size: None, + .. + } + ) + .then_some(e.parent); + (self.attr(ino.0, &e.node), provisional) + } + None => { + reply.error(Errno::ENOENT); + return; + } } - None => reply.error(Errno::ENOENT), + }; + reply.attr(&TTL, &attr); + if let Some(parent) = provisional_in { + self.core.upgrade_sizes_for_parent(parent); } } @@ -4643,6 +4888,8 @@ pub fn mount( trash_refreshing: Arc::new(AtomicBool::new(false)), thumb_gen: Arc::new(Mutex::new(HashSet::new())), no_thumbnail: Arc::new(Mutex::new(HashMap::new())), + size_upgrades: Arc::new(Mutex::new(HashSet::new())), + notifier: Arc::new(OnceLock::new()), transfers: TransferRegistry::new(), indexing: Arc::new(AtomicBool::new(false)), sync_progress: Arc::new(Mutex::new(HashMap::new())), @@ -4744,6 +4991,9 @@ pub fn mount( // thread; we then wait here for either a stop signal or the mount ending. let bg = Session::new(fs, mountpoint, &config)?.spawn()?; let notifier = bg.notifier(); + // Same channel, kept on the `Core` so background work (a size upgrade, say) + // can invalidate kernel-cached metadata without threading a handle through. + let _ = core.notifier.set(notifier.clone()); rt.spawn(run_event_sync( client, scope, @@ -4951,6 +5201,87 @@ mod local_uid_tests { } } +#[cfg(test)] +mod pending_size_tests { + use super::{Node, NodeKind, NodeUid, apply_pending_sizes, node_size}; + use proton_drive_rs::proton_sdk::ids::{LinkId, VolumeId}; + use std::collections::HashMap; + + fn uid(link: &str) -> NodeUid { + NodeUid::new(VolumeId::from("vol"), LinkId::from(link)) + } + + fn file(link: &str, claimed: i64) -> Node { + Node { + uid: uid(link), + parent_uid: Some(uid("parent")), + kind: NodeKind::File { + media_type: "text/plain".into(), + total_size_on_storage: 0, + active_revision_state: None, + claimed_size: Some(claimed), + claimed_modification_time: None, + }, + name: link.into(), + creation_time: 100, + modification_time: 100, + trashed: false, + is_shared: false, + is_shared_publicly: false, + signature_email: None, + verification: Default::default(), + } + } + + fn folder(link: &str) -> Node { + Node { + kind: NodeKind::Folder, + ..file(link, 0) + } + } + + /// B11: a re-enumeration mid-write must not revert the size to the + /// server's. A file that stats as 0 gets no `read` from the kernel at all, + /// so the staged blob is never served and the file reads as empty. + #[test] + fn a_queued_write_keeps_its_optimistic_size_through_a_re_enumeration() { + let mut nodes = vec![file("queued", 0), file("settled", 4096)]; + let sizes = HashMap::from([(uid("queued"), 3)]); + + apply_pending_sizes(&mut nodes, &sizes); + + assert_eq!( + node_size(&nodes[0]), + 3, + "the remote's pre-write size must not win over the queued write's" + ); + assert_eq!( + node_size(&nodes[1]), + 4096, + "a file with nothing queued keeps the size the server reported" + ); + } + + /// The map is keyed by uid and says nothing about kind; a folder that + /// somehow collides must not grow a `claimed_size`. + #[test] + fn folders_are_left_alone() { + let mut nodes = vec![folder("dir")]; + apply_pending_sizes(&mut nodes, &HashMap::from([(uid("dir"), 999)])); + assert!(matches!(nodes[0].kind, NodeKind::Folder)); + assert_eq!(node_size(&nodes[0]), 0); + } + + /// The common case: nothing queued, nothing touched. + #[test] + fn an_empty_pending_map_changes_nothing() { + let mut nodes = vec![file("a", 10), file("b", 20)]; + apply_pending_sizes(&mut nodes, &HashMap::new()); + assert_eq!(node_size(&nodes[0]), 10); + assert_eq!(node_size(&nodes[1]), 20); + } +} + #[cfg(test)] mod tests { use super::{Intervals, conflict_name}; diff --git a/crates/pdfs-gui/src/app/pages/verify.rs b/crates/pdfs-gui/src/app/pages/verify.rs index 9ed6469..8b9aa18 100644 --- a/crates/pdfs-gui/src/app/pages/verify.rs +++ b/crates/pdfs-gui/src/app/pages/verify.rs @@ -70,7 +70,7 @@ pub(crate) fn prompt_human_verification( let token_tx = Rc::new(RefCell::new(Some(token_tx))); let tx = token_tx.clone(); - let dlg = dialog.clone(); + let dlg_weak = dialog.downgrade(); content.connect_script_message_received(Some("hv"), move |_, value| { let Some(token) = extract_token(&value.to_str()) else { // Every `postMessage` on the page reaches here, most of them the @@ -81,7 +81,9 @@ pub(crate) fn prompt_human_verification( if let Some(tx) = tx.borrow_mut().take() { let _ = tx.send(token); } - dlg.close(); + if let Some(dlg) = dlg_weak.upgrade() { + dlg.close(); + } }); // Covers the close button, Escape, and the user giving up. @@ -104,7 +106,14 @@ pub(crate) fn prompt_human_verification( /// unrecognised shape as success would hand the API an empty token and fail the /// login with a confusing error. fn extract_token(raw: &str) -> Option { - let value: serde_json::Value = serde_json::from_str(raw).ok()?; + let mut value: serde_json::Value = serde_json::from_str(raw).ok()?; + // If the message was stringified twice (e.g. raw is a double-quoted JSON string), + // parse the inner string. + if let Some(inner) = value.as_str() { + if let Ok(parsed) = serde_json::from_str(inner) { + value = parsed; + } + } // The payload is nested under `payload` and the message names itself in // `type`; both spellings of the success type have shipped. let kind = value.get("type")?.as_str()?; @@ -133,6 +142,12 @@ mod tests { assert_eq!(extract_token(raw).as_deref(), Some("tok-1")); } + #[test] + fn a_double_serialized_completion_message_yields_its_token() { + let raw = r#""{\"type\":\"HUMAN_VERIFICATION_SUCCESS\",\"payload\":{\"token\":\"tok-1\",\"type\":\"captcha\"}}""#; + assert_eq!(extract_token(raw).as_deref(), Some("tok-1")); + } + /// The page posts plenty that is not a completion; none of it may be /// mistaken for one, or the login retries with a garbage token. #[test] diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a15c848..a183680 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -284,3 +284,63 @@ See [RECOVERY.md](RECOVERY.md) for what a lost machine means for the plaintext d - Treat the cache and state directories as being as sensitive as the Drive contents themselves. - Multi-user machines warrant checking `~/.cache` and `~/.local/state` are `0700` until §8.5 is addressed. - Purging the cache (`pdfs` settings, or `PurgeCache` over IPC) removes content but **not** `cache.db`, and deliberately never removes undrained `staging/` or `recovery/` blobs. + +--- + +## 9. Human Verification (CAPTCHA) Flow + +When logging in from an unfamiliar IP address or VPN, the Proton API may gate the sign-in with a human verification challenge (CAPTCHA). This client handles this asynchronously and interactively. + +### 9.1 Sequence of Verification and Re-Authentication + +```mermaid +sequenceDiagram + autonumber + actor User as User (GUI) + participant Core as pdfs-core (Auth) + participant UI as pdfs-gui (Login Page) + participant Web as WebKitWebView Dialog + participant API as Proton API Server + + User->>UI: Enter credentials & click Sign In + UI->>Core: login_interactive() + Core->>API: auth/v4 (Initial SRP Handshake) + API-->>Core: HTTP 422 (Error 9001: CAPTCHA Challenge URL) + Core->>UI: Error::HumanVerificationRequired(hv) + UI->>UI: Block login thread, dispatch to GTK main loop + UI->>Web: Create dialog & load verification URL + Note over User, Web: User completes CAPTCHA in embedded WebView + Web->>UI: window.postMessage(HUMAN_VERIFICATION_SUCCESS) + UI->>UI: Extract token (with double-serialization safety) + UI->>UI: Close dialog & send token to blocked login thread + UI->>Core: login_verified(with verification token) + Core->>API: auth/v4 (SRP retry with x-pm-human-verification-token) + API-->>Core: Returns session tokens + Core-->>UI: Login Successful +``` + +### 9.2 Key Technical Design Decisions + +1. **Weak Reference UI Binding:** To prevent memory leaks and strong reference cycles between the parent dialog, the child `WebKitWebView`, the script message manager, and the connection callback, the dialog is downgraded to a `WeakRef` inside the callback: + ```rust + let dlg_weak = dialog.downgrade(); + content.connect_script_message_received(Some("hv"), move |_, value| { + // ... + if let Some(dlg) = dlg_weak.upgrade() { + dlg.close(); + } + }); + ``` +2. **Double-Serialization Tolerance:** The JavaScript message listener forwards event data as a JSON string to the native handler. Since the underlying page may post either JS objects or pre-serialized JSON strings, the Rust side performs dual-phase parsing: + ```rust + let mut value = serde_json::from_str(raw).ok()?; + if let Some(inner) = value.as_str() { + if let Ok(parsed) = serde_json::from_str(inner) { + value = parsed; + } + } + ``` + This ensures compatibility with all versions of Proton's client verification scripts. +3. **SRP Handshake Reset:** Because a gated login burns the SRP handshake on the API side, the client cannot simply resume the previous request. Instead, `auth::login_interactive` restarts the SRP process from scratch with the verification credentials attached, keeping the complex handshake details isolated from the front-end. +4. **CLI Fallback:** Since the CLI has no native web browser engine, hitting the CAPTCHA gate fails immediately with a user-friendly message directing the user to sign in once via the GUI (`pdfs-app`) to persist the authenticated session keys to the system keyring. +