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