bibfetch: an edition's facts from its own record, and a cover whole to its end

A lookup by ISBN took the year, publisher, language and page count from
Open Library's index, which answers for the work and gathers them from
every edition: Penguin's English War and Peace came back Spanish,
Norton's, and 1845. The index now gives only the title, the authors and
the subjects, and the edition record the rest (#6).

A cover must run to its end marker, FF D9 for a JPEG and IEND for a PNG,
so a body that only starts like an image is refused (#4). A relative
redirect whose query holds another address is resolved as relative (#5).

Closes #4, #5, #6.

crates/bibfetch/README.md

@@ -86,15 +86,18 @@ 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
 assigned by a publisher and recorded by whoever gets round to it, so an
 answer that names the book but not who wrote it is common — OpenLibrary's
 edition record names its authors by key rather than by name, and its index
-names them. So the registers are asked in turn and the first complete answer
+names them. The index answers about the work, though, not the edition: the
-wins. An incomplete one is kept in case nothing better comes, and a register
+publishers, languages, years and page counts it holds belong to every edition
+at once, so only the title, the authors and the subjects are taken from it,
+and the rest comes from the edition record for that ISBN. 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.
 
 Google Books is asked last because it answers anonymous callers only until a
@@ -127,7 +130,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

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/source.rs

@@ -55,14 +55,13 @@ 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 whole work: the title,
-            // the title, the authors, the publisher, the language, a page
+            // the authors by name and the subjects. It answers about the
-            // count and the subjects in one request, which is the whole of
+            // work, not the edition, so its publishers, languages, years and
-            // what a catalogue wants; the edition record is then needed only
+            // page counts belong to every edition at once, and Penguin's
-            // for a subtitle or a publisher the index did not give. Asking it
+            // English War and Peace came back Spanish, Norton's, and 1845.
-            // second cost three requests per book where one usually does, and
+            // The edition record says those for this ISBN; it names its
-            // at three books in flight that is the difference between two
+            // authors only by key, which is why the index is still asked.
-            // days and two hours.
             "isbn" => &[
                 Source::OpenLibrarySearch,
                 Source::OpenLibrary,
@@ -90,8 +89,7 @@ impl Source {
             // 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!(
@@ -304,15 +302,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.
@@ -336,14 +326,14 @@ 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;
+    // Not the year, publisher, language or page count: the index gives them
-    record.container = doc.publisher.into_iter().next();
+    // for the work, gathered from every edition, and the edition record is
-    record.language = doc.language.into_iter().next();
+    // asked for this one's.
+    //
     // 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)
 }
 
@@ -594,6 +584,56 @@ mod tests {
         }
     }
 
+    #[test]
+    fn what_is_true_of_one_edition_comes_from_its_record_and_not_the_index() {
+        // ISBN 9780140447934, Penguin's English War and Peace, as the index
+        // answered on 2026-11-13: the work's languages, publishers and first
+        // year, gathered from every edition.
+        let index = r#"{"docs":[{"title":"War and Peace","author_name":["Leo Tolstoy"],
+            "first_publish_year":1845,"publisher":["W.W. Norton"],
+            "language":["spa","eng"],"number_of_pages_median":1296,
+            "subject":["Napoleonic Wars, 1800-1815"]}]}"#;
+        let alone = open_library_search("9780140447934", "isbn", index).expect("a record");
+        assert_eq!(alone.title.as_deref(), Some("War and Peace"));
+        assert_eq!(alone.authors, ["Leo Tolstoy"]);
+        assert_eq!(alone.subjects, ["Napoleonic Wars, 1800-1815"]);
+        assert_eq!(
+            (
+                alone.year,
+                alone.container.as_deref(),
+                alone.language.as_deref(),
+                alone.pages
+            ),
+            (None, None, None, None)
+        );
+        // Not complete, so `lookup` goes on to ask the edition record.
+        assert!(!alone.complete());
+
+        let edition = r#"{"title":"War and Peace","publishers":["Penguin Classics"],
+            "publish_date":"2007","languages":[{"key":"/languages/eng"}],
+            "number_of_pages":1392}"#;
+        let mut kept = alone;
+        kept.fill_from(open_library("9780140447934", "isbn", edition).expect("a record"));
+        assert_eq!(kept.year, Some(2007));
+        assert_eq!(kept.container.as_deref(), Some("Penguin Classics"));
+        assert_eq!(kept.language.as_deref(), Some("eng"));
+        assert_eq!(kept.pages, Some(1392));
+        assert_eq!(kept.authors, ["Leo Tolstoy"]);
+    }
+
+    #[test]
+    fn the_index_is_not_asked_for_what_belongs_to_editions() {
+        let url = Source::OpenLibrarySearch.url("1");
+        for field in [
+            "first_publish_year",
+            "publisher",
+            "language",
+            "number_of_pages",
+        ] {
+            assert!(!url.contains(field), "{field} in {url}");
+        }
+    }
+
     #[test]
     fn the_index_is_not_asked_for_the_works_cover() {
         assert!(!Source::OpenLibrarySearch.url("1").contains("cover_i"));