From 607f79e5e078f533c814871072179ada0fb5dccb Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 9 Nov 2026 08:39:01 +0800 Subject: [PATCH] M3: a protected file named before others at launch keeps its password prompt (#5756) (#5762) --- crates/ui-egui/src/app.rs | 6 +- crates/ui-egui/src/app/files.rs | 107 +++++++++++-- crates/ui-egui/src/control/host.rs | 11 +- crates/ui-egui/src/lib.rs | 18 +++ crates/ui-egui/src/platform/open_worker.rs | 2 +- .../ui-egui/src/platform/open_worker/tests.rs | 17 ++ crates/ui-egui/src/platform/pickers.rs | 1 + crates/ui-egui/tests/it/editing.rs | 147 ++++++++++++++++++ 8 files changed, 291 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 91bad575c4..e37b199df8 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -144,6 +144,10 @@ impl PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] pending_opens: Vec::new(), #[cfg(not(target_arch = "wasm32"))] + deferred_opens: Default::default(), + #[cfg(not(target_arch = "wasm32"))] + password_retry: None, + #[cfg(not(target_arch = "wasm32"))] next_open_token: 0, #[cfg(not(target_arch = "wasm32"))] launch_layout: None, @@ -537,7 +541,7 @@ impl eframe::App for PdfCraftApp { // Don't replace a pending prompt or import. A startup file may need its own password // prompt, so its bytes stay queued until the current interaction finishes. #[cfg(not(target_arch = "wasm32"))] - let opening = !self.pending_opens.is_empty(); + let opening = !self.pending_opens.is_empty() || !self.deferred_opens.is_empty(); #[cfg(target_arch = "wasm32")] let opening = false; if !opening diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 76dbfc3921..c8ac133a00 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -1,6 +1,17 @@ use super::*; use crate::file_filter::AddFilterAnyCase as _; +/// Whether an open's reply asks for a password: the file is encrypted and none, or a wrong one, +/// was given. +#[cfg(not(target_arch = "wasm32"))] +fn asks_for_password(kind: &open_worker::ResultKind) -> bool { + matches!( + kind, + open_worker::ResultKind::Prepared { result, .. } + if matches!(**result, Err(pdfcraft_render::OpenError::NeedsPassword | pdfcraft_render::OpenError::WrongPassword)) + ) +} + /// Whether `a` and `b` name the same file: equal, or equal once made absolute against the /// current directory (links are not resolved, as in `last_session`). pub(crate) fn same_file(a: &str, b: &str) -> bool { @@ -122,16 +133,36 @@ impl PdfCraftApp { recovered: request.recovered, then: request.then, display_override: None, + ready: None, }); Ok(token) } + /// Hand waiting files to the worker, oldest first, while its queue has room. Every queued + /// job has a `pending_opens` entry, so fewer entries than the bound means room. + #[cfg(not(target_arch = "wasm32"))] + fn submit_deferred_opens(&mut self) { + let mut queue = std::mem::take(&mut self.deferred_opens); + while self.deferred_opens.is_empty() && self.pending_opens.len() < open_worker::MAX_QUEUED_OPENS { + let Some(next) = queue.pop_front() else { break }; + self.open_path_request(&next.path, next.then, next.display_override); + } + // A file put back (the queue filled again) stays ahead of the ones behind it. + self.deferred_opens.append(&mut queue); + } + #[cfg(not(target_arch = "wasm32"))] pub(crate) fn poll_pending_opens(&mut self) { let pending_opens = std::mem::take(&mut self.pending_opens); + // A retry that is no longer waiting holds nothing back. + if let Some(token) = self.password_retry + && !pending_opens.iter().any(|pending| pending.token == token) + { + self.password_retry = None; + } let mut remaining = pending_opens.into_iter(); let mut waiting = Vec::new(); - while let Some(pending) = remaining.next() { + while let Some(mut pending) = remaining.next() { // A tool, password prompt or file picker belongs to the document just admitted. // Keep later replies queued so another open cannot change its recipient. if self.modal_open() || self.pickers.pending() { @@ -139,19 +170,41 @@ impl PdfCraftApp { waiting.extend(remaining); break; } - match pending.receiver.try_recv() { - Ok(result) if result.token == pending.token => self.finish_pending_open(pending, result.kind), - Ok(_) => waiting.push(pending), - Err(std::sync::mpsc::TryRecvError::Empty) => waiting.push(pending), - Err(std::sync::mpsc::TryRecvError::Disconnected) => { - self.notify_fmt("Couldn't open {name}: the background worker stopped", &[("name", &pending.name)]); - } + let retry = self.password_retry == Some(pending.token); + let kind = match pending.ready.take() { + Some(kind) => kind, + None => match pending.receiver.try_recv() { + Ok(result) if result.token == pending.token => result.kind, + Ok(_) | Err(std::sync::mpsc::TryRecvError::Empty) => { + waiting.push(pending); + continue; + } + Err(std::sync::mpsc::TryRecvError::Disconnected) => { + if retry { + self.password_retry = None; + } + self.notify_fmt("Couldn't open {name}: the background worker stopped", &[("name", &pending.name)]); + continue; + } + }, + }; + // While a closed prompt's password is being tried, a later file that asks for its own + // password keeps its reply: its prompt would hide the retry's outcome. Others admit. + if !retry && self.password_retry.is_some() && asks_for_password(&kind) { + pending.ready = Some(kind); + waiting.push(pending); + continue; } + if retry { + self.password_retry = None; + } + self.finish_pending_open(pending, kind); } // A completion may schedule another open; preserve those requests as well as the // earlier waiting replies, without consuming a blocked receiver's result. waiting.append(&mut self.pending_opens); self.pending_opens = waiting; + self.submit_deferred_opens(); self.settle_launch_layout(); } @@ -452,6 +505,9 @@ impl PdfCraftApp { && let Some(pending) = self.pending_opens.iter_mut().find(|pending| pending.token == *token) { pending.display_override = p.display_override; + // Replies behind it that ask for a password wait for its outcome + // (`poll_pending_opens`). + self.password_retry = Some(*token); } if let Err(error) = result { let mut prompt = p; @@ -594,10 +650,24 @@ impl PdfCraftApp { } return OpenOutcome::Pending; } - if display_override.is_some() - && let Some(prompt) = self.password_prompt.as_mut().filter(|p| p.path.as_deref().is_some_and(|p| same_file(p, path))) - { - prompt.display_override = display_override; + // The same file named again while its password is being asked for joins that prompt + // instead of queuing a second one; a display override or a tool it brings is kept. + if let Some(prompt) = self.password_prompt.as_mut().filter(|p| p.path.as_deref().is_some_and(|p| same_file(p, path))) { + if display_override.is_some() { + prompt.display_override = display_override; + } + if then.is_some() { + prompt.then = then; + } + return OpenOutcome::Pending; + } + if let Some(deferred) = self.deferred_opens.iter_mut().find(|d| same_file(&d.path, path)) { + if display_override.is_some() { + deferred.display_override = display_override; + } + if then.is_some() { + deferred.then = then; + } return OpenOutcome::Pending; } let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string()); @@ -606,8 +676,10 @@ impl PdfCraftApp { // Before the first frame there is no interaction to keep responsive; this also preserves // deterministic inline startup and test flows. Once live, reading and preparation use // the bounded worker and commit on the UI thread. - // Keep flagged launch files behind an existing password prompt or picker. - if (self.ctx.is_none() || self.run_inline) && !(display_override.is_some() && (self.modal_open() || self.pickers.pending())) { + // Keep later files behind an open password prompt: it belongs to an earlier file, and an + // inline open would clear it. Keep flagged launch files behind any dialog or picker too. + let held = self.password_prompt.is_some() || (display_override.is_some() && (self.modal_open() || self.pickers.pending())); + if (self.ctx.is_none() || self.run_inline) && !held { match std::fs::read(path) { Ok(bytes) => { let before = self.views.len(); @@ -649,6 +721,12 @@ impl PdfCraftApp { } } else { self.startup_superseded = true; + // The worker queue is bounded. A long file list (a launch behind a password prompt, + // or many files at once) waits here, in order, and is submitted as opens finish. + if self.pending_opens.len() >= open_worker::MAX_QUEUED_OPENS || !self.deferred_opens.is_empty() { + self.deferred_opens.push_back(DeferredOpen { path: path.to_string(), then, display_override }); + return OpenOutcome::Pending; + } match self.schedule_open(OpenRequest { name: name.clone(), path: Some(path.to_string()), @@ -686,6 +764,7 @@ impl PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] fn settle_launch_layout(&mut self) { if self.pending_opens.iter().any(|open| open.display_override.is_some()) + || self.deferred_opens.iter().any(|open| open.display_override.is_some()) || self.password_prompt.as_ref().is_some_and(|prompt| prompt.display_override.is_some()) { return; diff --git a/crates/ui-egui/src/control/host.rs b/crates/ui-egui/src/control/host.rs index c2e82f98e5..12c98007cc 100644 --- a/crates/ui-egui/src/control/host.rs +++ b/crates/ui-egui/src/control/host.rs @@ -67,8 +67,15 @@ impl Host for crate::PdfCraftApp { }) .collect(); #[cfg(not(target_arch = "wasm32"))] - let pending_opens: Vec = - self.pending_opens.iter().map(|pending| json!({ "token": pending.token, "name": pending.name, "path": pending.path })).collect(); + let pending_opens: Vec = self + .pending_opens + .iter() + .map(|pending| json!({ "token": pending.token, "name": pending.name, "path": pending.path })) + .chain(self.deferred_opens.iter().map(|deferred| { + let name = std::path::Path::new(&deferred.path).file_name().map(|n| n.to_string_lossy().into_owned()); + json!({ "token": null, "name": name, "path": deferred.path }) + })) + .collect(); #[cfg(target_arch = "wasm32")] let pending_opens: Vec = Vec::new(); json!({ diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1663c3be73..23625cb97b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -262,6 +262,17 @@ struct PendingOpen { recovered: Option, then: Option<&'static str>, display_override: Option<(canvas::PageLayout, bool)>, + /// A reply taken from `receiver` and kept back for later (a password prompt waiting for an + /// earlier file's password retry). + ready: Option, +} + +/// A file named while the worker's queue was full: submitted once it has room. +#[cfg(not(target_arch = "wasm32"))] +struct DeferredOpen { + path: String, + then: Option<&'static str>, + display_override: Option<(canvas::PageLayout, bool)>, } /// Puts `(html, plain_text)` on a clipboard; `false` when it couldn't, and plain-text Copy is @@ -419,6 +430,13 @@ pub struct PdfCraftApp { open_worker: Option, #[cfg(not(target_arch = "wasm32"))] pending_opens: Vec, + /// Files waiting for room in the worker's bounded queue, in the order they were named. + #[cfg(not(target_arch = "wasm32"))] + deferred_opens: std::collections::VecDeque, + /// The open retrying a closed password prompt's file with its password: replies behind it + /// wait for it. + #[cfg(not(target_arch = "wasm32"))] + password_retry: Option, #[cfg(not(target_arch = "wasm32"))] next_open_token: u64, /// Explicit CLI layout waits for flagged launch opens, never targeting restored tabs. diff --git a/crates/ui-egui/src/platform/open_worker.rs b/crates/ui-egui/src/platform/open_worker.rs index fe564db398..274071ea78 100644 --- a/crates/ui-egui/src/platform/open_worker.rs +++ b/crates/ui-egui/src/platform/open_worker.rs @@ -9,7 +9,7 @@ use crate::RecoveryStore; use pdfcraft_engine::{OpenContext, PreparedDocument, SourceKind}; use pdfcraft_render::OpenError; -const MAX_QUEUED_OPENS: usize = 16; +pub(crate) const MAX_QUEUED_OPENS: usize = 16; pub(crate) enum Input { Path(PathBuf), diff --git a/crates/ui-egui/src/platform/open_worker/tests.rs b/crates/ui-egui/src/platform/open_worker/tests.rs index c0fe2f03a2..efe911bff6 100644 --- a/crates/ui-egui/src/platform/open_worker/tests.rs +++ b/crates/ui-egui/src/platform/open_worker/tests.rs @@ -763,6 +763,7 @@ fn launch_layout_waits_for_cli_opens_and_preserves_restored_tabs() { recovered: None, then: None, display_override: Some((PageLayout::TwoUp, false)), + ready: None, }); replies.push(( sender, @@ -820,6 +821,7 @@ fn launch_layout_failed_or_cancelled_opens_do_not_target_existing_documents() { recovered: None, then: None, display_override: Some((PageLayout::TwoUp, false)), + ready: None, }); app.set_launch_layout("single").unwrap(); assert!(app.set_launch_layout("invalid").is_err()); @@ -904,3 +906,18 @@ fn a_new_file_from_the_os_brings_the_window_forward_before_it_is_prepared() { assert_eq!(doc.info.pages.len(), 2); let _ = std::fs::remove_file(path); } + +/// A file waiting for room in the open queue, named again with a tool (Open and run), keeps +/// that tool; a later repeat without one doesn't drop it. +#[test] +fn a_repeated_waiting_open_keeps_the_tool_it_brings() { + let file = tool_open_file("deferred-tool", "pdf", &fixture(1)); + let path = file.0.to_string_lossy().into_owned(); + let mut app = PdfCraftApp::new(); + app.deferred_opens.push_back(crate::DeferredOpen { path: path.clone(), then: None, display_override: None }); + assert_eq!(app.open_path_then(&path, Some("export.text")), crate::OpenOutcome::Pending); + assert_eq!(app.deferred_opens.len(), 1); + assert_eq!(app.deferred_opens.front().and_then(|d| d.then), Some("export.text")); + assert_eq!(app.open_path(&path), crate::OpenOutcome::Pending); + assert_eq!(app.deferred_opens.front().and_then(|d| d.then), Some("export.text")); +} diff --git a/crates/ui-egui/src/platform/pickers.rs b/crates/ui-egui/src/platform/pickers.rs index 80d44d975b..c680eaeaba 100644 --- a/crates/ui-egui/src/platform/pickers.rs +++ b/crates/ui-egui/src/platform/pickers.rs @@ -377,6 +377,7 @@ mod tests { recovered: None, then: None, display_override: None, + ready: None, }); let got = Arc::new(Mutex::new(Vec::new())); app.pickers.showing.store(true, Ordering::SeqCst); diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 4f3e15bd5d..500d504403 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -2727,6 +2727,153 @@ fn canceling_launch_display_override_does_not_affect_later_documents() { std::fs::remove_file(path).unwrap(); } +/// `pdfcraft locked.pdf other.pdf`: the files open before the first frame, and opening the +/// second one inline used to clear the first one's password prompt, so `locked.pdf` was never +/// opened or asked for. The later file now waits behind the prompt. +#[test] +fn a_startup_file_after_a_protected_one_keeps_its_password_prompt() { + for unlock in [true, false] { + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let locked = temp_path(&format!("startup-locked-{unlock}-{nonce}.pdf")); + let other = temp_path(&format!("startup-other-{unlock}-{nonce}.pdf")); + std::fs::write(&locked, protected("reader", "owner", -4)).unwrap(); + std::fs::write(&other, fixture(2)).unwrap(); + let (locked_path, other_path) = (locked.to_str().unwrap().to_string(), other.to_str().unwrap().to_string()); + let (first, second) = (locked_path.clone(), other_path.clone()); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_path(&first); + app.open_path(&second); + app + }); + h.run_steps(4); + let prompt = h.state().password_prompt.as_ref().map(|p| p.path.clone()); + assert_eq!(prompt, Some(Some(locked_path.clone())), "the protected file is asked for"); + assert!(h.state().views.is_empty(), "the later file waits behind the prompt"); + if unlock { + h.state_mut().submit_password(Some("reader".into())); + support::wait_until(&mut h, "both files", |app| app.views.len() == 2); + // The other file opened while the password was being typed; the unlocked one comes + // after it and is the one in view. + assert_eq!(open_paths(h.state()), vec![other_path, locked_path]); + assert_eq!(h.state().active_ids().map(|(i, _)| i), Some(1)); + } else { + h.state_mut().submit_password(None); + support::wait_until(&mut h, "the other file", |app| app.views.len() == 1); + assert_eq!(open_paths(h.state()), vec![other_path]); + } + std::fs::remove_file(locked).unwrap(); + std::fs::remove_file(other).unwrap(); + } +} + +/// `pdfcraft locked.pdf locked.pdf other.pdf`: the repeated file joins its open password +/// prompt. It used to be queued again behind the prompt, so the unlocked file asked for its +/// password a second time. +#[test] +fn a_protected_file_named_twice_at_startup_asks_for_its_password_once() { + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let locked = temp_path(&format!("startup-twice-locked-{nonce}.pdf")); + let other = temp_path(&format!("startup-twice-other-{nonce}.pdf")); + std::fs::write(&locked, protected("reader", "owner", -4)).unwrap(); + std::fs::write(&other, fixture(2)).unwrap(); + let (locked_path, other_path) = (locked.to_str().unwrap().to_string(), other.to_str().unwrap().to_string()); + let (first, second) = (locked_path.clone(), other_path.clone()); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_path(&first); + app.open_path(&first); + app.open_path(&second); + app + }); + h.run_steps(4); + assert!(h.state().password_prompt.is_some(), "the protected file is asked for"); + h.state_mut().submit_password(Some("reader".into())); + support::wait_until(&mut h, "both files, or a second prompt", |app| app.views.len() == 2 || app.password_prompt.is_some()); + assert!(h.state().password_prompt.is_none(), "the password is asked for once"); + let mut paths = open_paths(h.state()); + paths.sort(); + let mut expected = vec![locked_path, other_path]; + expected.sort(); + assert_eq!(paths, expected); + std::fs::remove_file(locked).unwrap(); + std::fs::remove_file(other).unwrap(); +} + +/// `pdfcraft locked-a.pdf locked-b.pdf`: B's reply is ready while A's password is asked for. +/// Once A's password is given, A's retry comes first: A opens (or asks again) before B's prompt +/// can take the dialog. B's prompt used to replace A's retry, so A waited for B's answer. +#[test] +fn a_password_retry_finishes_before_the_next_protected_files_prompt() { + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let a = temp_path(&format!("startup-retry-a-{nonce}.pdf")); + let b = temp_path(&format!("startup-retry-b-{nonce}.pdf")); + std::fs::write(&a, protected("reader", "owner", -4)).unwrap(); + std::fs::write(&b, protected("reader", "owner", -4)).unwrap(); + let (a_path, b_path) = (a.to_str().unwrap().to_string(), b.to_str().unwrap().to_string()); + let (first, second) = (a_path.clone(), b_path.clone()); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_path(&first); + app.open_path(&second); + app + }); + h.run_steps(4); + // B is prepared in the background meanwhile; its reply waits behind A's prompt. + std::thread::sleep(std::time::Duration::from_millis(300)); + h.run_steps(2); + assert_eq!(h.state().password_prompt.as_ref().and_then(|p| p.path.clone()), Some(a_path.clone())); + h.state_mut().submit_password(Some("reader".into())); + support::wait_until(&mut h, "the next prompt", |app| app.password_prompt.is_some()); + assert_eq!(open_paths(h.state()), vec![a_path], "A's retry finished first"); + assert_eq!(h.state().password_prompt.as_ref().and_then(|p| p.path.clone()), Some(b_path)); + std::fs::remove_file(a).unwrap(); + std::fs::remove_file(b).unwrap(); +} + +/// `pdfcraft locked.pdf` followed by more files than the open worker queues at once: the files +/// past its bound wait in order and open as it drains, instead of failing to open. +#[test] +fn a_long_startup_list_behind_a_password_prompt_opens_every_file() { + const OTHERS: usize = 24; + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let locked = temp_path(&format!("startup-long-locked-{nonce}.pdf")); + std::fs::write(&locked, protected("reader", "owner", -4)).unwrap(); + let others: Vec = (0..OTHERS).map(|i| temp_path(&format!("startup-long-{i:02}-{nonce}.pdf"))).collect(); + for path in &others { + std::fs::write(path, fixture(1)).unwrap(); + } + let locked_path = locked.to_str().unwrap().to_string(); + let other_paths: Vec = others.iter().map(|p| p.to_str().unwrap().to_string()).collect(); + let (first, rest) = (locked_path.clone(), other_paths.clone()); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_path(&first); + for path in &rest { + app.open_path(path); + } + app + }); + h.run_steps(4); + assert!(h.state().password_prompt.is_some(), "the protected file is asked for"); + h.state_mut().submit_password(Some("reader".into())); + support::wait_until(&mut h, "every file", |app| app.views.len() == OTHERS + 1); + let mut paths = open_paths(h.state()); + paths.sort(); + let mut expected = other_paths; + expected.push(locked_path); + expected.sort(); + assert_eq!(paths, expected); + std::fs::remove_file(locked).unwrap(); + for path in others { + std::fs::remove_file(path).unwrap(); + } +} + #[test] fn password_prompt_opens_and_security_tab_reports_the_details() { let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| {