From 19e5f3ddc363a8ef82a120a8a5960d6e9804a0f6 Mon Sep 17 00:00:00 2001 From: funkpower Date: Fri, 13 Nov 2026 14:36:32 -0500 Subject: [PATCH 1/2] bibfetch: a book's cover from Open Library, by its ISBN `bibfetch::cover(&agent, isbn, cover_id, size)` asks Open Library's covers store by the ISBN's thirteen-digit form, then its ten-digit form, then by a cover id a lookup record named, and returns the image or `Ok(None)`. The answer is checked before it is trusted: JPEG or PNG by content type and by magic bytes, between 1 KB and 10 MB, at most three redirects, none of them off HTTPS. One request at a time per process; a 429 or 503 is waited out for its Retry-After, up to a minute. `idnorm::isbn` converts between the two forms, and only for an ISBN whose check digit agrees. Lookup records carry `cover_id` from the edition record and the search index. The CLI gains `bibfetch cover -o `. --- crates/bibfetch/README.md | 39 +- crates/bibfetch/src/cover.rs | 880 ++++++++++++++++++++++++++++++++++ crates/bibfetch/src/lib.rs | 14 + crates/bibfetch/src/main.rs | 105 +++- crates/bibfetch/src/source.rs | 38 +- crates/idnorm/src/isbn.rs | 99 ++++ crates/idnorm/src/lib.rs | 1 + 7 files changed, 1162 insertions(+), 14 deletions(-) create mode 100644 crates/bibfetch/src/cover.rs create mode 100644 crates/idnorm/src/isbn.rs diff --git a/crates/bibfetch/README.md b/crates/bibfetch/README.md index 2dcad41..a4e40f9 100644 --- a/crates/bibfetch/README.md +++ b/crates/bibfetch/README.md @@ -107,6 +107,40 @@ register that is rate-limiting, short enough that one which is simply down does not hold up a pipeline. An answer this cannot read is not retried, because it will not read any better next time. +## Covers + +```sh +bibfetch cover 978-0-306-40615-7 -o cover.jpg +``` + +Fetches a book's cover from Open Library's covers store by its ISBN and +writes the image. The thirteen-digit form is asked first and the ten-digit +form second, because a book is often filed under only one of them; an ISBN +whose check digit is wrong is asked about as written, since converting it +would give another book's number. `--cover-id` names an Open Library cover id +to ask by last, which is what the `cover_id` field of a lookup record holds. + +What comes back is checked before it is written: a JPEG or PNG by its bytes +as well as by what the server says, between one kilobyte (Open Library's +stand-in for a missing cover is smaller) and ten megabytes, reached in at most +three redirects that never leave HTTPS. One request is made at a time, and a +`429` or `503` is waited out for as long as its `Retry-After` says, up to a +minute. + +| Option | Effect | +|---|---| +| `-o`, `--output ` | Where to write the image | +| `--size ` | `small`, `medium` or `large`. Default `large` | +| `--cover-id ` | An Open Library cover id to ask by when the ISBN has none | + +Exit `0` the cover was written, `1` Open Library has none, `65` the argument +was not an ISBN or the answer was not an image, `69` Open Library could not be +reached, `74` the file could not be written. + +From Rust, `bibfetch::cover(&agent, isbn, cover_id, size)` returns +`Ok(None)` when there is no cover, and a [`Cover`](src/cover.rs) with the +bytes, the media type and the address asked when there is. + ## Reaching the network This and [`emb`](../emb/README.md) are the only two tools here that do. Both @@ -114,8 +148,9 @@ say so in `--help`, and for both it is the whole of what they do rather than something that happens on the way to something else — which is the condition under which [the contract](../../docs/contract.md) allows it at all. -Nothing is cached, nothing is stored, and nothing is sent but the identifier -and the address you pass to `--polite`. +Nothing is cached, nothing is stored but a cover you asked to have written, +and nothing is sent but the identifier and the address you pass to +`--polite`. ## See also diff --git a/crates/bibfetch/src/cover.rs b/crates/bibfetch/src/cover.rs new file mode 100644 index 0000000..2ed1626 --- /dev/null +++ b/crates/bibfetch/src/cover.rs @@ -0,0 +1,880 @@ +//! A book's cover, by its ISBN, from Open Library's covers store. +//! +//! Open Library keeps covers apart from its records, at +//! `covers.openlibrary.org`, and answers by ISBN or by the cover's own id. An +//! ISBN is asked in its thirteen-digit form first and its ten-digit form +//! second, because a book is often filed under only one of them; the cover id +//! a record already named is asked last, since it may be the cover of another +//! edition of the work rather than of this one. +//! +//! What comes back is checked before it is trusted, because it is written to +//! disk next to a book: an image (JPEG or PNG, by what the bytes are as well +//! as what the server says), no larger than [`MAX_BYTES`], reached in at most +//! [`MAX_REDIRECTS`] redirects that never leave HTTPS. + +use std::sync::Mutex; +use std::time::Duration; + +use crate::Error; + +/// Where Open Library keeps its covers. +pub const OPEN_LIBRARY_COVERS: &str = "https://covers.openlibrary.org"; + +/// The largest cover accepted. A large cover from Open Library is a few +/// hundred kilobytes; ten megabytes is something that is not a cover. +pub const MAX_BYTES: u64 = 10 * 1024 * 1024; + +/// The smallest cover accepted. Open Library's stand-in for a missing cover +/// is a one-pixel image of a few dozen bytes, and nothing under a kilobyte +/// is a picture of a book. +pub const MIN_BYTES: usize = 1024; + +/// How many redirects to follow. A cover is served from the Internet +/// Archive two redirects away from the address asked. +pub const MAX_REDIRECTS: u32 = 3; + +/// The longest a `Retry-After` is honoured for. A register asking for more +/// than this is better treated as down than waited on. +const MAX_WAIT: Duration = Duration::from_secs(60); + +/// One request at a time, from the whole process. Open Library asks for +/// this, and limits covers asked by ISBN to a hundred every five minutes +/// from one address; threads asking at once would only meet the limit +/// sooner and all back off together. +static ONE_AT_A_TIME: Mutex<()> = Mutex::new(()); + +/// Which of Open Library's three sizes to ask for. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum CoverSize { + /// A thumbnail, about 45 pixels wide. + Small, + /// About 180 pixels wide. + Medium, + /// The largest Open Library keeps, often the original scan. + #[default] + Large, +} + +impl CoverSize { + fn letter(self) -> char { + match self { + CoverSize::Small => 'S', + CoverSize::Medium => 'M', + CoverSize::Large => 'L', + } + } +} + +/// A cover, checked to be an image. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Cover { + /// The image. + pub bytes: Vec, + /// `image/jpeg` or `image/png`, which the bytes agree with. + pub media_type: &'static str, + /// Which register it came from. + pub source: &'static str, + /// The address asked, which is stable; the one that finally served the + /// image is an archive's storage location and is not. + pub url: String, +} + +impl Cover { + /// The file extension its media type is written with. + pub fn extension(&self) -> &'static str { + match self.media_type { + "image/png" => "png", + _ => "jpg", + } + } +} + +/// Fetch the cover of the book with this ISBN, or of the cover id a record +/// named, at the default settings. +/// +/// `Ok(None)` means Open Library has no cover for it. The agent decides the +/// timeout and the User-Agent; [`crate::ask::Ask::agent`] makes one. +pub fn cover( + agent: &ureq::Agent, + isbn: &str, + cover_id: Option, + size: CoverSize, +) -> Result, Error> { + Covers::default().fetch(agent, isbn, cover_id, size) +} + +/// Where to ask for covers and how hard to try. +#[derive(Debug, Clone)] +pub struct Covers { + /// The covers store, without a trailing slash. + pub base: String, + /// How many times to ask an address that is unreachable or busy. + pub attempts: u32, +} + +impl Default for Covers { + fn default() -> Self { + Self { + base: OPEN_LIBRARY_COVERS.to_string(), + attempts: crate::ATTEMPTS, + } + } +} + +/// What one address said, short of a failure. +enum Answer { + Image(Cover), + Absent, +} + +/// Why one attempt at an address failed. +enum Fault { + /// Worth asking again, after the wait the server asked for if it did. + Again(Error, Option), + /// Not worth asking again: the answer will be the same. + Final(Error), +} + +impl Covers { + /// Fetch a cover by ISBN, then by cover id. + /// + /// The ISBN may be either form, with or without hyphens. One whose check + /// digit is wrong is asked about as written and not converted, since a + /// converted one would be another book's number. A failure at one address + /// does not stop the next being asked; it is returned only when no + /// address gave a cover. + pub fn fetch( + &self, + agent: &ureq::Agent, + isbn: &str, + cover_id: Option, + size: CoverSize, + ) -> Result, Error> { + let digits = idnorm::isbn::digits(isbn); + if !matches!(digits.len(), 10 | 13) { + return Err(Error::Invalid("not an ISBN".into())); + } + let forms: Vec = match idnorm::isbn::to_13(&digits) { + Some(thirteen) => std::iter::once(thirteen) + .chain(idnorm::isbn::to_10(&digits)) + .collect(), + None => vec![digits], + }; + + let letter = size.letter(); + let mut addresses: Vec = forms + .iter() + .map(|form| format!("{}/b/isbn/{form}-{letter}.jpg?default=false", self.base)) + .collect(); + if let Some(id) = cover_id { + addresses.push(format!( + "{}/b/id/{id}-{letter}.jpg?default=false", + self.base + )); + } + + let _turn = ONE_AT_A_TIME + .lock() + .unwrap_or_else(|held| held.into_inner()); + let mut failed = None; + for address in addresses { + match self.ask(agent, &address) { + Ok(Answer::Image(cover)) => return Ok(Some(cover)), + Ok(Answer::Absent) => {} + Err(why) => failed = Some(why), + } + } + match failed { + Some(why) => Err(why), + None => Ok(None), + } + } + + /// Ask one address, again while it is busy or unreachable. + /// + /// Waits as long as a `Retry-After` says, up to a minute, or else a + /// second and then two, the same as a record lookup does. + fn ask(&self, agent: &ureq::Agent, address: &str) -> Result { + let attempts = self.attempts.max(1); + let mut last = Error::Unreachable("no attempt was made".into()); + for attempt in 0..attempts { + match self.once(agent, address) { + Ok(answer) => return Ok(answer), + Err(Fault::Final(why)) => return Err(why), + Err(Fault::Again(why, wait)) => { + last = why; + if attempt + 1 < attempts { + std::thread::sleep( + wait.unwrap_or(Duration::from_secs(1 << attempt.min(5))), + ); + } + } + } + } + Err(last) + } + + /// One attempt at an address, following its redirects by hand so that + /// each one can be checked before it is followed. + fn once(&self, agent: &ureq::Agent, address: &str) -> Result { + let mut at = address.to_string(); + let mut hops = 0; + loop { + let response = agent + .get(&at) + .config() + .max_redirects(0) + .http_status_as_error(false) + .build() + .call() + .map_err(|e| Fault::Again(Error::Unreachable(format!("{at}: {e}")), None))?; + let status = response.status().as_u16(); + match status { + // A connection lost while reading is worth another try; a body + // that is not a cover is not. + 200 => { + return read(response, address).map_err(|why| match why { + Error::Unreachable(_) => Fault::Again(why, None), + why => Fault::Final(why), + }); + } + 301 | 302 | 303 | 307 | 308 => { + let Some(location) = header(&response, "location") else { + return Err(Fault::Final(Error::Unreachable(format!( + "{at}: a redirect with nowhere to go" + )))); + }; + if hops == MAX_REDIRECTS { + return Err(Fault::Final(Error::Unreachable(format!( + "{address}: more than {MAX_REDIRECTS} redirects" + )))); + } + let next = resolve(&at, &location); + if !next.starts_with("https://") && origin(&next) != origin(&self.base) { + return Err(Fault::Final(Error::Unreachable(format!( + "{at}: refused a redirect off HTTPS, to {next}" + )))); + } + at = next; + hops += 1; + } + // `default=false` makes a missing cover a 404 rather than a + // stand-in image. + 404 | 410 => return Ok(Answer::Absent), + 429 | 503 => { + let wait = header(&response, "retry-after") + .and_then(|value| value.trim().parse::().ok()) + .map(|seconds| Duration::from_secs(seconds).min(MAX_WAIT)); + return Err(Fault::Again( + Error::Unreachable(format!("{at}: busy ({status})")), + wait, + )); + } + 500..=599 => { + return Err(Fault::Again( + Error::Unreachable(format!("{at}: answered {status}")), + None, + )); + } + _ => { + return Err(Fault::Final(Error::Unreachable(format!( + "{at}: answered {status}" + )))); + } + } + } + } +} + +/// Read and check a body that is meant to be a cover. +fn read(mut response: ureq::http::Response, address: &str) -> Result { + let declared = header(&response, "content-type") + .map(|value| { + value + .split(';') + .next() + .unwrap_or_default() + .trim() + .to_ascii_lowercase() + }) + .unwrap_or_default(); + let media_type = match declared.as_str() { + "image/jpeg" => "image/jpeg", + "image/png" => "image/png", + other => { + return Err(Error::Shape(format!( + "{address}: not an image but {}", + if other.is_empty() { "untyped" } else { other } + ))); + } + }; + if response + .body() + .content_length() + .is_some_and(|length| length > MAX_BYTES) + { + return Err(Error::Shape(format!( + "{address}: larger than {MAX_BYTES} bytes" + ))); + } + let bytes = response + .body_mut() + .with_config() + .limit(MAX_BYTES) + .read_to_vec() + .map_err(|e| match e { + ureq::Error::BodyExceedsLimit(_) => { + Error::Shape(format!("{address}: larger than {MAX_BYTES} bytes")) + } + e => Error::Unreachable(format!("{address}: {e}")), + })?; + + let agrees = match media_type { + "image/jpeg" => bytes.starts_with(&[0xFF, 0xD8, 0xFF]), + _ => bytes.starts_with(b"\x89PNG\r\n\x1a\n"), + }; + if !agrees { + return Err(Error::Shape(format!( + "{address}: said {media_type} but is not one" + ))); + } + if bytes.len() < MIN_BYTES { + return Ok(Answer::Absent); + } + Ok(Answer::Image(Cover { + bytes, + media_type, + source: "openlibrary", + url: address.to_string(), + })) +} + +fn header(response: &ureq::http::Response, name: &str) -> Option { + response + .headers() + .get(name) + .and_then(|value| value.to_str().ok()) + .map(str::to_string) +} + +/// `scheme://host:port` of an absolute URL. +fn origin(url: &str) -> Option<&str> { + let after = url.find("://")? + 3; + let end = url[after..] + .find(['/', '?', '#']) + .map_or(url.len(), |at| after + at); + Some(&url[..end]) +} + +/// Where a `Location` points, read against the address that sent it. +fn resolve(from: &str, location: &str) -> String { + if location.contains("://") { + return location.to_string(); + } + if let Some(rest) = location.strip_prefix("//") { + let scheme = from.split("://").next().unwrap_or("https"); + return format!("{scheme}://{rest}"); + } + let Some(base) = origin(from) else { + return location.to_string(); + }; + if location.starts_with('/') { + return format!("{base}{location}"); + } + let path = &from[base.len()..]; + let path = path.split(['?', '#']).next().unwrap_or_default(); + let directory = path.rfind('/').map_or("/", |at| &path[..=at]); + format!("{base}{directory}{location}") +} + +#[cfg(test)] +mod tests { + use super::*; + + use std::io::{BufRead, BufReader, Write}; + use std::net::TcpListener; + use std::sync::{Arc, Mutex}; + + /// One reply from the stub: a status, headers and a body. + struct Reply { + status: u16, + headers: Vec<(&'static str, String)>, + body: Vec, + /// How long to sit on the request before answering. + delay: Duration, + /// Whether to say how long the body is, or leave the reader to + /// find out when the connection closes. + announce: bool, + } + + impl Reply { + fn status(status: u16) -> Self { + Reply { + status, + headers: Vec::new(), + body: Vec::new(), + delay: Duration::ZERO, + announce: true, + } + } + + fn image(media_type: &str, body: Vec) -> Self { + Reply { + status: 200, + headers: vec![("Content-Type", media_type.to_string())], + body, + delay: Duration::ZERO, + announce: true, + } + } + + fn redirect(to: String) -> Self { + Reply { + status: 302, + headers: vec![("Location", to)], + body: Vec::new(), + delay: Duration::ZERO, + announce: true, + } + } + + fn header(mut self, name: &'static str, value: &str) -> Self { + self.headers.push((name, value.to_string())); + self + } + } + + /// A covers store on a loopback port, answering each path with whatever + /// the route says, and keeping the paths it was asked for in order. + /// + /// Written out here rather than taken from a crate: a test that needs a + /// socket, a request line and a canned answer needs nothing else. + struct Stub { + base: String, + asked: Arc>>, + } + + impl Stub { + fn start(route: impl Fn(&str, &str) -> Reply + Send + Sync + 'static) -> Stub { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let base = format!("http://{}", listener.local_addr().expect("an address")); + let asked = Arc::new(Mutex::new(Vec::new())); + let route = Arc::new(route); + let (seen, own) = (Arc::clone(&asked), base.clone()); + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { continue }; + let (route, seen, own) = (Arc::clone(&route), Arc::clone(&seen), own.clone()); + std::thread::spawn(move || { + let mut reader = BufReader::new(stream.try_clone().expect("a clone")); + let mut line = String::new(); + if reader.read_line(&mut line).is_err() { + return; + } + let path = line.split_whitespace().nth(1).unwrap_or("").to_string(); + loop { + let mut header = String::new(); + match reader.read_line(&mut header) { + Ok(0) | Err(_) => break, + Ok(_) if header == "\r\n" => break, + Ok(_) => {} + } + } + seen.lock().expect("a lock").push(path.clone()); + let reply = route(&own, &path); + std::thread::sleep(reply.delay); + let mut head = + format!("HTTP/1.1 {} Stub\r\nConnection: close\r\n", reply.status); + let stated = reply + .headers + .iter() + .any(|(name, _)| *name == "Content-Length"); + if reply.announce && !stated { + head.push_str(&format!("Content-Length: {}\r\n", reply.body.len())); + } + for (name, value) in &reply.headers { + head.push_str(&format!("{name}: {value}\r\n")); + } + head.push_str("\r\n"); + let _ = stream.write_all(head.as_bytes()); + let _ = stream.write_all(&reply.body); + }); + } + }); + Stub { base, asked } + } + + fn covers(&self) -> Covers { + Covers { + base: self.base.clone(), + attempts: 3, + } + } + + fn asked(&self) -> Vec { + self.asked.lock().expect("a lock").clone() + } + } + + fn agent() -> ureq::Agent { + crate::ask::Ask { + timeout: Duration::from_secs(5), + ..Default::default() + } + .agent() + } + + fn jpeg(len: usize) -> Vec { + let mut bytes = vec![0xFF, 0xD8, 0xFF, 0xE0]; + bytes.resize(len, 0x11); + bytes + } + + fn png(len: usize) -> Vec { + let mut bytes = b"\x89PNG\r\n\x1a\n".to_vec(); + bytes.resize(len, 0x22); + bytes + } + + const ISBN13: &str = "/b/isbn/9780306406157-L.jpg?default=false"; + const ISBN10: &str = "/b/isbn/0306406152-L.jpg?default=false"; + + #[test] + fn a_cover_is_found_by_its_isbn() { + let stub = Stub::start(|_, path| match path { + ISBN13 => Reply::image("image/jpeg", jpeg(4096)), + _ => Reply::status(404), + }); + let cover = stub + .covers() + .fetch(&agent(), "978-0-306-40615-7", None, CoverSize::Large) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.media_type, "image/jpeg"); + assert_eq!(cover.extension(), "jpg"); + assert_eq!(cover.source, "openlibrary"); + assert_eq!(cover.bytes, jpeg(4096)); + assert_eq!(cover.url, format!("{}{ISBN13}", stub.base)); + assert_eq!(stub.asked(), [ISBN13]); + } + + #[test] + fn a_png_cover_is_a_cover_too() { + let stub = Stub::start(|_, _| Reply::image("image/png", png(2048))); + let cover = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Medium) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.media_type, "image/png"); + assert_eq!(cover.extension(), "png"); + assert_eq!(stub.asked(), ["/b/isbn/9780306406157-M.jpg?default=false"]); + } + + #[test] + fn no_cover_anywhere_is_none_and_not_a_failure() { + let stub = Stub::start(|_, _| Reply::status(404)); + let found = stub + .covers() + .fetch(&agent(), "9780306406157", Some(42), CoverSize::Large) + .expect("an answer"); + assert_eq!(found, None); + // Both forms of the ISBN, then the cover id, and each only once. + assert_eq!( + stub.asked(), + [ISBN13, ISBN10, "/b/id/42-L.jpg?default=false"] + ); + } + + #[test] + fn a_book_filed_under_its_ten_digit_isbn_is_still_found() { + let stub = Stub::start(|_, path| match path { + ISBN10 => Reply::image("image/jpeg", jpeg(2048)), + _ => Reply::status(404), + }); + // Asked with the ten-digit form, which is tried second. + let cover = stub + .covers() + .fetch(&agent(), "0-306-40615-2", None, CoverSize::Large) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.url, format!("{}{ISBN10}", stub.base)); + assert_eq!(stub.asked(), [ISBN13, ISBN10]); + } + + #[test] + fn a_cover_id_from_the_record_is_asked_when_the_isbn_has_none() { + let stub = Stub::start(|_, path| match path { + "/b/id/1162784-L.jpg?default=false" => Reply::image("image/jpeg", jpeg(2048)), + _ => Reply::status(404), + }); + let cover = stub + .covers() + .fetch(&agent(), "9780306406157", Some(1162784), CoverSize::Large) + .expect("an answer") + .expect("a cover"); + assert!(cover.url.ends_with("/b/id/1162784-L.jpg?default=false")); + } + + #[test] + fn a_mistyped_isbn_is_asked_as_written_and_not_converted() { + let stub = Stub::start(|_, _| Reply::status(404)); + let found = stub + .covers() + .fetch(&agent(), "978-0-306-40615-8", None, CoverSize::Large) + .expect("an answer"); + assert_eq!(found, None); + assert_eq!(stub.asked(), ["/b/isbn/9780306406158-L.jpg?default=false"]); + } + + #[test] + fn something_that_is_not_an_isbn_is_refused_without_asking() { + let stub = Stub::start(|_, _| Reply::status(404)); + let failed = stub + .covers() + .fetch(&agent(), "12345", None, CoverSize::Large) + .expect_err("not an isbn"); + assert!(matches!(failed, Error::Invalid(_)), "{failed:?}"); + assert!(stub.asked().is_empty()); + } + + #[test] + fn a_placeholder_sized_image_is_no_cover() { + let stub = Stub::start(|_, _| Reply::image("image/jpeg", jpeg(MIN_BYTES - 1))); + let found = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect("an answer"); + assert_eq!(found, None); + } + + #[test] + fn a_page_that_is_not_an_image_is_refused() { + let stub = Stub::start(|_, _| Reply::image("text/html; charset=utf-8", jpeg(4096))); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("not an image"); + assert!(matches!(failed, Error::Shape(_)), "{failed:?}"); + } + + #[test] + fn bytes_that_are_not_the_image_they_claim_are_refused() { + let stub = Stub::start(|_, _| Reply::image("image/jpeg", b"".repeat(500))); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("not a jpeg"); + assert!(matches!(failed, Error::Shape(_)), "{failed:?}"); + + let stub = Stub::start(|_, _| Reply::image("image/png", jpeg(4096))); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("a jpeg is not a png"); + assert!(matches!(failed, Error::Shape(_)), "{failed:?}"); + } + + #[test] + fn a_body_over_the_limit_is_refused() { + // Announced, and refused on the length; then not announced, so the + // cap has to hold while reading. + for announce in [true, false] { + let stub = Stub::start(move |_, _| Reply { + announce, + ..Reply::image("image/jpeg", jpeg(MAX_BYTES as usize + 1)) + }); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("too large"); + assert!( + matches!(&failed, Error::Shape(why) if why.contains("larger than")), + "announce {announce}: {failed:?}" + ); + } + } + + #[test] + fn a_body_of_unannounced_length_under_the_limit_is_read_whole() { + let stub = Stub::start(|_, _| Reply { + announce: false, + ..Reply::image("image/jpeg", jpeg(64 * 1024)) + }); + let cover = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.bytes.len(), 64 * 1024); + } + + #[test] + fn a_busy_register_is_waited_on_and_asked_again() { + let calls = Arc::new(Mutex::new(0u32)); + let count = Arc::clone(&calls); + let stub = Stub::start(move |_, _| { + let mut n = count.lock().expect("a lock"); + *n += 1; + match *n { + 1 => Reply::status(429).header("Retry-After", "0"), + 2 => Reply::status(503).header("Retry-After", "0"), + _ => Reply::image("image/jpeg", jpeg(2048)), + } + }); + let cover = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect("an answer"); + assert!(cover.is_some()); + assert_eq!(stub.asked(), [ISBN13, ISBN13, ISBN13]); + } + + #[test] + fn a_body_cut_short_is_asked_for_again() { + let calls = Arc::new(Mutex::new(0u32)); + let count = Arc::clone(&calls); + let stub = Stub::start(move |_, _| { + let mut n = count.lock().expect("a lock"); + *n += 1; + match *n { + // Promises more than it sends, then closes. + 1 => Reply::image("image/jpeg", jpeg(2048)).header("Content-Length", "9000"), + _ => Reply::image("image/jpeg", jpeg(2048)), + } + }); + let covers = Covers { + attempts: 2, + ..stub.covers() + }; + let cover = covers + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.bytes.len(), 2048); + assert_eq!(stub.asked(), [ISBN13, ISBN13]); + } + + #[test] + fn a_register_busy_every_time_is_a_failure_and_not_an_absence() { + let stub = Stub::start(|_, _| Reply::status(429).header("Retry-After", "0")); + let covers = Covers { + attempts: 2, + ..stub.covers() + }; + let failed = covers + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("busy throughout"); + assert!(matches!(failed, Error::Unreachable(_)), "{failed:?}"); + // Two attempts at each form of the ISBN. + assert_eq!(stub.asked(), [ISBN13, ISBN13, ISBN10, ISBN10]); + } + + #[test] + fn redirects_are_followed_to_the_image() { + let stub = Stub::start(|base, path| match path { + ISBN13 => Reply::redirect(format!("{base}/download/one")), + "/download/one" => Reply::redirect("/view?file=two".to_string()), + "/view?file=two" => Reply::image("image/jpeg", jpeg(2048)), + _ => Reply::status(404), + }); + let cover = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect("an answer") + .expect("a cover"); + // The address asked, not where the archive happened to serve it. + assert_eq!(cover.url, format!("{}{ISBN13}", stub.base)); + } + + #[test] + fn a_fourth_redirect_is_not_followed() { + let stub = Stub::start(|_, path| { + let n: u32 = path + .strip_prefix("/hop/") + .map_or(0, |n| n.parse().unwrap_or(0)); + Reply::redirect(format!("/hop/{}", n + 1)) + }); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("too many redirects"); + assert!( + matches!(&failed, Error::Unreachable(why) if why.contains("redirects")), + "{failed:?}" + ); + // Asked, then three redirects followed, for each ISBN form. + assert_eq!(stub.asked().iter().filter(|p| *p == ISBN13).count(), 1); + assert_eq!(stub.asked().len(), 8); + } + + #[test] + fn a_redirect_off_https_is_not_followed() { + let stub = Stub::start(|_, path| match path { + ISBN13 | ISBN10 => Reply::redirect("http://covers.example.invalid/x.jpg".to_string()), + _ => Reply::status(404), + }); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("plain http elsewhere"); + assert!( + matches!(&failed, Error::Unreachable(why) if why.contains("off HTTPS")), + "{failed:?}" + ); + } + + #[test] + fn a_register_that_does_not_answer_in_time_is_unreachable() { + let stub = Stub::start(|_, _| Reply { + delay: Duration::from_secs(3), + ..Reply::image("image/jpeg", jpeg(2048)) + }); + let agent = crate::ask::Ask { + timeout: Duration::from_millis(300), + ..Default::default() + } + .agent(); + let covers = Covers { + attempts: 1, + ..stub.covers() + }; + let started = std::time::Instant::now(); + let failed = covers + .fetch(&agent, "9780306406157", None, CoverSize::Large) + .expect_err("timed out"); + assert!(matches!(failed, Error::Unreachable(_)), "{failed:?}"); + assert!( + started.elapsed() < Duration::from_secs(3), + "waited the reply out" + ); + } + + #[test] + fn locations_are_read_against_the_address_that_sent_them() { + let from = "https://a.example/b/isbn/1-L.jpg?default=false"; + assert_eq!(resolve(from, "https://c.example/x"), "https://c.example/x"); + 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"); + assert_eq!( + origin("http://127.0.0.1:8080/a?b"), + Some("http://127.0.0.1:8080") + ); + assert_eq!(origin("https://a.example"), Some("https://a.example")); + } + + /// Run by hand: `cargo test -p bibfetch -- --ignored`. + #[test] + #[ignore = "asks Open Library over the network"] + fn a_known_book_has_a_cover_at_open_library() { + let agent = crate::ask::Ask::default().agent(); + // Error-correction coding for digital communications, Plenum 1981. + let cover = cover(&agent, "0-306-40615-2", None, CoverSize::Large) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.media_type, "image/jpeg"); + assert!(cover.bytes.len() > MIN_BYTES); + assert!(cover.url.starts_with(OPEN_LIBRARY_COVERS)); + } +} diff --git a/crates/bibfetch/src/lib.rs b/crates/bibfetch/src/lib.rs index cd47802..084bef3 100644 --- a/crates/bibfetch/src/lib.rs +++ b/crates/bibfetch/src/lib.rs @@ -14,6 +14,7 @@ //! ISSN is looked up as the journal of a work rather than on its own. pub mod ask; +pub mod cover; pub mod source; use std::time::Duration; @@ -21,6 +22,7 @@ use std::time::Duration; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +pub use cover::{Cover, CoverSize, cover}; pub use source::Source; /// How long to wait on a register before giving up. @@ -74,6 +76,11 @@ pub struct Record { /// How many pages the register says it has. #[serde(default, skip_serializing_if = "Option::is_none")] pub pages: Option, + /// The id Open Library files a cover of this book under, for + /// [`cover()`] to ask by when the ISBN has none. Its index names the + /// cover of the work, which may be another edition's. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cover_id: Option, } fn version() -> u32 { @@ -95,6 +102,8 @@ pub enum Error { Shape(String), /// Nothing here knows where to ask about this kind. NoRegister(String), + /// What was asked about is not an identifier of the kind it should be. + Invalid(String), } impl std::fmt::Display for Error { @@ -105,6 +114,7 @@ impl std::fmt::Display for Error { Error::NoRegister(kind) => { write!(f, "no register here answers about {kind} identifiers") } + Error::Invalid(why) => write!(f, "{why}"), } } } @@ -130,6 +140,7 @@ pub fn missing(id: &str, kind: &str, source: Source) -> Record { subjects: Vec::new(), language: None, pages: None, + cover_id: None, } } @@ -187,6 +198,9 @@ impl Record { if self.pages.is_none() { self.pages = other.pages; } + if self.cover_id.is_none() { + self.cover_id = other.cover_id; + } // Which register answered becomes which registers did. if !self.source.split('+').any(|s| s == other.source) { self.source = format!("{}+{}", self.source, other.source); diff --git a/crates/bibfetch/src/main.rs b/crates/bibfetch/src/main.rs index 7053914..069e691 100644 --- a/crates/bibfetch/src/main.rs +++ b/crates/bibfetch/src/main.rs @@ -29,33 +29,83 @@ Exit codes: 0 every identifier was found, 1 at least one was not, Example: idnorm refs.txt | bibfetch | jq -r 'select(.found) | .title' - idnorm --kind doi refs.txt | bibfetch --polite you@example.org" + idnorm --kind doi refs.txt | bibfetch --polite you@example.org + bibfetch cover 978-0-306-40615-7 -o cover.jpg" )] struct Args { + #[command(subcommand)] + command: Option, + /// Identifier records to look up, as `idnorm` writes them. Reads stdin /// when absent. path: Option, /// An address to put in the request, which the registers ask for and /// answer faster when they get. - #[arg(long, value_name = "EMAIL")] + #[arg(long, global = true, value_name = "EMAIL")] polite: Option, /// How long to wait on a register, in seconds. - #[arg(long, default_value_t = bibfetch::TIMEOUT.as_secs(), value_name = "SECONDS")] + #[arg(long, global = true, default_value_t = bibfetch::TIMEOUT.as_secs(), value_name = "SECONDS")] timeout: u64, /// How many times to ask before believing a failure. - #[arg(long, default_value_t = bibfetch::ATTEMPTS, value_name = "COUNT")] + #[arg(long, global = true, default_value_t = bibfetch::ATTEMPTS, value_name = "COUNT")] attempts: u32, } +#[derive(clap::Subcommand)] +enum Command { + /// Fetch a book's cover from Open Library by its ISBN. + /// + /// Exit codes: 0 the cover was written, 1 Open Library has none, + /// 65 not an ISBN or not an image, 69 Open Library could not be reached, + /// 74 the file could not be written. + Cover { + /// The ISBN, ten or thirteen digits, hyphens or not. + isbn: String, + /// Where to write the image. + #[arg(short, long, value_name = "FILE")] + output: PathBuf, + /// Which size to ask for. + #[arg(long, value_enum, default_value_t = Size::Large)] + size: Size, + /// An Open Library cover id to ask by when the ISBN has no cover. + #[arg(long, value_name = "ID")] + cover_id: Option, + }, +} + +#[derive(Clone, Copy, clap::ValueEnum)] +enum Size { + Small, + Medium, + Large, +} + fn main() -> ExitCode { let args: Args = match catena_cli::parse() { Ok(args) => args, Err(code) => return code, }; + let agent = bibfetch::ask::Ask { + polite: args.polite.clone(), + timeout: Duration::from_secs(args.timeout), + attempts: args.attempts, + } + .agent(); + + if let Some(Command::Cover { + isbn, + output, + size, + cover_id, + }) = &args.command + { + return cover(&agent, args.attempts, isbn, output, *size, *cover_id); + } + let text = match catena_cli::read_text(args.path.as_deref()) { Ok(text) => text, Err((code, why)) => { @@ -64,13 +114,6 @@ fn main() -> ExitCode { } }; - let agent = bibfetch::ask::Ask { - polite: args.polite.clone(), - timeout: Duration::from_secs(args.timeout), - attempts: args.attempts, - } - .agent(); - let mut records = Vec::new(); let mut missing = 0usize; @@ -114,6 +157,46 @@ fn main() -> ExitCode { }) } +/// Fetch one cover and write it where asked. +fn cover( + agent: &ureq::Agent, + attempts: u32, + isbn: &str, + output: &std::path::Path, + size: Size, + cover_id: Option, +) -> ExitCode { + let covers = bibfetch::cover::Covers { + attempts, + ..Default::default() + }; + let size = match size { + Size::Small => bibfetch::CoverSize::Small, + Size::Medium => bibfetch::CoverSize::Medium, + Size::Large => bibfetch::CoverSize::Large, + }; + match covers.fetch(agent, isbn, cover_id, size) { + Ok(Some(found)) => match std::fs::write(output, &found.bytes) { + Ok(()) => ExitCode::from(exit::OK), + Err(why) => { + eprintln!("bibfetch: {}: {why}", output.display()); + ExitCode::from(exit::IO) + } + }, + Ok(None) => { + eprintln!("bibfetch: {isbn}: Open Library has no cover"); + ExitCode::from(exit::NO) + } + Err(why) => { + eprintln!("bibfetch: {isbn}: {why}"); + ExitCode::from(match why { + bibfetch::Error::Unreachable(_) => exit::UNAVAILABLE, + _ => exit::DATA, + }) + } + } +} + /// The identifier and its kind out of one record. fn read_identifier(line: &str) -> Result<(String, String), String> { let value: serde_json::Value = diff --git a/crates/bibfetch/src/source.rs b/crates/bibfetch/src/source.rs index 877333f..8719066 100644 --- a/crates/bibfetch/src/source.rs +++ b/crates/bibfetch/src/source.rs @@ -88,7 +88,7 @@ impl Source { Source::OpenLibrarySearch => format!( "https://openlibrary.org/search.json?q=isbn:{}\ &fields=title,subtitle,author_name,first_publish_year,publisher,language,subject,\ -number_of_pages_median&limit=1", +number_of_pages_median,cover_i&limit=1", encode(id) ), Source::GoogleBooks => format!( @@ -245,6 +245,9 @@ struct OpenLibraryBook { languages: Vec, #[serde(default)] number_of_pages: Option, + /// Cover ids, best first. A deleted cover is left in the list as `-1`. + #[serde(default)] + covers: Vec, } #[derive(serde::Deserialize)] @@ -275,6 +278,10 @@ fn open_library(id: &str, kind: &str, body: &str) -> Result { .find_map(|l| l.key.rsplit('/').next().map(str::to_string)) .filter(|l| !l.is_empty()); record.pages = book.number_of_pages; + record.cover_id = book + .covers + .into_iter() + .find_map(|id| u64::try_from(id).ok().filter(|id| *id > 0)); Ok(record) } @@ -302,6 +309,8 @@ struct SearchDoc { subject: Vec, #[serde(rename = "number_of_pages_median", default)] pages: Option, + #[serde(default)] + cover_i: Option, } /// How many of a work's subject headings are worth carrying. @@ -333,6 +342,10 @@ fn open_library_search(id: &str, kind: &str, body: &str) -> Result 0); Ok(record) } @@ -520,6 +533,29 @@ mod tests { } } + #[test] + fn the_cover_id_a_record_names_is_kept_for_the_cover_lookup() { + // A deleted cover stays in an edition's list as -1, ahead of the + // ones still there. + let record = open_library("1", "isbn", r#"{"title":"A","covers":[-1,1162784,99]}"#) + .expect("a record"); + assert_eq!(record.cover_id, Some(1162784)); + let record = open_library("1", "isbn", r#"{"title":"A"}"#).expect("a record"); + assert_eq!(record.cover_id, None); + + let record = open_library_search("1", "isbn", r#"{"docs":[{"title":"A","cover_i":2468}]}"#) + .expect("a record"); + assert_eq!(record.cover_id, Some(2468)); + let record = open_library_search("1", "isbn", r#"{"docs":[{"title":"A","cover_i":-1}]}"#) + .expect("a record"); + assert_eq!(record.cover_id, None); + } + + #[test] + fn the_index_is_asked_for_the_cover_id() { + assert!(Source::OpenLibrarySearch.url("1").contains("cover_i")); + } + #[test] fn an_answer_that_is_not_what_it_should_be_says_so() { assert!(matches!( diff --git a/crates/idnorm/src/isbn.rs b/crates/idnorm/src/isbn.rs new file mode 100644 index 0000000..657275e --- /dev/null +++ b/crates/idnorm/src/isbn.rs @@ -0,0 +1,99 @@ +//! An ISBN's two forms, ten digits and thirteen. +//! +//! A book printed before 2007 carries a ten-digit ISBN, one printed since +//! carries thirteen, and every ten-digit one has a thirteen-digit twin: `978` +//! in front and the check digit worked out again. A catalogue may file a +//! book under either, so asking about one form and not the other misses +//! books that are there. +//! +//! Only a well-formed ISBN is converted. Working out a fresh check digit for +//! one whose own check digit is wrong would not correct the typing error, it +//! would turn the number into a different book's. + +/// Just the digits of an ISBN, with a check character of ten as `X`. +pub fn digits(text: &str) -> String { + bibclean::isbn::digits(text).to_ascii_uppercase() +} + +/// The thirteen-digit form of an ISBN, from either form. +/// +/// `None` when the text is not an ISBN whose check digit agrees. +pub fn to_13(text: &str) -> Option { + let digits = digits(text); + if !bibclean::isbn::valid(&digits) { + return None; + } + match digits.len() { + 13 => Some(digits), + 10 => { + let body = format!("978{}", &digits[..9]); + let sum: u32 = body + .bytes() + .enumerate() + .map(|(at, b)| u32::from(b - b'0') * if at % 2 == 0 { 1 } else { 3 }) + .sum(); + Some(format!("{body}{}", (10 - sum % 10) % 10)) + } + _ => None, + } +} + +/// The ten-digit form of an ISBN, from either form. +/// +/// `None` when the text is not an ISBN whose check digit agrees, or when it +/// is a thirteen-digit one starting `979`, which was never issued in ten. +pub fn to_10(text: &str) -> Option { + let digits = digits(text); + if !bibclean::isbn::valid(&digits) { + return None; + } + match digits.len() { + 10 => Some(digits), + 13 if digits.starts_with("978") => { + let body = &digits[3..12]; + let sum: u32 = body + .bytes() + .enumerate() + .map(|(at, b)| u32::from(b - b'0') * (10 - at as u32)) + .sum(); + let check = match (11 - sum % 11) % 11 { + 10 => 'X', + n => char::from(b'0' + n as u8), + }; + Some(format!("{body}{check}")) + } + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn each_form_becomes_the_other() { + assert_eq!(to_13("0-306-40615-2").as_deref(), Some("9780306406157")); + assert_eq!(to_10("978-0-306-40615-7").as_deref(), Some("0306406152")); + assert_eq!(to_13("9780306406157").as_deref(), Some("9780306406157")); + assert_eq!(to_10("0306406152").as_deref(), Some("0306406152")); + } + + #[test] + fn a_check_digit_of_ten_is_written_x() { + assert_eq!(to_10("9780804429573").as_deref(), Some("080442957X")); + assert_eq!(to_13("080442957x").as_deref(), Some("9780804429573")); + } + + #[test] + fn a_979_isbn_has_no_ten_digit_form() { + assert_eq!(to_10("979-10-90636-07-1"), None); + assert_eq!(to_13("979-10-90636-07-1").as_deref(), Some("9791090636071")); + } + + #[test] + fn a_mistyped_isbn_is_not_converted_into_a_different_book() { + assert_eq!(to_13("0-306-40615-3"), None); + assert_eq!(to_10("978-0-306-40615-8"), None); + assert_eq!(to_13("12345"), None); + } +} diff --git a/crates/idnorm/src/lib.rs b/crates/idnorm/src/lib.rs index 588ff95..8620b45 100644 --- a/crates/idnorm/src/lib.rs +++ b/crates/idnorm/src/lib.rs @@ -13,6 +13,7 @@ pub mod arxiv; pub mod doi; +pub mod isbn; pub mod issn; use schemars::JsonSchema; From 1abed4632d9fcf337a4cee94dbe95fe77c6a91bb Mon Sep 17 00:00:00 2001 From: funkpower Date: Fri, 13 Nov 2026 15:04:34 -0500 Subject: [PATCH 2/2] bibfetch: a cover id is the edition's own, and a fetch has a deadline Review id 3 on catena#2. P0: Record.cover_id came from the search index's cover_i, the cover chosen for the whole work, and the index is asked first, so cover() fell back to another edition's image (Penguin War and Peace, 9780140447934, got Standard Ebooks' Maude cover 12621906). cover_i is no longer asked for or read; cover_id is the first positive id in the edition record's covers, or None. An ISBN request that fails, as opposed to a 404, now ends the fetch with that failure instead of going on to the cover id. Redirects go only over HTTPS on the standard port to covers.openlibrary.org, archive.org or a host under archive.org, and never to an address by number; the covers store the fetch began at is still allowed. The 1 KB size floor is gone. With default=false a missing cover is a 404, and the stand-in without it is a 43-byte GIF the media type check refuses, so the floor could only drop real Small covers. Covers.deadline (DEADLINE, two minutes) bounds a whole fetch: every address, attempt, redirect, body and wait. It starts once the fetch holds the process lock. The timeout test no longer times a span that includes the lock wait. --- crates/bibfetch/README.md | 19 +- crates/bibfetch/src/cover.rs | 338 +++++++++++++++++++++++++++++----- crates/bibfetch/src/lib.rs | 8 +- crates/bibfetch/src/source.rs | 57 ++++-- 4 files changed, 349 insertions(+), 73 deletions(-) diff --git a/crates/bibfetch/README.md b/crates/bibfetch/README.md index a4e40f9..83b592e 100644 --- a/crates/bibfetch/README.md +++ b/crates/bibfetch/README.md @@ -118,14 +118,21 @@ writes the image. The thirteen-digit form is asked first and the ten-digit form second, because a book is often filed under only one of them; an ISBN whose check digit is wrong is asked about as written, since converting it would give another book's number. `--cover-id` names an Open Library cover id -to ask by last, which is what the `cover_id` field of a lookup record holds. +to ask by last, which is what the `cover_id` field of a lookup record holds: +the first cover in Open Library's record of that edition, never the cover its +index picked for the whole work, which is often another edition's. The cover +id is asked only when every form of the ISBN asked was answered with no cover; if +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, between one kilobyte (Open Library's -stand-in for a missing cover is smaller) and ten megabytes, reached in at most -three redirects that never leave HTTPS. One request is made at a time, and a -`429` or `503` is waited out for as long as its `Retry-After` says, up to a -minute. +as well as by what the server says, 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 +its `Retry-After` says, up to a minute, and the whole fetch, every attempt and +redirect included, stops after two minutes. The two minutes start once it is +the fetch's turn, so a fetch queued behind others in the same process can wait +that long again first. | Option | Effect | |---|---| diff --git a/crates/bibfetch/src/cover.rs b/crates/bibfetch/src/cover.rs index 2ed1626..4e99b1a 100644 --- a/crates/bibfetch/src/cover.rs +++ b/crates/bibfetch/src/cover.rs @@ -4,16 +4,18 @@ //! `covers.openlibrary.org`, and answers by ISBN or by the cover's own id. An //! ISBN is asked in its thirteen-digit form first and its ten-digit form //! second, because a book is often filed under only one of them; the cover id -//! a record already named is asked last, since it may be the cover of another -//! edition of the work rather than of this one. +//! of the edition's record is asked last, and only when every form of the +//! ISBN asked was answered and had none. //! //! What comes back is checked before it is trusted, because it is written to //! disk next to a book: an image (JPEG or PNG, by what the bytes are as well //! as what the server says), no larger than [`MAX_BYTES`], reached in at most -//! [`MAX_REDIRECTS`] redirects that never leave HTTPS. +//! [`MAX_REDIRECTS`] redirects, each over HTTPS to Open Library's covers or +//! the Internet Archive, all within [`DEADLINE`]. +use std::net::IpAddr; use std::sync::Mutex; -use std::time::Duration; +use std::time::{Duration, Instant}; use crate::Error; @@ -24,15 +26,26 @@ pub const OPEN_LIBRARY_COVERS: &str = "https://covers.openlibrary.org"; /// hundred kilobytes; ten megabytes is something that is not a cover. pub const MAX_BYTES: u64 = 10 * 1024 * 1024; -/// The smallest cover accepted. Open Library's stand-in for a missing cover -/// is a one-pixel image of a few dozen bytes, and nothing under a kilobyte -/// is a picture of a book. -pub const MIN_BYTES: usize = 1024; - /// How many redirects to follow. A cover is served from the Internet /// Archive two redirects away from the address asked. pub const MAX_REDIRECTS: u32 = 3; +/// The hosts a redirect may lead to, besides the covers store it began at. +/// Today a cover goes `covers.openlibrary.org`, then `archive.org`, then an +/// `ia*.us.archive.org` storage node. +const REDIRECT_HOSTS: [&str; 2] = ["covers.openlibrary.org", "archive.org"]; + +/// How long one fetch may take, start to finish: every address, attempt, +/// redirect, body and wait between attempts. One fetch runs at a time in a +/// process, and the clock starts once it is this fetch's turn, so time spent +/// waiting behind another fetch is not counted; a caller sharing the process +/// with other fetches can wait that long again before this one begins. +/// +/// Without it the worst case at the defaults ran to about eighteen minutes: +/// three addresses, three attempts, up to four requests an attempt at the +/// agent's timeout each, and a minute's `Retry-After` between attempts. +pub const DEADLINE: Duration = Duration::from_secs(120); + /// The longest a `Retry-After` is honoured for. A register asking for more /// than this is better treated as down than waited on. const MAX_WAIT: Duration = Duration::from_secs(60); @@ -110,6 +123,10 @@ pub struct Covers { pub base: String, /// How many times to ask an address that is unreachable or busy. pub attempts: u32, + /// How long the whole fetch may take; [`DEADLINE`] by default. Each + /// request is given whatever is left of it, or the agent's own timeout + /// if that is shorter, and a wait that would run past it is not waited. + pub deadline: Duration, } impl Default for Covers { @@ -117,6 +134,7 @@ impl Default for Covers { Self { base: OPEN_LIBRARY_COVERS.to_string(), attempts: crate::ATTEMPTS, + deadline: DEADLINE, } } } @@ -140,9 +158,10 @@ impl Covers { /// /// The ISBN may be either form, with or without hyphens. One whose check /// digit is wrong is asked about as written and not converted, since a - /// converted one would be another book's number. A failure at one address - /// does not stop the next being asked; it is returned only when no - /// address gave a cover. + /// converted one would be another book's number. A failure at one form + /// of the ISBN does not stop the other being asked, but it does stop the + /// cover id being asked: the ISBN's own cover may exist, and is the one + /// wanted. The failure is returned when no ISBN form gave a cover. pub fn fetch( &self, agent: &ureq::Agent, @@ -162,69 +181,99 @@ impl Covers { }; let letter = size.letter(); - let mut addresses: Vec = forms + let addresses: Vec = forms .iter() .map(|form| format!("{}/b/isbn/{form}-{letter}.jpg?default=false", self.base)) .collect(); - if let Some(id) = cover_id { - addresses.push(format!( - "{}/b/id/{id}-{letter}.jpg?default=false", - self.base - )); - } let _turn = ONE_AT_A_TIME .lock() .unwrap_or_else(|held| held.into_inner()); + // From here, not from the call: waiting for the lock is time another + // fetch spent, and counting it would let a busy process starve this + // one of the time its own requests need. + let until = Instant::now() + self.deadline; let mut failed = None; - for address in addresses { - match self.ask(agent, &address) { + for address in &addresses { + match self.ask(agent, address, until) { Ok(Answer::Image(cover)) => return Ok(Some(cover)), Ok(Answer::Absent) => {} Err(why) => failed = Some(why), } } - match failed { - Some(why) => Err(why), - None => Ok(None), + if let Some(why) = failed { + return Err(why); + } + let Some(id) = cover_id else { + return Ok(None); + }; + let address = format!("{}/b/id/{id}-{letter}.jpg?default=false", self.base); + match self.ask(agent, &address, until)? { + Answer::Image(cover) => Ok(Some(cover)), + Answer::Absent => Ok(None), } } /// Ask one address, again while it is busy or unreachable. /// /// Waits as long as a `Retry-After` says, up to a minute, or else a - /// second and then two, the same as a record lookup does. - fn ask(&self, agent: &ureq::Agent, address: &str) -> Result { + /// second and then two, the same as a record lookup does; but not past + /// `until`, where a wait that would end later is given up at once. + fn ask(&self, agent: &ureq::Agent, address: &str, until: Instant) -> Result { let attempts = self.attempts.max(1); let mut last = Error::Unreachable("no attempt was made".into()); for attempt in 0..attempts { - match self.once(agent, address) { + match self.once(agent, address, until) { Ok(answer) => return Ok(answer), Err(Fault::Final(why)) => return Err(why), Err(Fault::Again(why, wait)) => { - last = why; if attempt + 1 < attempts { - std::thread::sleep( - wait.unwrap_or(Duration::from_secs(1 << attempt.min(5))), - ); + let wait = wait.unwrap_or(Duration::from_secs(1 << attempt.min(5))); + if Instant::now() + wait >= until { + return Err(self.out_of_time(address, &why)); + } + std::thread::sleep(wait); } + last = why; } } } Err(last) } + fn out_of_time(&self, address: &str, after: &Error) -> Error { + Error::Unreachable(format!( + "{address}: past the {:?} deadline, after {after}", + self.deadline + )) + } + /// One attempt at an address, following its redirects by hand so that /// each one can be checked before it is followed. - fn once(&self, agent: &ureq::Agent, address: &str) -> Result { + fn once(&self, agent: &ureq::Agent, address: &str, until: Instant) -> Result { let mut at = address.to_string(); let mut hops = 0; loop { + // The agent's timeout bounds one request, body included; what is + // left of the deadline bounds it too, so no request outlives it. + let left = until.saturating_duration_since(Instant::now()); + if left.is_zero() { + return Err(Fault::Final(Error::Unreachable(format!( + "{at}: past the {:?} deadline", + self.deadline + )))); + } + let timeout = agent + .config() + .timeouts() + .global + .map_or(left, |own| own.min(left)); let response = agent .get(&at) .config() .max_redirects(0) .http_status_as_error(false) + .timeout_global(Some(timeout)) .build() .call() .map_err(|e| Fault::Again(Error::Unreachable(format!("{at}: {e}")), None))?; @@ -250,9 +299,10 @@ impl Covers { )))); } let next = resolve(&at, &location); - if !next.starts_with("https://") && origin(&next) != origin(&self.base) { + if !may_follow(&next, &self.base) { return Err(Fault::Final(Error::Unreachable(format!( - "{at}: refused a redirect off HTTPS, to {next}" + "{at}: refused a redirect to {next}, which is not HTTPS to \ +Open Library's covers or the Internet Archive" )))); } at = next; @@ -338,9 +388,15 @@ fn read(mut response: ureq::http::Response, address: &str) -> Result "{address}: said {media_type} but is not one" ))); } - if bytes.len() < MIN_BYTES { - return Ok(Answer::Absent); - } + // 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 + // a 404, for an ISBN and for a cover id alike (checked 2026-11-13). The + // stand-in would fail the media type check above in any case, being a + // GIF. So a floor could only drop real covers, and real Small ones sit + // close to any floor worth having: thumbnails measured 1032 to 2148 + // bytes, the smallest eight bytes over a kilobyte floor, so smaller ones + // not yet seen would have been dropped. Ok(Answer::Image(Cover { bytes, media_type, @@ -357,6 +413,44 @@ fn header(response: &ureq::http::Response, name: &str) -> Option bool { + if origin(next).is_some() && origin(next) == origin(base) { + return true; + } + // The same parser the request is made with, so the host checked is the + // host connected to. + let Ok(uri) = next.parse::() else { + return false; + }; + if uri.scheme_str() != Some("https") || uri.port_u16().is_some_and(|port| port != 443) { + return false; + } + let Some(authority) = uri.authority() else { + return false; + }; + if authority.as_str().contains('@') { + return false; + } + let host = authority.host().to_ascii_lowercase(); + if host + .trim_start_matches('[') + .trim_end_matches(']') + .parse::() + .is_ok() + { + return false; + } + REDIRECT_HOSTS.contains(&host.as_str()) || host.ends_with(".archive.org") +} + /// `scheme://host:port` of an absolute URL. fn origin(url: &str) -> Option<&str> { let after = url.find("://")? + 3; @@ -508,6 +602,7 @@ mod tests { Covers { base: self.base.clone(), attempts: 3, + deadline: DEADLINE, } } @@ -639,13 +734,17 @@ mod tests { } #[test] - fn a_placeholder_sized_image_is_no_cover() { - let stub = Stub::start(|_, _| Reply::image("image/jpeg", jpeg(MIN_BYTES - 1))); - let found = stub + fn open_librarys_stand_in_is_never_a_cover() { + // What Open Library serves for a missing cover when `default=false` + // is left off: a one-pixel GIF89a, 43 bytes. + let mut gif = b"GIF89a\x01\x00\x01\x00".to_vec(); + gif.resize(43, 0); + let stub = Stub::start(move |_, _| Reply::image("image/gif", gif.clone())); + let failed = stub .covers() - .fetch(&agent(), "9780306406157", None, CoverSize::Large) - .expect("an answer"); - assert_eq!(found, None); + .fetch(&agent(), "9780306406157", None, CoverSize::Small) + .expect_err("not a cover"); + assert!(matches!(failed, Error::Shape(_)), "{failed:?}"); } #[test] @@ -819,11 +918,43 @@ mod tests { .fetch(&agent(), "9780306406157", None, CoverSize::Large) .expect_err("plain http elsewhere"); assert!( - matches!(&failed, Error::Unreachable(why) if why.contains("off HTTPS")), + matches!(&failed, Error::Unreachable(why) if why.contains("refused")), "{failed:?}" ); } + #[test] + fn redirects_go_only_to_open_library_and_the_archive_over_https() { + let base = "https://covers.openlibrary.org"; + for next in [ + "https://covers.openlibrary.org/b/id/1-L.jpg", + "https://archive.org/download/l_covers_0001/x.jpg", + "https://ia800100.us.archive.org/view_archive.php?f=1", + "https://ARCHIVE.org/x", + "https://archive.org:443/x", + ] { + assert!(may_follow(next, base), "{next}"); + } + for next in [ + "http://archive.org/x", + "https://archive.org:8443/x", + "https://evilarchive.org/x", + "https://archive.org.example/x", + "https://openlibrary.org.example/x", + "https://covers.openlibrary.org@127.0.0.1/x", + "https://127.0.0.1/x", + "https://10.7.1.5/x", + "https://[::1]/x", + "https://example.com/x", + "ftp://archive.org/x", + ] { + assert!(!may_follow(next, base), "{next}"); + } + // The store it began at, whatever it is, for a test or a mirror. + assert!(may_follow("http://127.0.0.1:9/x", "http://127.0.0.1:9")); + assert!(!may_follow("http://127.0.0.1:10/x", "http://127.0.0.1:9")); + } + #[test] fn a_register_that_does_not_answer_in_time_is_unreachable() { let stub = Stub::start(|_, _| Reply { @@ -839,15 +970,126 @@ mod tests { attempts: 1, ..stub.covers() }; - let started = std::time::Instant::now(); + // No stopwatch: one started here would count the wait for the + // process lock, which other tests hold. The failure is proof enough, + // since a reply waited out is a cover. let failed = covers .fetch(&agent, "9780306406157", None, CoverSize::Large) .expect_err("timed out"); assert!(matches!(failed, Error::Unreachable(_)), "{failed:?}"); + } + + #[test] + fn a_slow_answer_is_cut_at_the_deadline() { + let stub = Stub::start(|_, _| Reply { + delay: Duration::from_secs(10), + ..Reply::image("image/jpeg", jpeg(2048)) + }); + // The agent would wait the reply out; the deadline will not. + let agent = crate::ask::Ask { + timeout: Duration::from_secs(30), + ..Default::default() + } + .agent(); + let covers = Covers { + attempts: 3, + deadline: Duration::from_millis(500), + ..stub.covers() + }; + let found = covers + .fetch(&agent, "9780306406157", Some(42), CoverSize::Large) + .map(|cover| cover.map(|cover| cover.url)); + assert!(matches!(found, Err(Error::Unreachable(_))), "{found:?}"); + // Out of time after the first request: no second attempt, no other + // form of the ISBN, no cover id. + assert_eq!(stub.asked(), [ISBN13]); + } + + #[test] + fn a_wait_past_the_deadline_is_not_waited() { + let stub = Stub::start(|_, _| Reply::status(429).header("Retry-After", "30")); + let covers = Covers { + attempts: 3, + deadline: Duration::from_secs(5), + ..stub.covers() + }; + let started = Instant::now(); + let failed = covers + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("busy"); assert!( - started.elapsed() < Duration::from_secs(3), - "waited the reply out" + matches!(&failed, Error::Unreachable(why) if why.contains("deadline")), + "{failed:?}" ); + // Each form asked once and given up on, not waited on for 30 s. The + // bound is loose because it includes any wait for the process lock. + assert_eq!(stub.asked(), [ISBN13, ISBN10]); + assert!( + started.elapsed() < Duration::from_secs(25), + "{:?}", + started.elapsed() + ); + } + + #[test] + fn an_isbn_that_fails_stops_the_cover_id_being_asked() { + // The ISBN-13 address fails (not a 404), the ISBN-10 has none, and + // the cover id has an image that might be another edition's. + let stub = Stub::start(|_, path| match path { + ISBN13 => Reply::status(500), + "/b/id/42-L.jpg?default=false" => Reply::image("image/jpeg", jpeg(4096)), + _ => Reply::status(404), + }); + let covers = Covers { + attempts: 1, + ..stub.covers() + }; + // Only the address of anything found, so a failure prints short. + let found = covers + .fetch(&agent(), "9780306406157", Some(42), CoverSize::Large) + .map(|cover| cover.map(|cover| cover.url)); + assert!(matches!(found, Err(Error::Unreachable(_))), "{found:?}"); + assert_eq!(stub.asked(), [ISBN13, ISBN10]); + } + + #[test] + fn a_redirect_to_an_address_by_number_is_refused() { + // A listener standing in for a service on the LAN; it only counts. + let lan = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let port = lan.local_addr().expect("an address").port(); + let reached = Arc::new(Mutex::new(0u32)); + let count = Arc::clone(&reached); + std::thread::spawn(move || { + for stream in lan.incoming() { + if stream.is_ok() { + *count.lock().expect("a lock") += 1; + } + } + }); + let stub = + Stub::start(move |_, _| Reply::redirect(format!("https://127.0.0.1:{port}/admin"))); + let failed = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Large) + .expect_err("refused"); + assert!( + matches!(&failed, Error::Unreachable(why) if why.contains("refused")), + "{failed:?}" + ); + assert_eq!(*reached.lock().expect("a lock"), 0); + } + + #[test] + fn a_small_thumbnail_is_a_cover() { + // Real Small covers run from about a kilobyte; one under it is + // still a cover. + let stub = Stub::start(|_, _| Reply::image("image/jpeg", jpeg(700))); + let cover = stub + .covers() + .fetch(&agent(), "9780306406157", None, CoverSize::Small) + .expect("an answer") + .expect("a cover"); + assert_eq!(cover.bytes.len(), 700); } #[test] @@ -874,7 +1116,7 @@ mod tests { .expect("an answer") .expect("a cover"); assert_eq!(cover.media_type, "image/jpeg"); - assert!(cover.bytes.len() > MIN_BYTES); + assert!(cover.bytes.len() > 1024); assert!(cover.url.starts_with(OPEN_LIBRARY_COVERS)); } } diff --git a/crates/bibfetch/src/lib.rs b/crates/bibfetch/src/lib.rs index 084bef3..8c84ce5 100644 --- a/crates/bibfetch/src/lib.rs +++ b/crates/bibfetch/src/lib.rs @@ -76,9 +76,11 @@ pub struct Record { /// How many pages the register says it has. #[serde(default, skip_serializing_if = "Option::is_none")] pub pages: Option, - /// The id Open Library files a cover of this book under, for - /// [`cover()`] to ask by when the ISBN has none. Its index names the - /// cover of the work, which may be another edition's. + /// The first cover id in Open Library's record of this edition, for + /// [`cover()`] to ask by when the ISBN has none. Taken from the edition + /// record only, never from the index's cover for the work, which is + /// often another edition's; `None` when the edition names no cover or + /// the lookup stopped before asking for the edition record. #[serde(default, skip_serializing_if = "Option::is_none")] pub cover_id: Option, } diff --git a/crates/bibfetch/src/source.rs b/crates/bibfetch/src/source.rs index 8719066..0ab0127 100644 --- a/crates/bibfetch/src/source.rs +++ b/crates/bibfetch/src/source.rs @@ -85,10 +85,13 @@ impl Source { } // `fields` or the index answers with a handful of defaults and // keeps the rest, which is how a language it holds went unread. + // Not `cover_i`: that is the cover chosen for the whole work, and + // on a work with hundreds of editions it is usually 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,\ -number_of_pages_median,cover_i&limit=1", +number_of_pages_median&limit=1", encode(id) ), Source::GoogleBooks => format!( @@ -278,6 +281,7 @@ fn open_library(id: &str, kind: &str, body: &str) -> Result { .find_map(|l| l.key.rsplit('/').next().map(str::to_string)) .filter(|l| !l.is_empty()); record.pages = book.number_of_pages; + // The edition's own cover, and the only place a cover id is taken from. record.cover_id = book .covers .into_iter() @@ -309,8 +313,6 @@ struct SearchDoc { subject: Vec, #[serde(rename = "number_of_pages_median", default)] pages: Option, - #[serde(default)] - cover_i: Option, } /// How many of a work's subject headings are worth carrying. @@ -342,10 +344,6 @@ fn open_library_search(id: &str, kind: &str, body: &str) -> Result 0); Ok(record) } @@ -542,18 +540,45 @@ mod tests { assert_eq!(record.cover_id, Some(1162784)); let record = open_library("1", "isbn", r#"{"title":"A"}"#).expect("a record"); assert_eq!(record.cover_id, None); - - let record = open_library_search("1", "isbn", r#"{"docs":[{"title":"A","cover_i":2468}]}"#) - .expect("a record"); - assert_eq!(record.cover_id, Some(2468)); - let record = open_library_search("1", "isbn", r#"{"docs":[{"title":"A","cover_i":-1}]}"#) - .expect("a record"); - assert_eq!(record.cover_id, None); } #[test] - fn the_index_is_asked_for_the_cover_id() { - assert!(Source::OpenLibrarySearch.url("1").contains("cover_i")); + fn the_cover_id_is_the_editions_and_not_the_works() { + // ISBN 9780140447934 today: the index (asked first) names the work's + // cover; the edition record has none. Merged as `ask::lookup` merges. + let mut kept = open_library_search( + "9780140447934", + "isbn", + r#"{"docs":[{"title":"War and Peace","cover_i":12621906}]}"#, + ) + .expect("a record"); + let edition = open_library("9780140447934", "isbn", r#"{"title":"War and Peace"}"#) + .expect("a record"); + kept.fill_from(edition); + // Cover 12621906 is OL34941467M, Standard Ebooks' Maude translation. + assert_eq!(kept.cover_id, None); + } + + #[test] + fn an_edition_with_no_covers_gives_no_cover_id() { + let index = r#"{"docs":[{"title":"War and Peace","cover_i":12621906}]}"#; + // `lookup` often stops at the index, before the edition is asked. + let alone = open_library_search("9780140447934", "isbn", index).expect("a record"); + assert_eq!(alone.cover_id, None); + // An edition whose only cover was deleted, and one with an empty list. + for edition in [ + r#"{"title":"War and Peace","covers":[-1]}"#, + r#"{"title":"War and Peace","covers":[]}"#, + ] { + let mut kept = open_library_search("9780140447934", "isbn", index).expect("a record"); + kept.fill_from(open_library("9780140447934", "isbn", edition).expect("a record")); + assert_eq!(kept.cover_id, None, "{edition}"); + } + } + + #[test] + fn the_index_is_not_asked_for_the_works_cover() { + assert!(!Source::OpenLibrarySearch.url("1").contains("cover_i")); } #[test]