From 13800cddbd38e755ff7c3cf8c80c398b534472fe Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Sat, 1 Aug 2026 13:19:08 -0400 Subject: [PATCH 1/7] feat(opds): serve acquisition feeds and stream safe library assets --- Cargo.lock | 16 + Cargo.toml | 7 +- crates/citadel-core/src/book.rs | 2 +- crates/citadel-core/src/url.rs | 2 +- crates/citadel-opds/Cargo.toml | 21 + crates/citadel-opds/src/assets.rs | 428 ++++++ crates/citadel-opds/src/catalog.rs | 1196 +++++++++++++++++ crates/citadel-opds/src/lib.rs | 6 + .../examples/bench_library_queries.rs | 2 +- crates/libcalibre/src/assets/mod.rs | 2 +- crates/libcalibre/src/error.rs | 20 +- crates/libcalibre/src/lib.rs | 4 +- crates/libcalibre/src/library.rs | 174 ++- crates/libcalibre/src/mime_type.rs | 49 + crates/libcalibre/src/operations/assets.rs | 166 ++- crates/libcalibre/src/operations/books.rs | 12 +- crates/libcalibre/src/queries/books.rs | 84 ++ .../libcalibre/tests/asset_resolution_test.rs | 179 +++ crates/libcalibre/tests/books_api_test.rs | 2 +- crates/libcalibre/tests/test_triggers.rs | 8 +- src-tauri/Cargo.toml | 1 + src-tauri/src/main.rs | 1 + src-tauri/src/opds/mod.rs | 1 + src-tauri/src/state.rs | 64 +- 24 files changed, 2366 insertions(+), 81 deletions(-) create mode 100644 crates/citadel-opds/Cargo.toml create mode 100644 crates/citadel-opds/src/assets.rs create mode 100644 crates/citadel-opds/src/catalog.rs create mode 100644 crates/citadel-opds/src/lib.rs create mode 100644 crates/libcalibre/tests/asset_resolution_test.rs create mode 100644 src-tauri/src/opds/mod.rs diff --git a/Cargo.lock b/Cargo.lock index aa81add4..73230897 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -653,12 +653,28 @@ dependencies = [ "urlencoding", ] +[[package]] +name = "citadel-opds" +version = "0.1.0" +dependencies = [ + "chrono", + "futures-util", + "libcalibre", + "quick-xml 0.38.4", + "serde", + "tempfile", + "tokio", + "urlencoding", + "uuid", +] + [[package]] name = "citadel-rs" version = "0.6.1" dependencies = [ "chrono", "citadel-core", + "citadel-opds", "diesel", "epub", "image", diff --git a/Cargo.toml b/Cargo.toml index 6f128ca7..1faf32d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,11 @@ [workspace] resolver = "2" -members = ["crates/citadel-core", "crates/libcalibre", "src-tauri"] +members = [ + "crates/citadel-core", + "crates/citadel-opds", + "crates/libcalibre", + "src-tauri", +] [workspace.package] rust-version = "1.97.1" diff --git a/crates/citadel-core/src/book.rs b/crates/citadel-core/src/book.rs index 2d1338d1..b7d38e15 100644 --- a/crates/citadel-core/src/book.rs +++ b/crates/citadel-core/src/book.rs @@ -82,7 +82,7 @@ impl LibraryBook { ) -> Self { Self { id: book.id.as_i32().to_string(), - uuid: Some(book.uuid.clone()), + uuid: book.uuid.clone(), title: book.title.clone(), author_list: book .authors diff --git a/crates/citadel-core/src/url.rs b/crates/citadel-core/src/url.rs index f7f3e60d..96be4d43 100644 --- a/crates/citadel-core/src/url.rs +++ b/crates/citadel-core/src/url.rs @@ -164,7 +164,7 @@ mod tests { ) -> libcalibre::library::Book { libcalibre::library::Book { id: libcalibre::BookId::from(1), - uuid: "test-uuid".to_string(), + uuid: Some("test-uuid".to_string()), title: "Title".to_string(), sortable_title: None, authors: vec![], diff --git a/crates/citadel-opds/Cargo.toml b/crates/citadel-opds/Cargo.toml new file mode 100644 index 00000000..ddeaaa01 --- /dev/null +++ b/crates/citadel-opds/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "citadel-opds" +version = "0.1.0" +edition = "2021" +rust-version.workspace = true +description = "Tauri-independent OPDS runtime for Citadel" + +[dependencies] +axum = "0.8.9" +bytes = "1" +chrono = { version = "0.4.31", features = ["serde"] } +futures-util = "0.3" +libcalibre = { path = "../libcalibre" } +quick-xml = "0.38" +serde = { version = "1.0", features = ["derive"] } +tokio = { version = "1.52.3", features = ["macros", "net", "rt-multi-thread", "sync", "time"] } +urlencoding = "2.1.3" +uuid = { version = "1.6.1", features = ["v4", "fast-rng"] } + +[dev-dependencies] +tempfile = "3.8" diff --git a/crates/citadel-opds/src/assets.rs b/crates/citadel-opds/src/assets.rs new file mode 100644 index 00000000..40885c53 --- /dev/null +++ b/crates/citadel-opds/src/assets.rs @@ -0,0 +1,428 @@ +use std::{ + fs::File, + io::{self, Read, Seek, SeekFrom, Take}, + path::Path, +}; + +use libcalibre::{mime_type::mime_type_from_extension, ResolvedBookAsset}; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AssetMethod { + Get, + Head, +} + +#[derive(Debug, Eq, PartialEq)] +pub struct AssetHeaders { + pub accept_ranges: &'static str, + pub content_disposition: String, + pub content_length: u64, + pub content_range: Option, + pub content_type: &'static str, +} + +/// A bounded synchronous file stream. CDL-23 can move reads onto its HTTP +/// runtime's blocking adapter without coupling asset resolution to a framework. +pub struct AssetBody { + reader: Take, +} + +impl Read for AssetBody { + fn read(&mut self, buffer: &mut [u8]) -> io::Result { + self.reader.read(buffer) + } +} + +pub struct AssetResponse { + pub status: u16, + pub headers: AssetHeaders, + pub body: Option, +} + +#[derive(Debug)] +pub enum AssetResponseError { + Io(io::Error), + RangeNotSatisfiable { length: u64 }, +} + +impl std::fmt::Display for AssetResponseError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Io(error) => write!(formatter, "Could not open asset: {error}"), + Self::RangeNotSatisfiable { length } => { + write!( + formatter, + "Range is not satisfiable for a {length}-byte asset" + ) + } + } + } +} + +impl std::error::Error for AssetResponseError {} + +impl AssetResponseError { + pub fn status(&self) -> u16 { + match self { + Self::Io(_) => 500, + Self::RangeNotSatisfiable { .. } => 416, + } + } + + pub fn content_range(&self) -> Option { + match self { + Self::RangeNotSatisfiable { length } => Some(format!("bytes */{length}")), + Self::Io(_) => None, + } + } +} + +impl From for AssetResponseError { + fn from(error: io::Error) -> Self { + Self::Io(error) + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct SelectedRange { + start: u64, + end: u64, +} + +impl SelectedRange { + fn length(self) -> u64 { + self.end - self.start + 1 + } +} + +pub fn prepare( + asset: &ResolvedBookAsset, + method: AssetMethod, + range_header: Option<&str>, +) -> Result { + prepare_path( + asset.canonical_path(), + asset.download_name(), + asset.format(), + method, + range_header, + ) +} + +fn prepare_path( + path: &Path, + download_name: &str, + format: &str, + method: AssetMethod, + range_header: Option<&str>, +) -> Result { + let mut file = File::open(path)?; + let full_length = file.metadata()?.len(); + let selected = match range_header { + Some(header) => Some(parse_range(header, full_length)?), + None => None, + }; + let (status, start, content_length, content_range) = match selected { + Some(range) => ( + 206, + range.start, + range.length(), + Some(format!( + "bytes {}-{}/{}", + range.start, range.end, full_length + )), + ), + None => (200, 0, full_length, None), + }; + + let body = if method == AssetMethod::Get { + file.seek(SeekFrom::Start(start))?; + Some(AssetBody { + reader: file.take(content_length), + }) + } else { + None + }; + + Ok(AssetResponse { + status, + headers: AssetHeaders { + accept_ranges: "bytes", + content_disposition: content_disposition(download_name), + content_length, + content_range, + content_type: mime_type(format), + }, + body, + }) +} + +fn parse_range(header: &str, length: u64) -> Result { + let invalid = || AssetResponseError::RangeNotSatisfiable { length }; + let value = header + .strip_prefix("bytes=") + .filter(|value| !value.contains(',')) + .ok_or_else(invalid)?; + let (start, end) = value.split_once('-').ok_or_else(invalid)?; + if length == 0 { + return Err(invalid()); + } + + match (start.is_empty(), end.is_empty()) { + (false, false) => { + let start = start.parse::().map_err(|_| invalid())?; + let requested_end = end.parse::().map_err(|_| invalid())?; + if start >= length || start > requested_end { + return Err(invalid()); + } + Ok(SelectedRange { + start, + end: requested_end.min(length - 1), + }) + } + (false, true) => { + let start = start.parse::().map_err(|_| invalid())?; + if start >= length { + return Err(invalid()); + } + Ok(SelectedRange { + start, + end: length - 1, + }) + } + (true, false) => { + let suffix = end.parse::().map_err(|_| invalid())?; + if suffix == 0 { + return Err(invalid()); + } + Ok(SelectedRange { + start: length.saturating_sub(suffix), + end: length - 1, + }) + } + (true, true) => Err(invalid()), + } +} + +pub fn mime_type(format: &str) -> &'static str { + mime_type_from_extension(format) +} + +fn content_disposition(filename: &str) -> String { + let fallback: String = filename + .chars() + .map(|character| { + if character.is_ascii_alphanumeric() + || matches!(character, ' ' | '.' | '-' | '_' | '(' | ')') + { + character + } else { + '_' + } + }) + .collect(); + let encoded = percent_encode(filename.as_bytes()); + format!("attachment; filename=\"{fallback}\"; filename*=UTF-8''{encoded}") +} + +fn percent_encode(bytes: &[u8]) -> String { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + let mut encoded = String::with_capacity(bytes.len()); + for &byte in bytes { + if byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'.' | b'_' | b'~') { + encoded.push(char::from(byte)); + } else { + encoded.push('%'); + encoded.push(char::from(HEX[usize::from(byte >> 4)])); + encoded.push(char::from(HEX[usize::from(byte & 0x0f)])); + } + } + encoded +} + +#[cfg(test)] +mod tests { + use std::io::Read; + + use super::*; + + fn file(bytes: &[u8]) -> tempfile::NamedTempFile { + let file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(file.path(), bytes).unwrap(); + file + } + + #[test] + fn get_streams_the_complete_file_with_download_headers() { + let file = file(b"0123456789"); + let mut response = + prepare_path(file.path(), "A Book.epub", "EPUB", AssetMethod::Get, None).unwrap(); + assert_eq!(response.status, 200); + assert_eq!(response.headers.content_length, 10); + assert_eq!(response.headers.content_type, "application/epub+zip"); + assert_eq!(response.headers.accept_ranges, "bytes"); + assert_eq!(response.headers.content_range, None); + let mut bytes = Vec::new(); + response + .body + .as_mut() + .unwrap() + .read_to_end(&mut bytes) + .unwrap(); + assert_eq!(bytes, b"0123456789"); + } + + #[test] + fn head_has_get_headers_and_no_body() { + let file = file(b"0123456789"); + let response = + prepare_path(file.path(), "A Book.pdf", "PDF", AssetMethod::Head, None).unwrap(); + assert_eq!(response.status, 200); + assert_eq!(response.headers.content_length, 10); + assert_eq!(response.headers.content_type, "application/pdf"); + assert!(response.body.is_none()); + } + + #[test] + fn streams_closed_open_and_suffix_ranges() { + let cases = [ + ("bytes=2-5", "bytes 2-5/10", b"2345".as_slice()), + ("bytes=7-", "bytes 7-9/10", b"789".as_slice()), + ("bytes=-3", "bytes 7-9/10", b"789".as_slice()), + ("bytes=8-99", "bytes 8-9/10", b"89".as_slice()), + ]; + for (header, expected_range, expected_bytes) in cases { + let file = file(b"0123456789"); + let mut response = prepare_path( + file.path(), + "book.epub", + "epub", + AssetMethod::Get, + Some(header), + ) + .unwrap(); + assert_eq!(response.status, 206); + assert_eq!( + response.headers.content_range.as_deref(), + Some(expected_range) + ); + assert_eq!(response.headers.content_length, expected_bytes.len() as u64); + let mut bytes = Vec::new(); + response + .body + .as_mut() + .unwrap() + .read_to_end(&mut bytes) + .unwrap(); + assert_eq!(bytes, expected_bytes); + } + } + + #[test] + fn head_honors_ranges_without_opening_a_body() { + let file = file(b"0123456789"); + let response = prepare_path( + file.path(), + "book.epub", + "epub", + AssetMethod::Head, + Some("bytes=2-5"), + ) + .unwrap(); + assert_eq!(response.status, 206); + assert_eq!(response.headers.content_length, 4); + assert_eq!( + response.headers.content_range.as_deref(), + Some("bytes 2-5/10") + ); + assert!(response.body.is_none()); + } + + #[test] + fn rejects_invalid_unsatisfiable_and_multiple_ranges_as_416() { + for header in [ + "items=0-1", + "bytes=", + "bytes=20-30", + "bytes=7-2", + "bytes=-0", + "bytes=0-1,4-5", + ] { + let file = file(b"0123456789"); + let error = prepare_path(file.path(), "book", "bin", AssetMethod::Get, Some(header)) + .err() + .unwrap(); + assert!(matches!( + &error, + AssetResponseError::RangeNotSatisfiable { length: 10 } + )); + assert_eq!(error.status(), 416); + assert_eq!(error.content_range().as_deref(), Some("bytes */10")); + } + } + + #[test] + fn empty_assets_reject_ranges_but_support_full_get() { + let file = file(b""); + let response = prepare_path(file.path(), "empty", "bin", AssetMethod::Get, None).unwrap(); + assert_eq!(response.status, 200); + assert_eq!(response.headers.content_length, 0); + assert!(matches!( + prepare_path( + file.path(), + "empty", + "bin", + AssetMethod::Get, + Some("bytes=0-") + ), + Err(AssetResponseError::RangeNotSatisfiable { length: 0 }) + )); + } + + #[test] + fn content_disposition_cannot_inject_headers_and_preserves_unicode() { + let header = content_disposition("Résumé\"\r\nX-Evil: yes.epub"); + assert!(!header.contains('\r')); + assert!(!header.contains('\n')); + assert!(header.contains("filename=\"R_sum____X-Evil_ yes.epub\"")); + assert!(header.contains("filename*=UTF-8''R%C3%A9sum%C3%A9%22%0D%0AX-Evil%3A%20yes.epub")); + } + + #[test] + fn maps_known_types_and_falls_back_for_unknown_formats() { + assert_eq!(mime_type("AZW3"), "application/vnd.amazon.ebook-kf8"); + assert_eq!(mime_type("cbz"), "application/vnd.comicbook+zip"); + assert_eq!(mime_type("unexpected"), "application/octet-stream"); + } + + #[test] + fn cover_assets_flow_through_the_same_response_preparation() { + let file = file(b"jpeg"); + let response = + prepare_path(file.path(), "cover.jpg", "JPG", AssetMethod::Get, None).unwrap(); + assert_eq!(response.status, 200); + assert_eq!(response.headers.content_type, "image/jpeg"); + assert!(response + .headers + .content_disposition + .contains("filename=\"cover.jpg\"")); + } + + #[test] + fn large_file_body_is_a_bounded_stream_not_an_allocated_payload() { + let file = file(b""); + file.as_file().set_len(64 * 1024 * 1024).unwrap(); + let mut response = + prepare_path(file.path(), "large.epub", "epub", AssetMethod::Get, None).unwrap(); + assert_eq!(response.headers.content_length, 64 * 1024 * 1024); + + let mut first_chunk = [0_u8; 1024]; + let read = response + .body + .as_mut() + .unwrap() + .read(&mut first_chunk) + .unwrap(); + assert_eq!(read, first_chunk.len()); + } +} diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs new file mode 100644 index 00000000..8e758a3a --- /dev/null +++ b/crates/citadel-opds/src/catalog.rs @@ -0,0 +1,1196 @@ +use std::{borrow::Cow, io::Read, sync::Arc}; + +use axum::{ + body::Body, + extract::{Path, Query, State}, + http::{header, HeaderValue, StatusCode}, + response::{IntoResponse, Response}, + routing::get, + Router, +}; +use bytes::Bytes; +use chrono::{NaiveDateTime, SecondsFormat}; +use libcalibre::{BookId, BookPage, CalibreError, ResolvedBookAsset}; +use quick_xml::{ + events::{BytesDecl, BytesEnd, BytesStart, BytesText, Event}, + Writer, +}; +use serde::Deserialize; + +use super::assets::{self, AssetMethod}; + +const PAGE_SIZE: i64 = 50; +const ACQUISITION_REL: &str = "http://opds-spec.org/acquisition"; +const IMAGE_REL: &str = "http://opds-spec.org/image"; +const ATOM_TYPE: &str = "application/atom+xml;profile=opds-catalog;kind=acquisition"; +const ATOM_CONTENT_TYPE: &str = + "application/atom+xml;profile=opds-catalog;kind=acquisition; charset=utf-8"; + +pub trait CatalogSource: Send + Sync + 'static { + fn active_library_id(&self) -> Result; + + fn book_page( + &self, + limit: i64, + offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError>; + fn book_file(&self, book_id: BookId, format: &str) -> Result; + fn book_cover(&self, book_id: BookId) -> Result; +} + +#[derive(Clone)] +struct CatalogState { + source: Arc, +} + +#[derive(Deserialize)] +struct PageQuery { + page: Option, +} + +pub fn router(source: Arc) -> Router { + Router::new() + .route("/opds", get(root_feed)) + .route("/opds/all", get(all_books_feed)) + .route( + "/opds/books/{book_id}/files/{format}/{filename}", + get(book_file).head(book_file_head), + ) + .route( + "/opds/books/{book_id}/cover", + get(book_cover).head(book_cover_head), + ) + .with_state(CatalogState { source }) +} + +async fn root_feed(state: State, query: Query) -> Response { + feed(state, query, "/opds").await +} + +async fn all_books_feed(state: State, query: Query) -> Response { + feed(state, query, "/opds/all").await +} + +async fn feed( + State(state): State, + Query(query): Query, + route: &'static str, +) -> Response { + let page_number = query.page.unwrap_or(1); + if page_number == 0 { + return public_error(StatusCode::BAD_REQUEST, "Invalid page"); + } + let offset = match page_number + .checked_sub(1) + .and_then(|page| page.checked_mul(PAGE_SIZE as u64)) + .and_then(|offset| i64::try_from(offset).ok()) + { + Some(offset) => offset, + None => return public_error(StatusCode::BAD_REQUEST, "Invalid page"), + }; + + let source = state.source.clone(); + let result = tokio::task::spawn_blocking(move || source.book_page(PAGE_SIZE, offset)).await; + let (library_uuid, updated_at, page) = match result { + Ok(Ok(page)) => page, + Ok(Err(error)) => return calibre_error(error), + Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Catalog unavailable"), + }; + + let last_page = page_count(page.total); + if page_number > last_page { + return public_error(StatusCode::NOT_FOUND, "Page not found"); + } + + match acquisition_feed( + &library_uuid, + updated_at, + &page, + page_number, + last_page, + route, + ) { + Ok(xml) => ( + [( + header::CONTENT_TYPE, + HeaderValue::from_static(ATOM_CONTENT_TYPE), + )], + xml, + ) + .into_response(), + Err(_) => public_error(StatusCode::INTERNAL_SERVER_ERROR, "Catalog unavailable"), + } +} + +async fn book_file( + state: State, + Path((book_id, format, _filename)): Path<(i32, String, String)>, + headers: axum::http::HeaderMap, +) -> Response { + asset_response( + state, + BookId(book_id), + Some(format), + AssetMethod::Get, + headers, + ) + .await +} + +async fn book_file_head( + state: State, + Path((book_id, format, _filename)): Path<(i32, String, String)>, + headers: axum::http::HeaderMap, +) -> Response { + asset_response( + state, + BookId(book_id), + Some(format), + AssetMethod::Head, + headers, + ) + .await +} + +async fn book_cover( + state: State, + Path(book_id): Path, + headers: axum::http::HeaderMap, +) -> Response { + asset_response(state, BookId(book_id), None, AssetMethod::Get, headers).await +} + +async fn book_cover_head( + state: State, + Path(book_id): Path, + headers: axum::http::HeaderMap, +) -> Response { + asset_response(state, BookId(book_id), None, AssetMethod::Head, headers).await +} + +async fn asset_response( + State(state): State, + book_id: BookId, + format: Option, + method: AssetMethod, + headers: axum::http::HeaderMap, +) -> Response { + let range = headers + .get(header::RANGE) + .and_then(|value| value.to_str().ok()) + .map(str::to_owned); + let source = state.source.clone(); + let resolved = tokio::task::spawn_blocking(move || match format { + Some(format) => source.book_file(book_id, &format), + None => source.book_cover(book_id), + }) + .await; + let asset = match resolved { + Ok(Ok(asset)) => asset, + Ok(Err(error)) => return calibre_error(error), + Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Asset unavailable"), + }; + + let prepared = + tokio::task::spawn_blocking(move || assets::prepare(&asset, method, range.as_deref())) + .await; + let prepared = match prepared { + Ok(Ok(response)) => response, + Ok(Err(error)) => { + let mut response = public_error( + StatusCode::from_u16(error.status()).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR), + if error.status() == 416 { + "Range not satisfiable" + } else { + "Asset unavailable" + }, + ); + if let Some(content_range) = error.content_range() { + if let Ok(value) = HeaderValue::from_str(&content_range) { + response.headers_mut().insert(header::CONTENT_RANGE, value); + } + } + return response; + } + Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Asset unavailable"), + }; + + let mut builder = Response::builder() + .status(prepared.status) + .header(header::ACCEPT_RANGES, prepared.headers.accept_ranges) + .header( + header::CONTENT_DISPOSITION, + prepared.headers.content_disposition, + ) + .header(header::CONTENT_LENGTH, prepared.headers.content_length) + .header(header::CONTENT_TYPE, prepared.headers.content_type); + if let Some(content_range) = prepared.headers.content_range { + builder = builder.header(header::CONTENT_RANGE, content_range); + } + + let body = match prepared.body { + Some(mut reader) => { + let (sender, receiver) = tokio::sync::mpsc::channel(2); + tokio::task::spawn_blocking(move || loop { + let mut buffer = vec![0_u8; 64 * 1024]; + match reader.read(&mut buffer) { + Ok(0) => break, + Ok(read) => { + buffer.truncate(read); + if sender + .blocking_send(Ok::<_, std::io::Error>(Bytes::from(buffer))) + .is_err() + { + break; + } + } + Err(error) => { + let _ = sender.blocking_send(Err(error)); + break; + } + } + }); + Body::from_stream(futures_util::stream::unfold( + receiver, + |mut receiver| async move { receiver.recv().await.map(|item| (item, receiver)) }, + )) + } + None => Body::empty(), + }; + builder + .body(body) + .unwrap_or_else(|_| public_error(StatusCode::INTERNAL_SERVER_ERROR, "Asset unavailable")) +} + +fn acquisition_feed( + library_uuid: &str, + updated_at: Option, + page: &BookPage, + page_number: u64, + last_page: u64, + route: &str, +) -> Result, quick_xml::Error> { + let mut writer = Writer::new(Vec::new()); + writer.write_event(Event::Decl(BytesDecl::new("1.0", Some("UTF-8"), None)))?; + let mut feed = BytesStart::new("feed"); + feed.push_attribute(("xmlns", "http://www.w3.org/2005/Atom")); + feed.push_attribute(("xmlns:dc", "http://purl.org/dc/terms/")); + writer.write_event(Event::Start(feed))?; + text_element(&mut writer, "id", &library_identity(library_uuid))?; + text_element(&mut writer, "title", "Citadel — All Books")?; + text_element(&mut writer, "updated", &feed_updated(updated_at))?; + writer.write_event(Event::Start(BytesStart::new("author")))?; + text_element(&mut writer, "name", "Citadel")?; + writer.write_event(Event::End(BytesEnd::new("author")))?; + + let self_href = page_href(route, page_number); + link(&mut writer, "self", &self_href, ATOM_TYPE)?; + link(&mut writer, "start", "/opds", ATOM_TYPE)?; + if route != "/opds" { + link(&mut writer, "up", "/opds", ATOM_TYPE)?; + } + if page_number > 1 { + link( + &mut writer, + "previous", + &page_href(route, page_number - 1), + ATOM_TYPE, + )?; + link(&mut writer, "first", route, ATOM_TYPE)?; + } + if page_number < last_page { + link( + &mut writer, + "next", + &page_href(route, page_number + 1), + ATOM_TYPE, + )?; + link(&mut writer, "last", &page_href(route, last_page), ATOM_TYPE)?; + } + + for book in &page.items { + writer.write_event(Event::Start(BytesStart::new("entry")))?; + text_element( + &mut writer, + "id", + &book_identity(library_uuid, book.uuid.as_deref(), book.id), + )?; + text_element(&mut writer, "title", &book.title)?; + text_element(&mut writer, "updated", ×tamp(book.updated_at))?; + for author in &book.authors { + writer.write_event(Event::Start(BytesStart::new("author")))?; + text_element(&mut writer, "name", &author.name)?; + writer.write_event(Event::End(BytesEnd::new("author")))?; + } + text_element(&mut writer, "published", ×tamp(book.created_at))?; + let mut content = BytesStart::new("content"); + content.push_attribute(("type", "text")); + writer.write_event(Event::Start(content))?; + writer.write_event(Event::Text(BytesText::new(&xml_text( + book.description.as_deref().unwrap_or(""), + ))))?; + writer.write_event(Event::End(BytesEnd::new("content")))?; + for language in &book.language_codes { + text_element(&mut writer, "dc:language", language)?; + } + for identifier in &book.identifiers { + text_element(&mut writer, "dc:identifier", &identifier.value)?; + } + for tag in &book.tags { + let mut category = BytesStart::new("category"); + category.push_attribute(("term", xml_text(tag).as_ref())); + writer.write_event(Event::Empty(category))?; + } + if book.has_cover { + link( + &mut writer, + IMAGE_REL, + &format!("/opds/books/{}/cover", book.id.as_i32()), + "image/jpeg", + )?; + } + for file in &book.files { + link( + &mut writer, + ACQUISITION_REL, + &format!( + "/opds/books/{}/files/{}/{}", + book.id.as_i32(), + urlencoding::encode(&file.format), + urlencoding::encode(&format!( + "{}.{}", + file.name, + file.format.to_ascii_lowercase() + )) + ), + assets::mime_type(&file.format), + )?; + } + writer.write_event(Event::End(BytesEnd::new("entry")))?; + } + + writer.write_event(Event::End(BytesEnd::new("feed")))?; + Ok(writer.into_inner()) +} + +fn text_element( + writer: &mut Writer>, + name: &str, + value: &str, +) -> Result<(), quick_xml::Error> { + writer.write_event(Event::Start(BytesStart::new(name)))?; + writer.write_event(Event::Text(BytesText::new(&xml_text(value))))?; + writer.write_event(Event::End(BytesEnd::new(name)))?; + Ok(()) +} + +fn link( + writer: &mut Writer>, + relation: &str, + href: &str, + media_type: &str, +) -> Result<(), quick_xml::Error> { + let mut element = BytesStart::new("link"); + element.push_attribute(("rel", relation)); + element.push_attribute(("href", href)); + element.push_attribute(("type", media_type)); + writer.write_event(Event::Empty(element))?; + Ok(()) +} + +fn page_count(total: i64) -> u64 { + if total <= 0 { + 1 + } else { + ((total as u64 - 1) / PAGE_SIZE as u64) + 1 + } +} + +fn page_href(route: &str, page: u64) -> String { + if page == 1 { + route.to_string() + } else { + format!("{route}?page={page}") + } +} + +fn library_identity(raw_uuid: &str) -> String { + match uuid::Uuid::parse_str(raw_uuid) { + Ok(uuid) => format!("urn:uuid:{uuid}"), + Err(_) => format!("urn:citadel:library:{}", hex(raw_uuid.as_bytes())), + } +} + +fn book_identity(library_uuid: &str, raw_uuid: Option<&str>, book_id: BookId) -> String { + match raw_uuid.and_then(|value| uuid::Uuid::parse_str(value).ok()) { + Some(uuid) => format!("urn:uuid:{uuid}"), + None => format!( + "urn:citadel:book:{}:{}", + hex(library_uuid.as_bytes()), + book_id.as_i32() + ), + } +} + +fn hex(bytes: &[u8]) -> String { + const HEX: &[u8; 16] = b"0123456789abcdef"; + let mut result = String::with_capacity(bytes.len() * 2); + for byte in bytes { + result.push(char::from(HEX[usize::from(byte >> 4)])); + result.push(char::from(HEX[usize::from(byte & 0x0f)])); + } + result +} + +fn xml_text(value: &str) -> Cow<'_, str> { + if value.chars().all(valid_xml_char) { + Cow::Borrowed(value) + } else { + Cow::Owned( + value + .chars() + .filter(|character| valid_xml_char(*character)) + .collect(), + ) + } +} + +fn valid_xml_char(character: char) -> bool { + matches!(character, '\u{9}' | '\u{a}' | '\u{d}') + || ('\u{20}'..='\u{d7ff}').contains(&character) + || ('\u{e000}'..='\u{fffd}').contains(&character) + || ('\u{10000}'..='\u{10ffff}').contains(&character) +} + +fn feed_updated(updated_at: Option) -> String { + updated_at + .map(timestamp) + .unwrap_or_else(|| "1970-01-01T00:00:00Z".to_string()) +} + +fn timestamp(timestamp: NaiveDateTime) -> String { + timestamp + .and_utc() + .to_rfc3339_opts(SecondsFormat::AutoSi, true) +} + +fn calibre_error(error: CalibreError) -> Response { + match error { + CalibreError::LibraryNotInitialized => { + public_error(StatusCode::SERVICE_UNAVAILABLE, "No active library") + } + CalibreError::BookNotFound(_) + | CalibreError::BookFileNotFound(_, _) + | CalibreError::BookCoverNotFound(_) + | CalibreError::AssetFileMissing(_) => { + public_error(StatusCode::NOT_FOUND, "Asset not found") + } + CalibreError::InvalidBookFormat(_) => { + public_error(StatusCode::BAD_REQUEST, "Invalid asset request") + } + CalibreError::Database(message) + if message.to_ascii_lowercase().contains("busy") + || message.to_ascii_lowercase().contains("locked") => + { + public_error(StatusCode::SERVICE_UNAVAILABLE, "Library busy") + } + _ => public_error(StatusCode::INTERNAL_SERVER_ERROR, "Catalog unavailable"), + } +} + +fn public_error(status: StatusCode, message: &'static str) -> Response { + ( + status, + [(header::CONTENT_TYPE, "text/plain; charset=utf-8")], + message, + ) + .into_response() +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::NaiveDate; + use diesel::{Connection, RunQueryDsl}; + use libcalibre::{ + library::Book, util::get_db_path, BookAdd, BookFileInfo, BookIdentifier, BookUpdate, + Library, LibraryAuthor, + }; + use quick_xml::{name::ResolveResult, NsReader}; + use std::{collections::HashMap, path::PathBuf, sync::Mutex}; + use tempfile::TempDir; + + fn book(title: &str, uuid: &str, id: i32) -> Book { + Book { + id: BookId(id), + uuid: Some(uuid.to_string()), + title: title.to_string(), + sortable_title: None, + authors: vec![LibraryAuthor { + id: libcalibre::AuthorId(1), + name: "A & Co".to_string(), + sort: "Writer, A".to_string(), + link: None, + }], + tags: Vec::new(), + series: None, + series_index: None, + description: Some("Words & symbols".to_string()), + language_codes: vec!["eng".to_string()], + identifiers: Vec::new(), + has_cover: true, + is_read: false, + files: vec![BookFileInfo { + id: 1, + format: "EPUB".to_string(), + name: "book".to_string(), + uncompressed_size: 3, + }], + created_at: NaiveDate::from_ymd_opt(2024, 1, 2) + .unwrap() + .and_hms_opt(3, 4, 5) + .unwrap(), + updated_at: NaiveDate::from_ymd_opt(2024, 2, 3) + .unwrap() + .and_hms_opt(4, 5, 6) + .unwrap(), + book_dir_path: "Author/Book".to_string(), + } + } + + struct MemorySource { + books: Vec, + } + + impl CatalogSource for MemorySource { + fn book_page( + &self, + limit: i64, + offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError> { + let items = self + .books + .iter() + .skip(offset as usize) + .take(limit as usize) + .cloned() + .collect(); + Ok(( + "550e8400-e29b-41d4-a716-446655440000".to_string(), + self.books.iter().map(|book| book.updated_at).max(), + BookPage { + items, + total: self.books.len() as i64, + }, + )) + } + + fn book_file( + &self, + book_id: BookId, + format: &str, + ) -> Result { + Err(CalibreError::BookFileNotFound(book_id, format.to_string())) + } + + fn book_cover(&self, book_id: BookId) -> Result { + Err(CalibreError::BookCoverNotFound(book_id)) + } + } + + struct LibrarySource { + library: Mutex, + } + + struct NoLibrary; + + struct FailureSource(fn() -> CalibreError); + + impl CatalogSource for NoLibrary { + fn book_page( + &self, + _limit: i64, + _offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError> { + Err(CalibreError::LibraryNotInitialized) + } + + fn book_file( + &self, + _book_id: BookId, + _format: &str, + ) -> Result { + Err(CalibreError::LibraryNotInitialized) + } + + fn book_cover(&self, _book_id: BookId) -> Result { + Err(CalibreError::LibraryNotInitialized) + } + } + + impl CatalogSource for FailureSource { + fn book_page( + &self, + _limit: i64, + _offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError> { + Err((self.0)()) + } + + fn book_file( + &self, + _book_id: BookId, + _format: &str, + ) -> Result { + Err((self.0)()) + } + + fn book_cover(&self, _book_id: BookId) -> Result { + Err((self.0)()) + } + } + + impl CatalogSource for LibrarySource { + fn book_page( + &self, + limit: i64, + offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError> { + let mut library = self.library.lock().unwrap(); + let uuid = library.library_uuid()?; + let updated = library.catalog_updated_at()?; + let page = library.query_acquirable_books(limit, offset)?; + Ok((uuid, updated, page)) + } + + fn book_file( + &self, + book_id: BookId, + format: &str, + ) -> Result { + self.library + .lock() + .unwrap() + .resolve_book_file(book_id, format) + } + + fn book_cover(&self, book_id: BookId) -> Result { + self.library.lock().unwrap().resolve_book_cover(book_id) + } + } + + fn test_library() -> (TempDir, Library) { + let directory = tempfile::tempdir().unwrap(); + let fixture = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../crates/libcalibre/tests/fixtures/empty_library/metadata.db"); + std::fs::copy(fixture, directory.path().join("metadata.db")).unwrap(); + let db_path = get_db_path(directory.path().to_str().unwrap()).unwrap(); + (directory, Library::new(db_path).unwrap()) + } + + async fn loopback(source: Arc) -> (String, tokio::task::JoinHandle<()>) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let task = tokio::spawn(async move { + axum::serve(listener, router(source)).await.unwrap(); + }); + (format!("http://{address}"), task) + } + + #[derive(Default)] + struct ParsedFeed { + ids: Vec, + titles: Vec, + content: Vec, + acquisition_hrefs: Vec, + image_hrefs: Vec, + next: Option, + previous: Option, + } + + fn parsed_feed(xml: &[u8]) -> ParsedFeed { + const ATOM: &[u8] = b"http://www.w3.org/2005/Atom"; + let mut reader = NsReader::from_reader(xml); + reader.config_mut().trim_text(false); + let mut parsed = ParsedFeed::default(); + let mut current = None::<(Vec, String)>; + let mut in_entry = false; + loop { + match reader.read_resolved_event() { + Ok((ResolveResult::Bound(namespace), Event::Start(element))) + if namespace.as_ref() == ATOM => + { + let name = element.local_name().as_ref().to_vec(); + if name == b"entry" { + in_entry = true; + } else if in_entry && matches!(name.as_slice(), b"id" | b"title" | b"content") { + current = Some((name, String::new())); + } + } + Ok((ResolveResult::Bound(namespace), Event::Empty(element))) + if namespace.as_ref() == ATOM && element.local_name().as_ref() == b"link" => + { + let rel = element + .attributes() + .flatten() + .find(|attribute| attribute.key.as_ref() == b"rel") + .and_then(|attribute| attribute.unescape_value().ok()) + .map(|value| value.into_owned()); + let href = element + .attributes() + .flatten() + .find(|attribute| attribute.key.as_ref() == b"href") + .and_then(|attribute| attribute.unescape_value().ok()) + .map(|value| value.into_owned()); + match (rel.as_deref(), href) { + (Some(ACQUISITION_REL), Some(href)) if in_entry => { + parsed.acquisition_hrefs.push(href) + } + (Some(IMAGE_REL), Some(href)) if in_entry => parsed.image_hrefs.push(href), + (Some("next"), Some(href)) => parsed.next = Some(href), + (Some("previous"), Some(href)) => parsed.previous = Some(href), + _ => {} + } + } + Ok((_, Event::Text(text))) if current.is_some() => { + current + .as_mut() + .unwrap() + .1 + .push_str(&text.decode().unwrap()); + } + Ok((_, Event::GeneralRef(reference))) if current.is_some() => { + let decoded = reference.decode().unwrap(); + let value = match decoded.as_ref() { + "amp" => "&", + "lt" => "<", + "gt" => ">", + "quot" => "\"", + "apos" => "'", + _ => "", + }; + current.as_mut().unwrap().1.push_str(value); + } + Ok((ResolveResult::Bound(namespace), Event::End(element))) + if namespace.as_ref() == ATOM => + { + if element.local_name().as_ref() == b"entry" { + in_entry = false; + } + if let Some((name, value)) = current.take() { + if element.local_name().as_ref() == name.as_slice() { + match name.as_slice() { + b"id" => parsed.ids.push(value), + b"title" => parsed.titles.push(value), + b"content" => parsed.content.push(value), + _ => {} + } + } else { + current = Some((name, value)); + } + } + } + Ok((_, Event::Eof)) => break, + Err(error) => panic!("invalid OPDS XML: {error}"), + _ => {} + } + } + parsed + } + + #[test] + fn writer_escapes_metadata_and_emits_acquisition_contract() { + let mut legacy_book = book("A \u{1} & More", "not-a-uuid", 42); + legacy_book.authors[0].name = "A \u{0} & Co".to_string(); + legacy_book.description = Some("Words \u{b} & symbols".to_string()); + legacy_book.tags = vec!["Tag \u{c} & two".to_string()]; + legacy_book.identifiers = vec![BookIdentifier { + id: 1, + label: "custom".to_string(), + value: "ID \u{7} & two".to_string(), + }]; + let page = BookPage { + items: vec![legacy_book], + total: 1, + }; + let xml = + String::from_utf8(acquisition_feed("bad-library", None, &page, 1, 1, "/opds").unwrap()) + .unwrap(); + assert!(xml.contains("A <Book> & More")); + assert!(xml.contains("A <Writer> & Co")); + assert!(xml.contains("Words <with> & symbols")); + assert!(xml.contains("term=\"Tag <one> & two\"")); + assert!(xml.contains("ID <one> & two")); + assert!(!xml.chars().any(|character| !valid_xml_char(character))); + assert!(xml.contains("urn:citadel:book:6261642d6c696272617279:42")); + assert!(xml.contains("rel=\"http://opds-spec.org/acquisition\"")); + assert!(xml.contains("type=\"application/epub+zip\"")); + assert!(xml.contains("href=\"/opds/books/42/files/EPUB/book.epub\"")); + assert!(xml.contains("rel=\"http://opds-spec.org/image\"")); + let parsed = parsed_feed(xml.as_bytes()); + assert_eq!(parsed.titles, ["A & More"]); + assert_eq!(parsed.content, ["Words & symbols"]); + assert_eq!(parsed.acquisition_hrefs.len(), 1); + } + + #[test] + fn pagination_links_are_bounded() { + let page = BookPage { + items: Vec::new(), + total: 101, + }; + let first = + String::from_utf8(acquisition_feed("id", None, &page, 1, 3, "/opds/all").unwrap()) + .unwrap(); + assert!(first.contains("rel=\"next\" href=\"/opds/all?page=2\"")); + assert!(!first.contains("rel=\"previous\"")); + let last = + String::from_utf8(acquisition_feed("id", None, &page, 3, 3, "/opds/all").unwrap()) + .unwrap(); + assert!(last.contains("rel=\"previous\" href=\"/opds/all?page=2\"")); + assert!(!last.contains("rel=\"next\"")); + } + + #[test] + fn valid_uuids_are_normalized_and_fallbacks_are_deterministic() { + assert_eq!( + library_identity("550E8400-E29B-41D4-A716-446655440000"), + "urn:uuid:550e8400-e29b-41d4-a716-446655440000" + ); + assert_eq!( + book_identity("lib", Some("bad"), BookId(7)), + book_identity("lib", Some("bad"), BookId(7)) + ); + } + + #[tokio::test] + async fn loopback_pagination_visits_every_entry_once() { + let books = (1..=101) + .map(|id| { + book( + &format!("Book {id:03}"), + &uuid::Uuid::new_v4().to_string(), + id, + ) + }) + .collect(); + let (base, server) = loopback(Arc::new(MemorySource { books })).await; + let client = reqwest::Client::new(); + let mut path = "/opds/all".to_string(); + let mut ids = Vec::new(); + let mut page_number = 1; + loop { + let response = client.get(format!("{base}{path}")).send().await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.headers()[header::CONTENT_TYPE], ATOM_CONTENT_TYPE); + assert!(response.headers().get(header::LAST_MODIFIED).is_none()); + let feed = parsed_feed(&response.bytes().await.unwrap()); + if page_number == 1 { + assert!(feed.previous.is_none()); + } else { + assert!(feed.previous.is_some()); + } + ids.extend(feed.ids); + match feed.next { + Some(next) => path = next, + None => break, + } + page_number += 1; + } + ids.sort(); + ids.dedup(); + assert_eq!(ids.len(), 101); + assert_eq!(page_number, 3); + server.abort(); + } + + #[tokio::test] + async fn loopback_serves_real_library_feed_head_get_and_range() { + let (directory, mut library) = test_library(); + let source_path = directory.path().join("L'été & 漢字.epub"); + std::fs::write(&source_path, b"0123456789").unwrap(); + let added = library + .add_book(BookAdd { + title: "Café & More".to_string(), + author_names: vec!["A & Author".to_string()], + tags: Some(vec!["One & Two".to_string()]), + series: None, + series_index: None, + publisher: None, + publication_date: None, + rating: None, + comments: Some("Description & intact".to_string()), + identifiers: HashMap::new(), + language: Some("eng".to_string()), + file_paths: vec![source_path], + }) + .unwrap(); + library + .upsert_book_identifier( + added.id, + "custom".to_string(), + "ID & 漢字".to_string(), + None, + ) + .unwrap(); + let after_identifier = library.get_book(added.id).unwrap().updated_at; + assert!(after_identifier > added.updated_at); + library + .update_book( + added.id, + BookUpdate { + title: None, + author_names: None, + author_ids: None, + description: Some("Description & intact".to_string()), + is_read: None, + tags: None, + series: None, + series_index: None, + language_codes: None, + publisher: None, + publication_date: None, + rating: None, + comments: None, + identifiers: None, + }, + ) + .unwrap(); + let after_metadata = library.get_book(added.id).unwrap().updated_at; + assert!(after_metadata > after_identifier); + library + .set_book_cover(added.id, b"cover-bytes".to_vec()) + .unwrap(); + assert!(library.get_book(added.id).unwrap().updated_at > after_metadata); + let metadata_only = library + .add_book(BookAdd { + title: "Metadata only".to_string(), + author_names: vec!["Nobody".to_string()], + tags: None, + series: None, + series_index: None, + publisher: None, + publication_date: None, + rating: None, + comments: None, + identifiers: HashMap::new(), + language: None, + file_paths: Vec::new(), + }) + .unwrap(); + + let database_path = library.database_path().to_string(); + drop(library); + let mut connection = diesel::SqliteConnection::establish(&database_path).unwrap(); + libcalibre::persistence::register_sql_functions(&mut connection).unwrap(); + diesel::sql_query(format!( + "UPDATE books SET uuid = NULL WHERE id = {}", + added.id.as_i32() + )) + .execute(&mut connection) + .unwrap(); + diesel::sql_query("UPDATE books SET last_modified = '2000-01-01 00:00:00'") + .execute(&mut connection) + .unwrap(); + diesel::sql_query(format!( + "INSERT INTO data (book, format, uncompressed_size, name) \ + VALUES ({}, 'PDF', 10, 'missing')", + metadata_only.id.as_i32() + )) + .execute(&mut connection) + .unwrap(); + drop(connection); + let mut library = + Library::new(get_db_path(directory.path().to_str().unwrap()).unwrap()).unwrap(); + assert_eq!(library.get_book(added.id).unwrap().uuid, None); + assert!( + library.catalog_updated_at().unwrap().unwrap() + > library.get_book(added.id).unwrap().updated_at + ); + + let (base, server) = loopback(Arc::new(LibrarySource { + library: Mutex::new(library), + })) + .await; + let client = reqwest::Client::new(); + let feed_head = client.head(format!("{base}/opds")).send().await.unwrap(); + assert_eq!(feed_head.status(), StatusCode::OK); + assert_eq!(feed_head.headers()[header::CONTENT_TYPE], ATOM_CONTENT_TYPE); + assert!(feed_head.headers().get(header::LAST_MODIFIED).is_none()); + assert!(feed_head.bytes().await.unwrap().is_empty()); + let response = client.get(format!("{base}/opds")).send().await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let bytes = response.bytes().await.unwrap(); + let xml = String::from_utf8(bytes.to_vec()).unwrap(); + assert!(xml.contains("A & Author")); + assert!(xml.contains("term=\"One & Two\"")); + assert!(xml.contains("ID <one> & 漢字")); + let feed = parsed_feed(&bytes); + assert_eq!(feed.titles, ["Café & More"]); + assert_eq!(feed.content, ["Description & intact"]); + assert!(feed.ids[0].starts_with("urn:citadel:book:")); + assert_eq!(feed.acquisition_hrefs.len(), 1); + let href = &feed.acquisition_hrefs[0]; + assert!(href.ends_with(".epub")); + assert!(href.contains("%")); + + assert_eq!(feed.image_hrefs.len(), 1); + let cover = client + .get(format!("{base}{}", feed.image_hrefs[0])) + .send() + .await + .unwrap(); + assert_eq!(cover.status(), StatusCode::OK); + assert_eq!(cover.headers()[header::CONTENT_TYPE], "image/jpeg"); + assert_eq!(cover.bytes().await.unwrap(), b"cover-bytes".as_slice()); + + let head = client.head(format!("{base}{href}")).send().await.unwrap(); + assert_eq!(head.status(), StatusCode::OK); + assert_eq!(head.headers()[header::CONTENT_TYPE], "application/epub+zip"); + assert_eq!(head.headers()[header::CONTENT_LENGTH], "10"); + assert!(head.bytes().await.unwrap().is_empty()); + + let get = client.get(format!("{base}{href}")).send().await.unwrap(); + assert_eq!(get.bytes().await.unwrap(), b"0123456789".as_slice()); + let range = client + .get(format!("{base}{href}")) + .header(header::RANGE, "bytes=2-5") + .send() + .await + .unwrap(); + assert_eq!(range.status(), StatusCode::PARTIAL_CONTENT); + assert_eq!(range.headers()[header::CONTENT_RANGE], "bytes 2-5/10"); + assert_eq!(range.bytes().await.unwrap(), b"2345".as_slice()); + + let missing = client + .get(format!( + "{base}/opds/books/{}/files/PDF/missing.pdf", + added.id.as_i32() + )) + .send() + .await + .unwrap(); + assert_eq!(missing.status(), StatusCode::NOT_FOUND); + assert_eq!(missing.text().await.unwrap(), "Asset not found"); + server.abort(); + } + + #[tokio::test] + async fn real_library_paging_has_no_holes_around_fileless_books() { + let (directory, mut library) = test_library(); + let source_path = directory.path().join("shared.epub"); + std::fs::write(&source_path, b"book").unwrap(); + for index in 0..55 { + let has_file = !matches!(index, 4 | 17 | 33 | 49); + library + .add_book(BookAdd { + title: format!("Book {index:02}"), + author_names: vec!["Author".to_string()], + tags: None, + series: None, + series_index: None, + publisher: None, + publication_date: None, + rating: None, + comments: None, + identifiers: HashMap::new(), + language: None, + file_paths: has_file.then(|| source_path.clone()).into_iter().collect(), + }) + .unwrap(); + } + + let (base, server) = loopback(Arc::new(LibrarySource { + library: Mutex::new(library), + })) + .await; + let client = reqwest::Client::new(); + let first = client.get(format!("{base}/opds")).send().await.unwrap(); + assert_eq!(first.status(), StatusCode::OK); + let first = parsed_feed(&first.bytes().await.unwrap()); + assert_eq!(first.ids.len(), 50); + let second_path = first + .next + .expect("51 resolvable books require a second page"); + let second = client + .get(format!("{base}{second_path}")) + .send() + .await + .unwrap(); + let second = parsed_feed(&second.bytes().await.unwrap()); + assert_eq!(second.ids.len(), 1); + assert!(second.next.is_none()); + assert!(second.previous.is_some()); + let mut ids = first.ids; + ids.extend(second.ids); + ids.sort(); + ids.dedup(); + assert_eq!(ids.len(), 51); + server.abort(); + } + + #[tokio::test] + async fn empty_and_unavailable_catalogs_return_valid_non_sensitive_responses() { + let (base, server) = loopback(Arc::new(MemorySource { books: Vec::new() })).await; + let response = reqwest::get(format!("{base}/opds")).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let bytes = response.bytes().await.unwrap(); + let feed = parsed_feed(&bytes); + assert!(feed.ids.is_empty()); + assert!(String::from_utf8(bytes.to_vec()) + .unwrap() + .contains("Citadel")); + server.abort(); + + let (base, server) = loopback(Arc::new(NoLibrary)).await; + let response = reqwest::get(format!("{base}/opds")).await.unwrap(); + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(response.text().await.unwrap(), "No active library"); + server.abort(); + } + + #[tokio::test] + async fn invalid_pages_and_library_failures_are_bounded_and_redacted() { + let client = reqwest::Client::new(); + let (base, server) = loopback(Arc::new(MemorySource { books: Vec::new() })).await; + for page in ["0", "18446744073709551615"] { + let response = client + .get(format!("{base}/opds?page={page}")) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + let response = client + .get(format!("{base}/opds?page=2")) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + let response = client + .get(format!("{base}/opds?page=nope")) + .send() + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + server.abort(); + + let (base, server) = loopback(Arc::new(FailureSource(|| { + CalibreError::Database("database is locked at /private/library/metadata.db".to_string()) + }))) + .await; + let response = client.get(format!("{base}/opds")).send().await.unwrap(); + assert_eq!(response.status(), StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(response.text().await.unwrap(), "Library busy"); + server.abort(); + + let (base, server) = loopback(Arc::new(FailureSource(|| { + CalibreError::DatabaseIntegrity("broken /private/library/metadata.db".to_string()) + }))) + .await; + let response = client.get(format!("{base}/opds")).send().await.unwrap(); + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!(response.text().await.unwrap(), "Catalog unavailable"); + server.abort(); + } +} diff --git a/crates/citadel-opds/src/lib.rs b/crates/citadel-opds/src/lib.rs new file mode 100644 index 00000000..565b0ce2 --- /dev/null +++ b/crates/citadel-opds/src/lib.rs @@ -0,0 +1,6 @@ +//! Tauri-independent OPDS catalog and asset streaming. + +pub mod assets; +pub mod catalog; + +pub use catalog::{router, CatalogSource}; diff --git a/crates/libcalibre/examples/bench_library_queries.rs b/crates/libcalibre/examples/bench_library_queries.rs index f9ed1aea..11ea2afd 100644 --- a/crates/libcalibre/examples/bench_library_queries.rs +++ b/crates/libcalibre/examples/bench_library_queries.rs @@ -317,7 +317,7 @@ fn run() -> Result<(), String> { return Err(format!("expected total {expected}, got {total}")); } let hydrated = &first_page.items[0]; - if hydrated.uuid.is_empty() || hydrated.authors.is_empty() { + if hydrated.uuid.as_deref().is_none_or(str::is_empty) || hydrated.authors.is_empty() { return Err("first page items are not fully hydrated".to_string()); } println!("Sanity check passed: total == {expected}, page is hydrated"); diff --git a/crates/libcalibre/src/assets/mod.rs b/crates/libcalibre/src/assets/mod.rs index 49395bc7..5aa75cf6 100644 --- a/crates/libcalibre/src/assets/mod.rs +++ b/crates/libcalibre/src/assets/mod.rs @@ -14,7 +14,7 @@ pub fn write(path: &Path, data: &[u8]) -> Result<(), CalibreError> { std::fs::write(path, data).map_err(|e| CalibreError::FileSystem(e.to_string())) } -pub fn asset_path(library_root: &String, book_path: &str, filename: &str) -> PathBuf { +pub fn asset_path(library_root: &str, book_path: &str, filename: &str) -> PathBuf { let library_path = Path::new(library_root); library_path.join(book_path).join(filename) } diff --git a/crates/libcalibre/src/error.rs b/crates/libcalibre/src/error.rs index cf1d708d..3edd232e 100644 --- a/crates/libcalibre/src/error.rs +++ b/crates/libcalibre/src/error.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::{fmt, path::PathBuf}; use crate::types::{AuthorId, BookId}; @@ -25,9 +25,21 @@ pub enum CalibreError { #[error("Book file not found for book {0} with format {1}")] BookFileNotFound(BookId, String), - /// Book file with given ID was not found. - #[error("Book file not found")] - BookCoverNotFound, + /// The book exists, but it has no cover entry. + #[error("Cover not found for book {0}")] + BookCoverNotFound(BookId), + + /// The database points at an asset which is absent from disk. + #[error("Asset file is missing: {0}")] + AssetFileMissing(PathBuf), + + /// The database-resolved asset escapes the canonical library root. + #[error("Asset path is outside the library root")] + AssetOutsideLibrary, + + /// A format identifier is empty or contains path syntax. + #[error("Invalid book format: {0}")] + InvalidBookFormat(String), #[error("Author cannot be deleted; they have associated books")] AuthorHasAssociatedBooks(Vec), diff --git a/crates/libcalibre/src/lib.rs b/crates/libcalibre/src/lib.rs index 9f426c42..3015110d 100644 --- a/crates/libcalibre/src/lib.rs +++ b/crates/libcalibre/src/lib.rs @@ -20,8 +20,8 @@ pub use custom_columns::{CustomColumn, CustomColumnKind, CustomColumnSpec, Custo pub use error::CalibreError; pub use library::{ Author as LibraryAuthor, AuthorAdd, AuthorUpdate, Book as LibraryBook, BookAdd, BookFileInfo, - BookIdentifier, BookPage, BookQuery, BookSortOrder, BookUpdate, Library, SeriesSummary, - TagSummary, + BookIdentifier, BookPage, BookQuery, BookSortOrder, BookUpdate, Library, ResolvedBookAsset, + SeriesSummary, TagSummary, }; pub use stats::{library_stats, LibraryStats}; pub use types::{AuthorId, BookFileId, BookId}; diff --git a/crates/libcalibre/src/library.rs b/crates/libcalibre/src/library.rs index 14310457..87573690 100644 --- a/crates/libcalibre/src/library.rs +++ b/crates/libcalibre/src/library.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, path::Path, path::PathBuf}; use chrono::{NaiveDate, NaiveDateTime}; -use diesel::{prelude::*, sql_query, RunQueryDsl, SqliteConnection}; +use diesel::{prelude::*, sql_query, OptionalExtension, RunQueryDsl, SqliteConnection}; use sanitise_file_name::sanitise; use crate::{ @@ -27,7 +27,7 @@ pub struct Book { /// Identifier unique only within this library pub id: BookId, /// Cross-library identifier - pub uuid: String, + pub uuid: Option, pub title: String, pub sortable_title: Option, pub authors: Vec, @@ -61,6 +61,8 @@ pub struct BookFileInfo { pub uncompressed_size: i32, } +pub use crate::operations::assets::ResolvedBookAsset; + #[derive(Clone, Debug)] pub struct BookIdentifier { pub id: i32, @@ -222,6 +224,41 @@ impl Library { &self.db_path.database_path } + pub fn library_uuid(&mut self) -> Result { + use crate::schema::library_id::dsl::{library_id, uuid}; + + library_id + .select(uuid) + .first::(&mut self.conn) + .optional()? + .ok_or_else(|| { + CalibreError::DatabaseIntegrity("library_id has no identity row".to_string()) + }) + } + + pub fn catalog_updated_at(&mut self) -> Result, CalibreError> { + use crate::schema::books::dsl::{books, last_modified}; + use diesel::dsl::max; + + let books_updated_at = books + .select(max(last_modified)) + .first::>(&mut self.conn) + .map_err(CalibreError::from)?; + let database_updated_at = [ + PathBuf::from(&self.db_path.database_path), + PathBuf::from(format!("{}-wal", self.db_path.database_path)), + ] + .into_iter() + .filter_map(|path| std::fs::metadata(path).ok()?.modified().ok()) + .map(|modified| chrono::DateTime::::from(modified).naive_utc()) + .max(); + + Ok(books_updated_at + .into_iter() + .chain(database_updated_at) + .max()) + } + // ========================================================================= // Books // ========================================================================= @@ -371,6 +408,7 @@ impl Library { } // 7. Generate metadata.opf from the freshly written DB state. + book_queries::touch(&mut self.conn, BookId(book_row.id))?; let _ = self.regenerate_metadata_opf(BookId(book_row.id)); self.get_book(BookId(book_row.id)) @@ -523,6 +561,23 @@ impl Library { ) } + pub fn resolve_book_file( + &mut self, + id: BookId, + format: &str, + ) -> Result { + operations::assets::resolve_book_file( + &self.db_path.library_path, + &mut self.conn, + id, + format, + ) + } + + pub fn resolve_book_cover(&mut self, id: BookId) -> Result { + operations::assets::resolve_book_cover(&self.db_path.library_path, &mut self.conn, id) + } + pub fn remove_book_file(&mut self, book_id: BookId, format: &str) -> Result<(), CalibreError> { operations::assets::remove_book_file( &self.db_path.library_path, @@ -591,7 +646,10 @@ impl Library { author_id: AuthorId, update: AuthorUpdate, ) -> Result { - operations::authors::update(&mut self.conn, author_id, update) + let book_ids = author_queries::find_books(&mut self.conn, author_id)?; + let author = operations::authors::update(&mut self.conn, author_id, update)?; + book_queries::touch_many(&mut self.conn, &book_ids)?; + Ok(author) } pub fn remove_author(&mut self, author_id: AuthorId) -> Result { @@ -612,27 +670,33 @@ impl Library { ) -> Result { use crate::schema::identifiers::dsl; - match existing_id { - Some(identifier_id) => { - diesel::update(dsl::identifiers.filter(dsl::id.eq(identifier_id))) - .set((dsl::type_.eq(&label), dsl::val.eq(&value))) - .returning(dsl::id) - .get_result::(&mut self.conn) - .map_err(CalibreError::from) - } - None => { - let lowercased_label = label.to_lowercase(); - diesel::insert_into(dsl::identifiers) - .values(( - dsl::book.eq(book_id.as_i32()), - dsl::type_.eq(lowercased_label), - dsl::val.eq(&value), - )) - .returning(dsl::id) - .get_result::(&mut self.conn) - .map_err(CalibreError::from) - } - } + self.conn.transaction(|conn| { + let identifier_id = match existing_id { + Some(identifier_id) => diesel::update( + dsl::identifiers + .filter(dsl::id.eq(identifier_id)) + .filter(dsl::book.eq(book_id.as_i32())), + ) + .set((dsl::type_.eq(&label), dsl::val.eq(&value))) + .returning(dsl::id) + .get_result::(conn) + .map_err(CalibreError::from), + None => { + let lowercased_label = label.to_lowercase(); + diesel::insert_into(dsl::identifiers) + .values(( + dsl::book.eq(book_id.as_i32()), + dsl::type_.eq(lowercased_label), + dsl::val.eq(&value), + )) + .returning(dsl::id) + .get_result::(conn) + .map_err(CalibreError::from) + } + }?; + book_queries::touch(conn, book_id)?; + Ok(identifier_id) + }) } pub fn delete_book_identifier( @@ -648,8 +712,9 @@ impl Library { .filter(dsl::id.eq(identifier_id)), ) .execute(&mut self.conn) - .map(|_| ()) - .map_err(CalibreError::from) + .map_err(CalibreError::from)?; + book_queries::touch(&mut self.conn, book_id)?; + Ok(()) } // ========================================================================= @@ -793,8 +858,9 @@ impl Library { pub fn randomize_library_uuid(&mut self) -> Result<(), CalibreError> { sql_query("UPDATE library_id SET uuid = uuid4()") .execute(&mut self.conn) - .map(|_| ()) - .map_err(CalibreError::from) + .map_err(CalibreError::from)?; + book_queries::touch_catalog(&mut self.conn)?; + Ok(()) } // ========================================================================= @@ -838,6 +904,58 @@ impl Library { Ok(BookPage { items, total }) } + /// Query books that have at least one safely resolvable acquisition file. + /// Candidate rows are scanned before paging so missing or escaping backing + /// files cannot create dead-only entries or pagination holes. + pub fn query_acquirable_books( + &mut self, + limit: i64, + offset: i64, + ) -> Result { + let mut resolvable_ids = Vec::new(); + let mut seen = std::collections::HashSet::new(); + for candidate in book_queries::acquisition_candidates(&mut self.conn)? { + let book_id = BookId(candidate.book_id); + if seen.contains(&book_id) { + continue; + } + if operations::assets::is_resolvable_book_file( + &self.db_path.library_path, + &candidate.book_path, + &candidate.name, + &candidate.format, + ) { + seen.insert(book_id); + resolvable_ids.push(book_id); + } + } + let total = i64::try_from(resolvable_ids.len()).unwrap_or(i64::MAX); + let start = usize::try_from(offset.max(0)).unwrap_or(usize::MAX); + let page_len = usize::try_from(limit.max(0)).unwrap_or(usize::MAX); + let book_ids = resolvable_ids + .into_iter() + .skip(start) + .take(page_len) + .collect(); + let mut items = self.get_books_with_read_states(book_ids)?; + for book in &mut items { + book.files.retain(|file| { + operations::assets::is_resolvable_book_file( + &self.db_path.library_path, + &book.book_dir_path, + &file.name, + &file.format, + ) + }); + book.has_cover = book.has_cover + && operations::assets::is_resolvable_book_cover( + &self.db_path.library_path, + &book.book_dir_path, + ); + } + Ok(BookPage { items, total }) + } + /// List every series in the library with its linked-book count, sorted /// by name. The returned ids feed [`BookQuery::series_id`]. pub fn list_series(&mut self) -> Result, CalibreError> { diff --git a/crates/libcalibre/src/mime_type.rs b/crates/libcalibre/src/mime_type.rs index f1af2b86..09a17a79 100644 --- a/crates/libcalibre/src/mime_type.rs +++ b/crates/libcalibre/src/mime_type.rs @@ -60,6 +60,36 @@ impl MIMETYPE { } } +/// Authoritative extension → MIME mapping for every format Citadel serves, +/// shared by book downloads and OPDS asset responses. [`MIMETYPE`] covers the +/// formats Citadel imports; the remaining entries exist because adopted +/// Calibre libraries can hold those book formats (served as-is) and because +/// cover files are images. `txt` carries a charset here for HTTP responses; +/// [`MIMETYPE::as_str`] stays bare for non-HTTP use. +pub fn mime_type_from_extension(extension: &str) -> &'static str { + match extension.to_ascii_lowercase().as_str() { + "epub" => "application/epub+zip", + "mobi" => "application/x-mobipocket-ebook", + "azw" => "application/vnd.amazon.ebook", + "azw3" => "application/vnd.amazon.ebook-kf8", + "pdf" => "application/pdf", + "txt" => "text/plain; charset=utf-8", + "html" | "htm" => "text/html; charset=utf-8", + "cbz" => "application/vnd.comicbook+zip", + "cbr" => "application/vnd.comicbook-rar", + "fb2" => "application/x-fictionbook+xml", + "djvu" | "djv" => "image/vnd.djvu", + "docx" => "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "odt" => "application/vnd.oasis.opendocument.text", + "rtf" => "application/rtf", + "jpg" | "jpeg" => "image/jpeg", + "png" => "image/png", + "gif" => "image/gif", + "webp" => "image/webp", + _ => "application/octet-stream", + } +} + impl PartialEq for MIMETYPE { fn eq(&self, other: &Self) -> bool { matches!( @@ -74,3 +104,22 @@ impl PartialEq for MIMETYPE { ) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn maps_importable_cover_and_unknown_extensions() { + assert_eq!(mime_type_from_extension("EPUB"), "application/epub+zip"); + assert_eq!( + mime_type_from_extension("Azw3"), + "application/vnd.amazon.ebook-kf8" + ); + assert_eq!(mime_type_from_extension("jpg"), "image/jpeg"); + assert_eq!( + mime_type_from_extension("unexpected"), + "application/octet-stream" + ); + } +} diff --git a/crates/libcalibre/src/operations/assets.rs b/crates/libcalibre/src/operations/assets.rs index a0a99145..fbea1cee 100644 --- a/crates/libcalibre/src/operations/assets.rs +++ b/crates/libcalibre/src/operations/assets.rs @@ -1,3 +1,5 @@ +use std::path::{Path, PathBuf}; + use diesel::{Connection, SqliteConnection}; use crate::{ @@ -7,53 +9,144 @@ use crate::{ CalibreError, UpdateBookData, }; -pub fn get_book_cover( - library_root: &String, +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ResolvedBookAsset { + canonical_path: PathBuf, + download_name: String, + format: String, +} + +impl ResolvedBookAsset { + pub fn canonical_path(&self) -> &Path { + &self.canonical_path + } + + pub fn download_name(&self) -> &str { + &self.download_name + } + + pub fn format(&self) -> &str { + &self.format + } +} + +fn canonical_asset(library_root: &str, path: PathBuf) -> Result { + let canonical_root = Path::new(library_root).canonicalize()?; + let canonical_path = match path.canonicalize() { + Ok(path) => path, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Err(CalibreError::AssetFileMissing(path)); + } + Err(error) => return Err(CalibreError::IoError(error)), + }; + + if !canonical_path.starts_with(canonical_root) { + return Err(CalibreError::AssetOutsideLibrary); + } + + Ok(canonical_path) +} + +fn normalized_format(file_format: &str) -> Result { + let trimmed = file_format.trim(); + let normalized = trimmed + .strip_prefix('.') + .unwrap_or(trimmed) + .to_ascii_uppercase(); + if normalized.is_empty() || !normalized.bytes().all(|byte| byte.is_ascii_alphanumeric()) { + return Err(CalibreError::InvalidBookFormat(file_format.to_string())); + } + Ok(normalized) +} + +pub(crate) fn is_resolvable_book_file( + library_root: &str, + book_path: &str, + file_name: &str, + file_format: &str, +) -> bool { + let Ok(format) = normalized_format(file_format) else { + return false; + }; + let download_name = format!("{}.{}", file_name, format.to_ascii_lowercase()); + let path = assets::asset_path(library_root, book_path, &download_name); + canonical_asset(library_root, path).is_ok() +} + +pub(crate) fn is_resolvable_book_cover(library_root: &str, book_path: &str) -> bool { + let path = assets::asset_path(library_root, book_path, COVER_FILENAME); + canonical_asset(library_root, path).is_ok() +} + +pub fn resolve_book_cover( + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, -) -> Result, CalibreError> { +) -> Result { let book = books::get(conn, book_id)?.ok_or(CalibreError::BookNotFound(book_id))?; - let file_path = assets::asset_path(library_root, &book.path, COVER_FILENAME); + if book.has_cover != Some(true) { + return Err(CalibreError::BookCoverNotFound(book_id)); + } - assets::read(&file_path) + let path = assets::asset_path(library_root, &book.path, COVER_FILENAME); + Ok(ResolvedBookAsset { + canonical_path: canonical_asset(library_root, path)?, + download_name: COVER_FILENAME.to_string(), + format: "JPG".to_string(), + }) } -pub fn get_book_file_path( - library_root: &String, +pub fn resolve_book_file( + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, file_format: &str, -) -> Result { +) -> Result { let book = books::get(conn, book_id)?.ok_or(CalibreError::BookNotFound(book_id))?; - let file = book_files::find_by_book_and_format(conn, book_id, file_format.to_string())?; + let requested_format = normalized_format(file_format)?; + let file = book_files::find_by_book_and_format(conn, book_id, requested_format.clone())? + .ok_or_else(|| CalibreError::BookFileNotFound(book_id, requested_format.clone()))?; + let stored_format = file.format.to_ascii_uppercase(); + let download_name = format!("{}.{}", file.name, stored_format.to_ascii_lowercase()); + let path = assets::asset_path(library_root, &book.path, &download_name); + + Ok(ResolvedBookAsset { + canonical_path: canonical_asset(library_root, path)?, + download_name, + format: stored_format, + }) +} - match file { - Some(_) => { - let book_filename = filename(book.path.clone(), file_format.to_string()); - let file_path = assets::asset_path(library_root, &book.path, &book_filename); - Ok(file_path) - } - None => { - return Err(CalibreError::BookFileNotFound( - book_id, - file_format.to_string(), - )) - } - } +pub fn get_book_cover( + library_root: &str, + conn: &mut SqliteConnection, + book_id: BookId, +) -> Result, CalibreError> { + let asset = resolve_book_cover(library_root, conn, book_id)?; + assets::read(asset.canonical_path()) +} + +pub fn get_book_file_path( + library_root: &str, + conn: &mut SqliteConnection, + book_id: BookId, + file_format: &str, +) -> Result { + resolve_book_file(library_root, conn, book_id, file_format).map(|asset| asset.canonical_path) } pub fn get_book_file( - library_root: &String, + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, file_format: &str, ) -> Result, CalibreError> { let file_path = get_book_file_path(library_root, conn, book_id, file_format)?; - return assets::read(&file_path); + assets::read(&file_path) } pub fn add_book_file_from_bytes( - library_root: &String, + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, file_format: String, @@ -73,7 +166,10 @@ pub fn add_book_file_from_bytes( }; match book_files::create(conn, new_file) { - Ok(_) => Ok(()), + Ok(_) => { + books::touch(conn, book_id)?; + Ok(()) + } Err(e) => { // If database entry fails, remove the written file let _ = std::fs::remove_file(&file_path); @@ -83,7 +179,7 @@ pub fn add_book_file_from_bytes( } pub fn add_book_file_from_path( - library_root: &String, + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, file_format: String, @@ -107,7 +203,10 @@ pub fn add_book_file_from_path( }; match book_files::create(conn, new_file) { - Ok(_) => Ok(()), + Ok(_) => { + books::touch(conn, book_id)?; + Ok(()) + } Err(e) => { // If database entry fails, remove the copied file let _ = std::fs::remove_file(&dest_path); @@ -117,7 +216,7 @@ pub fn add_book_file_from_path( } pub fn set_book_cover( - library_root: &String, + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, cover_data: Vec, @@ -133,12 +232,13 @@ pub fn set_book_cover( ..Default::default() }; books::update(conn, book_id, update)?; + books::touch(conn, book_id)?; Ok(()) }) } pub fn remove_book_file( - library_root: &String, + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, file_format: &str, @@ -168,6 +268,7 @@ pub fn remove_book_file( eprintln!("WARNING: Failed to delete file {:?}: {}", file_path, e); } } + books::touch(conn, book_id)?; } Ok(()) @@ -175,7 +276,7 @@ pub fn remove_book_file( } pub(crate) fn delete_entire_book( - library_root: &String, + library_root: &str, conn: &mut SqliteConnection, book_id: BookId, book_path: &str, @@ -192,6 +293,7 @@ pub(crate) fn delete_entire_book( } books::delete(conn, book_id)?; + books::touch_catalog(conn)?; let book_dir = std::path::Path::new(library_root).join(book_path); match std::fs::remove_dir_all(&book_dir) { @@ -212,7 +314,7 @@ fn filename(book_path: String, file_format: String) -> String { let base_name = book_path; if ext.is_empty() { - format!("{}", base_name) + base_name } else { format!("{}.{}", base_name, ext) } diff --git a/crates/libcalibre/src/operations/books.rs b/crates/libcalibre/src/operations/books.rs index 0a96653f..3085b883 100644 --- a/crates/libcalibre/src/operations/books.rs +++ b/crates/libcalibre/src/operations/books.rs @@ -152,6 +152,7 @@ pub fn update_book( } } + books::touch(conn, book_id)?; Ok(()) }) } @@ -205,9 +206,10 @@ pub fn get_book(conn: &mut SqliteConnection, book_id: BookId) -> Result Result, CalibreError> { + sql_query( + "SELECT books.id AS book_id, books.path AS book_path, \ + data.format AS format, data.name AS name \ + FROM books JOIN data ON data.book = books.id \ + ORDER BY books.sort ASC, books.id ASC, data.id ASC", + ) + .load(conn) + .map_err(CalibreError::from) +} + fn like_pattern(text: &str) -> String { let escaped = text .replace('\\', "\\\\") @@ -271,6 +296,65 @@ pub(crate) fn update( .map_err(CalibreError::from) } +pub(crate) fn touch(conn: &mut SqliteConnection, book_id: BookId) -> Result { + use crate::schema::books::dsl::{books, id, last_modified}; + + let previous = books + .filter(id.eq(book_id.as_i32())) + .select(last_modified) + .first::(conn)?; + let updated_at = std::cmp::max( + chrono::Utc::now().naive_utc(), + previous + chrono::TimeDelta::microseconds(1), + ); + diesel::update(books.filter(id.eq(book_id.as_i32()))) + .set(last_modified.eq(updated_at)) + .execute(conn) + .map_err(CalibreError::from) +} + +pub(crate) fn touch_many( + conn: &mut SqliteConnection, + book_ids: &[BookId], +) -> Result { + use crate::schema::books::dsl::{books, id, last_modified}; + + if book_ids.is_empty() { + return Ok(0); + } + let previous = books + .filter(id.eq_any(book_ids.iter().map(|book_id| book_id.as_i32()))) + .select(diesel::dsl::max(last_modified)) + .first::>(conn)?; + let updated_at = previous + .map(|previous| { + std::cmp::max( + chrono::Utc::now().naive_utc(), + previous + chrono::TimeDelta::microseconds(1), + ) + }) + .unwrap_or_else(|| chrono::Utc::now().naive_utc()); + let ids = book_ids.iter().map(|book_id| book_id.as_i32()); + diesel::update(books.filter(id.eq_any(ids))) + .set(last_modified.eq(updated_at)) + .execute(conn) + .map_err(CalibreError::from) +} + +pub(crate) fn touch_catalog(conn: &mut SqliteConnection) -> Result { + use crate::schema::books::dsl::{books, id}; + + match books + .select(id) + .order(id.asc()) + .first::(conn) + .optional()? + { + Some(book_id) => touch(conn, BookId(book_id)), + None => Ok(0), + } +} + pub(crate) fn delete(conn: &mut SqliteConnection, book_id: BookId) -> Result { use crate::schema::books::dsl::*; diff --git a/crates/libcalibre/tests/asset_resolution_test.rs b/crates/libcalibre/tests/asset_resolution_test.rs new file mode 100644 index 00000000..482c0b34 --- /dev/null +++ b/crates/libcalibre/tests/asset_resolution_test.rs @@ -0,0 +1,179 @@ +mod common; + +use std::fs; + +use common::{setup_with_library, standard_test_book}; +use libcalibre::{error::CalibreError, util::get_db_path, BookId, Library}; +use rusqlite::{params, Connection}; + +fn reopen(temp: &tempfile::TempDir) -> Library { + let path = get_db_path(temp.path().to_str().unwrap()).unwrap(); + Library::new(path).unwrap() +} + +fn add_file_row( + temp: &tempfile::TempDir, + book_id: BookId, + book_path: &str, + name: &str, + format: &str, + bytes: &[u8], +) { + let db = Connection::open(temp.path().join("metadata.db")).unwrap(); + db.execute( + "INSERT INTO data (book, format, uncompressed_size, name) VALUES (?1, ?2, ?3, ?4)", + params![book_id.as_i32(), format, bytes.len() as i32, name], + ) + .unwrap(); + let dir = temp.path().join(book_path); + fs::create_dir_all(&dir).unwrap(); + fs::write( + dir.join(format!("{name}.{}", format.to_ascii_lowercase())), + bytes, + ) + .unwrap(); +} + +#[test] +fn resolves_the_matching_data_name_and_normalizes_format() { + let (temp, mut library) = setup_with_library(); + let book = library.add_book(standard_test_book()).unwrap(); + drop(library); + add_file_row( + &temp, + book.id, + &book.book_dir_path, + "Authoritative Calibre Name", + "EPUB", + b"epub", + ); + + let mut library = reopen(&temp); + let asset = library.resolve_book_file(book.id, " .ePuB ").unwrap(); + assert_eq!(asset.download_name(), "Authoritative Calibre Name.epub"); + assert_eq!(asset.format(), "EPUB"); + assert!(asset.canonical_path().is_absolute()); + assert_eq!(fs::read(asset.canonical_path()).unwrap(), b"epub"); +} + +#[test] +fn distinguishes_missing_book_format_and_backing_file() { + let (temp, mut library) = setup_with_library(); + let book = library.add_book(standard_test_book()).unwrap(); + drop(library); + + let mut library = reopen(&temp); + assert!(matches!( + library.resolve_book_file(BookId::from(999_999), "epub"), + Err(CalibreError::BookNotFound(_)) + )); + assert!(matches!( + library.resolve_book_file(book.id, "epub"), + Err(CalibreError::BookFileNotFound(_, ref format)) if format == "EPUB" + )); + drop(library); + + add_file_row(&temp, book.id, &book.book_dir_path, "Gone", "EPUB", b"gone"); + fs::remove_file(temp.path().join(&book.book_dir_path).join("Gone.epub")).unwrap(); + let mut library = reopen(&temp); + assert!(matches!( + library.resolve_book_file(book.id, "epub"), + Err(CalibreError::AssetFileMissing(_)) + )); +} + +#[test] +fn rejects_format_path_syntax_before_filesystem_resolution() { + let (_temp, mut library) = setup_with_library(); + let book = library.add_book(standard_test_book()).unwrap(); + assert!(matches!( + library.resolve_book_file(book.id, "../epub"), + Err(CalibreError::InvalidBookFormat(_)) + )); +} + +#[test] +fn distinguishes_coverless_book_from_missing_cover_file() { + let (temp, mut library) = setup_with_library(); + let book = library.add_book(standard_test_book()).unwrap(); + assert!(matches!( + library.resolve_book_cover(book.id), + Err(CalibreError::BookCoverNotFound(id)) if id == book.id + )); + library.set_book_cover(book.id, b"cover".to_vec()).unwrap(); + let cover_path = library.resolve_book_cover(book.id).unwrap(); + assert_eq!(cover_path.download_name(), "cover.jpg"); + assert_eq!(cover_path.format(), "JPG"); + fs::remove_file(cover_path.canonical_path()).unwrap(); + drop(library); + let mut library = reopen(&temp); + assert!(matches!( + library.resolve_book_cover(book.id), + Err(CalibreError::AssetFileMissing(_)) + )); +} + +#[test] +fn rejects_a_malformed_database_book_path_that_escapes_the_library() { + let (temp, mut library) = setup_with_library(); + let book = library.add_book(standard_test_book()).unwrap(); + drop(library); + + let outside_dir = temp + .path() + .parent() + .unwrap() + .join(format!("citadel-escaped-book-{}", std::process::id())); + fs::create_dir_all(&outside_dir).unwrap(); + fs::write(outside_dir.join("Outside.epub"), b"outside").unwrap(); + let escaped_book_path = format!("../{}", outside_dir.file_name().unwrap().to_string_lossy()); + let db = Connection::open(temp.path().join("metadata.db")).unwrap(); + db.execute_batch("DROP TRIGGER IF EXISTS books_update_trg") + .unwrap(); + db.execute( + "UPDATE books SET path = ?1 WHERE id = ?2", + params![escaped_book_path, book.id.as_i32()], + ) + .unwrap(); + db.execute( + "INSERT INTO data (book, format, uncompressed_size, name) VALUES (?1, 'EPUB', 7, 'Outside')", + [book.id.as_i32()], + ) + .unwrap(); + drop(db); + + let mut library = reopen(&temp); + assert!(matches!( + library.resolve_book_file(book.id, "epub"), + Err(CalibreError::AssetOutsideLibrary) + )); + fs::remove_dir_all(outside_dir).unwrap(); +} + +#[cfg(unix)] +#[test] +fn rejects_a_symlink_that_escapes_the_library() { + use std::os::unix::fs::symlink; + + let (temp, mut library) = setup_with_library(); + let book = library.add_book(standard_test_book()).unwrap(); + drop(library); + add_file_row( + &temp, + book.id, + &book.book_dir_path, + "Escaped", + "EPUB", + b"placeholder", + ); + let asset_path = temp.path().join(&book.book_dir_path).join("Escaped.epub"); + fs::remove_file(&asset_path).unwrap(); + let outside = tempfile::NamedTempFile::new().unwrap(); + symlink(outside.path(), asset_path).unwrap(); + + let mut library = reopen(&temp); + assert!(matches!( + library.resolve_book_file(book.id, "epub"), + Err(CalibreError::AssetOutsideLibrary) + )); +} diff --git a/crates/libcalibre/tests/books_api_test.rs b/crates/libcalibre/tests/books_api_test.rs index 9a4a1335..704990ee 100644 --- a/crates/libcalibre/tests/books_api_test.rs +++ b/crates/libcalibre/tests/books_api_test.rs @@ -53,7 +53,7 @@ fn test_add_book() { let book = result.unwrap(); assert_eq!(book.title, "Test Book"); - assert!(!book.uuid.is_empty()); + assert!(book.uuid.as_deref().is_some_and(|uuid| !uuid.is_empty())); } #[test] diff --git a/crates/libcalibre/tests/test_triggers.rs b/crates/libcalibre/tests/test_triggers.rs index 98cf84f7..e6d4d9d8 100644 --- a/crates/libcalibre/tests/test_triggers.rs +++ b/crates/libcalibre/tests/test_triggers.rs @@ -33,8 +33,12 @@ fn test_books_insert_trigger_generates_sort_and_uuid() { assert_eq!(book.sortable_title, Some("Great Gatsby, The".to_string())); // Verify UUID was generated (should be a valid UUID format) - assert_eq!(book.uuid.len(), 36); // UUID format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx - assert!(book.uuid.contains('-')); + let uuid = book + .uuid + .as_deref() + .expect("insert trigger should set UUID"); + assert_eq!(uuid.len(), 36); // UUID format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + assert!(uuid.contains('-')); } /// Test that the books_update_trg trigger updates sort when title changes diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 51257b59..1bc24d6b 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -20,6 +20,7 @@ diesel = { version = "2.1.0", features = ["sqlite", "chrono", "returning_clauses epub = "2.1.1" mobi = "0.8.0" citadel-core = { path = "../crates/citadel-core" } +citadel-opds = { path = "../crates/citadel-opds" } libcalibre = { path = "../crates/libcalibre" } log = "0.4" quick-xml = "0.38" diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index d741b232..91975345 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -16,6 +16,7 @@ pub mod libs { mod book; mod menu; mod metadata; +pub mod opds; mod state; fn run_tauri_backend() -> std::io::Result<()> { diff --git a/src-tauri/src/opds/mod.rs b/src-tauri/src/opds/mod.rs new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/src-tauri/src/opds/mod.rs @@ -0,0 +1 @@ + diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index 4736b0c4..954aba8d 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -1,6 +1,9 @@ use std::sync::Mutex; -use libcalibre::Library; +use chrono::NaiveDateTime; +use libcalibre::{BookId, BookPage, CalibreError, Library, ResolvedBookAsset}; + +use citadel_opds::CatalogSource; pub struct CitadelState { library: Mutex>, @@ -58,4 +61,63 @@ impl CitadelState { .expect("Library mutex poisoned") .is_some() } + + /// Resolve under the library mutex and return only owned data. Opening and + /// streaming the file must happen after this method returns. + pub fn resolve_book_asset( + &self, + book_id: BookId, + format: &str, + ) -> Result { + let mut library = self.library.lock().expect("Library mutex poisoned"); + library + .as_mut() + .ok_or(CalibreError::LibraryNotInitialized)? + .resolve_book_file(book_id, format) + } + + pub fn resolve_book_cover(&self, book_id: BookId) -> Result { + let mut library = self.library.lock().expect("Library mutex poisoned"); + library + .as_mut() + .ok_or(CalibreError::LibraryNotInitialized)? + .resolve_book_cover(book_id) + } + + pub fn opds_book_page( + &self, + limit: i64, + offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError> { + let mut library = self.library.lock().expect("Library mutex poisoned"); + let library = library + .as_mut() + .ok_or(CalibreError::LibraryNotInitialized)?; + let library_uuid = library.library_uuid()?; + let updated_at = library.catalog_updated_at()?; + let page = library.query_acquirable_books(limit, offset)?; + Ok((library_uuid, updated_at, page)) + } +} + +impl CatalogSource for CitadelState { + fn active_library_id(&self) -> Result { + CitadelState::active_library_id(self) + } + + fn book_page( + &self, + limit: i64, + offset: i64, + ) -> Result<(String, Option, BookPage), CalibreError> { + self.opds_book_page(limit, offset) + } + + fn book_file(&self, book_id: BookId, format: &str) -> Result { + self.resolve_book_asset(book_id, format) + } + + fn book_cover(&self, book_id: BookId) -> Result { + self.resolve_book_cover(book_id) + } } From 3b6690c3f0d56a9b828892d46cd728650c0a3c6d Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Thu, 17 Sep 2026 21:23:40 -0700 Subject: [PATCH 2/7] refactor(opds): extract xml and identity helpers --- Cargo.lock | 4 ++ crates/citadel-opds/Cargo.toml | 2 + crates/citadel-opds/src/catalog.rs | 72 +++++++++-------------------- crates/citadel-opds/src/identity.rs | 29 ++++++++++++ crates/citadel-opds/src/lib.rs | 2 + crates/citadel-opds/src/xml.rs | 21 +++++++++ 6 files changed, 80 insertions(+), 50 deletions(-) create mode 100644 crates/citadel-opds/src/identity.rs create mode 100644 crates/citadel-opds/src/xml.rs diff --git a/Cargo.lock b/Cargo.lock index 73230897..5c522990 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -657,10 +657,14 @@ dependencies = [ name = "citadel-opds" version = "0.1.0" dependencies = [ + "axum", + "bytes", "chrono", + "diesel", "futures-util", "libcalibre", "quick-xml 0.38.4", + "reqwest 0.12.28", "serde", "tempfile", "tokio", diff --git a/crates/citadel-opds/Cargo.toml b/crates/citadel-opds/Cargo.toml index ddeaaa01..c5c8c820 100644 --- a/crates/citadel-opds/Cargo.toml +++ b/crates/citadel-opds/Cargo.toml @@ -18,4 +18,6 @@ urlencoding = "2.1.3" uuid = { version = "1.6.1", features = ["v4", "fast-rng"] } [dev-dependencies] +diesel = { version = "2.2.4", features = ["sqlite"] } +reqwest = "0.12" tempfile = "3.8" diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index 8e758a3a..5db26930 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, io::Read, sync::Arc}; +use std::{io::Read, sync::Arc}; use axum::{ body::Body, @@ -18,6 +18,8 @@ use quick_xml::{ use serde::Deserialize; use super::assets::{self, AssetMethod}; +use crate::identity::{book_identity, library_identity}; +use crate::xml::xml_text; const PAGE_SIZE: i64 = 50; const ACQUISITION_REL: &str = "http://opds-spec.org/acquisition"; @@ -414,54 +416,6 @@ fn page_href(route: &str, page: u64) -> String { } } -fn library_identity(raw_uuid: &str) -> String { - match uuid::Uuid::parse_str(raw_uuid) { - Ok(uuid) => format!("urn:uuid:{uuid}"), - Err(_) => format!("urn:citadel:library:{}", hex(raw_uuid.as_bytes())), - } -} - -fn book_identity(library_uuid: &str, raw_uuid: Option<&str>, book_id: BookId) -> String { - match raw_uuid.and_then(|value| uuid::Uuid::parse_str(value).ok()) { - Some(uuid) => format!("urn:uuid:{uuid}"), - None => format!( - "urn:citadel:book:{}:{}", - hex(library_uuid.as_bytes()), - book_id.as_i32() - ), - } -} - -fn hex(bytes: &[u8]) -> String { - const HEX: &[u8; 16] = b"0123456789abcdef"; - let mut result = String::with_capacity(bytes.len() * 2); - for byte in bytes { - result.push(char::from(HEX[usize::from(byte >> 4)])); - result.push(char::from(HEX[usize::from(byte & 0x0f)])); - } - result -} - -fn xml_text(value: &str) -> Cow<'_, str> { - if value.chars().all(valid_xml_char) { - Cow::Borrowed(value) - } else { - Cow::Owned( - value - .chars() - .filter(|character| valid_xml_char(*character)) - .collect(), - ) - } -} - -fn valid_xml_char(character: char) -> bool { - matches!(character, '\u{9}' | '\u{a}' | '\u{d}') - || ('\u{20}'..='\u{d7ff}').contains(&character) - || ('\u{e000}'..='\u{fffd}').contains(&character) - || ('\u{10000}'..='\u{10ffff}').contains(&character) -} - fn feed_updated(updated_at: Option) -> String { updated_at .map(timestamp) @@ -510,6 +464,8 @@ fn public_error(status: StatusCode, message: &'static str) -> Response { #[cfg(test)] mod tests { use super::*; + use crate::identity::{book_identity, library_identity}; + use crate::xml::valid_xml_char; use chrono::NaiveDate; use diesel::{Connection, RunQueryDsl}; use libcalibre::{ @@ -563,6 +519,10 @@ mod tests { } impl CatalogSource for MemorySource { + fn active_library_id(&self) -> Result { + Ok("550e8400-e29b-41d4-a716-446655440000".to_string()) + } + fn book_page( &self, limit: i64, @@ -607,6 +567,10 @@ mod tests { struct FailureSource(fn() -> CalibreError); impl CatalogSource for NoLibrary { + fn active_library_id(&self) -> Result { + Err(CalibreError::LibraryNotInitialized) + } + fn book_page( &self, _limit: i64, @@ -629,6 +593,10 @@ mod tests { } impl CatalogSource for FailureSource { + fn active_library_id(&self) -> Result { + Err((self.0)()) + } + fn book_page( &self, _limit: i64, @@ -651,6 +619,10 @@ mod tests { } impl CatalogSource for LibrarySource { + fn active_library_id(&self) -> Result { + self.library.lock().unwrap().library_uuid() + } + fn book_page( &self, limit: i64, @@ -682,7 +654,7 @@ mod tests { fn test_library() -> (TempDir, Library) { let directory = tempfile::tempdir().unwrap(); let fixture = PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .join("../crates/libcalibre/tests/fixtures/empty_library/metadata.db"); + .join("../../crates/libcalibre/tests/fixtures/empty_library/metadata.db"); std::fs::copy(fixture, directory.path().join("metadata.db")).unwrap(); let db_path = get_db_path(directory.path().to_str().unwrap()).unwrap(); (directory, Library::new(db_path).unwrap()) diff --git a/crates/citadel-opds/src/identity.rs b/crates/citadel-opds/src/identity.rs new file mode 100644 index 00000000..b1fa6516 --- /dev/null +++ b/crates/citadel-opds/src/identity.rs @@ -0,0 +1,29 @@ +use libcalibre::BookId; + +pub(crate) fn library_identity(raw_uuid: &str) -> String { + match uuid::Uuid::parse_str(raw_uuid) { + Ok(uuid) => format!("urn:uuid:{uuid}"), + Err(_) => format!("urn:citadel:library:{}", hex(raw_uuid.as_bytes())), + } +} + +pub(crate) fn book_identity(library_uuid: &str, raw_uuid: Option<&str>, book_id: BookId) -> String { + match raw_uuid.and_then(|value| uuid::Uuid::parse_str(value).ok()) { + Some(uuid) => format!("urn:uuid:{uuid}"), + None => format!( + "urn:citadel:book:{}:{}", + hex(library_uuid.as_bytes()), + book_id.as_i32() + ), + } +} + +fn hex(bytes: &[u8]) -> String { + const HEX: &[u8; 16] = b"0123456789abcdef"; + let mut result = String::with_capacity(bytes.len() * 2); + for byte in bytes { + result.push(char::from(HEX[usize::from(byte >> 4)])); + result.push(char::from(HEX[usize::from(byte & 0x0f)])); + } + result +} diff --git a/crates/citadel-opds/src/lib.rs b/crates/citadel-opds/src/lib.rs index 565b0ce2..1fd5750c 100644 --- a/crates/citadel-opds/src/lib.rs +++ b/crates/citadel-opds/src/lib.rs @@ -2,5 +2,7 @@ pub mod assets; pub mod catalog; +mod identity; +mod xml; pub use catalog::{router, CatalogSource}; diff --git a/crates/citadel-opds/src/xml.rs b/crates/citadel-opds/src/xml.rs new file mode 100644 index 00000000..baa2e364 --- /dev/null +++ b/crates/citadel-opds/src/xml.rs @@ -0,0 +1,21 @@ +use std::borrow::Cow; + +pub(crate) fn xml_text(value: &str) -> Cow<'_, str> { + if value.chars().all(valid_xml_char) { + Cow::Borrowed(value) + } else { + Cow::Owned( + value + .chars() + .filter(|character| valid_xml_char(*character)) + .collect(), + ) + } +} + +pub(crate) fn valid_xml_char(character: char) -> bool { + matches!(character, '\u{9}' | '\u{a}' | '\u{d}') + || ('\u{20}'..='\u{d7ff}').contains(&character) + || ('\u{e000}'..='\u{fffd}').contains(&character) + || ('\u{10000}'..='\u{10ffff}').contains(&character) +} From 4e0ba78f5619445165e6fdbbe0951d8095d717bc Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Thu, 17 Sep 2026 21:28:42 -0700 Subject: [PATCH 3/7] refactor(opds): split feed handlers and introduce the Feed model --- crates/citadel-opds/src/catalog.rs | 393 +++++++++++++++-------------- crates/citadel-opds/src/xml.rs | 96 +++++++ 2 files changed, 302 insertions(+), 187 deletions(-) diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index 5db26930..25f979c4 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -11,22 +11,18 @@ use axum::{ use bytes::Bytes; use chrono::{NaiveDateTime, SecondsFormat}; use libcalibre::{BookId, BookPage, CalibreError, ResolvedBookAsset}; -use quick_xml::{ - events::{BytesDecl, BytesEnd, BytesStart, BytesText, Event}, - Writer, -}; use serde::Deserialize; -use super::assets::{self, AssetMethod}; +use super::assets::{self, AssetMethod, AssetResponseError}; use crate::identity::{book_identity, library_identity}; -use crate::xml::xml_text; const PAGE_SIZE: i64 = 50; const ACQUISITION_REL: &str = "http://opds-spec.org/acquisition"; -const IMAGE_REL: &str = "http://opds-spec.org/image"; +pub(crate) const IMAGE_REL: &str = "http://opds-spec.org/image"; const ATOM_TYPE: &str = "application/atom+xml;profile=opds-catalog;kind=acquisition"; const ATOM_CONTENT_TYPE: &str = "application/atom+xml;profile=opds-catalog;kind=acquisition; charset=utf-8"; +pub(crate) const IMAGE_TYPE: &str = "image/jpeg"; pub trait CatalogSource: Send + Sync + 'static { fn active_library_id(&self) -> Result; @@ -50,6 +46,34 @@ struct PageQuery { page: Option, } +pub(crate) struct Feed { + pub(crate) id: String, + pub(crate) title: String, + pub(crate) updated: String, + pub(crate) links: Vec, + pub(crate) entries: Vec, +} + +pub(crate) struct FeedLink { + pub(crate) rel: &'static str, + pub(crate) href: String, + pub(crate) media_type: &'static str, +} + +pub(crate) struct FeedEntry { + pub(crate) id: String, + pub(crate) title: String, + pub(crate) updated: String, + pub(crate) authors: Vec, + pub(crate) published: String, + pub(crate) languages: Vec, + pub(crate) identifiers: Vec, + pub(crate) categories: Vec, + pub(crate) acquisition_links: Vec<(String, String, String)>, + pub(crate) image_link: Option, + pub(crate) content: Option, +} + pub fn router(source: Arc) -> Router { Router::new() .route("/opds", get(root_feed)) @@ -78,17 +102,9 @@ async fn feed( Query(query): Query, route: &'static str, ) -> Response { - let page_number = query.page.unwrap_or(1); - if page_number == 0 { - return public_error(StatusCode::BAD_REQUEST, "Invalid page"); - } - let offset = match page_number - .checked_sub(1) - .and_then(|page| page.checked_mul(PAGE_SIZE as u64)) - .and_then(|offset| i64::try_from(offset).ok()) - { - Some(offset) => offset, - None => return public_error(StatusCode::BAD_REQUEST, "Invalid page"), + let (page_number, offset) = match page_params(&query) { + Ok(params) => params, + Err(response) => return response, }; let source = state.source.clone(); @@ -124,6 +140,22 @@ async fn feed( } } +fn page_params(query: &PageQuery) -> Result<(u64, i64), Response> { + let page_number = query.page.unwrap_or(1); + if page_number == 0 { + return Err(public_error(StatusCode::BAD_REQUEST, "Invalid page")); + } + let offset = match page_number + .checked_sub(1) + .and_then(|page| page.checked_mul(PAGE_SIZE as u64)) + .and_then(|offset| i64::try_from(offset).ok()) + { + Some(offset) => offset, + None => return Err(public_error(StatusCode::BAD_REQUEST, "Invalid page")), + }; + Ok((page_number, offset)) +} + async fn book_file( state: State, Path((book_id, format, _filename)): Path<(i32, String, String)>, @@ -181,13 +213,8 @@ async fn asset_response( .get(header::RANGE) .and_then(|value| value.to_str().ok()) .map(str::to_owned); - let source = state.source.clone(); - let resolved = tokio::task::spawn_blocking(move || match format { - Some(format) => source.book_file(book_id, &format), - None => source.book_cover(book_id), - }) - .await; - let asset = match resolved { + + let asset = match resolve_asset(state.source.clone(), book_id, format).await { Ok(Ok(asset)) => asset, Ok(Err(error)) => return calibre_error(error), Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Asset unavailable"), @@ -198,22 +225,7 @@ async fn asset_response( .await; let prepared = match prepared { Ok(Ok(response)) => response, - Ok(Err(error)) => { - let mut response = public_error( - StatusCode::from_u16(error.status()).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR), - if error.status() == 416 { - "Range not satisfiable" - } else { - "Asset unavailable" - }, - ); - if let Some(content_range) = error.content_range() { - if let Ok(value) = HeaderValue::from_str(&content_range) { - response.headers_mut().insert(header::CONTENT_RANGE, value); - } - } - return response; - } + Ok(Err(error)) => return asset_error_response(error), Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Asset unavailable"), }; @@ -231,32 +243,7 @@ async fn asset_response( } let body = match prepared.body { - Some(mut reader) => { - let (sender, receiver) = tokio::sync::mpsc::channel(2); - tokio::task::spawn_blocking(move || loop { - let mut buffer = vec![0_u8; 64 * 1024]; - match reader.read(&mut buffer) { - Ok(0) => break, - Ok(read) => { - buffer.truncate(read); - if sender - .blocking_send(Ok::<_, std::io::Error>(Bytes::from(buffer))) - .is_err() - { - break; - } - } - Err(error) => { - let _ = sender.blocking_send(Err(error)); - break; - } - } - }); - Body::from_stream(futures_util::stream::unfold( - receiver, - |mut receiver| async move { receiver.recv().await.map(|item| (item, receiver)) }, - )) - } + Some(reader) => stream_body(reader), None => Body::empty(), }; builder @@ -264,6 +251,65 @@ async fn asset_response( .unwrap_or_else(|_| public_error(StatusCode::INTERNAL_SERVER_ERROR, "Asset unavailable")) } +async fn resolve_asset( + source: Arc, + book_id: BookId, + format: Option, +) -> Result, tokio::task::JoinError> { + tokio::task::spawn_blocking(move || match format { + Some(format) => source.book_file(book_id, &format), + None => source.book_cover(book_id), + }) + .await +} + +fn asset_error_response(error: AssetResponseError) -> Response { + let mut response = public_error( + StatusCode::from_u16(error.status()).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR), + if error.status() == 416 { + "Range not satisfiable" + } else { + "Asset unavailable" + }, + ); + if let Some(content_range) = error.content_range() { + if let Ok(value) = HeaderValue::from_str(&content_range) { + response.headers_mut().insert(header::CONTENT_RANGE, value); + } + } + response +} + +fn stream_body(reader: assets::AssetBody) -> Body { + let (sender, receiver) = tokio::sync::mpsc::channel(2); + tokio::task::spawn_blocking(move || { + let mut reader = reader; + loop { + let mut buffer = vec![0_u8; 64 * 1024]; + match reader.read(&mut buffer) { + Ok(0) => break, + Ok(read) => { + buffer.truncate(read); + if sender + .blocking_send(Ok::<_, std::io::Error>(Bytes::from(buffer))) + .is_err() + { + break; + } + } + Err(error) => { + let _ = sender.blocking_send(Err(error)); + break; + } + } + } + }); + Body::from_stream(futures_util::stream::unfold( + receiver, + |mut receiver| async move { receiver.recv().await.map(|item| (item, receiver)) }, + )) +} + fn acquisition_feed( library_uuid: &str, updated_at: Option, @@ -272,132 +318,105 @@ fn acquisition_feed( last_page: u64, route: &str, ) -> Result, quick_xml::Error> { - let mut writer = Writer::new(Vec::new()); - writer.write_event(Event::Decl(BytesDecl::new("1.0", Some("UTF-8"), None)))?; - let mut feed = BytesStart::new("feed"); - feed.push_attribute(("xmlns", "http://www.w3.org/2005/Atom")); - feed.push_attribute(("xmlns:dc", "http://purl.org/dc/terms/")); - writer.write_event(Event::Start(feed))?; - text_element(&mut writer, "id", &library_identity(library_uuid))?; - text_element(&mut writer, "title", "Citadel — All Books")?; - text_element(&mut writer, "updated", &feed_updated(updated_at))?; - writer.write_event(Event::Start(BytesStart::new("author")))?; - text_element(&mut writer, "name", "Citadel")?; - writer.write_event(Event::End(BytesEnd::new("author")))?; - - let self_href = page_href(route, page_number); - link(&mut writer, "self", &self_href, ATOM_TYPE)?; - link(&mut writer, "start", "/opds", ATOM_TYPE)?; + let mut links = vec![ + FeedLink { + rel: "self", + href: page_href(route, page_number), + media_type: ATOM_TYPE, + }, + FeedLink { + rel: "start", + href: "/opds".to_string(), + media_type: ATOM_TYPE, + }, + ]; if route != "/opds" { - link(&mut writer, "up", "/opds", ATOM_TYPE)?; + links.push(FeedLink { + rel: "up", + href: "/opds".to_string(), + media_type: ATOM_TYPE, + }); } if page_number > 1 { - link( - &mut writer, - "previous", - &page_href(route, page_number - 1), - ATOM_TYPE, - )?; - link(&mut writer, "first", route, ATOM_TYPE)?; + links.push(FeedLink { + rel: "previous", + href: page_href(route, page_number - 1), + media_type: ATOM_TYPE, + }); + links.push(FeedLink { + rel: "first", + href: route.to_string(), + media_type: ATOM_TYPE, + }); } if page_number < last_page { - link( - &mut writer, - "next", - &page_href(route, page_number + 1), - ATOM_TYPE, - )?; - link(&mut writer, "last", &page_href(route, last_page), ATOM_TYPE)?; - } - - for book in &page.items { - writer.write_event(Event::Start(BytesStart::new("entry")))?; - text_element( - &mut writer, - "id", - &book_identity(library_uuid, book.uuid.as_deref(), book.id), - )?; - text_element(&mut writer, "title", &book.title)?; - text_element(&mut writer, "updated", ×tamp(book.updated_at))?; - for author in &book.authors { - writer.write_event(Event::Start(BytesStart::new("author")))?; - text_element(&mut writer, "name", &author.name)?; - writer.write_event(Event::End(BytesEnd::new("author")))?; - } - text_element(&mut writer, "published", ×tamp(book.created_at))?; - let mut content = BytesStart::new("content"); - content.push_attribute(("type", "text")); - writer.write_event(Event::Start(content))?; - writer.write_event(Event::Text(BytesText::new(&xml_text( - book.description.as_deref().unwrap_or(""), - ))))?; - writer.write_event(Event::End(BytesEnd::new("content")))?; - for language in &book.language_codes { - text_element(&mut writer, "dc:language", language)?; - } - for identifier in &book.identifiers { - text_element(&mut writer, "dc:identifier", &identifier.value)?; - } - for tag in &book.tags { - let mut category = BytesStart::new("category"); - category.push_attribute(("term", xml_text(tag).as_ref())); - writer.write_event(Event::Empty(category))?; - } - if book.has_cover { - link( - &mut writer, - IMAGE_REL, - &format!("/opds/books/{}/cover", book.id.as_i32()), - "image/jpeg", - )?; - } - for file in &book.files { - link( - &mut writer, - ACQUISITION_REL, - &format!( - "/opds/books/{}/files/{}/{}", - book.id.as_i32(), - urlencoding::encode(&file.format), - urlencoding::encode(&format!( - "{}.{}", - file.name, - file.format.to_ascii_lowercase() - )) - ), - assets::mime_type(&file.format), - )?; - } - writer.write_event(Event::End(BytesEnd::new("entry")))?; + links.push(FeedLink { + rel: "next", + href: page_href(route, page_number + 1), + media_type: ATOM_TYPE, + }); + links.push(FeedLink { + rel: "last", + href: page_href(route, last_page), + media_type: ATOM_TYPE, + }); } - writer.write_event(Event::End(BytesEnd::new("feed")))?; - Ok(writer.into_inner()) -} - -fn text_element( - writer: &mut Writer>, - name: &str, - value: &str, -) -> Result<(), quick_xml::Error> { - writer.write_event(Event::Start(BytesStart::new(name)))?; - writer.write_event(Event::Text(BytesText::new(&xml_text(value))))?; - writer.write_event(Event::End(BytesEnd::new(name)))?; - Ok(()) -} - -fn link( - writer: &mut Writer>, - relation: &str, - href: &str, - media_type: &str, -) -> Result<(), quick_xml::Error> { - let mut element = BytesStart::new("link"); - element.push_attribute(("rel", relation)); - element.push_attribute(("href", href)); - element.push_attribute(("type", media_type)); - writer.write_event(Event::Empty(element))?; - Ok(()) + let entries = page + .items + .iter() + .map(|book| FeedEntry { + id: book_identity(library_uuid, book.uuid.as_deref(), book.id), + title: book.title.clone(), + updated: timestamp(book.updated_at), + authors: book + .authors + .iter() + .map(|author| author.name.clone()) + .collect(), + published: timestamp(book.created_at), + languages: book.language_codes.clone(), + identifiers: book + .identifiers + .iter() + .map(|identifier| identifier.value.clone()) + .collect(), + categories: book.tags.clone(), + acquisition_links: book + .files + .iter() + .map(|file| { + ( + ACQUISITION_REL.to_string(), + format!( + "/opds/books/{}/files/{}/{}", + book.id.as_i32(), + urlencoding::encode(&file.format), + urlencoding::encode(&format!( + "{}.{}", + file.name, + file.format.to_ascii_lowercase() + )) + ), + assets::mime_type(&file.format).to_string(), + ) + }) + .collect(), + image_link: book + .has_cover + .then(|| format!("/opds/books/{}/cover", book.id.as_i32())), + content: Some(book.description.clone().unwrap_or_default()), + }) + .collect(); + + let feed = Feed { + id: library_identity(library_uuid), + title: "Citadel — All Books".to_string(), + updated: feed_updated(updated_at), + links, + entries, + }; + crate::xml::write_feed(&feed) } fn page_count(total: i64) -> u64 { @@ -472,7 +491,7 @@ mod tests { library::Book, util::get_db_path, BookAdd, BookFileInfo, BookIdentifier, BookUpdate, Library, LibraryAuthor, }; - use quick_xml::{name::ResolveResult, NsReader}; + use quick_xml::{events::Event, name::ResolveResult, NsReader}; use std::{collections::HashMap, path::PathBuf, sync::Mutex}; use tempfile::TempDir; diff --git a/crates/citadel-opds/src/xml.rs b/crates/citadel-opds/src/xml.rs index baa2e364..09f75f54 100644 --- a/crates/citadel-opds/src/xml.rs +++ b/crates/citadel-opds/src/xml.rs @@ -1,5 +1,101 @@ use std::borrow::Cow; +use quick_xml::{ + events::{BytesDecl, BytesEnd, BytesStart, BytesText, Event}, + Writer, +}; + +use super::catalog::{Feed, FeedEntry, IMAGE_REL, IMAGE_TYPE}; + +pub(crate) fn write_feed(feed: &Feed) -> Result, quick_xml::Error> { + let mut writer = Writer::new(Vec::new()); + writer.write_event(Event::Decl(BytesDecl::new("1.0", Some("UTF-8"), None)))?; + let mut element = BytesStart::new("feed"); + element.push_attribute(("xmlns", "http://www.w3.org/2005/Atom")); + element.push_attribute(("xmlns:dc", "http://purl.org/dc/terms/")); + writer.write_event(Event::Start(element))?; + text_element(&mut writer, "id", &feed.id)?; + text_element(&mut writer, "title", &feed.title)?; + text_element(&mut writer, "updated", &feed.updated)?; + writer.write_event(Event::Start(BytesStart::new("author")))?; + text_element(&mut writer, "name", "Citadel")?; + writer.write_event(Event::End(BytesEnd::new("author")))?; + + for link in &feed.links { + write_link(&mut writer, link.rel, &link.href, link.media_type)?; + } + for entry in &feed.entries { + write_entry(&mut writer, entry)?; + } + + writer.write_event(Event::End(BytesEnd::new("feed")))?; + Ok(writer.into_inner()) +} + +fn write_entry(writer: &mut Writer>, entry: &FeedEntry) -> Result<(), quick_xml::Error> { + writer.write_event(Event::Start(BytesStart::new("entry")))?; + text_element(writer, "id", &entry.id)?; + text_element(writer, "title", &entry.title)?; + text_element(writer, "updated", &entry.updated)?; + for author in &entry.authors { + writer.write_event(Event::Start(BytesStart::new("author")))?; + text_element(writer, "name", author)?; + writer.write_event(Event::End(BytesEnd::new("author")))?; + } + text_element(writer, "published", &entry.published)?; + let mut content = BytesStart::new("content"); + content.push_attribute(("type", "text")); + writer.write_event(Event::Start(content))?; + writer.write_event(Event::Text(BytesText::new(&xml_text( + entry.content.as_deref().unwrap_or(""), + ))))?; + writer.write_event(Event::End(BytesEnd::new("content")))?; + for language in &entry.languages { + text_element(writer, "dc:language", language)?; + } + for identifier in &entry.identifiers { + text_element(writer, "dc:identifier", identifier)?; + } + for term in &entry.categories { + let mut category = BytesStart::new("category"); + category.push_attribute(("term", xml_text(term).as_ref())); + writer.write_event(Event::Empty(category))?; + } + if let Some(href) = &entry.image_link { + write_link(writer, IMAGE_REL, href, IMAGE_TYPE)?; + } + for (rel, href, media_type) in &entry.acquisition_links { + write_link(writer, rel, href, media_type)?; + } + writer.write_event(Event::End(BytesEnd::new("entry")))?; + Ok(()) +} + +fn write_link( + writer: &mut Writer>, + relation: &str, + href: &str, + media_type: &str, +) -> Result<(), quick_xml::Error> { + let mut element = BytesStart::new("link"); + element.push_attribute(("rel", relation)); + element.push_attribute(("href", href)); + element.push_attribute(("type", media_type)); + writer.write_event(Event::Empty(element))?; + Ok(()) +} + +fn text_element( + writer: &mut Writer>, + name: &str, + value: &str, +) -> Result<(), quick_xml::Error> { + writer.write_event(Event::Start(BytesStart::new(name)))?; + writer.write_event(Event::Text(BytesText::new(&xml_text(value))))?; + writer.write_event(Event::End(BytesEnd::new(name)))?; + Ok(()) +} + pub(crate) fn xml_text(value: &str) -> Cow<'_, str> { if value.chars().all(valid_xml_char) { Cow::Borrowed(value) From 1fe1ed76ad3537fdf26021c0b0ebc6cd08a3736a Mon Sep 17 00:00:00 2001 From: Phil Denhoff Date: Thu, 17 Sep 2026 21:31:25 -0700 Subject: [PATCH 4/7] =?UTF-8?q?fix(opds):=20review=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20encoding,=20page=20math,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/citadel-opds/Cargo.toml | 2 +- crates/citadel-opds/src/assets.rs | 19 ++------------- crates/citadel-opds/src/catalog.rs | 32 ++++++++++++++------------ crates/citadel-opds/src/lib.rs | 2 +- crates/citadel-opds/src/xml.rs | 37 ++++++++++++++++++++++++++++++ src-tauri/src/main.rs | 1 - src-tauri/src/opds/mod.rs | 1 - 7 files changed, 58 insertions(+), 36 deletions(-) delete mode 100644 src-tauri/src/opds/mod.rs diff --git a/crates/citadel-opds/Cargo.toml b/crates/citadel-opds/Cargo.toml index c5c8c820..731416ae 100644 --- a/crates/citadel-opds/Cargo.toml +++ b/crates/citadel-opds/Cargo.toml @@ -3,7 +3,7 @@ name = "citadel-opds" version = "0.1.0" edition = "2021" rust-version.workspace = true -description = "Tauri-independent OPDS runtime for Citadel" +description = "OPDS catalog, authentication, and networking runtime for Citadel" [dependencies] axum = "0.8.9" diff --git a/crates/citadel-opds/src/assets.rs b/crates/citadel-opds/src/assets.rs index 40885c53..261a4bf9 100644 --- a/crates/citadel-opds/src/assets.rs +++ b/crates/citadel-opds/src/assets.rs @@ -21,7 +21,7 @@ pub struct AssetHeaders { pub content_type: &'static str, } -/// A bounded synchronous file stream. CDL-23 can move reads onto its HTTP +/// A bounded synchronous file stream. A follow-up can move reads onto the HTTP /// runtime's blocking adapter without coupling asset resolution to a framework. pub struct AssetBody { reader: Take, @@ -221,25 +221,10 @@ fn content_disposition(filename: &str) -> String { } }) .collect(); - let encoded = percent_encode(filename.as_bytes()); + let encoded = urlencoding::encode(filename); format!("attachment; filename=\"{fallback}\"; filename*=UTF-8''{encoded}") } -fn percent_encode(bytes: &[u8]) -> String { - const HEX: &[u8; 16] = b"0123456789ABCDEF"; - let mut encoded = String::with_capacity(bytes.len()); - for &byte in bytes { - if byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'.' | b'_' | b'~') { - encoded.push(char::from(byte)); - } else { - encoded.push('%'); - encoded.push(char::from(HEX[usize::from(byte >> 4)])); - encoded.push(char::from(HEX[usize::from(byte & 0x0f)])); - } - } - encoded -} - #[cfg(test)] mod tests { use std::io::Read; diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index 25f979c4..025a945f 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -16,7 +16,7 @@ use serde::Deserialize; use super::assets::{self, AssetMethod, AssetResponseError}; use crate::identity::{book_identity, library_identity}; -const PAGE_SIZE: i64 = 50; +const PAGE_SIZE: u64 = 50; const ACQUISITION_REL: &str = "http://opds-spec.org/acquisition"; pub(crate) const IMAGE_REL: &str = "http://opds-spec.org/image"; const ATOM_TYPE: &str = "application/atom+xml;profile=opds-catalog;kind=acquisition"; @@ -108,7 +108,8 @@ async fn feed( }; let source = state.source.clone(); - let result = tokio::task::spawn_blocking(move || source.book_page(PAGE_SIZE, offset)).await; + let result = + tokio::task::spawn_blocking(move || source.book_page(PAGE_SIZE as i64, offset)).await; let (library_uuid, updated_at, page) = match result { Ok(Ok(page)) => page, Ok(Err(error)) => return calibre_error(error), @@ -147,7 +148,7 @@ fn page_params(query: &PageQuery) -> Result<(u64, i64), Response> { } let offset = match page_number .checked_sub(1) - .and_then(|page| page.checked_mul(PAGE_SIZE as u64)) + .and_then(|page| page.checked_mul(PAGE_SIZE)) .and_then(|offset| i64::try_from(offset).ok()) { Some(offset) => offset, @@ -420,11 +421,7 @@ fn acquisition_feed( } fn page_count(total: i64) -> u64 { - if total <= 0 { - 1 - } else { - ((total as u64 - 1) / PAGE_SIZE as u64) + 1 - } + u64::try_from(total).unwrap_or(0).div_ceil(PAGE_SIZE) } fn page_href(route: &str, page: u64) -> String { @@ -854,6 +851,16 @@ mod tests { ); } + #[test] + fn page_count_is_zero_for_empty_totals_and_one_for_a_single_book() { + assert_eq!(page_count(0), 0); + assert_eq!(page_count(1), 1); + assert_eq!(page_count(-7), 0); + assert_eq!(page_count(50), 1); + assert_eq!(page_count(51), 2); + assert_eq!(page_count(101), 3); + } + #[tokio::test] async fn loopback_pagination_visits_every_entry_once() { let books = (1..=101) @@ -1124,13 +1131,8 @@ mod tests { async fn empty_and_unavailable_catalogs_return_valid_non_sensitive_responses() { let (base, server) = loopback(Arc::new(MemorySource { books: Vec::new() })).await; let response = reqwest::get(format!("{base}/opds")).await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - let bytes = response.bytes().await.unwrap(); - let feed = parsed_feed(&bytes); - assert!(feed.ids.is_empty()); - assert!(String::from_utf8(bytes.to_vec()) - .unwrap() - .contains("Citadel")); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!(response.text().await.unwrap(), "Page not found"); server.abort(); let (base, server) = loopback(Arc::new(NoLibrary)).await; diff --git a/crates/citadel-opds/src/lib.rs b/crates/citadel-opds/src/lib.rs index 1fd5750c..d1a3c948 100644 --- a/crates/citadel-opds/src/lib.rs +++ b/crates/citadel-opds/src/lib.rs @@ -1,4 +1,4 @@ -//! Tauri-independent OPDS catalog and asset streaming. +//! Framework-free OPDS catalog, authentication, networking, and service lifecycle for Citadel. pub mod assets; pub mod catalog; diff --git a/crates/citadel-opds/src/xml.rs b/crates/citadel-opds/src/xml.rs index 09f75f54..3ab0d5c9 100644 --- a/crates/citadel-opds/src/xml.rs +++ b/crates/citadel-opds/src/xml.rs @@ -115,3 +115,40 @@ pub(crate) fn valid_xml_char(character: char) -> bool { || ('\u{e000}'..='\u{fffd}').contains(&character) || ('\u{10000}'..='\u{10ffff}').contains(&character) } + +#[cfg(test)] +mod tests { + use super::*; + + fn feed_with_title(title: &str) -> Feed { + Feed { + id: "urn:test:feed".to_string(), + title: title.to_string(), + updated: "2024-01-01T00:00:00Z".to_string(), + links: Vec::new(), + entries: vec![FeedEntry { + id: "urn:test:book".to_string(), + title: title.to_string(), + updated: "2024-01-01T00:00:00Z".to_string(), + authors: Vec::new(), + published: "2024-01-01T00:00:00Z".to_string(), + languages: Vec::new(), + identifiers: Vec::new(), + categories: Vec::new(), + acquisition_links: Vec::new(), + image_link: None, + content: None, + }], + } + } + + #[test] + fn titles_with_invalid_xml_chars_are_stripped_so_the_document_stays_valid() { + let xml = + String::from_utf8(write_feed(&feed_with_title("Bad \u{0} & More")).unwrap()) + .unwrap(); + assert!(xml.contains("Bad <Title> & More")); + assert!(!xml.contains('\u{0}')); + assert!(xml.chars().all(valid_xml_char)); + } +} diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index 91975345..d741b232 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -16,7 +16,6 @@ pub mod libs { mod book; mod menu; mod metadata; -pub mod opds; mod state; fn run_tauri_backend() -> std::io::Result<()> { diff --git a/src-tauri/src/opds/mod.rs b/src-tauri/src/opds/mod.rs deleted file mode 100644 index 8b137891..00000000 --- a/src-tauri/src/opds/mod.rs +++ /dev/null @@ -1 +0,0 @@ - From aff8aa93dac551f60cc2dbf146c5cff2b468b46f Mon Sep 17 00:00:00 2001 From: Phil Denhoff <phil@digits.com> Date: Thu, 17 Sep 2026 21:58:37 -0700 Subject: [PATCH 5/7] refactor(opds): count books as u64 at the source boundary --- crates/citadel-opds/src/catalog.rs | 7 +++---- crates/libcalibre/examples/bench_library_queries.rs | 7 ++++--- crates/libcalibre/src/library.rs | 4 ++-- crates/libcalibre/src/queries/books.rs | 9 +++++++-- src-tauri/src/libs/calibre/book.rs | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index 025a945f..f89335f5 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -420,8 +420,8 @@ fn acquisition_feed( crate::xml::write_feed(&feed) } -fn page_count(total: i64) -> u64 { - u64::try_from(total).unwrap_or(0).div_ceil(PAGE_SIZE) +fn page_count(total: u64) -> u64 { + total.div_ceil(PAGE_SIZE) } fn page_href(route: &str, page: u64) -> String { @@ -556,7 +556,7 @@ mod tests { self.books.iter().map(|book| book.updated_at).max(), BookPage { items, - total: self.books.len() as i64, + total: self.books.len() as u64, }, )) } @@ -855,7 +855,6 @@ mod tests { fn page_count_is_zero_for_empty_totals_and_one_for_a_single_book() { assert_eq!(page_count(0), 0); assert_eq!(page_count(1), 1); - assert_eq!(page_count(-7), 0); assert_eq!(page_count(50), 1); assert_eq!(page_count(51), 2); assert_eq!(page_count(101), 3); diff --git a/crates/libcalibre/examples/bench_library_queries.rs b/crates/libcalibre/examples/bench_library_queries.rs index 11ea2afd..efc53c78 100644 --- a/crates/libcalibre/examples/bench_library_queries.rs +++ b/crates/libcalibre/examples/bench_library_queries.rs @@ -32,7 +32,7 @@ struct Args { library: PathBuf, runs: usize, warmup: usize, - expect_total: Option<i64>, + expect_total: Option<u64>, } fn parse_args() -> Result<Args, String> { @@ -333,8 +333,9 @@ fn run() -> Result<(), String> { "Filters: author {prolific_author} ({author_books} books), series {busy_series} ({series_books} books)" ); - let middle_offset = (total / 2 / PAGE_SIZE) * PAGE_SIZE; - let last_offset = ((total - 1).max(0) / PAGE_SIZE) * PAGE_SIZE; + let total_i64 = i64::try_from(total).unwrap_or(i64::MAX); + let middle_offset = (total_i64 / 2 / PAGE_SIZE) * PAGE_SIZE; + let last_offset = ((total_i64 - 1).max(0) / PAGE_SIZE) * PAGE_SIZE; let page_query = |offset: i64, sort: BookSortOrder| BookQuery { sort, diff --git a/crates/libcalibre/src/library.rs b/crates/libcalibre/src/library.rs index 87573690..fdeef473 100644 --- a/crates/libcalibre/src/library.rs +++ b/crates/libcalibre/src/library.rs @@ -183,7 +183,7 @@ pub struct BookPage { pub items: Vec<Book>, /// Total number of books matching the query's filters, ignoring /// limit/offset. - pub total: i64, + pub total: u64, } /// One series in the library, with its linked-book count. Returned by @@ -929,7 +929,7 @@ impl Library { resolvable_ids.push(book_id); } } - let total = i64::try_from(resolvable_ids.len()).unwrap_or(i64::MAX); + let total = u64::try_from(resolvable_ids.len()).unwrap_or(u64::MAX); let start = usize::try_from(offset.max(0)).unwrap_or(usize::MAX); let page_len = usize::try_from(limit.max(0)).unwrap_or(usize::MAX); let book_ids = resolvable_ids diff --git a/crates/libcalibre/src/queries/books.rs b/crates/libcalibre/src/queries/books.rs index 27147617..4105bc30 100644 --- a/crates/libcalibre/src/queries/books.rs +++ b/crates/libcalibre/src/queries/books.rs @@ -250,10 +250,12 @@ pub(crate) fn query_page( } /// COUNT over the same WHERE as [`query_page`], ignoring limit/offset. +/// SQLite COUNT is non-negative, so the boundary conversion to u64 is total +/// in practice; a nonsensical negative maps to 0. pub(crate) fn query_count( conn: &mut SqliteConnection, filters: &BookPageFilters, -) -> Result<i64, CalibreError> { +) -> Result<u64, CalibreError> { let where_sql = filter_where_sql(filters); let sql = format!("SELECT COUNT(*) AS total FROM books WHERE {where_sql}"); @@ -270,7 +272,10 @@ pub(crate) fn query_count( None => sql_query(sql).load(conn).map_err(CalibreError::from)?, }; - Ok(rows.first().map(|row| row.total).unwrap_or(0)) + Ok(rows + .first() + .map(|row| u64::try_from(row.total).unwrap_or(0)) + .unwrap_or(0)) } pub(crate) fn create(conn: &mut SqliteConnection, book: NewBook) -> Result<BookRow, CalibreError> { diff --git a/src-tauri/src/libs/calibre/book.rs b/src-tauri/src/libs/calibre/book.rs index b03beac6..c2b2865f 100644 --- a/src-tauri/src/libs/calibre/book.rs +++ b/src-tauri/src/libs/calibre/book.rs @@ -59,7 +59,7 @@ pub fn query_page( library_root: String, lib: &mut Library, query: libcalibre::BookQuery, -) -> Result<(Vec<LibraryBook>, i64), libcalibre::CalibreError> { +) -> Result<(Vec<LibraryBook>, u64), libcalibre::CalibreError> { let page = lib.query_books(query)?; let author_book_counts = lib.author_book_counts()?; From 21024577a1850ae294e54e693c44bd61d71f1347 Mon Sep 17 00:00:00 2001 From: Phil Denhoff <phil@digits.com> Date: Thu, 17 Sep 2026 22:01:05 -0700 Subject: [PATCH 6/7] fix(opds): empty catalogs serve a valid empty acquisition feed --- crates/citadel-opds/src/catalog.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index f89335f5..b60af222 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -117,7 +117,7 @@ async fn feed( }; let last_page = page_count(page.total); - if page_number > last_page { + if page_number > last_page && !(page_number == 1 && page.total == 0) { return public_error(StatusCode::NOT_FOUND, "Page not found"); } @@ -1130,8 +1130,13 @@ mod tests { async fn empty_and_unavailable_catalogs_return_valid_non_sensitive_responses() { let (base, server) = loopback(Arc::new(MemorySource { books: Vec::new() })).await; let response = reqwest::get(format!("{base}/opds")).await.unwrap(); - assert_eq!(response.status(), StatusCode::NOT_FOUND); - assert_eq!(response.text().await.unwrap(), "Page not found"); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.headers()[header::CONTENT_TYPE], ATOM_CONTENT_TYPE); + let feed = parsed_feed(&response.bytes().await.unwrap()); + assert!(feed.ids.is_empty()); + assert!(feed.titles.is_empty()); + assert!(feed.next.is_none()); + assert!(feed.previous.is_none()); server.abort(); let (base, server) = loopback(Arc::new(NoLibrary)).await; From 07d11ea9620d348101d585daf8177f9f8b942eef Mon Sep 17 00:00:00 2001 From: Phil Denhoff <phil@digits.com> Date: Thu, 17 Sep 2026 22:10:17 -0700 Subject: [PATCH 7/7] fix(opds): loud count conversion, always-valid page 1, empty-feed Atom assertions --- crates/citadel-opds/src/catalog.rs | 35 +++++++++++++++++++++++++----- crates/libcalibre/src/library.rs | 2 +- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/crates/citadel-opds/src/catalog.rs b/crates/citadel-opds/src/catalog.rs index b60af222..0cb07e9d 100644 --- a/crates/citadel-opds/src/catalog.rs +++ b/crates/citadel-opds/src/catalog.rs @@ -116,8 +116,8 @@ async fn feed( Err(_) => return public_error(StatusCode::INTERNAL_SERVER_ERROR, "Catalog unavailable"), }; - let last_page = page_count(page.total); - if page_number > last_page && !(page_number == 1 && page.total == 0) { + let last_page = page_count(page.total).max(1); + if page_number > last_page { return public_error(StatusCode::NOT_FOUND, "Page not found"); } @@ -482,7 +482,7 @@ mod tests { use super::*; use crate::identity::{book_identity, library_identity}; use crate::xml::valid_xml_char; - use chrono::NaiveDate; + use chrono::{DateTime, NaiveDate}; use diesel::{Connection, RunQueryDsl}; use libcalibre::{ library::Book, util::get_db_path, BookAdd, BookFileInfo, BookIdentifier, BookUpdate, @@ -687,6 +687,9 @@ mod tests { #[derive(Default)] struct ParsedFeed { + feed_id: Option<String>, + feed_title: Option<String>, + feed_updated: Option<String>, ids: Vec<String>, titles: Vec<String>, content: Vec<String>, @@ -713,6 +716,9 @@ mod tests { in_entry = true; } else if in_entry && matches!(name.as_slice(), b"id" | b"title" | b"content") { current = Some((name, String::new())); + } else if !in_entry && matches!(name.as_slice(), b"id" | b"title" | b"updated") + { + current = Some((name, String::new())); } } Ok((ResolveResult::Bound(namespace), Event::Empty(element))) @@ -768,9 +774,22 @@ mod tests { if let Some((name, value)) = current.take() { if element.local_name().as_ref() == name.as_slice() { match name.as_slice() { - b"id" => parsed.ids.push(value), - b"title" => parsed.titles.push(value), + b"id" => { + if in_entry { + parsed.ids.push(value); + } else { + parsed.feed_id = Some(value); + } + } + b"title" => { + if in_entry { + parsed.titles.push(value); + } else { + parsed.feed_title = Some(value); + } + } b"content" => parsed.content.push(value), + b"updated" => parsed.feed_updated = Some(value), _ => {} } } else { @@ -1135,6 +1154,12 @@ mod tests { let feed = parsed_feed(&response.bytes().await.unwrap()); assert!(feed.ids.is_empty()); assert!(feed.titles.is_empty()); + let feed_id = feed.feed_id.expect("feed-level id present"); + assert!(!feed_id.is_empty()); + let feed_title = feed.feed_title.expect("feed-level title present"); + assert!(!feed_title.is_empty()); + let feed_updated = feed.feed_updated.expect("feed-level updated present"); + DateTime::parse_from_rfc3339(&feed_updated).expect("feed updated is valid RFC 3339"); assert!(feed.next.is_none()); assert!(feed.previous.is_none()); server.abort(); diff --git a/crates/libcalibre/src/library.rs b/crates/libcalibre/src/library.rs index fdeef473..87a680d7 100644 --- a/crates/libcalibre/src/library.rs +++ b/crates/libcalibre/src/library.rs @@ -929,7 +929,7 @@ impl Library { resolvable_ids.push(book_id); } } - let total = u64::try_from(resolvable_ids.len()).unwrap_or(u64::MAX); + let total = u64::try_from(resolvable_ids.len()).expect("usize fits in u64"); let start = usize::try_from(offset.max(0)).unwrap_or(usize::MAX); let page_len = usize::try_from(limit.max(0)).unwrap_or(usize::MAX); let book_ids = resolvable_ids