From 0e83c6ffc1bb257d547cf72575098c81d5558f54 Mon Sep 17 00:00:00 2001 From: funkpower Date: Fri, 13 Nov 2026 15:49:18 -0500 Subject: [PATCH] bookmeta: an EPUB cover by the word, not the letters The last route took any image whose id or address contained "cover", so images/discover-ch1.png listed before images/cover.jpg won, and recovery-map.png alone became the cover. A name now says cover only with it as a word of its own: words break at anything not a letter or digit and where a lower case letter meets a capital, so CoverImage and Book_COVER still do. An image named cover outright comes first, then manifest order. It is still a guess. The guide's cover page took its first image, which can be a logo. Where the page has several, the one whose file name, alt, id or class says cover is taken; where none does, the first is, with a note saying how many there were. Each member was read no further than MAX_COVER, but nothing bounded how many: two hundred cover-named images of 16 MiB took seven seconds to inflate and left two hundred notes. One search now reads at most eight members and stops with one note. A percent-escape takes two hex digits and nothing else. from_str_radix also takes a sign, so a%+fb decoded to a, 0x0f, b. --- crates/bookmeta/README.md | 20 ++- crates/bookmeta/src/epub.rs | 289 ++++++++++++++++++++++++++++++++++-- 2 files changed, 290 insertions(+), 19 deletions(-) diff --git a/crates/bookmeta/README.md b/crates/bookmeta/README.md index 7a4ac6e..4f9b8f8 100644 --- a/crates/bookmeta/README.md +++ b/crates/bookmeta/README.md @@ -130,10 +130,18 @@ An EPUB's is the first of these that reaches an image: 2. The item `` names (EPUB 2). Some converters write the image's address there rather than its id, and that is followed too. -3. The guide's ``, a page, and the first `` - or SVG `` on it. A guide that points straight at an image is - taken at its word. -4. Any image in the manifest whose id or address says `cover`. +3. The guide's ``, a page, and the `` or + SVG `` on it. Where the page has several, the one whose file + name, `alt`, `id` or `class` says `cover` is taken; where none does, the + first is, with a note that there were others. A guide that points straight + at an image is taken at its word. +4. Any image in the manifest whose id or file name says `cover`, one named + `cover` outright first and the rest in manifest order. + +A name says `cover` when it has it as a word of its own, in any case. Words +break at anything not a letter or digit and where a lower case letter meets a +capital, so `CoverImage`, `Book_COVER` and `cover-thumb` say it, and +`discover-ch1` and `recovery-map` do not. A route that names something unusable — not in the archive, not an image — leaves a note, and the next route is asked. Addresses are resolved against the @@ -145,7 +153,9 @@ What the bytes say beats what the manifest says: a JPEG listed as `image/png` comes back as `image/jpeg`, with a note. A cover larger than `bookmeta::MAX_COVER` (16 MiB) is left out with a note, in either format, and an EPUB's is never decompressed further than that, whatever size the archive -claims for it. +claims for it. Nor are more than eight members read looking for one, +however many candidates the manifest lists: the search stops there, with a +note. `Cover::guessed` says the file left the choice open: a Kindle book's first image record, or an EPUB image picked because its name says `cover`. Either diff --git a/crates/bookmeta/src/epub.rs b/crates/bookmeta/src/epub.rs index 979040a..1e6fd4e 100644 --- a/crates/bookmeta/src/epub.rs +++ b/crates/bookmeta/src/epub.rs @@ -22,6 +22,13 @@ use crate::{Cover, MAX_COVER, Metadata, Source, image_type}; /// document or is trying to be a decompression bomb. const MAX_PACKAGE: u64 = 4 * 1024 * 1024; +/// How many members one search for a cover will decompress. +/// +/// Each is bounded by [`MAX_COVER`], but a manifest can list as many +/// cover-named images as it likes, and reading every one of them is time a +/// single book is not owed. A real cover is found in one or two reads. +const MAX_READS: usize = 8; + /// Read what an EPUB says about itself. pub fn read(path: &str, bytes: &[u8]) -> Metadata { let mut meta = Metadata::empty(path, Source::Opf); @@ -295,6 +302,8 @@ fn cover( archive, items: &items, tried: Vec::new(), + reads: 0, + spent: false, meta, }; @@ -344,12 +353,21 @@ fn cover( } } - // Last: an image whose id or address says cover. - for item in items.iter().filter(|item| { - item.media_type.starts_with("image/") - && (item.id.to_lowercase().contains("cover") - || item.href.to_lowercase().contains("cover")) - }) { + // Last: an image whose id or file name has the word cover in it, one + // named `cover` outright first, then in manifest order. + let file_stem = |item: &Item| stem(item.path.as_deref().unwrap_or(&item.href)).to_string(); + let mut named: Vec<&Item> = items + .iter() + .filter(|item| { + item.media_type.starts_with("image/") + && (says_cover(&item.id) || says_cover(&file_stem(item))) + }) + .collect(); + named.sort_by_key(|item| !file_stem(item).eq_ignore_ascii_case("cover")); + for item in named { + if look.spent { + break; + } if let Some(mut found) = look.image(base, &item.href) { found.guessed = true; return Some(found); @@ -364,12 +382,19 @@ struct Look<'a, R> { archive: &'a mut zip::ZipArchive, items: &'a [Item], tried: Vec, + /// How many members have been decompressed, against [`MAX_READS`]. + reads: usize, + /// Whether the reads have run out, and said so. + spent: bool, meta: &'a mut Metadata, } impl Look<'_, R> { /// Where an address leads, or a note saying it leads nowhere it may. fn locate(&mut self, base: &str, href: &str) -> Option { + if self.spent { + return None; + } let Some(path) = resolve(base, href) else { self.meta.note(format!( "the cover's address, {href}, is not a path inside the archive" @@ -392,6 +417,11 @@ impl Look<'_, R> { /// The image a cover page shows, or the page itself where the guide /// points straight at an image. + /// + /// A page with several images may lead with a logo or an ornament, so + /// the one whose file name, `alt`, `id` or `class` says cover is taken + /// over the first; with none of them saying so, the first is taken, and + /// a note says the page had others. fn page(&mut self, base: &str, href: &str) -> Option { let path = self.locate(base, href)?; let bytes = self.fetch(&path, MAX_PACKAGE as usize, "cover page")?; @@ -399,23 +429,55 @@ impl Look<'_, R> { return self.check(&path, bytes); } let text = String::from_utf8_lossy(&bytes); - let Some(src) = tags(&text).find_map(|(name, tag)| match name { - "img" => attr(tag, "src"), - "image" => attr(tag, "xlink:href").or_else(|| attr(tag, "href")), - _ => None, - }) else { + let images: Vec<(String, bool)> = tags(&text) + .filter_map(|(name, tag)| { + let src = match name { + "img" => attr(tag, "src"), + "image" => attr(tag, "xlink:href").or_else(|| attr(tag, "href")), + _ => None, + }?; + let named = says_cover(stem(src.split('#').next().unwrap_or(""))) + || ["alt", "id", "class"] + .iter() + .any(|name| attr(tag, name).is_some_and(|value| says_cover(&value))); + Some((src, named)) + }) + .collect(); + let src = if let Some((src, _)) = images.iter().find(|(_, named)| *named) { + src + } else if let Some((first, _)) = images.first() { + if images.len() > 1 { + self.meta.note(format!( + "the cover page, {path}, shows {} images, and the first was taken", + images.len() + )); + } + first + } else { self.meta .note(format!("the cover page, {path}, shows no image")); return None; }; - self.image(directory_of(&path), &src) + self.image(directory_of(&path), src) } /// One member's bytes, read no further than `cap`. /// /// The size the archive declares is checked first and the read is - /// bounded anyway, because the declaration may be a lie. + /// bounded anyway, because the declaration may be a lie. No more than + /// [`MAX_READS`] members are read in all; the first asked for after + /// that ends the search, with a note. fn fetch(&mut self, path: &str, cap: usize, what: &str) -> Option> { + if self.spent { + return None; + } + if self.reads == MAX_READS { + self.spent = true; + self.meta.note(format!( + "the search for a cover stopped after reading {MAX_READS} members" + )); + return None; + } let file = match self.archive.by_name(path) { Ok(file) => file, Err(zip::result::ZipError::FileNotFound) => { @@ -434,6 +496,7 @@ impl Look<'_, R> { self.meta.note(too_large); return None; } + self.reads += 1; let mut bytes = Vec::new(); if file.take(cap as u64 + 1).read_to_end(&mut bytes).is_err() { self.meta @@ -480,6 +543,50 @@ fn directory_of(path: &str) -> &str { path.rsplit_once('/').map_or("", |(dir, _)| dir) } +/// A path's file name without its extension. +fn stem(path: &str) -> &str { + let name = path.rsplit_once('/').map_or(path, |(_, name)| name); + match name.rsplit_once('.') { + Some((stem, _)) if !stem.is_empty() => stem, + _ => name, + } +} + +/// Whether a name has `cover` in it as a word of its own, in any case. +/// +/// Words are split at anything not a letter or a digit and where a lower +/// case letter meets a capital, so `CoverImage`, `Book_COVER` and +/// `cover-thumb` say cover, and `discover` and `recovery` do not. +fn says_cover(name: &str) -> bool { + words(name) + .iter() + .any(|word| word.eq_ignore_ascii_case("cover")) +} + +/// The words of a name, as [`says_cover`] splits them. +fn words(name: &str) -> Vec<&str> { + let mut out = Vec::new(); + let mut start: Option = None; + let mut prev = ' '; + for (at, c) in name.char_indices() { + let begins = + c.is_alphanumeric() && (start.is_none() || (prev.is_lowercase() && c.is_uppercase())); + if !c.is_alphanumeric() || begins { + if let Some(from) = start.take() { + out.push(&name[from..at]); + } + } + if begins { + start = Some(at); + } + prev = c; + } + if let Some(from) = start { + out.push(&name[from..]); + } + out +} + /// Where an address written in a member in `base` leads inside the archive. /// /// The fragment comes off and percent-escapes are undone. An address with a @@ -523,7 +630,13 @@ fn percent_decode(text: &str) -> Option { let mut at = 0; while at < raw.len() { if raw[at] == b'%' { - let hex = std::str::from_utf8(raw.get(at + 1..at + 3)?).ok()?; + // Two hex digits and nothing else: `from_str_radix` would also + // take a sign, and read `%+f` as 15. + let hex = raw.get(at + 1..at + 3)?; + if !hex.iter().all(u8::is_ascii_hexdigit) { + return None; + } + let hex = std::str::from_utf8(hex).ok()?; out.push(u8::from_str_radix(hex, 16).ok()?); at += 3; } else { @@ -878,6 +991,147 @@ mod tests { assert_eq!(meta.cover.expect("a cover").bytes, JPEG); } + #[test] + fn a_name_says_cover_only_as_a_word_of_its_own() { + for yes in [ + "cover", + "CoverImage", + "Book_COVER", + "cover-thumb", + "frontCover", + "x.cover", + ] { + assert!(says_cover(yes), "{yes}"); + } + for no in [ + "discover-ch1", + "recovery-map", + "covers", + "Coverage", + "uncover", + "", + ] { + assert!(!says_cover(no), "{no}"); + } + assert_eq!(words("CoverImage_v2"), ["Cover", "Image", "v2"]); + } + + #[test] + fn a_name_that_only_contains_cover_is_not_a_cover() { + let package = opf( + r#" + "#, + "", + ); + let meta = book( + &package, + &[ + ("OEBPS/images/discover-ch1.png", PNG), + ("OEBPS/images/cover.jpg", JPEG), + ], + ); + let cover = meta.cover.expect("a cover"); + assert_eq!(cover.bytes, JPEG); + assert!(cover.guessed); + + let alone = opf( + r#""#, + "", + ); + let meta = book(&alone, &[("OEBPS/images/recovery-map.png", PNG)]); + assert_eq!(meta.cover, None); + assert!(meta.notes.is_empty(), "{:?}", meta.notes); + } + + #[test] + fn an_image_named_cover_outright_beats_one_listed_before_it() { + let package = opf( + r#" + "#, + "", + ); + let meta = book( + &package, + &[ + ("OEBPS/images/thumb.png", PNG), + ("OEBPS/images/Cover.jpg", JPEG), + ], + ); + let cover = meta.cover.expect("a cover"); + assert_eq!(cover.bytes, JPEG); + assert!(cover.guessed); + } + + #[test] + fn the_guide_cover_page_prefers_the_image_that_says_cover() { + let package = opf( + "", + r#""#, + ); + for art in [ + r#""#, + r#"Cover"#, + r#""#, + r#""#, + ] { + let page = format!(r#"{art}"#); + let meta = book( + &package, + &[ + ("OEBPS/text/cover.xhtml", page.as_bytes()), + ("OEBPS/images/logo.png", PNG), + ("OEBPS/images/front-cover.jpg", JPEG), + ], + ); + assert_eq!(meta.cover.expect(art).bytes, JPEG, "{art}"); + assert!(meta.notes.is_empty(), "{art}: {:?}", meta.notes); + } + } + + #[test] + fn a_guide_cover_page_with_several_unnamed_images_takes_the_first_and_says_so() { + let package = opf( + "", + r#""#, + ); + let page = br#""#; + let meta = book( + &package, + &[ + ("OEBPS/text/cover.xhtml", page), + ("OEBPS/images/logo.png", PNG), + ("OEBPS/images/art.jpg", JPEG), + ], + ); + assert_eq!(meta.cover.expect("a cover").bytes, PNG); + assert_eq!( + meta.notes, + ["the cover page, OEBPS/text/cover.xhtml, shows 2 images, and the first was taken"] + ); + } + + #[test] + fn a_search_for_a_cover_reads_no_more_than_its_budget() { + let count = MAX_READS + 4; + let manifest: String = (0..count) + .map(|n| format!(r#""#)) + .collect(); + let names: Vec = (0..count).map(|n| format!("OEBPS/c{n}.jpg")).collect(); + let members: Vec<(&str, &[u8])> = names + .iter() + .map(|name| (name.as_str(), &b"not an image"[..])) + .collect(); + let meta = book(&opf(&manifest, ""), &members); + assert_eq!(meta.cover, None); + let mut expected: Vec = (0..MAX_READS) + .map(|n| format!("the cover, OEBPS/c{n}.jpg, is not an image")) + .collect(); + expected.push(format!( + "the search for a cover stopped after reading {MAX_READS} members" + )); + assert_eq!(meta.notes, expected); + } + #[test] fn a_book_that_names_no_cover_has_none_and_says_nothing() { let package = opf( @@ -938,6 +1192,8 @@ mod tests { archive: &mut archive, items: &[], tried: Vec::new(), + reads: 0, + spent: false, meta: &mut meta, }; let got = look.fetch("big.jpg", cap, "cover").map(|bytes| bytes.len()); @@ -1030,6 +1286,8 @@ mod tests { Some("OEBPS/text/b.jpg") ); assert_eq!(resolve("", "a%2Fb.jpg").as_deref(), Some("a/b.jpg")); + assert_eq!(percent_decode("a%2fb%2F").as_deref(), Some("a/b/")); + assert_eq!(percent_decode("a%+fb"), None); for hostile in [ "../../c.jpg", "../../../etc/passwd", @@ -1038,6 +1296,9 @@ mod tests { "file:///etc/passwd", "%ZZ.jpg", "%ff.jpg", + "a%+fb", + "a%-1b", + "a% fb", "c%", "#only-a-fragment", "",