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

@@ -130,10 +130,18 @@ An EPUB's is the first of these that reaches an image:
 2. The item `<meta name="cover" content="…">` names (EPUB 2). Some
    converters write the image's address there rather than its id, and that is
    followed too.
-3. The guide's `<reference type="cover">`, a page, and the first `<img src>`
-   or SVG `<image href>` 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 `<reference type="cover">`, a page, and the `<img src>` or
+   SVG `<image href>` 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

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<R: std::io::Read + std::io::Seek>(
         archive,
         items: &items,
         tried: Vec::new(),
+        reads: 0,
+        spent: false,
         meta,
     };
 
@@ -344,12 +353,21 @@ fn cover<R: std::io::Read + std::io::Seek>(
         }
     }
 
-    // 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<R>,
     items: &'a [Item],
     tried: Vec<String>,
+    /// 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<R: std::io::Read + std::io::Seek> Look<'_, R> {
     /// Where an address leads, or a note saying it leads nowhere it may.
     fn locate(&mut self, base: &str, href: &str) -> Option<String> {
+        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<R: std::io::Read + std::io::Seek> 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<Cover> {
         let path = self.locate(base, href)?;
         let bytes = self.fetch(&path, MAX_PACKAGE as usize, "cover page")?;
@@ -399,23 +429,55 @@ impl<R: std::io::Read + std::io::Seek> 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<Vec<u8>> {
+        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<R: std::io::Read + std::io::Seek> 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<usize> = 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<String> {
     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#"<item id="ch1" href="images/discover-ch1.png" media-type="image/png"/>
+               <item id="i2" href="images/cover.jpg" media-type="image/jpeg"/>"#,
+            "",
+        );
+        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#"<item id="map" href="images/recovery-map.png" media-type="image/png"/>"#,
+            "",
+        );
+        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#"<item id="cover-thumb" href="images/thumb.png" media-type="image/png"/>
+               <item id="i2" href="images/Cover.jpg" media-type="image/jpeg"/>"#,
+            "",
+        );
+        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#"<guide><reference type="cover" href="text/cover.xhtml"/></guide>"#,
+        );
+        for art in [
+            r#"<img src="../images/front-cover.jpg"/>"#,
+            r#"<img alt="Cover" src="../images/front-cover.jpg"/>"#,
+            r#"<img id="coverImage" src="../images/front-cover.jpg"/>"#,
+            r#"<img class="full cover" src="../images/front-cover.jpg"/>"#,
+        ] {
+            let page = format!(r#"<html><body><img src="../images/logo.png"/>{art}</body></html>"#);
+            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#"<guide><reference type="cover" href="text/cover.xhtml"/></guide>"#,
+        );
+        let page = br#"<html><body><img src="../images/logo.png"/><img src="../images/art.jpg"/></body></html>"#;
+        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#"<item id="cover-{n}" href="c{n}.jpg" media-type="image/jpeg"/>"#))
+            .collect();
+        let names: Vec<String> = (0..count).map(|n| format!("OEBPS/c{n}.jpg")).collect();
+        let members: Vec<(&str, &[u8])> = names
+            .iter()
+            .map(|name| (name.as_str(), &b"<html>not an image</html>"[..]))
+            .collect();
+        let meta = book(&opf(&manifest, ""), &members);
+        assert_eq!(meta.cover, None);
+        let mut expected: Vec<String> = (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",
             "",

crates/booksniff/README.md

@@ -89,12 +89,12 @@ one bad file does not stop a sweep.
 | `ole2` | The compound-file header every binary Word document starts with |
 | `epub` | A zip holding `mimetype` and `META-INF/container.xml` |
 | `zip-wrapped-pdf` | A zip whose only real member is one PDF |
-| `zip-wrapped-fb2` | A zip whose only real member is named `.fb2` — the `.fb2.zip` most FictionBooks are passed around as |
-| `cbz` | A zip whose every real member is a page image (JPEG, PNG, GIF or WebP), and which carries a `ComicInfo.xml` or is itself named `.cbz` or `.cbr` |
+| `zip-wrapped-fb2` | A zip whose only real member is one FictionBook — the `.fb2.zip` most of them are passed around as |
+| `cbz` | A zip whose every real member is a page image: JPEG, PNG, GIF or WebP |
 | `azw3`, `mobi` | A Palm database tagged `BOOKMOBI`, told apart by the format version inside the first record |
 | `lit` | `ITOLITLS`, Microsoft Reader's signature |
-| `rtf` | `{\rtf` at the front, after any byte-order mark and blank space, followed by the version digit, a space or the next control word |
-| `fb2` | XML whose root element is `FictionBook`, found by stepping over the declaration, comments and doctype in the first four kilobytes; UTF-16 with a byte-order mark is decoded first |
+| `rtf` | `{\rtf` at the front, after any byte-order mark and blank space |
+| `fb2` | XML whose root element is `FictionBook`, found by stepping over the declaration, comments and doctype in the first four kilobytes |
 | `html` | Markup at the front, after any byte-order mark |
 | `txt` | Text that is none of the above |
 | `unknown` | Anything else, with a note saying what was seen |
@@ -110,17 +110,8 @@ kind is there to stop exactly that.
 A comic is the pages and nothing else. Directories, the `ComicInfo.xml` a
 comic reader writes into the archive, and the desktop clutter every archive
 picks up are passed over; anything more — one PDF among the pages, a readme —
-and the zip is not a comic, and says what it holds instead.
-
-A zip of pictures is as often a holiday as a comic, so images alone are not
-enough. It is a comic when it carries a `ComicInfo.xml`, or when its own name
-ends `.cbz` or `.cbr`. Otherwise it is `unknown`, with a note saying it is a
-zip of so many images and would be a comic book if its name said `.cbz`.
-
-Members are judged by name. Only the archive's directory is read, so no
-member is ever unpacked and a page called `001.jpg` is taken to be an image
-without its bytes being looked at. The same goes for a wrapped FictionBook:
-the one member is taken to be a FictionBook because its name ends `.fb2`.
+and the zip is not a comic, and says what it holds instead. Only names are
+read, out of the archive's directory, so no member is ever unpacked.
 
 FictionBook is read as bytes rather than as text. A great many are written in
 windows-1251, which spells its markup in ASCII but is not UTF-8, and would

crates/booksniff/src/fb2.rs

@@ -9,22 +9,9 @@
 //! windows-1251, which is not UTF-8 but spells its markup in ASCII all the
 //! same, and a reader that insisted on UTF-8 would call every one of them
 //! binary.
-//!
-//! A FictionBook saved as UTF-16 is the one case where the markup is not
-//! ASCII bytes. Its byte-order mark says so, and the front is decoded before
-//! the same walk is made over it.
 
 use crate::BOM;
 
-/// The UTF-16 byte-order mark, little-endian.
-const UTF16_LE: &[u8] = b"\xff\xfe";
-/// The UTF-16 byte-order mark, big-endian.
-const UTF16_BE: &[u8] = b"\xfe\xff";
-
-/// Most UTF-16 code units decoded from the front: the same window, in
-/// characters, as the bytes read for every other encoding.
-const UTF16_UNITS: usize = 2048;
-
 /// What the root element of a FictionBook is called.
 const ROOT: &[u8] = b"FictionBook";
 
@@ -34,41 +21,9 @@ const ROOT: &[u8] = b"FictionBook";
 /// does not begin inside it is not found, so the answer is no rather than a
 /// guess.
 pub fn is_fictionbook(head: &[u8]) -> bool {
-    match utf16(head) {
-        Some(decoded) => is_fictionbook_root(decoded.as_bytes()),
-        None => is_fictionbook_root(head),
-    }
-}
-
-fn is_fictionbook_root(head: &[u8]) -> bool {
     root_element(head).is_some_and(|name| local_name(name) == ROOT)
 }
 
-/// The front of a UTF-16 file as UTF-8, if a byte-order mark says it is one.
-///
-/// Only a bounded prefix is decoded. A half character at the end, or a
-/// surrogate without its pair, becomes a replacement character, which the
-/// walk takes as part of a name and so never as the end of one.
-fn utf16(head: &[u8]) -> Option<String> {
-    let (body, big_endian) = match head.strip_prefix(UTF16_LE) {
-        Some(body) => (body, false),
-        None => (head.strip_prefix(UTF16_BE)?, true),
-    };
-    let units = body.chunks_exact(2).take(UTF16_UNITS).map(|pair| {
-        let pair = [pair[0], pair[1]];
-        if big_endian {
-            u16::from_be_bytes(pair)
-        } else {
-            u16::from_le_bytes(pair)
-        }
-    });
-    Some(
-        char::decode_utf16(units)
-            .map(|c| c.unwrap_or(char::REPLACEMENT_CHARACTER))
-            .collect(),
-    )
-}
-
 /// The name of the first element, after the prolog.
 fn root_element(head: &[u8]) -> Option<&[u8]> {
     let mut rest = head.strip_prefix(BOM).unwrap_or(head);
@@ -191,60 +146,6 @@ mod tests {
         assert!(!is_fictionbook(b"<!-- never closed <FictionBook>"));
     }
 
-    fn utf16(text: &str, big_endian: bool) -> Vec<u8> {
-        let mut out = if big_endian {
-            UTF16_BE.to_vec()
-        } else {
-            UTF16_LE.to_vec()
-        };
-        for unit in text.encode_utf16() {
-            out.extend_from_slice(&if big_endian {
-                unit.to_be_bytes()
-            } else {
-                unit.to_le_bytes()
-            });
-        }
-        out
-    }
-
-    const UTF16_BOOK: &str = "<?xml version=\"1.0\" encoding=\"UTF-16\"?>\r\n\
-        <FictionBook xmlns=\"http://www.gribuser.ru/xml/fictionbook/2.0\">\
-        <description><book-title>Мельница</book-title>";
-
-    #[test]
-    fn a_little_endian_utf16_fictionbook_is_recognised() {
-        assert!(is_fictionbook(&utf16(UTF16_BOOK, false)));
-        assert!(!is_fictionbook(&utf16(
-            "<?xml version=\"1.0\"?><rss>",
-            false
-        )));
-    }
-
-    #[test]
-    fn a_big_endian_utf16_fictionbook_is_recognised() {
-        assert!(is_fictionbook(&utf16(UTF16_BOOK, true)));
-        assert!(!is_fictionbook(&utf16(
-            "<?xml version=\"1.0\"?><rss>",
-            true
-        )));
-    }
-
-    #[test]
-    fn utf16_read_in_the_wrong_order_is_not_a_fictionbook() {
-        let mut swapped = utf16(UTF16_BOOK, false);
-        swapped[..2].copy_from_slice(UTF16_BE);
-        assert!(!is_fictionbook(&swapped));
-    }
-
-    #[test]
-    fn a_utf16_root_the_window_cut_short_is_not_taken() {
-        let head = utf16("<FictionBook>", false);
-        for cut in 0..head.len() {
-            assert!(!is_fictionbook(&head[..cut]), "cut at {cut}");
-        }
-        assert!(is_fictionbook(&head));
-    }
-
     #[test]
     fn a_truncated_or_mangled_prolog_does_not_panic() {
         let head = b"\xef\xbb\xbf<?xml?><!--c--><!DOCTYPE x [ <!ENTITY a '>'> ] \"]\">\

crates/booksniff/src/lib.rs

@@ -36,10 +36,7 @@ const ARCHIVE_NOISE: &[&str] = &["__macosx/", ".ds_store", "thumbs.db"];
 
 /// What a comic book archive carries besides its pages: the catalogue entry
 /// comic readers write into it.
-const COMIC_CATALOGUE: &str = "comicinfo.xml";
-
-/// The extensions that say an archive is a comic book.
-const COMIC_EXTENSIONS: &[&str] = &["cbz", "cbr"];
+const COMIC_NOISE: &[&str] = &["comicinfo.xml"];
 
 /// The extensions of the images a comic book archive is made of.
 const PAGE_IMAGES: &[&str] = &[".jpg", ".jpeg", ".png", ".gif", ".webp"];
@@ -128,23 +125,8 @@ fn archive(source: &impl Bytes, doc: &mut Doc) -> Kind {
         doc.note(format!("a zip holding one FictionBook, {fb2:?}"));
         return Kind::ZipWrappedFb2;
     }
-    if let Some(pages) = comic_pages(&names) {
-        // A zip of pictures is as often a holiday as a comic, and only the
-        // names have been read. It is a comic when something says so: the
-        // catalogue entry a comic reader writes, or a name that claims it.
-        let named_comic = doc
-            .declared_ext
-            .as_deref()
-            .is_some_and(|ext| COMIC_EXTENSIONS.contains(&ext));
-        if pages.catalogued || named_comic {
-            return Kind::Cbz;
-        }
-        doc.note(format!(
-            "a zip of {} image{}, which would be a comic book if its name said .cbz",
-            pages.count,
-            if pages.count == 1 { "" } else { "s" }
-        ));
-        return Kind::Unknown;
+    if is_comic(&names) {
+        return Kind::Cbz;
     }
     if lower
         .iter()
@@ -178,39 +160,23 @@ fn sole_member<'a>(names: &'a [String], extension: &str) -> Option<&'a str> {
     }
 }
 
-/// The pages of an archive that is images and nothing else.
-struct Pages {
-    /// How many images there are.
-    count: usize,
-    /// Whether a comic reader's catalogue entry is there beside them.
-    catalogued: bool,
-}
-
-/// The pages, if an archive is images and nothing else.
+/// Whether an archive is the pages of a comic and nothing else.
 ///
 /// Every real member has to be an image. One PDF or one text file among the
-/// pages makes it something else, and saying what would be a guess. Images
-/// are known by their names alone: only the archive's directory is read.
-fn comic_pages(names: &[String]) -> Option<Pages> {
-    let mut catalogued = false;
+/// pages makes it something else, and saying what would be a guess.
+fn is_comic(names: &[String]) -> bool {
     let pages: Vec<String> = real_members(names)
         .into_iter()
         .map(|name| name.to_lowercase())
         .filter(|name| {
             let file = name.rsplit('/').next().unwrap_or(name);
-            let catalogue = file == COMIC_CATALOGUE;
-            catalogued |= catalogue;
-            !catalogue
+            !COMIC_NOISE.contains(&file)
         })
         .collect();
-    let all_images = !pages.is_empty()
+    !pages.is_empty()
         && pages
             .iter()
-            .all(|name| PAGE_IMAGES.iter().any(|image| name.ends_with(image)));
-    all_images.then_some(Pages {
-        count: pages.len(),
-        catalogued,
-    })
+            .all(|name| PAGE_IMAGES.iter().any(|image| name.ends_with(image)))
 }
 
 /// The members that carry content: no directories, no desktop clutter.
@@ -232,7 +198,7 @@ fn front(head: &[u8], doc: &mut Doc) -> Kind {
     // design but not always in practice, and a FictionBook is as often
     // windows-1251 as UTF-8.
     let lead = head.strip_prefix(BOM).unwrap_or(head).trim_ascii_start();
-    if is_rtf(lead) {
+    if lead.starts_with(RTF) {
         return Kind::Rtf;
     }
     if fb2::is_fictionbook(head) {
@@ -241,17 +207,6 @@ fn front(head: &[u8], doc: &mut Doc) -> Kind {
     text(head, doc)
 }
 
-/// Whether `lead` opens an RTF document.
-///
-/// `{\rtf` has to be the whole control word: what follows it is the version,
-/// normally `1`, or the space or next control word that ends it. `{\rtfoo` is
-/// some other word that happens to start the same way.
-fn is_rtf(lead: &[u8]) -> bool {
-    lead.strip_prefix(RTF)
-        .and_then(|rest| rest.first())
-        .is_some_and(|next| next.is_ascii_digit() || next.is_ascii_whitespace() || *next == b'\\')
-}
-
 /// Text, markup, or nothing this knows.
 fn text(head: &[u8], doc: &mut Doc) -> Kind {
     if head.is_empty() {
@@ -477,23 +432,10 @@ mod tests {
     #[test]
     fn rtf_cut_short_of_its_control_word_is_not_rtf() {
         assert_eq!(sniff(&br"{\rt".as_slice(), "a.rtf").kind, Kind::Txt);
-        assert_eq!(sniff(&br"{\rtf".as_slice(), "a.rtf").kind, Kind::Txt);
         assert_eq!(kind_of(br"{\rtx1", "a.rtf"), Kind::Txt);
         assert_eq!(kind_of(br"text {\rtf1", "a.rtf"), Kind::Txt);
     }
 
-    #[test]
-    fn rtf_is_the_whole_control_word() {
-        assert_eq!(kind_of(br"{\rtf\ansi\deff0", "a.rtf"), Kind::Rtf);
-        assert_eq!(kind_of(b"{\\rtf \\ansi", "a.rtf"), Kind::Rtf);
-        assert_eq!(kind_of(b"{\\rtf\r\n\\ansi", "a.rtf"), Kind::Rtf);
-        assert_eq!(kind_of(br"{\rtf2", "a.rtf"), Kind::Rtf);
-
-        assert_eq!(kind_of(br"{\rtfoo bar}", "a.rtf"), Kind::Txt);
-        assert_eq!(kind_of(br"{\rtf}", "a.rtf"), Kind::Txt);
-        assert_eq!(kind_of(br"{\rtf-1", "a.rtf"), Kind::Txt);
-    }
-
     #[test]
     fn rtf_named_as_a_word_document_is_remarked_on() {
         let doc = sniff(&padded(br"{\rtf1\ansi").as_slice(), "letter.doc");
@@ -529,28 +471,6 @@ mod tests {
         assert_eq!(kind_of(feed, "feed.xml"), Kind::Html);
     }
 
-    #[test]
-    fn a_utf16_fictionbook_is_recognised_in_either_byte_order() {
-        let book = "<?xml version=\"1.0\" encoding=\"UTF-16\"?>\n<FictionBook>";
-        for big_endian in [false, true] {
-            let mut bytes = if big_endian {
-                vec![0xfe, 0xff]
-            } else {
-                vec![0xff, 0xfe]
-            };
-            for unit in book.encode_utf16() {
-                bytes.extend_from_slice(&if big_endian {
-                    unit.to_be_bytes()
-                } else {
-                    unit.to_le_bytes()
-                });
-            }
-            let doc = sniff(&padded(&bytes).as_slice(), "mill.fb2");
-            assert_eq!(doc.kind, Kind::Fb2, "big-endian: {big_endian}");
-            assert!(doc.notes.is_empty(), "{:?}", doc.notes);
-        }
-    }
-
     #[test]
     fn a_fictionbook_named_as_something_else_is_remarked_on() {
         let doc = sniff(&padded(b"<FictionBook>").as_slice(), "mill.txt");
@@ -606,55 +526,11 @@ mod tests {
     }
 
     #[test]
-    fn a_zip_of_photographs_is_not_a_comic() {
-        let doc = archive_of(&["IMG_0001.jpg", "IMG_0002.jpg"], "holiday.zip");
-        assert_eq!(doc.kind, Kind::Unknown);
-        assert!(
-            doc.notes.iter().any(|note| note
-                == "a zip of 2 images, which would be a comic book if its name said .cbz"),
-            "{:?}",
-            doc.notes
-        );
-
-        let one = archive_of(&["IMG_0001.jpg"], "holiday.zip");
-        assert!(
-            one.notes
-                .iter()
-                .any(|note| note.starts_with("a zip of 1 image,")),
-            "{:?}",
-            one.notes
-        );
-    }
-
-    #[test]
-    fn the_same_images_named_as_a_comic_are_one() {
-        let doc = archive_of(&["IMG_0001.jpg", "IMG_0002.jpg"], "holiday.cbz");
+    fn a_comic_saved_as_a_zip_is_remarked_on() {
+        let doc = archive_of(&["001.jpg", "002.jpg"], "issue-1.zip");
         assert_eq!(doc.kind, Kind::Cbz);
         assert!(
-            !doc.notes.iter().any(|note| note.contains("named .")),
-            "{:?}",
-            doc.notes
-        );
-
-        // A comic zipped and named as if it were RAR is still a comic, and
-        // the name is remarked on.
-        let rar = archive_of(&["001.jpg", "002.jpg"], "issue-1.cbr");
-        assert_eq!(rar.kind, Kind::Cbz);
-        assert!(
-            rar.notes.iter().any(|note| note.contains("named .cbr")),
-            "{:?}",
-            rar.notes
-        );
-    }
-
-    #[test]
-    fn a_comic_saved_as_a_zip_is_known_by_its_catalogue_and_remarked_on() {
-        let doc = archive_of(&["001.jpg", "002.jpg", "ComicInfo.xml"], "issue-1.zip");
-        assert_eq!(doc.kind, Kind::Cbz);
-        assert!(
-            doc.notes
-                .iter()
-                .any(|note| note == "named .zip, but the bytes say a comic book archive"),
+            doc.notes.iter().any(|note| note.contains("comic")),
             "{:?}",
             doc.notes
         );