From 8e8865bba18922e29dd57ed9c62fa910ab701c41 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 30 Oct 2026 03:08:39 +0100 Subject: [PATCH] thumbnail: refuse an output that names the input document (#1782) designcraft-cli thumbnail a.designcraft a.designcraft replaced the document with PNG bytes. The output is now refused before anything is read or written when it is the input by the same path, through a symlink, or on Unix as a hard link (same device and inode). --- apps/designcraft-cli/src/main.rs | 20 ++++++++++++ apps/designcraft-cli/tests/it/cli.rs | 47 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/apps/designcraft-cli/src/main.rs b/apps/designcraft-cli/src/main.rs index bf7b6b351..a26c3d977 100644 --- a/apps/designcraft-cli/src/main.rs +++ b/apps/designcraft-cli/src/main.rs @@ -203,6 +203,9 @@ fn thumbnail(args: &[String]) -> Result<(), String> { [s] => Some(s.parse::().ok().filter(|s| *s > 0).ok_or_else(|| format!("bad SIZE `{s}`: expected a positive number of pixels"))?), _ => return Err(USAGE.into()), }; + if names_the_same_file(input.as_ref(), out.as_ref()) { + return Err(format!("{out} is the document {input}: the thumbnail would replace it")); + } let file = std::fs::File::open(input).map_err(|e| format!("{input}: {e}"))?; let preview = designcraft_format::read_preview(std::io::BufReader::new(file)) .ok_or_else(|| format!("{input} has no preview (a DesignCraft file saved before they had one, or not a DesignCraft file)"))?; @@ -210,6 +213,23 @@ fn thumbnail(args: &[String]) -> Result<(), String> { designcraft_engine::atomic::write_atomic(std::path::Path::new(out), &png).map_err(|e| format!("{out}: {e}")) } +/// Whether `out` is the file `input` names: the same path, a symlink to it or, on Unix, a hard +/// link (the same device and inode). An `out` that doesn't exist yet can't be the input. +fn names_the_same_file(input: &std::path::Path, out: &std::path::Path) -> bool { + let (Ok(a), Ok(b)) = (std::fs::canonicalize(input), std::fs::canonicalize(out)) else { return false }; + if a == b { + return true; + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if let (Ok(a), Ok(b)) = (std::fs::metadata(&a), std::fs::metadata(&b)) { + return (a.dev(), a.ino()) == (b.dev(), b.ino()); + } + } + false +} + /// `png` with its longer side brought down to `size`. A picture already that small is returned /// as it is; it is decoded anyway, so a broken one is an error rather than an unreadable thumbnail. fn fit_png(png: Vec, size: Option) -> Result, String> { diff --git a/apps/designcraft-cli/tests/it/cli.rs b/apps/designcraft-cli/tests/it/cli.rs index 98b4f5edf..f19551a71 100644 --- a/apps/designcraft-cli/tests/it/cli.rs +++ b/apps/designcraft-cli/tests/it/cli.rs @@ -616,3 +616,50 @@ fn thumbnail_of_a_file_without_a_preview_fails_and_writes_nothing() { } let _ = std::fs::remove_dir_all(&dir); } + +/// `thumbnail a.designcraft a.designcraft` would replace the document with a PNG. An output that +/// names the input, by the same path, through a symlink or (on Unix) as a hard link, is refused +/// before anything is read or written, and the document keeps its bytes. +#[test] +fn thumbnail_refuses_an_output_that_names_the_input() { + let dir = std::env::temp_dir().join(format!("dc-cli-thumb-self-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let doc = dir.join("a.designcraft"); + make_document(&doc); + let saved = std::fs::read(&doc).unwrap(); + let mut outs = vec![doc.clone(), dir.join(".").join("a.designcraft")]; + #[cfg(unix)] + { + let link = dir.join("link.png"); + std::os::unix::fs::symlink(&doc, &link).unwrap(); + outs.push(link); + let hard = dir.join("hard.png"); + std::fs::hard_link(&doc, &hard).unwrap(); + outs.push(hard); + } + for out_path in &outs { + let out = cli().arg("thumbnail").arg(&doc).arg(out_path).arg("128").output().unwrap(); + let err = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(1), "{}: {err}", out_path.display()); + assert!(err.contains("is the document"), "{}: {err}", out_path.display()); + assert!(std::fs::read(&doc).unwrap() == saved, "{} changed the document", out_path.display()); + assert!(std::fs::read(out_path).unwrap() == saved, "{} was written", out_path.display()); + } + let _ = std::fs::remove_dir_all(&dir); +} + +/// Another existing file at the output path is still replaced by the thumbnail. +#[test] +fn thumbnail_replaces_another_existing_file() { + let dir = std::env::temp_dir().join(format!("dc-cli-thumb-other-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let doc = dir.join("a.designcraft"); + make_document(&doc); + let png = dir.join("old.png"); + std::fs::write(&png, b"an older thumbnail").unwrap(); + let out = cli().arg("thumbnail").arg(&doc).arg(&png).arg("128").output().unwrap(); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + let (w, h) = png_size(&std::fs::read(&png).unwrap()); + assert_eq!(w.max(h), 128, "{w}×{h}"); + let _ = std::fs::remove_dir_all(&dir); +}