diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 2bf6d90b6..0d4d51852 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -53,7 +53,118 @@ pub fn open_path(path: &std::path::Path) -> Result { return Err(format!("{}: file is larger than 2 GB", path.display())); } let bytes = std::fs::read(path).map_err(|e| format!("{}: {e}", path.display()))?; - open_bytes(&path.to_string_lossy(), &bytes) + let name = path.to_string_lossy(); + if matches!(ext_of(&name).as_str(), "html" | "htm" | "xhtml") { + // Pictures referenced by a relative path load from beside the HTML file. + let images = LocalImages::new(path.parent().unwrap_or(std::path::Path::new(""))); + let mut doc = wordcraft_formats::html::import_with(&wordcraft_formats::html::decode(&bytes), &|src| images.load(src)); + doc.ensure_nonempty(); + return Ok(doc); + } + open_bytes(&name, &bytes) +} + +/// The pictures an HTML file names by a path relative to its folder. +/// +/// Only regular files inside the folder (after resolving `..` and symlinks) are read. URLs +/// (`http:`, `file:`…) and absolute paths are not followed. Each file is read at most once and +/// at most `max_file` bytes of it; every reference, cached or not, counts against a budget for +/// the whole document, so one large picture can't be multiplied by referencing it many times. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) struct LocalImages { + /// The folder, canonical; `None` when it can't be resolved (nothing loads then). + dir: Option, + max_file: u64, + /// Bytes the document may still load. + left: std::cell::Cell, + cache: std::cell::RefCell>)>>, +} + +#[cfg(not(target_arch = "wasm32"))] +impl LocalImages { + /// Largest picture file read. + const MAX_FILE: u64 = 256 << 20; + /// Picture bytes one document may load, counting every reference. + const BUDGET: u64 = 1 << 30; + /// Files whose bytes are kept for further references. + const CACHE: usize = 64; + + pub(crate) fn new(dir: &std::path::Path) -> LocalImages { + LocalImages::with_limits(dir, LocalImages::MAX_FILE, LocalImages::BUDGET) + } + + pub(crate) fn with_limits(dir: &std::path::Path, max_file: u64, budget: u64) -> LocalImages { + let dir = if dir.as_os_str().is_empty() { std::path::Path::new(".") } else { dir }; + LocalImages { dir: std::fs::canonicalize(dir).ok(), max_file, left: std::cell::Cell::new(budget), cache: Default::default() } + } + + /// Bytes of the file `src` names, or `None` when it isn't a regular file inside the folder, + /// is too large, or the budget is spent. + pub(crate) fn load(&self, src: &str) -> Option>> { + let path = self.resolve(src)?; + let cached = self.cache.borrow().iter().find(|(p, _)| *p == path).map(|(_, d)| d.clone()); + if let Some(data) = cached { + self.charge(data.len())?; + return Some(data); + } + let limit = self.max_file.min(self.left.get()); + let bytes = read_regular_file(&path, limit)?; + // What was read is spent even when it's over the limit. + self.charge(bytes.len())?; + if bytes.len() as u64 > limit { + return None; + } + let data = std::sync::Arc::new(bytes); + let mut cache = self.cache.borrow_mut(); + if cache.len() < LocalImages::CACHE { + cache.push((path, data.clone())); + } + Some(data) + } + + /// Take `n` bytes from the budget; `None` (and the budget emptied) when there aren't enough. + fn charge(&self, n: usize) -> Option<()> { + let left = self.left.get().checked_sub(n as u64); + self.left.set(left.unwrap_or(0)); + left.map(|_| ()) + } + + /// The canonical path `src` names, if it's inside the folder. + fn resolve(&self, src: &str) -> Option { + let src = src.split(['#', '?']).next().unwrap_or(""); + if src.is_empty() || src.starts_with("//") || src.split_once(':').is_some_and(|(scheme, _)| !scheme.contains('/')) { + return None; + } + let rel = String::from_utf8(wordcraft_formats::model::percent_decode(src)).ok()?; + let rel = std::path::Path::new(&rel); + if rel.components().any(|c| matches!(c, std::path::Component::Prefix(_) | std::path::Component::RootDir)) { + return None; + } + let dir = self.dir.as_ref()?; + // Canonical paths have `..` and symlinks resolved, so this keeps both inside the folder. + let path = std::fs::canonicalize(dir.join(rel)).ok()?; + path.starts_with(dir).then_some(path) + } +} + +/// Up to `limit + 1` bytes of the regular file at `path` (more than `limit` means too large), or +/// `None` for anything else. A named pipe blocks when opened and a device reports a length of 0 +/// and may never end, so the path is checked before opening, the opened handle is checked again, +/// and the read is bounded whatever length the file reports. +#[cfg(not(target_arch = "wasm32"))] +fn read_regular_file(path: &std::path::Path, limit: u64) -> Option> { + use std::io::Read; + if !std::fs::metadata(path).ok()?.is_file() { + return None; + } + let file = std::fs::File::open(path).ok()?; + let meta = file.metadata().ok()?; + if !meta.is_file() { + return None; + } + let mut out = Vec::with_capacity(usize::try_from(meta.len().min(limit)).unwrap_or(0)); + file.take(limit.saturating_add(1)).read_to_end(&mut out).ok()?; + Some(out) } #[cfg(target_arch = "wasm32")] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e7c8edcc9..ecac474e6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -632,6 +632,161 @@ fn caret_navigation() { assert_eq!(s.sel.focus.off, 10); } +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn html_pictures_load_relative_to_the_file() { + // Issue #98: `` beside an HTML file is embedded when it is opened. + let dir = std::env::temp_dir().join(format!("wordcraft-html-img-{}", std::process::id())); + std::fs::create_dir_all(dir.join("img")).unwrap(); + let mut png = Vec::new(); + image::RgbaImage::from_pixel(4, 2, image::Rgba([10, 20, 30, 255])) + .write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png) + .unwrap(); + std::fs::write(dir.join("img/my logo.png"), &png).unwrap(); + std::fs::write(dir.join("secret.txt"), b"not a picture").unwrap(); + let html = r#"

Logo

T

Web

"#; + std::fs::write(dir.join("page.html"), html).unwrap(); + let doc = crate::io::open_path(&dir.join("page.html")); + let _ = std::fs::remove_dir_all(&dir); + let doc = doc.unwrap(); + assert_eq!(doc.media.len(), 1); + let text = doc.plain_text(StoryRef::Body); + assert!(!text.contains("Logo") && text.contains('T') && text.contains("Web"), "{text:?}"); +} + +/// A fresh, empty scratch folder for one test. +#[cfg(not(target_arch = "wasm32"))] +fn scratch_dir(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("wordcraft-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +#[cfg(not(target_arch = "wasm32"))] +fn tiny_png() -> Vec { + let mut png = Vec::new(); + image::RgbaImage::from_pixel(2, 2, image::Rgba([200, 30, 30, 255])) + .write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png) + .unwrap(); + png +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn html_pictures_outside_the_folder_are_not_loaded() { + // #98 review: `..` must not reach files outside the HTML file's folder. + let root = scratch_dir("html-img-escape"); + std::fs::create_dir_all(root.join("site/img")).unwrap(); + std::fs::write(root.join("outside.png"), tiny_png()).unwrap(); + std::fs::write(root.join("site/inside.png"), tiny_png()).unwrap(); + let html = r#"

Up

Sneak

In

"#; + std::fs::write(root.join("site/page.html"), html).unwrap(); + let doc = crate::io::open_path(&root.join("site/page.html")); + let _ = std::fs::remove_dir_all(&root); + let doc = doc.unwrap(); + assert_eq!(doc.media.len(), 1, "only the picture inside the folder loads"); + let text = doc.plain_text(StoryRef::Body); + assert!(text.contains("Up") && text.contains("Sneak") && !text.contains("In"), "{text:?}"); +} + +#[cfg(unix)] +#[test] +fn html_pictures_through_a_symlink_out_of_the_folder_are_not_loaded() { + // #98 review: a symlink inside the folder must not lead outside it. + let root = scratch_dir("html-img-symlink"); + std::fs::create_dir_all(root.join("site")).unwrap(); + std::fs::write(root.join("outside.png"), tiny_png()).unwrap(); + std::os::unix::fs::symlink(root.join("outside.png"), root.join("site/link.png")).unwrap(); + std::fs::write(root.join("site/page.html"), r#"

Link

"#).unwrap(); + let doc = crate::io::open_path(&root.join("site/page.html")); + let _ = std::fs::remove_dir_all(&root); + let doc = doc.unwrap(); + assert!(doc.media.is_empty()); + assert!(doc.plain_text(StoryRef::Body).contains("Link")); +} + +#[cfg(unix)] +#[test] +fn html_picture_that_is_a_named_pipe_is_skipped_without_blocking() { + // #98 review: opening a FIFO blocks until a writer appears; it must never be opened. + let dir = scratch_dir("html-img-fifo"); + let fifo = dir.join("pipe.png"); + let made = std::process::Command::new("mkfifo").arg(&fifo).status(); + if !made.is_ok_and(|s| s.success()) { + let _ = std::fs::remove_dir_all(&dir); + eprintln!("mkfifo unavailable; skipping"); + return; + } + std::fs::write(dir.join("page.html"), r#"

Pipe

"#).unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + let page = dir.join("page.html"); + std::thread::spawn(move || { + let _ = tx.send(crate::io::open_path(&page)); + }); + let got = rx.recv_timeout(std::time::Duration::from_secs(10)); + if got.is_err() { + // Unblock the stuck reader so the thread ends, then fail. + let _ = std::fs::OpenOptions::new().write(true).open(&fifo); + } + let _ = std::fs::remove_dir_all(&dir); + let doc = got.expect("opening the HTML file blocked on a named pipe").unwrap(); + assert!(doc.media.is_empty()); + assert!(doc.plain_text(StoryRef::Body).contains("Pipe")); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn html_picture_size_limit_holds_whatever_length_the_file_reports() { + use crate::io::LocalImages; + let dir = scratch_dir("html-img-limit"); + std::fs::write(dir.join("ten.bin"), [7u8; 10]).unwrap(); + std::fs::write(dir.join("eleven.bin"), [7u8; 11]).unwrap(); + let images = LocalImages::with_limits(&dir, 10, 1000); + let ten = images.load("ten.bin").map(|d| d.len()); + let eleven = images.load("eleven.bin"); + let _ = std::fs::remove_dir_all(&dir); + assert_eq!(ten, Some(10)); + assert!(eleven.is_none()); + #[cfg(unix)] + { + // A device reports a length of 0 and never ends: it is not a regular file, and the read is + // bounded anyway. + let dev = LocalImages::with_limits(std::path::Path::new("/dev"), 1 << 20, 1 << 20); + assert!(dev.load("zero").is_none()); + assert!(dev.load("null").is_none()); + } +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn html_pictures_share_one_read_and_a_per_document_budget() { + // #98 review: a picture referenced many times is read once, and every reference counts + // against the document's budget so a large picture can't be multiplied without bound. + use crate::io::LocalImages; + let dir = scratch_dir("html-img-budget"); + let png = tiny_png(); + let n = png.len() as u64; + std::fs::write(dir.join("a.png"), &png).unwrap(); + let images = LocalImages::with_limits(&dir, 1 << 20, 3 * n); + let first = images.load("a.png").unwrap(); + std::fs::write(dir.join("a.png"), b"changed on disk").unwrap(); + let second = images.load("./a.png").unwrap(); + assert!(std::sync::Arc::ptr_eq(&first, &second), "the second reference reuses the first read"); + assert_eq!(*second, png); + assert!(images.load("a.png").is_some()); + assert!(images.load("a.png").is_none(), "budget spent"); + // Files over the per-picture limit still cost what was read. + std::fs::write(dir.join("big.bin"), vec![1u8; 100]).unwrap(); + std::fs::write(dir.join("small.bin"), [1u8; 5]).unwrap(); + let images = LocalImages::with_limits(&dir, 10, 25); + assert!(images.load("big.bin").is_none()); + assert!(images.load("big.bin").is_none()); + let small = images.load("small.bin"); + let _ = std::fs::remove_dir_all(&dir); + assert!(small.is_none(), "two oversized reads used up the budget"); +} + fn para_style(s: &Session, i: usize) -> Option { s.doc.para_at(&Pos::body(i, 0)).and_then(|p| p.props.style.clone()) } diff --git a/crates/formats/src/html.rs b/crates/formats/src/html.rs index 03d5b8d16..926508133 100644 --- a/crates/formats/src/html.rs +++ b/crates/formats/src/html.rs @@ -4,13 +4,15 @@ //! HTML's implicit closes for `p`, `li`, `tr`, `td`/`th` and headings. It understands `p`, `div`, //! `h1`–`h6`, `b`/`strong`, `i`/`em`, `u`, `s`/`strike`/`del`, `sup`/`sub`, `code`/`kbd`, `a` //! (links and `id`/`name` anchors), `br`, `hr`, `ul`/`ol`/`li` (nested), `table`/`tr`/`td`/`th` -//! (colspan/rowspan), `img` with `data:` URIs, `blockquote`, `pre`, `font`, and the CSS +//! (colspan/rowspan), `img` (`data:` URIs, other sources through a caller's [`ImageLoader`]), `blockquote`, `pre`, `font`, and the CSS //! properties `color`, `background-color`, `font-weight`, `font-style`, `font-size`, //! `font-family`, `text-decoration`, `vertical-align`, `text-align`, `white-space` and page //! breaks. Scripts and styles are skipped; nesting is capped. //! //! Export writes clean semantic HTML with inline CSS and pictures as `data:` URIs. +use std::sync::Arc; + use wordcraft_doc::{Align, Document}; use crate::model::{ @@ -362,7 +364,13 @@ fn declares_border(attrs: &[(String, String)], is_table: bool) -> bool { }) } -struct Builder { +/// Loads the bytes of an image an `img` names by a source other than a `data:` URI (a path +/// relative to the HTML file). `None` when it can't be found. A picture used more than once can +/// share one buffer. +pub type ImageLoader<'a> = &'a dyn Fn(&str) -> Option>>; + +struct Builder<'r> { + images: ImageLoader<'r>, containers: Vec>, cells: Vec, tables: Vec, @@ -552,9 +560,10 @@ fn attr<'a>(attrs: &'a [(String, String)], name: &str) -> Option<&'a str> { attrs.iter().find(|(k, _)| k == name).map(|(_, v)| v.as_str()) } -impl Builder { - fn new() -> Builder { +impl<'r> Builder<'r> { + fn new(images: ImageLoader<'r>) -> Builder<'r> { Builder { + images, containers: vec![Vec::new()], cells: Vec::new(), tables: Vec::new(), @@ -906,7 +915,9 @@ impl Builder { } } } - let img = attr(attrs, "src").and_then(data_uri).and_then(|d| make_img(d, w, h, &alt)); + let img = attr(attrs, "src") + .and_then(|src| if src.trim_start().starts_with("data:") { data_uri(src).map(Arc::new) } else { (self.images)(src.trim()) }) + .and_then(|d| make_img(d, w, h, &alt)); let f = self.fmt(); match img { Some(im) => self.start_para().inlines.push(Inline::Image(im)), @@ -1068,8 +1079,13 @@ impl Builder { /// Parse HTML text into the flow model. pub fn parse(s: &str) -> Flow { + parse_with(s, &|_| None) +} + +/// [`parse`], loading pictures that aren't `data:` URIs through `images`. +pub fn parse_with(s: &str, images: ImageLoader) -> Flow { let mut lx = Lexer { s, pos: 0 }; - let mut b = Builder::new(); + let mut b = Builder::new(images); let mut guard = 0usize; while let Some(t) = lx.next() { guard += 1; @@ -1098,6 +1114,11 @@ pub fn import(s: &str) -> Document { model::to_doc(&parse(s)) } +/// [`import`], loading pictures that aren't `data:` URIs through `images`. +pub fn import_with(s: &str, images: ImageLoader) -> Document { + model::to_doc(&parse_with(s, images)) +} + // --------------------------------------------------------------------------------------------- // Export diff --git a/crates/formats/src/model.rs b/crates/formats/src/model.rs index 34e435f91..f3371422b 100644 --- a/crates/formats/src/model.rs +++ b/crates/formats/src/model.rs @@ -795,7 +795,8 @@ pub fn image_px(data: &[u8]) -> Option<(u32, u32)> { /// A picture from bytes with an optional display size (points); the natural size (96 dpi) is /// used otherwise, shrunk to fit a 6.5" column. -pub fn make_img(data: Vec, w: Option, h: Option, alt: &str) -> Option { +pub fn make_img(data: impl Into>>, w: Option, h: Option, alt: &str) -> Option { + let data = data.into(); let ext = sniff_image(&data)?; let (pw, ph) = image_px(&data).map(|(a, b)| (a as f32 * 0.75, b as f32 * 0.75)).unwrap_or((72.0, 72.0)); let (pw, ph) = (pw.max(1.0), ph.max(1.0)); @@ -809,7 +810,7 @@ pub fn make_img(data: Vec, w: Option, h: Option, alt: &str) -> Opt h *= 468.0 / w; w = 468.0; } - Some(Img { data: Arc::new(data), ext: ext.to_string(), w: w.clamp(1.0, 1584.0), h: h.clamp(1.0, 1584.0), alt: alt.to_string() }) + Some(Img { data, ext: ext.to_string(), w: w.clamp(1.0, 1584.0), h: h.clamp(1.0, 1584.0), alt: alt.to_string() }) } const B64: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; diff --git a/crates/formats/src/tests.rs b/crates/formats/src/tests.rs index c65f64cc5..fcd36d4dc 100644 --- a/crates/formats/src/tests.rs +++ b/crates/formats/src/tests.rs @@ -520,3 +520,21 @@ proptest::proptest! { proptest::prop_assert_eq!(got.trim(), text.trim(), "md: {}", String::from_utf8_lossy(&md)); } } + +#[test] +fn html_img_sources_go_through_the_loader() { + // Issue #98: an `img` named by a relative path was dropped, leaving only its alt text. + let html = r#"

Sample Logo

Missing

Text after.

"#; + let pic = png(); + let seen = std::cell::RefCell::new(Vec::new()); + let d = crate::html::import_with(html, &|src| { + seen.borrow_mut().push(src.to_string()); + (src == "logo.png").then(|| std::sync::Arc::new(pic.clone())) + }); + assert_eq!(*seen.borrow(), ["logo.png", "gone.png"]); + assert_eq!(d.media.len(), 1); + let text = d.plain_text(wordcraft_doc::StoryRef::Body); + assert!(text.contains("Missing") && !text.contains("Sample Logo"), "{text:?}"); + // Without a loader only data: URIs load. + assert!(import("html", html.as_bytes()).unwrap().unwrap().media.is_empty()); +}