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 <noreply@anthropic.com>
This commit is contained in:
Jeffrey Thayne
2026-10-12 15:15:01 -06:00
co-authored by Claude Opus 5.5
parent 2d2c6bbc83
commit bb7e81425f
3 changed files with 99 additions and 4 deletions
+90 -4
View File
@@ -18,7 +18,7 @@ use crate::{EngineError, Result, Session};
pub fn specs() -> Vec<CommandSpec> {
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<Value> {
#[cfg(not(target_arch = "wasm32"))]
fn read_text(path: &str) -> Result<Vec<u8>> {
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<T: Send + 'static>(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<usize, String> {
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);
}
}
+8
View File
@@ -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 {
+1
View File
@@ -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));