From bb7e81425ff86c76d99e1c09a6016245f4aa644b Mon Sep 17 00:00:00 2001 From: Jeffrey Thayne <36243577+thaynej@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:15:01 -0600 Subject: [PATCH] Links: never read a linked text file that isn't a regular file Checking a linked text file's status (every links.list, which the Links panel polls) compared sizes and then read the whole file, so a link to a FIFO or a device such as /dev/zero (size 0) never returned. Only regular files are read now, and no more than the Data Merge source limit (256 MiB): anything else shows as missing; a file over the limit shows as modified when its size changed and as tooLarge ("Too large to check") when it didn't. Update and Relink read through the same guard, and placing a file too large to check doesn't link it. Unix test: a FIFO, /dev/zero, a folder and an over-limit file each return promptly without being read (each call runs on a thread with a timeout, so a regression fails instead of hanging). Thanks @aiongg. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/links.rs | 94 +++++++++++++++++++++++-- crates/engine/src/cmd/place_text.rs | 8 +++ crates/ui-egui/src/panels/properties.rs | 1 + 3 files changed, 99 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 1f9512953..e1c482a07 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -18,7 +18,7 @@ use crate::{EngineError, Result, Session}; pub fn specs() -> Vec { vec![ - cmd!(query "links.list", "Links", [], None, "{} → [{asset, name, path, status: ok|modified|missing|embedded, pixels, uses: [{id, page, ppi}]}], then linked text files: [{story, name, path, status, edited (the story was changed in the layout), uses: [{id, page}]}]", has_doc, |s, _| { + cmd!(query "links.list", "Links", [], None, "{} → [{asset, name, path, status: ok|modified|missing|embedded, pixels, uses: [{id, page, ppi}]}], then linked text files: [{story, name, path, status (also tooLarge: unchanged size, too big to check), edited (the story was changed in the layout), uses: [{id, page}]}]", has_doc, |s, _| { Ok(Value::Array(list(&s.doc()?.doc))) }), cmd!( @@ -145,14 +145,26 @@ pub fn status(a: &designcraft_doc::Asset) -> &'static str { } } -/// Link status of a linked text file. +/// The largest linked text file read (the Data Merge source limit). Only regular files are read, +/// so a link to a FIFO or a device (e.g. `/dev/zero`) can't block or fill memory. +const MAX_TEXT_LINK_BYTES: u64 = super::datamerge::MAX_DATA_FILE_BYTES; + +/// Can a linked text file this size be checked and updated? +pub(crate) fn checkable_size(len: usize) -> bool { + len as u64 <= MAX_TEXT_LINK_BYTES +} + +/// Link status of a linked text file: `tooLarge` when it is the recorded size but over +/// [`MAX_TEXT_LINK_BYTES`], so its contents aren't checked. pub fn text_status(l: &TextFileLink) -> &'static str { #[cfg(not(target_arch = "wasm32"))] { match std::fs::metadata(&l.path) { Err(_) => "missing", + Ok(m) if !m.is_file() => "missing", Ok(m) if m.len() != l.size => "modified", - Ok(_) => match std::fs::read(&l.path) { + Ok(m) if m.len() > MAX_TEXT_LINK_BYTES => "tooLarge", + Ok(_) => match read_text(&l.path) { Ok(b) if fnv_hex(&b) == l.hash => "ok", Ok(_) => "modified", Err(_) => "missing", @@ -210,7 +222,23 @@ fn list(d: &Document) -> Vec { #[cfg(not(target_arch = "wasm32"))] fn read_text(path: &str) -> Result> { - std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}"))) + use std::io::Read; + let err = |e: std::io::Error| EngineError::Other(format!("{path}: {e}")); + let m = std::fs::metadata(path).map_err(err)?; + if !m.is_file() { + return Err(EngineError::Other(format!("{path}: not a file"))); + } + let too_large = || EngineError::Other(format!("{path}: larger than {} MiB", MAX_TEXT_LINK_BYTES / (1024 * 1024))); + if m.len() > MAX_TEXT_LINK_BYTES { + return Err(too_large()); + } + // At most one byte over the limit, in case the file grew since. + let mut b = Vec::new(); + std::fs::File::open(path).map_err(err)?.take(MAX_TEXT_LINK_BYTES + 1).read_to_end(&mut b).map_err(err)?; + if b.len() as u64 > MAX_TEXT_LINK_BYTES { + return Err(too_large()); + } + Ok(b) } #[cfg(target_arch = "wasm32")] @@ -712,3 +740,61 @@ mod text_link_tests { let _ = std::fs::remove_dir_all(&dir); } } + +#[cfg(test)] +#[cfg(unix)] +mod text_link_read_tests { + use std::time::Duration; + + use designcraft_doc::TextFileLink; + + use super::{MAX_TEXT_LINK_BYTES, read_text, text_status}; + + /// `f` on another thread: a call that blocks fails the test instead of hanging it. + fn promptly(f: impl FnOnce() -> T + Send + 'static) -> T { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(f()); + }); + // A timeout means the call blocked. + rx.recv_timeout(Duration::from_secs(10)).unwrap() + } + + fn status_of(path: &std::path::Path, size: u64) -> &'static str { + let l = TextFileLink { path: path.to_string_lossy().to_string(), size, hash: "0".into(), ..Default::default() }; + promptly(move || text_status(&l)) + } + + fn read(path: &std::path::Path) -> Result { + let p = path.to_string_lossy().to_string(); + promptly(move || read_text(&p).map(|b| b.len()).map_err(|e| e.to_string())) + } + + #[test] + fn links_to_fifos_devices_folders_and_huge_files_are_not_read() { + let dir = std::env::temp_dir().join(format!("dc-text-link-read-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + // A FIFO with no writer: reading it would block for ever. + let fifo = dir.join("pipe.docx"); + assert!(std::process::Command::new("mkfifo").arg(&fifo).status().unwrap().success()); + assert_eq!(status_of(&fifo, 0), "missing"); + assert!(read(&fifo).is_err()); + // A device that never ends, recorded with size 0 (as its metadata reports). + let zero = std::path::Path::new("/dev/zero"); + assert_eq!(status_of(zero, 0), "missing"); + assert!(read(zero).is_err()); + // A folder. + assert_eq!(status_of(&dir, 0), "missing"); + assert!(read(&dir).is_err()); + // Over the limit (sparse, so cheap): not read; the same size is reported as too large + // to check rather than OK, another size as modified. + let big = dir.join("big.txt"); + std::fs::File::create(&big).unwrap().set_len(MAX_TEXT_LINK_BYTES + 1).unwrap(); + assert_eq!(status_of(&big, MAX_TEXT_LINK_BYTES + 1), "tooLarge"); + assert_eq!(status_of(&big, 10), "modified"); + assert!(read(&big).unwrap_err().contains("larger than")); + assert!(!super::checkable_size(MAX_TEXT_LINK_BYTES as usize + 1), "placing it doesn't link it"); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/engine/src/cmd/place_text.rs b/crates/engine/src/cmd/place_text.rs index 698f38809..4d4577a5f 100644 --- a/crates/engine/src/cmd/place_text.rs +++ b/crates/engine/src/cmd/place_text.rs @@ -251,6 +251,11 @@ pub(super) fn place_text(s: &mut Session, p: &Value, name: &str, bytes: &[u8], p let map = map_styles(&s.doc()?.doc, &mut imp, &map, conflict); // Create Links When Placing Text Files (only a file on disk can be linked). let link = path.filter(|_| p.get("link").and_then(Value::as_bool).unwrap_or(s.prefs.link_text_files)); + // A file too large for the Links panel to check isn't linked. + let (link, too_large) = match link { + Some(_) if !super::links::checkable_size(bytes.len()) => (None, true), + l => (l, false), + }; let autoflow_on = p.get("autoflow").and_then(Value::as_bool).unwrap_or(false); let st = s.doc()?; let caret = st.selection.text.filter(|t| t.cell.is_none()); @@ -260,6 +265,9 @@ pub(super) fn place_text(s: &mut Session, p: &Value, name: &str, bytes: &[u8], p let page = p.get("page").and_then(Value::as_u64).map(|v| (v as usize).saturating_sub(1)); let rect = super::rect_param(p, "rect"); let mut warnings = imp.warnings.clone(); + if too_large { + warnings.push("not linked: the file is too large to keep linked".into()); + } s.edit(|d, sel| { add_styles(d, &imp, conflict); let sid = if let Some(t) = caret { diff --git a/crates/ui-egui/src/panels/properties.rs b/crates/ui-egui/src/panels/properties.rs index 7d683a8ff..3fc97c2ee 100644 --- a/crates/ui-egui/src/panels/properties.rs +++ b/crates/ui-egui/src/panels/properties.rs @@ -1850,6 +1850,7 @@ pub fn links_panel(app: &mut DesignApp, ui: &mut egui::Ui) { "missing" => ("\u{26A0} Missing", egui::Color32::from_rgb(0xe5, 0x4b, 0x4b)), "modified" => ("\u{25B2} Modified", egui::Color32::from_rgb(0xe8, 0xb3, 0x2c)), "embedded" => ("Embedded", t.text_dim), + "tooLarge" => ("Too large to check", egui::Color32::from_rgb(0xe8, 0xb3, 0x2c)), _ => ("OK", t.text_dim), }; crate::rtl::label(ui, egui::RichText::new(crate::i18n::tr(&app.ui.language, txt)).color(col).size(11.0));