From 72d167005fb16f992612bfdc78a0fb96601642ab Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 12 Sep 2026 23:45:01 -0400 Subject: [PATCH] fix(scanner): resolve EPUB covers via structured OPF parsing, Calibre chain The cover lookup scraped the OPF with attribute-order-sensitive regexes. Real books serialize attributes in any order - Grand Central's '3 Days to Live' puts href before id on manifest items and content before name on the cover meta - so all three regex paths missed and the book fell through to filename guessing, extracting no cover at all. Attribute order is meaningless in XML; the regexes were never safe. Replace them with a structured parse (encoding/xml, namespace and attribute-order agnostic; see the new media_scanner_opf.go) and follow Calibre's read_raster_cover resolution order: 1. manifest item with properties=cover-image (non-(X)HTML media only) 2. resolved through the manifest, same media guard 3. first spine item that is itself a raster image (store manga) 4. NEW cover-page fallback: books declaring no raster cover at all - the classic EPUB2/Adobe cover.xhtml wrapper - are mined for / SVG references (Calibre renders the page with Qt; extracting the referenced image covers the practical cases without a rendering engine) 5. existing zip filename guessing stays as the last resort, and the old regex chain survives as findCoverInOPFLegacy for OPFs too malformed for a real XML parse. Hrefs are now URL-decoded and posix-normalized against the OPF's own path (path.Join semantics), so '../art/cover.jpg' from a nested cover page and %20-encoded names resolve correctly. Tests: attribute-order chaos modeled on the failing Patterson book, SVG-wrapped cover pages via guide references, image-first spines, and path resolution edge cases. Verified live against the real '3 Days to Live' EPUB, which previously produced no cover. --- internal/services/media_scanner.go | 84 +++++++-- internal/services/media_scanner_opf.go | 199 ++++++++++++++++++++ internal/services/media_scanner_opf_test.go | 164 ++++++++++++++++ 3 files changed, 434 insertions(+), 13 deletions(-) create mode 100644 internal/services/media_scanner_opf.go create mode 100644 internal/services/media_scanner_opf_test.go diff --git a/internal/services/media_scanner.go b/internal/services/media_scanner.go index 2a29a92..da0785f 100644 --- a/internal/services/media_scanner.go +++ b/internal/services/media_scanner.go @@ -23,8 +23,10 @@ import ( _ "image/png" "io" "io/fs" + "net/url" "os" "os/exec" + "path" "path/filepath" "regexp" "strconv" @@ -1949,7 +1951,47 @@ func findCoverImageInZip(files []*zip.File) string { } // findCoverInOPF parses OPF content to find cover image reference -func findCoverInOPF(opfContent []byte, files []*zip.File, opfDir string) string { +// findCoverInOPF locates the cover image for an EPUB, following Calibre's +// read_raster_cover resolution order (see media_scanner_opf.go): +// 1. manifest item with properties="cover-image" +// 2. resolved through the manifest +// 3. the first spine item being a raster image itself (store manga) +// 4. NEW: the cover page (guide type="cover" or first spine item) mined for +// / SVG - covers books that declare no +// raster cover at all, e.g. classic EPUB2/Adobe cover.xhtml wrappers +// 5. filename guessing in the zip (pre-existing fallback) +// +// XML parsing is attribute-order agnostic; if the OPF is too malformed for +// encoding/xml, the legacy regex chain runs as a compatibility fallback. +func findCoverInOPF(opfContent []byte, files []*zip.File, opfPath string) string { + opf, err := parseOPFXML(opfContent) + if err != nil { + return findCoverInOPFLegacy(opfContent, files, opfPath) + } + + if href := opf.findRasterCoverInOPF(); href != "" { + return resolveOPFPath(opfPath, href) + } + + // Cover-page fallback: Calibre renders the page; we extract the image it + // references (the practical case - the page wraps a raster in img/SVG). + if pageHref := opf.coverPageHref(); pageHref != "" { + if pageContent, err := readFileFromZip(files, resolveOPFPath(opfPath, pageHref)); err == nil { + if imgRef := findImageReferenceInPage(pageContent); imgRef != "" { + imgPath := resolveOPFPath(resolveOPFPath(opfPath, pageHref), imgRef) + if zipHasFile(files, imgPath) { + return imgPath + } + } + } + } + + return findCoverImageInZip(files) +} + +// findCoverInOPFLegacy is the pre-XML cover lookup, kept solely as a +// fallback for OPFs too malformed for a real XML parse. +func findCoverInOPFLegacy(opfContent []byte, files []*zip.File, opfPath string) string { contentStr := string(opfContent) // Look for item with properties="cover-image" @@ -1961,7 +2003,7 @@ func findCoverInOPF(opfContent []byte, files []*zip.File, opfDir string) string hrefRE := regexp.MustCompile(fmt.Sprintf(`]*id="%s"[^>]*href="([^"]+)"`, coverID)) hrefMatches := hrefRE.FindStringSubmatch(contentStr) if len(hrefMatches) > 1 { - return resolveOPFPath(opfDir, hrefMatches[1]) + return resolveOPFPath(opfPath, hrefMatches[1]) } } @@ -1975,7 +2017,7 @@ func findCoverInOPF(opfContent []byte, files []*zip.File, opfDir string) string hrefRE := regexp.MustCompile(fmt.Sprintf(`]*id="%s"[^>]*href="([^"]+)"`, coverID)) hrefMatches := hrefRE.FindStringSubmatch(contentStr) if len(hrefMatches) > 1 { - return resolveOPFPath(opfDir, hrefMatches[1]) + return resolveOPFPath(opfPath, hrefMatches[1]) } } } @@ -1984,18 +2026,34 @@ func findCoverInOPF(opfContent []byte, files []*zip.File, opfDir string) string return findCoverImageInZip(files) } -// resolveOPFPath resolves a relative path against the OPF directory -func resolveOPFPath(opfDir, href string) string { - if opfDir == "" { - return href +// zipHasFile reports whether the zip contains an entry with exactly this name. +func zipHasFile(files []*zip.File, name string) bool { + name = filepath.ToSlash(name) + for _, f := range files { + if filepath.ToSlash(f.Name) == name { + return true + } } - // Handle ../ in href - if strings.HasPrefix(href, "../") { - // Simple case: just use the href as-is for now - return href + return false +} + +// resolveOPFPath resolves an OPF-relative href against the OPF document's own +// path inside the zip. Hrefs are URL-decoded and normalized with posix path +// semantics ("../" walks up), matching Calibre's +// posixpath.normpath(posixpath.join(base, href)). +func resolveOPFPath(opfPath, href string) string { + if unescaped, err := url.PathUnescape(href); err == nil { + href = unescaped } - // Join the directory with the href - return filepath.Join(filepath.Dir(opfDir), href) + href = strings.TrimPrefix(filepath.ToSlash(href), "/") + base := "" + if dir := path.Dir(filepath.ToSlash(opfPath)); dir != "." { + base = dir + } + if base == "" { + return path.Clean(href) + } + return path.Clean(path.Join(base, href)) } // readFileFromZip reads a file from the zip by name diff --git a/internal/services/media_scanner_opf.go b/internal/services/media_scanner_opf.go new file mode 100644 index 0000000..9e750ad --- /dev/null +++ b/internal/services/media_scanner_opf.go @@ -0,0 +1,199 @@ +package services + +import ( + "bytes" + "encoding/xml" + "strings" +) + +// Calibre-modeled OPF parsing. The scanner previously scraped OPF content +// with attribute-order-sensitive regexes; real books serialize attributes in +// any order (e.g. Pragmatic/Pattinson EPUBs put id before properties and +// content before name), which silently defeated cover detection. Everything +// here is parsed with encoding/xml so attribute order and namespace prefix +// choices are irrelevant. + +type opfDCValue struct { + ID string `xml:"id,attr"` + Value string `xml:",chardata"` +} + +type opfIdentifier struct { + Scheme string `xml:"http://www.idpf.org/2007/opf scheme,attr"` + Value string `xml:",chardata"` +} + +type opfMeta struct { + ID string `xml:"id,attr"` + Name string `xml:"name,attr"` + Content string `xml:"content,attr"` + Property string `xml:"property,attr"` + Refines string `xml:"refines,attr"` + Value string `xml:",chardata"` +} + +type opfItem struct { + ID string `xml:"id,attr"` + Href string `xml:"href,attr"` + MediaType string `xml:"media-type,attr"` + Properties string `xml:"properties,attr"` +} + +// opfDocument is a structured view of an OPF package document. +type opfDocument struct { + Metadata struct { + Titles []opfDCValue `xml:"http://purl.org/dc/elements/1.1/ title"` + Creators []string `xml:"http://purl.org/dc/elements/1.1/ creator"` + Subjects []string `xml:"http://purl.org/dc/elements/1.1/ subject"` + Descriptions []string `xml:"http://purl.org/dc/elements/1.1/ description"` + Publishers []string `xml:"http://purl.org/dc/elements/1.1/ publisher"` + Dates []string `xml:"http://purl.org/dc/elements/1.1/ date"` + Languages []string `xml:"http://purl.org/dc/elements/1.1/ language"` + Identifiers []opfIdentifier `xml:"http://purl.org/dc/elements/1.1/ identifier"` + Contributors []string `xml:"http://purl.org/dc/elements/1.1/ contributor"` + Metas []opfMeta `xml:"meta"` + } `xml:"metadata"` + Manifest struct { + Items []opfItem `xml:"item"` + } `xml:"manifest"` + Spine struct { + Itemrefs []struct { + IDRef string `xml:"idref,attr"` + } `xml:"itemref"` + } `xml:"spine"` + Guide struct { + References []struct { + Type string `xml:"type,attr"` + Href string `xml:"href,attr"` + } `xml:"reference"` + } `xml:"guide"` +} + +func parseOPFXML(content []byte) (*opfDocument, error) { + var doc opfDocument + if err := xml.Unmarshal(content, &doc); err != nil { + return nil, err + } + return &doc, nil +} + +// itemByID returns manifest items with id, href and media-type, keyed by id. +func (d *opfDocument) itemByID() map[string]opfItem { + m := make(map[string]opfItem, len(d.Manifest.Items)) + for _, it := range d.Manifest.Items { + if it.ID != "" && it.Href != "" && it.MediaType != "" { + m[it.ID] = it + } + } + return m +} + +// firstSpineItem returns the manifest item for the first spine idref. +func (d *opfDocument) firstSpineItem() (opfItem, bool) { + if len(d.Spine.Itemrefs) == 0 { + return opfItem{}, false + } + item, ok := d.itemByID()[d.Spine.Itemrefs[0].IDRef] + return item, ok +} + +// isRasterMedia reports whether a manifest media-type is an image but not an +// (X)HTML document - Calibre's guard against cover *pages* masquerading as +// cover images. +func isRasterMedia(mediaType string) bool { + mt := strings.ToLower(strings.TrimSpace(mediaType)) + if mt == "" { + return false + } + if strings.Contains(mt, "xml") || strings.Contains(mt, "html") { + return false + } + return strings.HasPrefix(mt, "image/") +} + +// findRasterCoverInOPF ports Calibre's read_raster_cover resolution order: +// 1. manifest item with properties containing "cover-image" +// 2. resolved through the manifest +// 3. the first spine item being a raster image itself (store manga) +// +// Returns the OPF-relative href of the cover image, or "". +func (d *opfDocument) findRasterCoverInOPF() string { + // 1. properties="cover-image" (space-separated property list) + for _, it := range d.Manifest.Items { + for _, prop := range strings.Fields(it.Properties) { + if strings.EqualFold(prop, "cover-image") && isRasterMedia(it.MediaType) { + return it.Href + } + } + } + + // 2. meta name="cover" content= + byID := d.itemByID() + for _, m := range d.Metadata.Metas { + if !strings.EqualFold(m.Name, "cover") { + continue + } + if it, ok := byID[strings.TrimSpace(m.Content)]; ok && isRasterMedia(it.MediaType) { + return it.Href + } + } + + // 3. first spine item is itself an image (jpeg/webp/png per Calibre) + if it, ok := d.firstSpineItem(); ok { + mt := strings.ToLower(it.MediaType) + if mt == "image/jpeg" || mt == "image/webp" || mt == "image/png" { + return it.Href + } + } + + return "" +} + +// coverPageHref returns the OPF-relative href of the cover *page* document to +// mine for an embedded image: the guide's type="cover" reference when +// present, otherwise the first spine item (Calibre renders the latter). +func (d *opfDocument) coverPageHref() string { + for _, ref := range d.Guide.References { + if strings.EqualFold(ref.Type, "cover") && ref.Href != "" { + return ref.Href + } + } + if it, ok := d.firstSpineItem(); ok { + if it.Href != "" && !isRasterMedia(it.MediaType) { + return it.Href + } + } + return "" +} + +// findImageReferenceInPage extracts the first raster image reference from a +// cover (X)HTML page: or SVG . +// Token-based parsing keeps it tolerant of mixed namespaces and fragments. +// Returns the reference relative to the page document, or "". +func findImageReferenceInPage(pageContent []byte) string { + decoder := xml.NewDecoder(bytes.NewReader(pageContent)) + for { + tok, err := decoder.Token() + if err != nil { + return "" + } + start, ok := tok.(xml.StartElement) + if !ok { + continue + } + switch strings.ToLower(start.Name.Local) { + case "img": + for _, a := range start.Attr { + if strings.EqualFold(a.Name.Local, "src") && strings.TrimSpace(a.Value) != "" { + return strings.TrimSpace(a.Value) + } + } + case "image": + for _, a := range start.Attr { + if strings.EqualFold(a.Name.Local, "href") && strings.TrimSpace(a.Value) != "" { + return strings.TrimSpace(a.Value) + } + } + } + } +} diff --git a/internal/services/media_scanner_opf_test.go b/internal/services/media_scanner_opf_test.go new file mode 100644 index 0000000..0a6fb76 --- /dev/null +++ b/internal/services/media_scanner_opf_test.go @@ -0,0 +1,164 @@ +package services + +import ( + "archive/zip" + "os" + "path/filepath" + "testing" +) + +// helper to build an EPUB zip from a file map for cover tests +func writeEPUB(t *testing.T, path string, files map[string]string) { + t.Helper() + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + defer f.Close() + w := zip.NewWriter(f) + mimetype, err := w.CreateHeader(&zip.FileHeader{Name: "mimetype", Method: zip.Store}) + if err != nil { + t.Fatal(err) + } + mimetype.Write([]byte("application/epub+zip")) + for name, content := range files { + fw, err := w.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := fw.Write([]byte(content)); err != nil { + t.Fatal(err) + } + } + if err := w.Close(); err != nil { + t.Fatal(err) + } +} + +const containerXML = `` + +const tinyJPEG = "\xff\xd8\xff\xe0\x00\x10JFIF\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xd9" + +// TestFindCoverInOPFAttributeOrder guards the regression where attribute +// order defeated regex scraping: this OPF mirrors Grand Central's "3 Days to +// Live" serialization (href before id, content before name on the meta tag). +func TestFindCoverInOPFAttributeOrder(t *testing.T) { + opf := ` + + + 3 Days to Live + + + + + + +` + files := map[string]string{ + "META-INF/container.xml": containerXML, + "OEBPS/package.opf": opf, + "OEBPS/images/9781538752760.jpg": tinyJPEG, + } + epubPath := filepath.Join(t.TempDir(), "book.epub") + writeEPUB(t, epubPath, files) + + s := NewMediaScanner(nil) + coverPath, err := s.extractEPUBCover(epubPath) + if err != nil { + t.Fatalf("extractEPUBCover() error: %v", err) + } + if coverPath == "" { + t.Fatal("cover not extracted - attribute order still defeats resolution") + } + if _, err := os.Stat(coverPath); err != nil { + t.Fatalf("cover file not written: %v", err) + } +} + +// TestFindCoverInOPFCoverPage covers books that declare no raster cover at +// all: the classic EPUB2/Adobe structure where cover.xhtml wraps the image +// (here via SVG), reachable through the guide reference or first spine item. +func TestFindCoverInOPFCoverPage(t *testing.T) { + opf := ` + + + Old Adobe Book + + + + + + + +` + coverPage := ` + + +
+ +` + files := map[string]string{ + "META-INF/container.xml": containerXML, + "OEBPS/package.opf": opf, + "OEBPS/text/cover.xhtml": coverPage, + "OEBPS/art/cover-wrap.jpg": tinyJPEG, + } + epubPath := filepath.Join(t.TempDir(), "adobe.epub") + writeEPUB(t, epubPath, files) + + s := NewMediaScanner(nil) + coverPath, err := s.extractEPUBCover(epubPath) + if err != nil { + t.Fatalf("extractEPUBCover() error: %v", err) + } + if coverPath == "" { + t.Fatal("cover-page fallback failed to find SVG-wrapped image") + } +} + +// TestFindCoverInOPFImageFirstSpine covers store manga whose first spine +// item is a raster image itself (Calibre's third resolution step). +func TestFindCoverInOPFImageFirstSpine(t *testing.T) { + opf := ` + + Manga Vol 1 + + + + +` + files := map[string]string{ + "META-INF/container.xml": containerXML, + "OEBPS/package.opf": opf, + "OEBPS/pages/0001.jpg": tinyJPEG, + } + epubPath := filepath.Join(t.TempDir(), "manga.epub") + writeEPUB(t, epubPath, files) + + s := NewMediaScanner(nil) + coverPath, err := s.extractEPUBCover(epubPath) + if err != nil { + t.Fatalf("extractEPUBCover() error: %v", err) + } + if coverPath == "" { + t.Fatal("image-first spine cover not detected") + } +} + +// TestResolveOPFPath checks URL decoding and posix normalization of +// OPF-relative hrefs. +func TestResolveOPFPath(t *testing.T) { + tests := []struct { + opfPath, href, want string + }{ + {"OEBPS/package.opf", "images/cover.jpg", "OEBPS/images/cover.jpg"}, + {"package.opf", "cover.jpg", "cover.jpg"}, + {"OEBPS/package.opf", "../cover.jpg", "cover.jpg"}, + {"OEBPS/package.opf", "my%20covers/a%20cover.jpg", "OEBPS/my covers/a cover.jpg"}, + } + for _, tt := range tests { + if got := resolveOPFPath(tt.opfPath, tt.href); got != tt.want { + t.Errorf("resolveOPFPath(%q, %q) = %q, want %q", tt.opfPath, tt.href, got, tt.want) + } + } +}