Merge pull request 'bibfetch: a cover whole to its end marker, and relative redirects with an address in the query (#4, #5)' (#13) from issues-3-6 into main

# Conflicts:
#	crates/bibfetch/src/source.rs

Cargo.lock

@@ -101,7 +101,7 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53"
 
 [[package]]
 name = "avplan"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -126,7 +126,7 @@ checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5"
 
 [[package]]
 name = "bibclean"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -143,7 +143,7 @@ dependencies = [
 
 [[package]]
 name = "bibfetch"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -196,7 +196,7 @@ dependencies = [
 
 [[package]]
 name = "bookmeta"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -210,7 +210,7 @@ dependencies = [
 
 [[package]]
 name = "booksniff"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -250,7 +250,7 @@ checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04"
 
 [[package]]
 name = "catena"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "bibclean",
  "bibfetch",
@@ -277,7 +277,7 @@ dependencies = [
 
 [[package]]
 name = "catena-cli"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "clap",
  "clap_mangen",
@@ -312,7 +312,7 @@ checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600"
 
 [[package]]
 name = "chunkmd"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -334,7 +334,7 @@ dependencies = [
 
 [[package]]
 name = "citefind"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "chunkmd",
@@ -399,7 +399,7 @@ dependencies = [
 
 [[package]]
 name = "colophon"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "bibclean",
  "catena-cli",
@@ -448,7 +448,7 @@ dependencies = [
 
 [[package]]
 name = "corpus-core"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "schemars",
  "serde",
@@ -492,7 +492,7 @@ dependencies = [
 
 [[package]]
 name = "cuekit"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "corpus-core",
  "regex",
@@ -504,7 +504,7 @@ dependencies = [
 
 [[package]]
 name = "cuenorm"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -514,7 +514,7 @@ dependencies = [
 
 [[package]]
 name = "cuepack"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -525,7 +525,7 @@ dependencies = [
 
 [[package]]
 name = "cueseg"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -612,7 +612,7 @@ dependencies = [
 
 [[package]]
 name = "emb"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -876,7 +876,7 @@ dependencies = [
 
 [[package]]
 name = "idnorm"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "bibclean",
  "catena-cli",
@@ -942,7 +942,7 @@ dependencies = [
 
 [[package]]
 name = "langid"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1022,7 +1022,7 @@ dependencies = [
 
 [[package]]
 name = "mdscrub"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1034,7 +1034,7 @@ dependencies = [
 
 [[package]]
 name = "mdsplit"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1113,7 +1113,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe"
 
 [[package]]
 name = "pdfgutter"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1126,7 +1126,7 @@ dependencies = [
 
 [[package]]
 name = "pdftriage"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1370,7 +1370,7 @@ checksum = "323c417e1d9665a65b263ec744ba09030cfb277e9daa0b018a4ab62e57bc8189"
 
 [[package]]
 name = "rrf"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1517,7 +1517,7 @@ dependencies = [
 
 [[package]]
 name = "seriesof"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",
@@ -1634,7 +1634,7 @@ checksum = "adb6935a6f5c20170eeceb1a3835a49e12e19d792f6dd344ccc76a985ca5a6ca"
 
 [[package]]
 name = "textkit"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "regex",
  "tiktoken-rs",
@@ -1724,7 +1724,7 @@ checksum = "fd3ca314f692efd6c868f8408f53fe444634a845f96c028b97d35f6a1f79f0ee"
 
 [[package]]
 name = "tocfind"
-version = "0.3.1"
+version = "0.5.0"
 dependencies = [
  "catena-cli",
  "clap",

Cargo.toml

@@ -3,39 +3,39 @@ resolver = "3"
 members = ["crates/*"]
 
 [workspace.package]
-version = "0.3.1"
+version = "0.5.0"
 edition = "2024"
 rust-version = "1.85"
 license = "MPL-2.0"
 repository = "https://git.pumpin.online/funkpower/catena"
 
 [workspace.dependencies]
-catena-cli = { path = "crates/catena-cli", version = "0.3.1" }
+catena-cli = { path = "crates/catena-cli", version = "0.5.0" }
-corpus-core = { path = "crates/corpus-core", version = "0.3.1" }
+corpus-core = { path = "crates/corpus-core", version = "0.5.0" }
-cuekit = { path = "crates/cuekit", version = "0.3.1" }
+cuekit = { path = "crates/cuekit", version = "0.5.0" }
-textkit = { path = "crates/textkit", version = "0.3.1" }
+textkit = { path = "crates/textkit", version = "0.5.0" }
-chunkmd = { path = "crates/chunkmd", version = "0.3.1" }
+chunkmd = { path = "crates/chunkmd", version = "0.5.0" }
-mdscrub = { path = "crates/mdscrub", version = "0.3.1" }
+mdscrub = { path = "crates/mdscrub", version = "0.5.0" }
-mdsplit = { path = "crates/mdsplit", version = "0.3.1" }
+mdsplit = { path = "crates/mdsplit", version = "0.5.0" }
-pdftriage = { path = "crates/pdftriage", version = "0.3.1" }
+pdftriage = { path = "crates/pdftriage", version = "0.5.0" }
-cuenorm = { path = "crates/cuenorm", version = "0.3.1" }
+cuenorm = { path = "crates/cuenorm", version = "0.5.0" }
-cueseg = { path = "crates/cueseg", version = "0.3.1" }
+cueseg = { path = "crates/cueseg", version = "0.5.0" }
-rrf = { path = "crates/rrf", version = "0.3.1" }
+rrf = { path = "crates/rrf", version = "0.5.0" }
-cuepack = { path = "crates/cuepack", version = "0.3.1" }
+cuepack = { path = "crates/cuepack", version = "0.5.0" }
-bibclean = { path = "crates/bibclean", version = "0.3.1" }
+bibclean = { path = "crates/bibclean", version = "0.5.0" }
-avplan = { path = "crates/avplan", version = "0.3.1" }
+avplan = { path = "crates/avplan", version = "0.5.0" }
-pdfgutter = { path = "crates/pdfgutter", version = "0.3.1" }
+pdfgutter = { path = "crates/pdfgutter", version = "0.5.0" }
-catena = { path = "crates/bindings", version = "0.3.1" }
+catena = { path = "crates/bindings", version = "0.5.0" }
-seriesof = { path = "crates/seriesof", version = "0.3.1" }
+seriesof = { path = "crates/seriesof", version = "0.5.0" }
-idnorm = { path = "crates/idnorm", version = "0.3.1" }
+idnorm = { path = "crates/idnorm", version = "0.5.0" }
-bibfetch = { path = "crates/bibfetch", version = "0.3.1" }
+bibfetch = { path = "crates/bibfetch", version = "0.5.0" }
-bookmeta = { path = "crates/bookmeta", version = "0.3.1" }
+bookmeta = { path = "crates/bookmeta", version = "0.5.0" }
-langid = { path = "crates/langid", version = "0.3.1" }
+langid = { path = "crates/langid", version = "0.5.0" }
-tocfind = { path = "crates/tocfind", version = "0.3.1" }
+tocfind = { path = "crates/tocfind", version = "0.5.0" }
-booksniff = { path = "crates/booksniff", version = "0.3.1" }
+booksniff = { path = "crates/booksniff", version = "0.5.0" }
-citefind = { path = "crates/citefind", version = "0.3.1" }
+citefind = { path = "crates/citefind", version = "0.5.0" }
-colophon = { path = "crates/colophon", version = "0.3.1" }
+colophon = { path = "crates/colophon", version = "0.5.0" }
-emb = { path = "crates/emb", version = "0.3.1" }
+emb = { path = "crates/emb", version = "0.5.0" }
 
 clap = { version = "4", features = ["derive"] }
 clap_mangen = "0.3"

crates/bibfetch/README.md

@@ -86,7 +86,7 @@ is a failure — nobody replied, and the identifier may be perfectly good.
 |---|---|---|
 | `doi` | Crossref | It assigns most scholarly DOIs |
 | `arxiv` | the arXiv | Its own register of preprints |
-| `isbn` | OpenLibrary's edition record, then its index, then Google Books | Books, without a key |
+| `isbn` | OpenLibrary's index, then its edition record, then Google Books | Books, without a key |
 
 One register is enough for a DOI or a preprint: the body that assigns the
 identifier is the only one that has it. A book is different. An ISBN is
@@ -97,6 +97,14 @@ names them. So the registers are asked in turn and the first complete answer
 wins. An incomplete one is kept in case nothing better comes, and a register
 being down is a failure only when every register is.
 
+OpenLibrary's index answers about the work, not the edition: its publishers,
+languages, years and page counts are every edition's at once, and the first
+of each is usually another edition's. So for a book it gives only the title,
+the authors and the subjects, and the `container`, `language`, `year` and
+`pages` of a record come from the edition record for that ISBN, or from
+Google Books when the edition record does not say. A field neither gives is
+left out rather than filled from the work.
+
 Google Books is asked last because it answers anonymous callers only until a
 daily quota runs out, after which it replies `429` to everyone sharing the
 address.
@@ -127,7 +135,8 @@ id is asked only when every form of the ISBN asked was answered with no cover; i
 asking by ISBN failed, that failure is the result.
 
 What comes back is checked before it is written: a JPEG or PNG by its bytes
-as well as by what the server says, no larger than ten megabytes, reached in at
+as well as by what the server says, whole to its end marker (`FF D9` for a
+JPEG, `IEND` for a PNG), no larger than ten megabytes, reached in at
 most three redirects, each over HTTPS to `covers.openlibrary.org`,
 `archive.org` or a host under `archive.org`, and never to an address by number.
 One request is made at a time, a `429` or `503` is waited out for as long as
@@ -172,6 +181,15 @@ title and perhaps a name. This asks which books a register has under those,
 and prints one record per candidate: Open Library's index first, and Google
 Books only when Open Library has nothing.
 
+Open Library's title search is strict. Given an author as well, it finds
+nothing for *The Ladder of Divine Ascent* and four editions of *Ladder of
+Divine Ascent*. So when the title as given finds nothing, the title without a
+leading "The", "A" or "An" is asked next, and then the title and author as one
+free-text query. Each is asked only when the one before found nothing, so a
+title that matches costs one request. The looser questions can bring back
+books that only share words with the title, which is one more reason the
+candidates are for the caller to judge.
+
 Each candidate is one edition. Open Library's index answers with works, and
 names the edition of each that best matches the search. The ISBN, publisher,
 language, year and cover are that edition's and no other's: a field the

crates/bibfetch/src/cover.rs

@@ -386,6 +386,23 @@ by ISBN every five minutes from one address"
     }
 }
 
+/// Whether an image's bytes run to its end marker: `FF D9` for a JPEG, which
+/// some encoders follow with zero padding, and the `IEND` chunk for a PNG,
+/// which is its last twelve bytes.
+fn ends_whole(media_type: &str, bytes: &[u8]) -> bool {
+    match media_type {
+        "image/jpeg" => {
+            let end = bytes.iter().rposition(|&b| b != 0).map_or(0, |at| at + 1);
+            end >= 5 && bytes[..end].ends_with(&[0xFF, 0xD9])
+        }
+        _ => {
+            bytes.len() >= 8 + 12
+                && bytes[bytes.len() - 12..].starts_with(&[0, 0, 0, 0])
+                && &bytes[bytes.len() - 8..bytes.len() - 4] == b"IEND"
+        }
+    }
+}
+
 /// Read and check a body that is meant to be a cover.
 fn read(mut response: ureq::http::Response<ureq::Body>, address: &str) -> Result<Answer, Error> {
     let declared = header(&response, "content-type")
@@ -438,6 +455,14 @@ fn read(mut response: ureq::http::Response<ureq::Body>, address: &str) -> Result
             "{address}: said {media_type} but is not one"
         )));
     }
+    // The start of an image is not an image. A body sent with no length,
+    // or a stub, can begin right and stop short; what ends it says it is
+    // whole.
+    if !ends_whole(media_type, &bytes) {
+        return Err(Error::Shape(format!(
+            "{address}: a {media_type} that stops before its end"
+        )));
+    }
     // No floor on the size. Open Library's stand-in for a missing cover is a
     // 43-byte, one-pixel GIF, and it serves that only when `default=false`
     // is left off: with it, as every address here has it, a missing cover is
@@ -512,7 +537,12 @@ fn origin(url: &str) -> Option<&str> {
 
 /// Where a `Location` points, read against the address that sent it.
 fn resolve(from: &str, location: &str) -> String {
-    if location.contains("://") {
+    // Absolute only when the scheme comes first: a relative address can
+    // carry another address in its query.
+    if location
+        .find("://")
+        .is_some_and(|at| !location[..at].contains(['/', '?', '#']))
+    {
         return location.to_string();
     }
     if let Some(rest) = location.strip_prefix("//") {
@@ -670,15 +700,19 @@ mod tests {
         .agent()
     }
 
+    /// A JPEG's start, filler, and its end marker, `len` bytes in all.
     fn jpeg(len: usize) -> Vec<u8> {
         let mut bytes = vec![0xFF, 0xD8, 0xFF, 0xE0];
-        bytes.resize(len, 0x11);
+        bytes.resize(len - 2, 0x11);
+        bytes.extend([0xFF, 0xD9]);
         bytes
     }
 
+    /// A PNG's signature, filler, and its `IEND` chunk, `len` bytes in all.
     fn png(len: usize) -> Vec<u8> {
         let mut bytes = b"\x89PNG\r\n\x1a\n".to_vec();
-        bytes.resize(len, 0x22);
+        bytes.resize(len - 12, 0x22);
+        bytes.extend(b"\0\0\0\0IEND\xAE\x42\x60\x82");
         bytes
     }
 
@@ -1170,6 +1204,38 @@ mod tests {
         assert_eq!(stub.asked(), [ISBN13, ISBN10]);
     }
 
+    #[test]
+    fn a_magic_number_alone_is_not_a_cover() {
+        let mut cut_jpeg = jpeg(4096);
+        cut_jpeg.truncate(4000);
+        let mut cut_png = png(4096);
+        cut_png.truncate(4000);
+        for (media_type, body) in [
+            ("image/jpeg", vec![0xFF, 0xD8, 0xFF]),
+            ("image/png", b"\x89PNG\r\n\x1a\n".to_vec()),
+            ("image/jpeg", cut_jpeg),
+            ("image/png", cut_png),
+        ] {
+            let stub = Stub::start(move |_, _| Reply::image(media_type, body.clone()));
+            let found = stub
+                .covers()
+                .fetch(&agent(), "9780306406157", None, CoverSize::Large)
+                .map(|cover| cover.map(|cover| cover.bytes.len()));
+            assert!(
+                matches!(found, Err(Error::Shape(_))),
+                "{media_type}: {found:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn a_jpeg_padded_after_its_end_marker_is_whole() {
+        let mut padded = jpeg(2048);
+        padded.extend([0, 0, 0]);
+        assert!(ends_whole("image/jpeg", &padded));
+        assert!(!ends_whole("image/jpeg", &[0xFF, 0xD8, 0xFF, 0, 0]));
+    }
+
     #[test]
     fn a_redirect_to_an_address_by_number_is_refused() {
         // A listener standing in for a service on the LAN; it only counts.
@@ -1217,6 +1283,15 @@ mod tests {
         assert_eq!(resolve(from, "//c.example/x"), "https://c.example/x");
         assert_eq!(resolve(from, "/x?y=1"), "https://a.example/x?y=1");
         assert_eq!(resolve(from, "x.jpg"), "https://a.example/b/isbn/x.jpg");
+        // An address in a relative one's query does not make it absolute.
+        assert_eq!(
+            resolve(from, "/view_archive.php?from=https://archive.org/x"),
+            "https://a.example/view_archive.php?from=https://archive.org/x"
+        );
+        assert_eq!(
+            resolve(from, "x?u=https://c.example/"),
+            "https://a.example/b/isbn/x?u=https://c.example/"
+        );
         assert_eq!(
             origin("http://127.0.0.1:8080/a?b"),
             Some("http://127.0.0.1:8080")

crates/bibfetch/src/lib.rs

@@ -265,11 +265,10 @@ mod merge_tests {
         let mut index = rec("openlibrary");
         index.title = Some("The First World War".into());
         index.authors = vec!["Michael Howard".into()];
-        index.year = Some(2007);
 
         edition.fill_from(index);
         assert_eq!(edition.authors, ["Michael Howard"]);
-        assert_eq!(edition.year, Some(2007));
+        assert_eq!(edition.language.as_deref(), Some("eng"));
         // The fuller title arrived first and is not displaced by the shorter.
         assert_eq!(
             edition.title.as_deref(),

crates/bibfetch/src/search.rs

@@ -40,7 +40,13 @@ const GOOGLE_BOOKS: &str = "https://www.googleapis.com/books/v1/volumes";
 /// named.
 ///
 /// Open Library's index first, and Google Books only when that answered with
-/// nothing. Once Google Books has been asked, its failure is the answer:
+/// nothing. Open Library's title search is strict: given an author too, it
+/// finds nothing for "The Ladder of Divine Ascent" that it finds for "Ladder
+/// of Divine Ascent". So when the title as given finds nothing, the title
+/// without a leading article is asked, and then the title and author as one
+/// free-text query, before Google Books is. Each later question is asked only
+/// when the one before it found nothing, and a looser one can return books
+/// that only share words with the title, which the caller judges. Once Google Books has been asked, its failure is the answer:
 /// [`Error::Limited`] when it refused for its quota, [`Error::Unreachable`]
 /// when it could not be reached, since saying no register has the title
 /// would be a claim no register made. Asks nothing when `limit` is zero; a
@@ -79,8 +85,14 @@ fn search_with(
         None => Err(Error::Unreachable(format!("{url}: answered 404"))),
     };
 
-    let open_library =
+    let mut from_open_library = Ok(Vec::new());
-        fetch(open_library_url(title, author, limit)).and_then(|body| open_library(&body, limit));
+    for url in open_library_urls(title, author, limit) {
+        from_open_library = fetch(url).and_then(|body| open_library(&body, limit));
+        if !from_open_library.as_ref().is_ok_and(Vec::is_empty) {
+            break;
+        }
+    }
+    let open_library = from_open_library;
     if open_library.as_ref().is_ok_and(|found| !found.is_empty()) {
         return open_library;
     }
@@ -141,7 +153,36 @@ fn refused(url: &str, status: u16) -> Option<Error> {
     })
 }
 
-/// Where to ask Open Library.
+/// Every question to put to Open Library, strictest first.
+fn open_library_urls(title: &str, author: Option<&str>, limit: usize) -> Vec<String> {
+    let mut urls = vec![open_library_url(title, author, limit)];
+    if let Some(rest) = without_article(title) {
+        urls.push(open_library_url(rest, author, limit));
+    }
+    let words = match author {
+        Some(author) => format!("{title} {author}"),
+        None => title.to_string(),
+    };
+    urls.push(format!(
+        "https://openlibrary.org/search.json?q={}&fields={OPEN_LIBRARY_FIELDS}&limit={limit}",
+        source::encode(&words),
+    ));
+    urls
+}
+
+/// A title without its leading English article, when it has one and
+/// something follows it.
+fn without_article(title: &str) -> Option<&str> {
+    let (first, rest) = title.split_once(char::is_whitespace)?;
+    let rest = rest.trim_start();
+    (["the", "a", "an"]
+        .iter()
+        .any(|article| first.eq_ignore_ascii_case(article))
+        && !rest.is_empty())
+    .then_some(rest)
+}
+
+/// Where to ask Open Library by title and author.
 fn open_library_url(title: &str, author: Option<&str>, limit: usize) -> String {
     let author = author
         .map(|author| format!("&author={}", source::encode(author)))
@@ -630,8 +671,19 @@ mod tests {
             recorded(&mut asked, &bodies),
         )
         .expect("candidates");
-        assert_eq!(asked.len(), 2);
+        // The title as given, without its article, as free text, then Google.
-        assert!(asked[1].starts_with("https://www.googleapis.com/books/v1/volumes?q="));
+        assert_eq!(asked.len(), 4, "{asked:?}");
+        assert!(
+            asked[1].contains("?title=Google%20Story&author=Vise&"),
+            "{}",
+            asked[1]
+        );
+        assert!(
+            asked[2].contains("?q=The%20Google%20Story%20Vise&"),
+            "{}",
+            asked[2]
+        );
+        assert!(asked[3].starts_with("https://www.googleapis.com/books/v1/volumes?q="));
 
         assert_eq!(found.len(), 2);
         let first = &found[0];
@@ -686,7 +738,8 @@ mod tests {
         ];
         let failed = search_with("Nothing", None, 5, recorded(&mut asked, &bodies));
         assert!(matches!(failed, Err(Error::Unreachable(_))), "{failed:?}");
-        assert_eq!(asked.len(), 2);
+        // By title, as free text, then Google.
+        assert_eq!(asked.len(), 3, "{asked:?}");
     }
 
     #[test]
@@ -765,8 +818,50 @@ mod tests {
         ];
         search_with("Anathem", Some("  "), 5, recorded(&mut asked, &bodies)).expect("ok");
         assert!(!asked[0].contains("author="), "{}", asked[0]);
-        assert!(!asked[1].contains("inauthor"), "{}", asked[1]);
+        assert!(asked[1].contains("?q=Anathem&"), "{}", asked[1]);
-        assert!(asked[1].contains("q=intitle%3AAnathem&"), "{}", asked[1]);
+        assert!(!asked[2].contains("inauthor"), "{}", asked[2]);
+        assert!(asked[2].contains("q=intitle%3AAnathem&"), "{}", asked[2]);
+    }
+
+    #[test]
+    fn a_leading_article_is_dropped_when_the_title_as_given_finds_nothing() {
+        let mut asked = Vec::new();
+        let mut answered = 0;
+        let found = search_with(
+            "The Ladder of Divine Ascent",
+            Some("John Climacus"),
+            5,
+            |url: &str| {
+                asked.push(url.to_string());
+                answered += 1;
+                Ok(Some(
+                    if answered == 2 { TOLSTOY } else { NOTHING }.to_string(),
+                ))
+            },
+        )
+        .expect("candidates");
+        assert!(!found.is_empty());
+        assert_eq!(
+            asked.len(),
+            2,
+            "nothing is asked after a question finds something"
+        );
+        assert!(
+            asked[1].contains("?title=Ladder%20of%20Divine%20Ascent&author=John%20Climacus&"),
+            "{}",
+            asked[1]
+        );
+    }
+
+    #[test]
+    fn only_a_leading_article_with_words_after_it_is_dropped() {
+        assert_eq!(without_article("The Republic"), Some("Republic"));
+        assert_eq!(without_article("an  Essay"), Some("Essay"));
+        assert_eq!(without_article("A"), None);
+        assert_eq!(without_article("The "), None);
+        assert_eq!(without_article("Theology"), None);
+        assert_eq!(without_article("Anathem"), None);
+        assert_eq!(open_library_urls("Anathem", None, 5).len(), 2);
     }
 
     #[test]
@@ -838,7 +933,9 @@ intitle:y intitle:z inauthor:O'Brien inauthor:Sons";
         let found = search_with("?!", Some("Someone"), 5, recorded(&mut asked, &bodies))
             .expect("an answer");
         assert!(found.is_empty());
-        assert_eq!(asked.len(), 1);
+        // By title and as free text; never Google.
+        assert_eq!(asked.len(), 2, "{asked:?}");
+        assert!(asked.iter().all(|url| url.contains("openlibrary.org")));
     }
 
     #[test]
@@ -859,13 +956,25 @@ intitle:y intitle:z inauthor:O'Brien inauthor:Sons";
         let found =
             search_with("The Google Story", None, 1, recorded(&mut asked, &bodies)).expect("ok");
         assert_eq!(found.len(), 1);
-        assert!(asked[0].ends_with("&limit=1"), "{}", asked[0]);
+        let google = asked.last().expect("Google was asked");
-        assert!(asked[1].ends_with("&maxResults=1"), "{}", asked[1]);
+        assert!(
+            asked[..asked.len() - 1]
+                .iter()
+                .all(|url| url.ends_with("&limit=1")),
+            "{asked:?}"
+        );
+        assert!(google.ends_with("&maxResults=1"), "{google}");
 
         let mut asked = Vec::new();
         search_with("The Google Story", None, 500, recorded(&mut asked, &bodies)).expect("ok");
-        assert!(asked[0].ends_with("&limit=40"), "{}", asked[0]);
+        let google = asked.last().expect("Google was asked");
-        assert!(asked[1].ends_with("&maxResults=40"), "{}", asked[1]);
+        assert!(
+            asked[..asked.len() - 1]
+                .iter()
+                .all(|url| url.ends_with("&limit=40")),
+            "{asked:?}"
+        );
+        assert!(google.ends_with("&maxResults=40"), "{google}");
 
         let mut asked = Vec::new();
         let found =

crates/bibfetch/src/source.rs

@@ -17,9 +17,10 @@ pub enum Source {
     Arxiv,
     /// OpenLibrary, for books by ISBN.
     OpenLibrary,
-    /// OpenLibrary's index, rather than its edition record. Asked when the
+    /// OpenLibrary's index, rather than its edition record. Asked for what is
-    /// edition record has the book but not who wrote it, which is often: an
+    /// true of the work — the title, the authors by name, the subjects —
-    /// edition names its authors by key, and the index names them.
+    /// because an edition names its authors by key and the index names them.
+    /// Never for what differs between editions.
     OpenLibrarySearch,
     /// Google Books, for books by ISBN. Asked last, because it answers
     /// anonymous callers only until a daily quota runs out.
@@ -55,14 +56,14 @@ impl Source {
         match kind {
             "doi" => &[Source::Crossref],
             "arxiv" => &[Source::Arxiv],
-            // The index first. Asked for the fields by name it answers with
+            // The index first, for what is true of the work: the title, the
-            // the title, the authors, the publisher, the language, a page
+            // authors by name, the subjects. Then the edition record, for
-            // count and the subjects in one request, which is the whole of
+            // what is true of this edition only: the publisher, the language,
-            // what a catalogue wants; the edition record is then needed only
+            // the year and the length. The index answers about the work, and
-            // for a subtitle or a publisher the index did not give. Asking it
+            // its publishers, languages and years are every edition's at
-            // second cost three requests per book where one usually does, and
+            // once, so they are not asked of it; the lookup goes on to the
-            // at three books in flight that is the difference between two
+            // edition record for them. Google Books last, for whatever the
-            // days and two hours.
+            // edition record does not say.
             "isbn" => &[
                 Source::OpenLibrarySearch,
                 Source::OpenLibrary,
@@ -83,15 +84,16 @@ impl Source {
             Source::OpenLibrary => {
                 format!("https://openlibrary.org/isbn/{}.json", encode(id))
             }
-            // `fields` or the index answers with a handful of defaults and
+            // Only what is true of the work. Not `publisher`, `language`,
-            // keeps the rest, which is how a language it holds went unread.
+            // `first_publish_year` or `number_of_pages_median`: those are every
-            // Not `cover_i`: that is the cover chosen for the whole work, and
+            // edition's at once, and the first of each is usually another
-            // on a work with hundreds of editions it is usually another
+            // edition's (the Penguin *War and Peace* came back Spanish, from
-            // edition's, in another translation or language.
+            // W.W. Norton, in 1845). Not `cover_i`: that is the cover chosen
+            // for the whole work, often another edition's in another
+            // translation or language.
             Source::OpenLibrarySearch => format!(
                 "https://openlibrary.org/search.json?q=isbn:{}\
-&fields=title,subtitle,author_name,first_publish_year,publisher,language,subject,\
+&fields=title,subtitle,author_name,subject&limit=1",
-number_of_pages_median&limit=1",
                 encode(id)
             ),
             Source::GoogleBooks => format!(
@@ -295,6 +297,8 @@ struct SearchReply {
     docs: Vec<SearchDoc>,
 }
 
+/// What the index says about a work. Only the fields true of every edition
+/// are read, so an answer that carries the others anyway cannot leak them.
 #[derive(Deserialize)]
 struct SearchDoc {
     #[serde(default)]
@@ -304,15 +308,7 @@ struct SearchDoc {
     #[serde(default)]
     author_name: Vec<String>,
     #[serde(default)]
-    first_publish_year: Option<i64>,
-    #[serde(default)]
-    publisher: Vec<String>,
-    #[serde(default)]
-    language: Vec<String>,
-    #[serde(default)]
     subject: Vec<String>,
-    #[serde(rename = "number_of_pages_median", default)]
-    pages: Option<u32>,
 }
 
 /// How many of a work's subject headings are worth carrying.
@@ -324,6 +320,11 @@ pub(crate) const SUBJECT_LIMIT: usize = 12;
 
 /// A search that matched nothing answers with no documents rather than a
 /// status, so an empty reply is the absence and not a failure.
+///
+/// The record says what is true of the work and nothing that belongs to one
+/// edition: no publisher, language, year or page count. Those come from the
+/// edition record, which [`crate::ask::lookup`] asks next because a record
+/// without them is never complete.
 fn open_library_search(id: &str, kind: &str, body: &str) -> Result<Record, Error> {
     let reply: SearchReply = serde_json::from_str(body).map_err(|e| Error::Shape(e.to_string()))?;
     let Some(doc) = reply.docs.into_iter().next() else {
@@ -336,14 +337,10 @@ fn open_library_search(id: &str, kind: &str, body: &str) -> Result<Record, Error
         (title, _) => title,
     };
     record.authors = doc.author_name;
-    record.year = doc.first_publish_year;
-    record.container = doc.publisher.into_iter().next();
-    record.language = doc.language.into_iter().next();
     // The index lists every subject any edition of the work carries, which
     // runs to hundreds on a well-known book. A shelf needs a handful.
     record.subjects = doc.subject;
     record.subjects.truncate(SUBJECT_LIMIT);
-    record.pages = doc.pages;
     Ok(record)
 }
 
@@ -475,9 +472,8 @@ mod tests {
     fn each_kind_goes_to_the_body_that_assigns_it() {
         assert_eq!(Source::for_kind("doi"), Some(Source::Crossref));
         assert_eq!(Source::for_kind("arxiv"), Some(Source::Arxiv));
-        // The index, not the edition record: it answers with the whole of
+        // The index first, for the authors by name; the edition record names
-        // what a catalogue wants in one request, where the edition record
+        // them by key, and is asked next for what only the edition knows.
-        // takes up to twenty-eight seconds and names its authors by key.
         assert_eq!(Source::for_kind("isbn"), Some(Source::OpenLibrarySearch));
         // No register answers about an ISSN without a key.
         assert_eq!(Source::for_kind("issn"), None);
@@ -599,6 +595,96 @@ mod tests {
         assert!(!Source::OpenLibrarySearch.url("1").contains("cover_i"));
     }
 
+    /// What the index said for ISBN 9780140447934, the Penguin Classics *War
+    /// and Peace*, on 2026-11-13, asked for the fields the lookup asked for
+    /// before #6: the publishers, languages and first year of every edition
+    /// of the work.
+    const WAR_AND_PEACE_INDEX: &str = r#"{"docs":[{"title":"War and Peace",
+        "author_name":["Leo Tolstoy"],"first_publish_year":1845,
+        "publisher":["W.W. Norton","IndyPublish.com","Canterbury Classics"],
+        "language":["spa","gre","dut","eng"],"subject":["Fiction"],
+        "number_of_pages_median":1296}]}"#;
+
+    /// The edition record for the same ISBN, `/isbn/9780140447934.json`, with
+    /// the English language its `languages` field should name.
+    const WAR_AND_PEACE_EDITION: &str = r#"{"title":"War and Peace (Penguin Classics)",
+        "publishers":["Penguin Books"],"publish_date":"September 27, 2007",
+        "languages":[{"key":"/languages/eng"}],"number_of_pages":1440}"#;
+
+    /// Index first, then the edition record, merged as `ask::lookup` merges.
+    fn looked_up(edition: &str) -> Record {
+        let mut kept =
+            open_library_search("9780140447934", "isbn", WAR_AND_PEACE_INDEX).expect("a record");
+        kept.fill_from(open_library("9780140447934", "isbn", edition).expect("a record"));
+        kept
+    }
+
+    #[test]
+    fn the_index_says_nothing_only_an_edition_can() {
+        let record =
+            open_library_search("9780140447934", "isbn", WAR_AND_PEACE_INDEX).expect("a record");
+        // What is true of the work is kept.
+        assert_eq!(record.title.as_deref(), Some("War and Peace"));
+        assert_eq!(record.authors, ["Leo Tolstoy"]);
+        // What belongs to one edition is not taken from all of them.
+        assert_eq!(record.language, None);
+        assert_eq!(record.container, None);
+        assert_eq!(record.year, None);
+        assert_eq!(record.pages, None);
+    }
+
+    #[test]
+    fn a_lookup_by_isbn_gives_the_editions_language_publisher_and_year() {
+        let record = looked_up(WAR_AND_PEACE_EDITION);
+        assert_eq!(record.language.as_deref(), Some("eng"));
+        assert_eq!(record.container.as_deref(), Some("Penguin Books"));
+        assert_eq!(record.year, Some(2007));
+        assert_eq!(record.pages, Some(1440));
+        assert_eq!(record.authors, ["Leo Tolstoy"]);
+    }
+
+    #[test]
+    fn what_the_edition_does_not_say_is_missing_rather_than_the_works() {
+        // The real edition record today names no language at all.
+        let record = looked_up(
+            r#"{"title":"War and Peace (Penguin Classics)","publishers":["Penguin Books"],
+            "publish_date":"September 27, 2007","number_of_pages":1440}"#,
+        );
+        assert_eq!(record.language, None);
+        assert_eq!(record.container.as_deref(), Some("Penguin Books"));
+        assert_eq!(record.year, Some(2007));
+        // Nor a date: the work's first year is not this edition's.
+        let record = looked_up(r#"{"title":"War and Peace (Penguin Classics)"}"#);
+        assert_eq!(record.year, None);
+        assert_eq!(record.container, None);
+        assert_eq!(record.pages, None);
+    }
+
+    #[test]
+    fn the_index_alone_never_ends_a_lookup() {
+        // `lookup` stops at the first complete record; the index must not be
+        // one, or the edition record is never asked.
+        let record =
+            open_library_search("9780140447934", "isbn", WAR_AND_PEACE_INDEX).expect("a record");
+        assert!(!record.complete());
+    }
+
+    #[test]
+    fn the_index_is_not_asked_for_what_its_editions_differ_in() {
+        let url = Source::OpenLibrarySearch.url("9780140447934");
+        for field in [
+            "publisher",
+            "language",
+            "first_publish_year",
+            "number_of_pages_median",
+        ] {
+            assert!(
+                !url.split(['=', ',', '&']).any(|f| f == field),
+                "{field} in {url}"
+            );
+        }
+    }
+
     #[test]
     fn an_answer_that_is_not_what_it_should_be_says_so() {
         assert!(matches!(

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>`
+3. The guide's `<reference type="cover">`, a page, and the `<img src>` or
-   or SVG `<image href>` on it. A guide that points straight at an image is
+   SVG `<image href>` on it. Where the page has several, the one whose file
-   taken at its word.
+   name, `alt`, `id` or `class` says `cover` is taken; where none does, the
-4. Any image in the manifest whose id or address says `cover`.
+   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.
+    // Last: an image whose id or file name has the word cover in it, one
-    for item in items.iter().filter(|item| {
+    // 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/")
-            && (item.id.to_lowercase().contains("cover")
+                && (says_cover(&item.id) || says_cover(&file_stem(item)))
-                || item.href.to_lowercase().contains("cover"))
+        })
-    }) {
+        .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 {
+        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,
-        }) else {
+                }?;
+                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 one FictionBook — the `.fb2.zip` most of them are passed around as |
+| `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 |
+| `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` |
 | `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 |
+| `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 |
+| `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 |
 | `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,8 +110,17 @@ 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. Only names are
+and the zip is not a comic, and says what it holds instead.
-read, out of the archive's directory, so no member is ever unpacked.
+
+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`.
 
 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,9 +9,22 @@
 //! 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";
 
@@ -21,9 +34,41 @@ 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);
@@ -146,6 +191,60 @@ 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,7 +36,10 @@ 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_NOISE: &[&str] = &["comicinfo.xml"];
+const COMIC_CATALOGUE: &str = "comicinfo.xml";
+
+/// The extensions that say an archive is a comic book.
+const COMIC_EXTENSIONS: &[&str] = &["cbz", "cbr"];
 
 /// The extensions of the images a comic book archive is made of.
 const PAGE_IMAGES: &[&str] = &[".jpg", ".jpeg", ".png", ".gif", ".webp"];
@@ -125,9 +128,24 @@ fn archive(source: &impl Bytes, doc: &mut Doc) -> Kind {
         doc.note(format!("a zip holding one FictionBook, {fb2:?}"));
         return Kind::ZipWrappedFb2;
     }
-    if is_comic(&names) {
+    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 lower
         .iter()
         .any(|name| name.ends_with(".htm") || name.ends_with(".html") || name.ends_with(".xhtml"))
@@ -160,23 +178,39 @@ fn sole_member<'a>(names: &'a [String], extension: &str) -> Option<&'a str> {
     }
 }
 
-/// Whether an archive is the pages of a comic and nothing else.
+/// 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.
 ///
 /// 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.
+/// pages makes it something else, and saying what would be a guess. Images
-fn is_comic(names: &[String]) -> bool {
+/// are known by their names alone: only the archive's directory is read.
+fn comic_pages(names: &[String]) -> Option<Pages> {
+    let mut catalogued = false;
     let pages: Vec<String> = real_members(names)
         .into_iter()
         .map(|name| name.to_lowercase())
         .filter(|name| {
             let file = name.rsplit('/').next().unwrap_or(name);
-            !COMIC_NOISE.contains(&file)
+            let catalogue = file == COMIC_CATALOGUE;
+            catalogued |= catalogue;
+            !catalogue
         })
         .collect();
-    !pages.is_empty()
+    let all_images = !pages.is_empty()
         && pages
             .iter()
-            .all(|name| PAGE_IMAGES.iter().any(|image| name.ends_with(image)))
+            .all(|name| PAGE_IMAGES.iter().any(|image| name.ends_with(image)));
+    all_images.then_some(Pages {
+        count: pages.len(),
+        catalogued,
+    })
 }
 
 /// The members that carry content: no directories, no desktop clutter.
@@ -198,7 +232,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 lead.starts_with(RTF) {
+    if is_rtf(lead) {
         return Kind::Rtf;
     }
     if fb2::is_fictionbook(head) {
@@ -207,6 +241,17 @@ 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() {
@@ -432,10 +477,23 @@ 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");
@@ -471,6 +529,28 @@ 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");
@@ -526,11 +606,55 @@ mod tests {
     }
 
     #[test]
-    fn a_comic_saved_as_a_zip_is_remarked_on() {
+    fn a_zip_of_photographs_is_not_a_comic() {
-        let doc = archive_of(&["001.jpg", "002.jpg"], "issue-1.zip");
+        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");
         assert_eq!(doc.kind, Kind::Cbz);
         assert!(
-            doc.notes.iter().any(|note| note.contains("comic")),
+            !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
         );