diff --git a/core/artwork/e2e/acquire_serve_test.go b/core/artwork/e2e/acquire_serve_test.go index d6c374e39..0a9363b5d 100644 --- a/core/artwork/e2e/acquire_serve_test.go +++ b/core/artwork/e2e/acquire_serve_test.go @@ -6,6 +6,7 @@ import ( "errors" "io" "os" + "path/filepath" "time" "github.com/navidrome/navidrome/conf" @@ -116,6 +117,25 @@ var _ = Describe("Acquisition → serve loop", func() { albumRepo.SetData(model.Albums{{ID: albumID, Name: "Album", FolderIDs: []string{"f1"}, LibraryID: 0}}) } + It("acquires and serves a cover whose format has no registered decoder (#5950)", func() { + libDir := GinkgoT().TempDir() + Expect(os.MkdirAll(filepath.Join(libDir, "an-album"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libDir, "an-album", "cover.jxl"), jxlFixture, 0600)).To(Succeed()) + + conf.Server.CoverArtPriority = "cover.*" + libRepo.SetData(model.Libraries{{ID: 0, Path: libDir}}) + folderRepo.result = []model.Folder{{Path: "an-album", ImageFiles: []string{"cover.jxl"}}} + albumRepo.SetData(model.Albums{{ID: "al1", Name: "Album", FolderIDs: []string{"f1"}, LibraryID: 0}}) + + bump("al", "al1") + runWorkerUntil(ctx, worker, itemFound(model.KindAlbumArtwork, "al1")) + + img, err := svc.Get(ctx, model.MustParseArtworkID("al-al1"), 0, false) + Expect(err).ToNot(HaveOccurred()) + Expect(img.Placeholder).To(BeFalse()) + Expect(readAll(img)).To(Equal(jxlFixture)) + }) + It("acquires album folder art and serves the exact bytes under its hash", func() { seedFolderAlbum("al1") bump("al", "al1") @@ -317,6 +337,9 @@ func mustGet(img *artwork.Image, err error) *artwork.Image { } // Raw bytes on purpose: encoding a GIF here would register image/gif in the test binary, masking +// jxlFixture is a JPEG XL bare codestream header: a real image format, with no stdlib decoder. +var jxlFixture = []byte{0xff, 0x0a, 0x00, 0x10, 0x00} + // the production import the spec above guards. var gifFixture = []byte{ 0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x04, 0x00, 0x04, 0x00, 0x80, 0x00, diff --git a/core/artwork/processor.go b/core/artwork/processor.go index 6ce76bf45..4d38ced95 100644 --- a/core/artwork/processor.go +++ b/core/artwork/processor.go @@ -123,12 +123,20 @@ func (p *processor) acquire(ctx context.Context, item model.ArtworkQueueItem) (o art, err := repo.GetImage(hash) switch { - case err == nil: + case err == nil && art.Width > 0: log.Debug(ctx, "Artwork: Reusing a known image, skipping decode", "kind", item.ItemKind, "id", item.ItemID, "hash", hash) - case errors.Is(err, model.ErrNotFound): + // A row with no dimensions was stored when no decoder matched; retry in case one exists now. + case err == nil, errors.Is(err, model.ErrNotFound): decodeStart := time.Now() art, err = decodeArtwork(ctx, hash, data) + // Extension-matched local bytes we cannot decode are most likely a codec we lack; an + // external body carries no such guarantee, and empty bytes are no image at all. + if errors.Is(err, image.ErrFormat) && len(data) > 0 && isLocalSource(res.source) { + log.Debug(ctx, "Artwork: No decoder for this image format, storing it without placeholders", + "kind", item.ItemKind, "id", item.ItemID, "source", res.source, "bytes", len(data)) + art, err = undecodedArtwork(hash), nil + } if err != nil { log.Warn(ctx, "Artwork: Failed to decode resolved image", "kind", item.ItemKind, "id", item.ItemID, err) return outcomeFailed, nil @@ -234,6 +242,12 @@ func decodeCapped(data []byte) (image.Image, string, error) { return img, format, nil } +// undecodedArtwork is the row for bytes no decoder matched: servable, but with no dimensions +// and none of the placeholders a decode would have produced. +func undecodedArtwork(hash string) *model.Artwork { + return &model.Artwork{Hash: hash, Mime: mimeForFormat("")} +} + // decodeArtwork builds a new Artwork row from raw bytes: dimensions, mime and the two // placeholder hashes, both encoded from one shared downscaled thumbnail. func decodeArtwork(ctx context.Context, hash string, data []byte) (*model.Artwork, error) { @@ -288,6 +302,11 @@ func isFileBacked(source string) bool { return source == "folder" || source == "upload" } +// isLocalSource reports whether the bytes came off disk rather than off the network. +func isLocalSource(source string) bool { + return isFileBacked(source) || source == "embedded" +} + // placeBytes reports the item's backing-file provenance and writes the bytes into the store // for the sources that have none. func placeBytes(store *ImageStore, art *model.Artwork, res resolution, data []byte) (sourcePath string, refMtime int64, err error) { diff --git a/core/artwork/processor_test.go b/core/artwork/processor_test.go index 8d30975cd..1ada8415d 100644 --- a/core/artwork/processor_test.go +++ b/core/artwork/processor_test.go @@ -1,6 +1,7 @@ package artwork import ( + "bytes" "context" "encoding/binary" "errors" @@ -24,6 +25,9 @@ import ( . "github.com/onsi/gomega" ) +// jxlCodestream is a JPEG XL bare codestream header: a real image format, with no stdlib decoder. +var jxlCodestream = []byte{0xff, 0x0a, 0x00, 0x10, 0x00} + // DecodeConfig reads only the header, so the pixel data can be omitted entirely. func pngHeaderWithDims(w, h uint32) []byte { ihdr := make([]byte, 13) @@ -259,6 +263,59 @@ var _ = Describe("processor.acquire", func() { Expect(ia.Source).To(Equal("folder")) }) + It("undecodable local file: acquires it anyway, with no placeholder metadata", func() { + libRoot := GinkgoT().TempDir() + Expect(os.MkdirAll(filepath.Join(libRoot, "album"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "album", "cover.jpg"), jxlCodestream, 0600)).To(Succeed()) + libRepo.SetData(model.Libraries{{ID: 0, Path: testFileLibPath(libRoot)}}) + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alU", Name: "Album", FolderIDs: []string{"f1"}}}) + folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}} + + out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alU"}) + Expect(out).To(Equal(outcomeFound)) + + ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alU", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(ia.Source).To(Equal("folder")) + art, err := artRepo.GetImage(ia.Hash) + Expect(err).ToNot(HaveOccurred()) + Expect(art.Width).To(BeZero()) + Expect(art.BlurHash).To(BeEmpty()) + }) + + It("empty local file: fails without writing state", func() { + libRoot := GinkgoT().TempDir() + Expect(os.MkdirAll(filepath.Join(libRoot, "album"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "album", "cover.jpg"), nil, 0600)).To(Succeed()) + libRepo.SetData(model.Libraries{{ID: 0, Path: testFileLibPath(libRoot)}}) + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alE", Name: "Album", FolderIDs: []string{"f1"}}}) + folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}} + + out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alE"}) + Expect(out).To(Equal(outcomeFailed)) + + _, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alE", model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + }) + + // An agent answering 200 with a non-image body must keep retrying, not pin garbage as a cover. + It("undecodable external body: fails without writing state", func() { + conf.Server.CoverArtPriority = "external" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("rate limited")) + })) + DeferCleanup(srv.Close) + + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alX", Name: "Album"}}) + imageAgents(&fakeImageAgent{name: "deezerFake", imgs: []agents.ExternalImage{{URL: srv.URL, Size: 500}}}) + + out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alX"}) + Expect(out).To(Equal(outcomeFailed)) + + _, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "alX", model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + }) + It("found-external: persists source as external: and stores the fetched bytes", func() { conf.Server.CoverArtPriority = "external" imgBytes, err := os.ReadFile(filepath.Join(repoRoot, "tests/fixtures/artist/an-album/cover.jpg")) @@ -374,7 +431,8 @@ var _ = Describe("processor.acquire", func() { conf.Server.DataFolder = conf.NewDir(tmpDir) Expect(os.MkdirAll(filepath.Join(tmpDir, "artwork", "radio"), 0755)).To(Succeed()) imgPath := filepath.Join(tmpDir, "artwork", "radio", "ra1_test.jpg") - Expect(os.WriteFile(imgPath, []byte("not actually an image"), 0600)).To(Succeed()) + // Truncated PNG: a known format, so this is a real decode failure, not a missing decoder. + Expect(os.WriteFile(imgPath, pngHeaderWithDims(100, 100), 0600)).To(Succeed()) radioRepo := tests.CreateMockedRadioRepo() radioRepo.Data = map[string]*model.Radio{"ra1": {ID: "ra1", Name: "Radio", UploadedImage: "ra1_test.jpg"}} @@ -428,6 +486,54 @@ var _ = Describe("processor.acquire", func() { Expect(err.Error()).To(ContainSubstring("dimensions")) }) + It("unknown format: reports ErrFormat so the caller can decide", func() { + _, err := decodeArtwork(ctx, "jxl", jxlCodestream) + Expect(err).To(MatchError(image.ErrFormat)) + }) + + It("undecodedArtwork: carries the hash and mime, and nothing a decode would add", func() { + art := undecodedArtwork("jxl") + Expect(art.Hash).To(Equal("jxl")) + Expect(art.Mime).To(Equal("application/octet-stream")) + Expect(art.Width).To(BeZero()) + Expect(art.Height).To(BeZero()) + Expect(art.BlurHash).To(BeEmpty()) + Expect(art.ThumbHash).To(BeEmpty()) + Expect(art.DominantColor).To(BeEmpty()) + }) + + It("corrupt image of a known format: still fails", func() { + data := pngHeaderWithDims(100, 100) // header declares a decodable size, body is missing + _, err := decodeArtwork(ctx, "truncated", data) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("decode image")) + }) + + // Without this a metadata-less row would be reused forever, so a decoder added later + // could never upgrade it. + It("metadata-less row: re-decodes on reuse instead of skipping", func() { + libRoot := GinkgoT().TempDir() + imgBytes, err := os.ReadFile(filepath.Join(repoRoot, "tests/fixtures/artist/an-album/cover.jpg")) + Expect(err).ToNot(HaveOccurred()) + Expect(os.MkdirAll(filepath.Join(libRoot, "album"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "album", "cover.jpg"), imgBytes, 0600)).To(Succeed()) + libRepo.SetData(model.Libraries{{ID: 0, Path: testFileLibPath(libRoot)}}) + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alM", Name: "Album", FolderIDs: []string{"f1"}}}) + folderRepo.result = []model.Folder{{Path: "album", ImageFiles: []string{"cover.jpg"}}} + + hash, err := hashImage(bytes.NewReader(imgBytes)) + Expect(err).ToNot(HaveOccurred()) + Expect(artRepo.PutImage(&model.Artwork{Hash: hash, Mime: "application/octet-stream"})).To(Succeed()) + + out, _ := proc.acquire(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alM"}) + Expect(out).To(Equal(outcomeFound)) + + upgraded, err := artRepo.GetImage(hash) + Expect(err).ToNot(HaveOccurred()) + Expect(upgraded.Width).To(BeNumerically(">", 0)) + Expect(upgraded.BlurHash).ToNot(BeEmpty()) + }) + It("store write failure: fails without writing state", func() { ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{ {ID: "al7", Name: "Album", EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", FolderIDs: []string{"f1"}}, diff --git a/resources/mime_types.yaml b/resources/mime_types.yaml index f67b26af4..83abf2e5c 100644 --- a/resources/mime_types.yaml +++ b/resources/mime_types.yaml @@ -37,6 +37,9 @@ types: .webp: image/webp .png: image/png .bmp: image/bmp + .jxl: image/jxl + .heic: image/heic + .heif: image/heif # List of audio formats that are considered lossless lossless: