From 6e60bbccbe6ff10eb4ea4757df425fff2ffbd0ca Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Wed, 14 Oct 2026 11:09:11 +0200 Subject: [PATCH] EPUB: read the cover image through a capped reader (256 MiB) coverImage was read with an uncapped std::fs::read, so a huge file or a device like /dev/zero could exhaust memory. Read it through the data merge's capped reader, which now also stops the read itself at the cap (a device reports no length), as in aiongg's integration branch (#259, 6eae3620), with /dev/zero as a test. --- crates/engine/src/cmd/datamerge/mod.rs | 24 +++++++++++++++--------- crates/engine/src/cmd/datamerge/tests.rs | 9 +++++++++ crates/engine/src/cmd/export.rs | 2 +- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/datamerge/mod.rs b/crates/engine/src/cmd/datamerge/mod.rs index 3b3e1dd3b..4c258f1c9 100644 --- a/crates/engine/src/cmd/datamerge/mod.rs +++ b/crates/engine/src/cmd/datamerge/mod.rs @@ -767,17 +767,23 @@ fn read_path(path: &str) -> std::result::Result, String> { read_path_capped(path, MAX_DATA_FILE_BYTES) } -fn read_path_capped(path: &str, cap: u64) -> std::result::Result, String> { - match std::fs::metadata(path) { - Ok(m) if m.len() > cap => return Err(format!("{path}: the data file is larger than {} MiB", cap / (1024 * 1024))), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Err(format!("{path}: the data file is missing")), - _ => {} +/// The file at `path`, refused when it is larger than `cap` bytes. The read itself stops past +/// `cap`, so a device or a file that grows while it is read can't exhaust memory. +pub(crate) fn read_path_capped(path: &str, cap: u64) -> std::result::Result, String> { + use std::io::Read; + let too_big = || format!("{path}: the file is larger than {} MiB", cap / (1024 * 1024)); + let missing = |e: std::io::Error| { + if e.kind() == std::io::ErrorKind::NotFound { format!("{path}: the file is missing") } else { format!("{path}: {e}") } + }; + if std::fs::metadata(path).map_err(missing)?.len() > cap { + return Err(too_big()); } - match std::fs::read(path) { - Ok(b) => Ok(b), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Err(format!("{path}: the data file is missing")), - Err(e) => Err(format!("{path}: {e}")), + let mut b = Vec::new(); + std::fs::File::open(path).map_err(missing)?.take(cap.saturating_add(1)).read_to_end(&mut b).map_err(missing)?; + if b.len() as u64 > cap { + return Err(too_big()); } + Ok(b) } fn parent_dir(path: &str) -> Option { diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index f4c2004e4..98849eae1 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -1086,6 +1086,15 @@ fn oversized_data_file_is_refused_before_reading() { assert_eq!(read_path_capped(&path, 4096).unwrap().len(), 2048); } +/// A device reports no length; the read still stops at the cap (an EPUB `coverImage` of +/// `/dev/zero` must not read until memory runs out). +#[cfg(unix)] +#[test] +fn endless_file_is_refused_at_the_cap() { + let msg = read_path_capped("/dev/zero", 1024).unwrap_err(); + assert!(msg.contains("larger than"), "{msg}"); +} + /// #198: with facing pages on, one record per page must not stack the copies at identical /// coordinates on one spread (leaving a later page empty). #[test] diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index 0b4a91d32..c29a3a66e 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -132,7 +132,7 @@ fn export_epub(s: &mut Session, p: &Value) -> Result { let ropts = designcraft_render::RenderOptions { printing_only: true, ..Default::default() }; // A cover image file (PNG or JPEG), else the first page rendered when `cover` asks for it. let cover = match p.get("coverImage").and_then(Value::as_str).filter(|_| !fixed) { - Some(path) => Some(std::fs::read(path).map_err(|e| crate::EngineError::Other(format!("{path}: {e}")))?), + Some(path) => Some(super::datamerge::read_path_capped(path, super::datamerge::MAX_IMAGE_FILE_BYTES).map_err(crate::EngineError::Other)?), None => (want_cover && !fixed).then(|| rr.render_page(&st.doc, &s.cache, 0, 2.0, false, &ropts).map(|img| img.to_png())).flatten(), }; if let Some(c) = &cover