M2: keep the system clipboard away from the control channel

Commands run while handling a control request (engine.execute,
ui.menu.invoke, ...) no longer read the system clipboard into Paste or
write Copy/Cut text to it; only user-initiated runs do. Regression test
included. append_story allocates note, endnote, editorial and table ids
with saturating_add (no overflow panic on hostile ids), with a test.
Also fixes a clippy nonminimal_bool in cmd/text.rs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-07 22:11:44 -04:00
co-authored by Claude Opus 5.5
parent 7861297237
commit 2d889af2e2
5 changed files with 82 additions and 10 deletions
+1 -1
View File
@@ -178,7 +178,7 @@ impl Story {
}
pub(crate) fn next_note_id(&self) -> u64 {
self.notes.iter().map(|n| n.id).max().unwrap_or(0) + 1
self.notes.iter().map(|n| n.id).max().unwrap_or(0).saturating_add(1)
}
/// Insert empty footnotes for `n` new reference characters before which `k` references sit.
+26 -6
View File
@@ -136,13 +136,15 @@ impl Story {
}
if self.text.is_empty() {
let (id, frames, rev, link) = (self.id, std::mem::take(&mut self.frames), self.rev, self.link.take());
*self = Story { id, frames, rev: rev + 1, link, ..other };
*self = Story { id, frames, rev: rev.saturating_add(1), link, ..other };
return;
}
if !other.tables.is_empty() {
let first = self.tables.keys().max().copied().unwrap_or(0) + 1;
let mut next = self.tables.keys().max().copied().unwrap_or(0).saturating_add(1);
let mut ids = std::collections::HashMap::new();
for (new, (old, t)) in (first..).zip(std::mem::take(&mut other.tables)) {
for (old, t) in std::mem::take(&mut other.tables) {
let new = next;
next = next.saturating_add(1);
let mut t = Arc::unwrap_or_clone(t);
t.id = new;
ids.insert(old, new);
@@ -165,18 +167,18 @@ impl Story {
self.notes.push(Arc::new(crate::notes::Footnote { id, text: n.text.clone() }));
}
for n in other.endnotes {
let id = self.endnotes.iter().map(|x| x.id).max().unwrap_or(0) + 1;
let id = self.endnotes.iter().map(|x| x.id).max().unwrap_or(0).saturating_add(1);
self.endnotes.push(Arc::new(crate::notes::Footnote { id, text: n.text.clone() }));
}
for n in other.editorial {
let id = self.editorial.iter().map(|x| x.id).max().unwrap_or(0) + 1;
let id = self.editorial.iter().map(|x| x.id).max().unwrap_or(0).saturating_add(1);
self.editorial.push(Arc::new(crate::endnotes::EditorialNote { id, ..(*n).clone() }));
}
self.anchors.extend(other.anchors);
self.xrefs.extend(other.xrefs);
self.index_refs.extend(other.index_refs);
self.objects.extend(other.objects);
self.rev += 1;
self.rev = self.rev.saturating_add(1);
}
}
@@ -211,4 +213,22 @@ mod tests {
assert_eq!(part.notes.len(), 1);
assert!(a.extract(2..8).notes.is_empty());
}
#[test]
fn append_story_with_maximal_ids_does_not_overflow() {
let mut a = Story::with_text(StoryId(1), "Alpha", ParaFormat::default());
a.tables.insert(u64::MAX, Arc::new(crate::table::Table::new(u64::MAX, 1, 1, 0, 0, 100.0)));
let note = |id| Arc::new(crate::notes::Footnote { id, text: Story::with_text(StoryId(0), "n", ParaFormat::default()) });
a.endnotes.push(note(u64::MAX));
a.notes.push(note(u64::MAX));
a.rev = u64::MAX;
let mut b = Story::with_text(StoryId(2), "Beta", ParaFormat::default());
b.tables.insert(1, Arc::new(crate::table::Table::new(1, 1, 1, 0, 0, 100.0)));
b.endnotes.push(note(1));
b.notes.push(note(1));
a.append_story(b);
assert_eq!(a.text, "Alpha\nBeta");
assert_eq!(a.endnotes.len(), 2);
assert_eq!(a.notes.len(), 2);
}
}
+1 -1
View File
@@ -810,7 +810,7 @@ pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result<Value> {
let doc = s.doc()?.uid;
let mut f = typing_format(s).cloned().unwrap_or(TypingFormat { doc, at, attrs: CharAttrs::default(), cleared: vec![] });
f.attrs.merge(&a);
f.cleared.retain(|k| !attrs.get(k).is_some_and(|v| !v.is_null()) && !cleared.contains(k));
f.cleared.retain(|k| attrs.get(k).is_none_or(|v| v.is_null()) && !cleared.contains(k));
f.cleared.extend(cleared);
s.typing_format = Some(f);
return ok();
+46
View File
@@ -99,7 +99,15 @@ fn key_from(name: &str) -> Option<egui::Key> {
})
}
/// Handle one control request. Commands it runs leave the user's system clipboard alone.
pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome {
let was = std::mem::replace(&mut app.in_control, true);
let out = handle_request(app, ctx, req);
app.in_control = was;
out
}
fn handle_request(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome {
let p = &req.params;
let s = |k: &str| p.get(k).and_then(Value::as_str);
match req.method.as_str() {
@@ -367,3 +375,41 @@ pub fn save_screenshot(app: &mut DesignApp, image: &egui::ColorImage, path: Opti
None => json!({"ok": false, "error": "no writer configured"}),
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::sync::{
Arc,
atomic::{AtomicUsize, Ordering},
};
#[test]
fn control_paste_never_reads_the_system_clipboard() {
let calls = Arc::new(AtomicUsize::new(0));
let seen = calls.clone();
let services = crate::Services {
clipboard_text: Some(Box::new(move || {
seen.fetch_add(1, Ordering::SeqCst);
Some("SECRET".into())
})),
..Default::default()
};
let mut app = DesignApp::new(designcraft_engine::Session::new(), services);
app.run("file.new", json!({})).unwrap();
let r = app.run("frame.create", json!({"rect": [72, 72, 300, 200], "content": "text", "text": "words"})).unwrap();
let sid = r["story"].as_u64().unwrap();
assert!(app.session.active().unwrap().selection.text.is_some(), "the new frame is being typed in");
let text = |app: &DesignApp| app.session.active().unwrap().doc.story(designcraft_doc::StoryId(sid)).unwrap().text.clone();
let ctx = egui::Context::default();
let (req, _rx) = ControlRequest::new("engine.execute", json!({"command": "edit.paste"}));
let _ = handle(&mut app, &ctx, &req);
assert_eq!(calls.load(Ordering::SeqCst), 0, "the control channel must not read the system clipboard");
assert!(!text(&app).contains("SECRET"));
assert!(!app.in_control);
// The same command chosen by the user does paste the system clipboard.
let _ = app.run("edit.paste", json!({}));
assert_eq!(calls.load(Ordering::SeqCst), 1);
assert!(text(&app).contains("SECRET"));
}
}
+8 -2
View File
@@ -423,6 +423,9 @@ pub struct DesignApp {
pub last_recovery: f64,
/// The egui context (set by the first frame): copied text goes to the system clipboard through it.
egui_ctx: Option<egui::Context>,
/// A control-channel request is being handled: commands it runs never read or write the
/// user's system clipboard (a control client must not see or replace it).
pub(crate) in_control: bool,
}
impl DesignApp {
@@ -460,6 +463,7 @@ impl DesignApp {
last_time: 0.0,
last_recovery: 0.0,
egui_ctx: None,
in_control: false,
}
}
@@ -503,13 +507,15 @@ impl DesignApp {
if let Some(r) = menus::run_ui(self, id, &params) {
return r;
}
let params = self.with_system_clipboard(id, params);
// Only user-initiated runs touch the system clipboard, never the control channel's.
let system_clipboard = !self.in_control;
let params = if system_clipboard { self.with_system_clipboard(id, params) } else { params };
let r = self.session.execute(id, &params).map_err(|e| e.to_string());
self.after_engine();
match &r {
Err(e) => self.status(e.clone()),
// Copied text goes to the system clipboard too, whichever way Copy was chosen.
Ok(v) if matches!(id, "edit.copy" | "edit.cut") => {
Ok(v) if system_clipboard && matches!(id, "edit.copy" | "edit.cut") => {
if let (Some(t), Some(ctx)) = (v.get("text").and_then(Value::as_str), &self.egui_ctx) {
ctx.copy_text(t.to_string());
ctx.request_repaint();