From 01e9e6f092cc4befe03f9cd909b8cfa3a0000f2d Mon Sep 17 00:00:00 2001 From: tomsideguide Date: Wed, 19 Aug 2026 10:14:35 -0700 Subject: [PATCH 1/2] fix(epub): read a repeated spine part once --- src/formats/epub/mod.rs | 99 ++++++++++++++++++++++++++++++++--------- 1 file changed, 78 insertions(+), 21 deletions(-) diff --git a/src/formats/epub/mod.rs b/src/formats/epub/mod.rs index e5c17e06..5d3b9194 100644 --- a/src/formats/epub/mod.rs +++ b/src/formats/epub/mod.rs @@ -44,32 +44,33 @@ pub fn parse(bytes: &[u8]) -> Result { // still publication content, and unusable parts degrade at parse time. // Intra-book links target these; links to any other resource stay // Relative. - let spine_hrefs: Vec<&str> = opf + // A part holds one position in a reading order, and each repeat would + // cost another parse of it and another copy of its anchor. + let mut spine_entries = 0usize; + let mut spine_paths: Vec = Vec::new(); + let mut spine_parts: HashSet = HashSet::new(); + for href in opf .descendants_any("itemref") .filter_map(|ir| ir.attr_any("idref")) .filter_map(|idref| manifest.get(idref)) .map(|(href, _)| href.as_str()) - .collect(); - let spine_parts: HashSet = spine_hrefs - .iter() - .filter_map(|href| path::resolve(&opf_path, href).ok().map(|t| t.path)) - .collect(); + { + spine_entries += 1; + let Ok(target) = path::resolve(&opf_path, href) else { + log::warn!("skipping chapter with unresolvable href {href:?}"); + continue; + }; + if spine_parts.insert(target.path.clone()) { + spine_paths.push(target.path); + } + } let assets = RefCell::new(AssetSink::new()); let mut css_cache: HashMap> = HashMap::new(); - let mut failed = 0usize; - for href in &spine_hrefs { - let chapter_path = match path::resolve(&opf_path, href) { - Ok(t) => t.path, - Err(e) => { - log::warn!("skipping chapter with unresolvable href {href:?}: {e}"); - failed += 1; - continue; - } - }; - let Some(tree) = pkg.borrow_mut().optional_xml_part(&chapter_path)? else { + let mut converted = 0usize; + for chapter_path in &spine_paths { + let Some(tree) = pkg.borrow_mut().optional_xml_part(chapter_path)? else { log::warn!("skipping unusable chapter {chapter_path}"); - failed += 1; continue; }; let Some(body) = tree @@ -78,10 +79,9 @@ pub fn parse(bytes: &[u8]) -> Result { .and_then(|h| h.child_elems().find(|e| e.local == "body")) else { log::warn!("skipping chapter {chapter_path}: no body element"); - failed += 1; continue; }; - let css = chapter_stylesheet(&tree, &chapter_path, &pkg, &mut css_cache)?; + let css = chapter_stylesheet(&tree, chapter_path, &pkg, &mut css_cache)?; let ctx = ChapterCtx { pkg: &pkg, assets: &assets, @@ -91,8 +91,9 @@ pub fn parse(bytes: &[u8]) -> Result { // Chapter-start anchor: renders only when a link targets this chapter. doc.blocks.push(Block::Paragraph(vec![Inline::Anchor(chapter_path.clone())])); doc.blocks.extend(crate::shared::html::to_blocks(body, &css, &ctx)?); + converted += 1; } - if !spine_hrefs.is_empty() && failed == spine_hrefs.len() { + if spine_entries > 0 && converted == 0 { return Err(ConvertError::malformed("no chapter in the book could be read")); } @@ -206,3 +207,59 @@ fn scoped(chapter_path: &str, fragment: Option<&str>) -> AnchorId { _ => chapter_path.to_string(), } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Cursor, Write}; + + #[test] + fn a_part_repeated_across_the_spine_is_read_once() { + let items: String = (0..64) + .map(|i| { + format!(r#""#) + }) + .collect(); + let refs: String = (0..64).map(|i| format!(r#""#)).collect(); + let parts = [ + ( + "META-INF/container.xml", + r#" + + "# + .to_string(), + ), + ( + "c.opf", + format!( + r#" + + {items}{refs}"# + ), + ), + ( + "ch.xhtml", + r#" +

chapter text

"# + .to_string(), + ), + ]; + let mut w = zip::ZipWriter::new(Cursor::new(Vec::new())); + for (name, body) in &parts { + w.start_file(*name, zip::write::SimpleFileOptions::default()).unwrap(); + w.write_all(body.as_bytes()).unwrap(); + } + + let doc = parse(&w.finish().unwrap().into_inner()).unwrap(); + let text = doc + .blocks + .iter() + .filter_map(|b| match b { + Block::Paragraph(inlines) => Some(crate::model::inlines_to_plain_text(inlines)), + _ => None, + }) + .collect::(); + assert_eq!(text, "chapter text", "sixty-four itemrefs naming one part"); + } +} From 903e00946260594e29e545d4d2cea753d06cab33 Mon Sep 17 00:00:00 2001 From: tomsideguide Date: Wed, 19 Aug 2026 10:25:54 -0700 Subject: [PATCH 2/2] fix(epub): keep the resolve error in the skipped-chapter warning --- src/formats/epub/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/formats/epub/mod.rs b/src/formats/epub/mod.rs index 5d3b9194..26296c9a 100644 --- a/src/formats/epub/mod.rs +++ b/src/formats/epub/mod.rs @@ -56,9 +56,12 @@ pub fn parse(bytes: &[u8]) -> Result { .map(|(href, _)| href.as_str()) { spine_entries += 1; - let Ok(target) = path::resolve(&opf_path, href) else { - log::warn!("skipping chapter with unresolvable href {href:?}"); - continue; + let target = match path::resolve(&opf_path, href) { + Ok(t) => t, + Err(e) => { + log::warn!("skipping chapter with unresolvable href {href:?}: {e}"); + continue; + } }; if spine_parts.insert(target.path.clone()) { spine_paths.push(target.path);