navidrome/core/artwork/sources.go
Deluan Quintão 8b4125267e
fix(artwork): only use image files as local artwork sources (#6180)
* fix(playlists): limit local cover paths to images in owner's libraries

A local #EXTALBUMARTURL path (absolute or file://) was only checked against the union of all
libraries. The artwork resolver then opened it with no further check and served the bytes as the
playlist cover, undecoded. Any user who can upload an M3U could read any file under any library
root, including libraries they were not granted, through getCoverArt (GHSA-vwq6-xrw5-phpg).

resolveImageURL now requires an image extension, and for uploaded playlists (no folder) the
library holding the cover must pass the owner's HasLibraryAccess. Scanner and CLI imports keep
the all-libraries check, since those files are admin-controlled.

resolveLocalFile, used by every file-backed artwork source, now ignores paths without an image
extension, which covers playlists stored before this fix that were not resolved yet. openOriginal
refuses a stored file-backed row whose path is not an image, so the existing dangling path
re-resolves it and the playlist falls back to the generated grid. No migration is needed.

* fix(artwork): skip non-image files matched by folder cover patterns

Album and disc folder sources opened any file in the folder's image list that matched a
cover pattern, without checking its extension. openOriginal now refuses to serve file-backed
rows whose path is not an image, so a stored row like that would be refused, re-resolved to
the same file, and refused again on every view. The list comes from the scanner, which only
records image files, but a database scanned where the OS mime table knows more image types
than the serving process could still reach this.

Both fromExternalFile variants now skip matches that are not image files, so the album falls
back to its next source instead. Also correct the parser comment: a playlist without a folder
can come from an API upload or from a CLI import of a file outside all libraries.

* fix(artwork): check stored source type before using the resize cache

The image-extension check for file-backed rows ran inside openOriginal, which the resize
cache skips on a hit. Before the fix, a resized request for a playlist pointing at a non-image
file cached the raw bytes, because a failed resize falls back to the original data. After the
upgrade the same request still hit that entry and returned the file.

serveHash now refuses a file-backed row whose path is not an image before calling serveSource,
so both full-size and resized requests go through dangling and re-resolve the item. The stale
cache entry is keyed by the old hash and is no longer reachable once the row changes.

* test: register mime_types.yaml in test binaries

Artwork resolution now skips candidates that are not image files, and model.IsImageFile answers
from the process mime table. The server registers the extra image types from
resources/mime_types.yaml through a conf hook, but a test binary only does that if it links
conf/mime, so the artwork e2e suite fell back to the host table: .jxl resolves on macOS and
Linux and does not on Windows, where the #5950 cover spec then found no source.

tests.Init now imports conf/mime for its side effect, so every suite that loads the test config
sees the same image types as the server.

* fix(artwork): drop the image-file guard from the disc art reader

The guard was added to both fromExternalFile variants, but disc artwork keeps no state row and
is never queued, so it cannot hit the refuse-and-re-resolve loop the guard exists to prevent.
The only case where it can fire is a real image whose extension this process's mime table does
not know, and there it drops a disc cover that used to work. The album variant keeps the guard,
since those resolutions are stored and re-served.
2026-09-20 13:40:14 -04:00

171 lines
5.5 KiB
Go

package artwork
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"io/fs"
"net/http"
"net/url"
"path/filepath"
"regexp"
"strings"
"time"
"github.com/navidrome/navidrome/core/ffmpeg"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/httpclient"
"go.senan.xyz/taglib"
)
// errSourceUnreadable marks a candidate the resolver knows exists but could not read. Failing
// to open it is not evidence the entity has no artwork, so callers must not settle on absent.
var errSourceUnreadable = errors.New("artwork source unreadable")
type sourceFunc func() (r io.ReadCloser, path string, err error)
func fromExternalFile(ctx context.Context, libFS fs.FS, files []string, pattern string) sourceFunc {
return func() (io.ReadCloser, string, error) {
var openErr error
for _, file := range files {
_, name := filepath.Split(file)
match, err := filepath.Match(pattern, strings.ToLower(name))
if err != nil {
log.Warn(ctx, "Artwork: Error matching cover art file to pattern", "pattern", pattern, "file", file)
continue
}
if !match || !model.IsImageFile(name) {
continue
}
f, err := libFS.Open(file)
if err != nil {
log.Warn(ctx, "Artwork: Could not open cover art file", "file", file, err)
openErr = fmt.Errorf("%w: %s: %w", errSourceUnreadable, file, err)
continue
}
return f, file, nil
}
if openErr != nil {
return nil, "", openErr
}
return nil, "", fmt.Errorf("pattern '%s' not matched by files %v", pattern, files)
}
}
// These regexes are used to match the picture type in the file, in the order they are listed.
var picTypeRegexes = []*regexp.Regexp{
regexp.MustCompile(`(?i).*cover.*front.*|.*front.*cover.*`),
regexp.MustCompile(`(?i).*front.*`),
regexp.MustCompile(`(?i).*cover.*`),
}
func fromTag(ctx context.Context, libFS fs.FS, relPath string) sourceFunc {
return func() (io.ReadCloser, string, error) {
if relPath == "" {
return nil, "", nil
}
f, err := libFS.Open(relPath)
if err != nil {
return nil, "", fmt.Errorf("%w: %s: %w", errSourceUnreadable, relPath, err)
}
rs, ok := f.(io.ReadSeeker)
if !ok {
f.Close()
return nil, "", fmt.Errorf("FS file %s is not seekable; cannot read tags", relPath)
}
tf, err := taglib.OpenStream(rs,
taglib.WithReadStyle(taglib.ReadStyleFast),
taglib.WithFilename(relPath),
)
if err != nil {
f.Close()
return nil, "", fmt.Errorf("%w: %s: %w", errSourceUnreadable, relPath, err)
}
// Close in LIFO order: tf first (it holds rs internally), then f.
defer f.Close()
defer tf.Close()
images := tf.Properties().Images
if len(images) == 0 {
return nil, "", fmt.Errorf("no embedded image found in %s", relPath)
}
imageIndex := findBestImageIndex(ctx, images, relPath)
data, err := tf.Image(imageIndex)
if err != nil || len(data) == 0 {
return nil, "", fmt.Errorf("could not load embedded image from %s", relPath)
}
return io.NopCloser(bytes.NewReader(data)), relPath, nil
}
}
func findBestImageIndex(ctx context.Context, images []taglib.ImageDesc, path string) int {
for _, regex := range picTypeRegexes {
for i, img := range images {
if regex.MatchString(img.Type) {
log.Trace(ctx, "Artwork: Found embedded image", "type", img.Type, "path", path)
return i
}
}
}
log.Trace(ctx, "Artwork: Could not find a front image. Getting the first one", "type", images[0].Type, "path", path)
return 0
}
// fromFFmpegTag is intentionally absolute-path-based. ffmpeg is a subprocess
// and cannot read from arbitrary fs.FS implementations; piping via stdin is a
// non-trivial refactor with stream/seek implications.
//
// TODO(artwork-musicfs): when the storage backing the library is not local
// (e.g. a future S3 backend, or FakeFS in tests), short-circuit this source
// func to return (nil, "", nil) so callers fall through cleanly.
func fromFFmpegTag(ctx context.Context, ffmpeg ffmpeg.FFmpeg, path string) sourceFunc {
return func() (io.ReadCloser, string, error) {
if path == "" {
return nil, "", nil
}
r, err := ffmpeg.ExtractImage(ctx, path)
if err != nil {
return nil, "", err
}
// Validate that the stream actually contains image data by reading the first byte.
// ffmpeg.ExtractImage returns a pipe reader that may fail asynchronously if the
// file has no video/image stream (e.g., an MP3 without embedded art).
buf := make([]byte, 1)
n, err := r.Read(buf)
if n == 0 || err != nil {
r.Close()
return nil, "", fmt.Errorf("ffmpeg produced no image data for %s: %w", path, err)
}
return readCloser{Reader: io.MultiReader(bytes.NewReader(buf[:n]), r), Closer: r}, path, nil
}
}
// readCloser combines a Reader and a Closer into an io.ReadCloser.
type readCloser struct {
io.Reader
io.Closer
}
func fromURL(ctx context.Context, imageUrl *url.URL) (io.ReadCloser, string, error) {
hc := httpclient.New(5 * time.Second)
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageUrl.String(), nil)
resp, err := hc.Do(req) //nolint:gosec
if err != nil {
return nil, "", err
}
// An agent-advertised URL that 404s is a definitive miss, not a fault: settle absent
// instead of retrying forever and tripping the artwork breaker.
if resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusGone {
resp.Body.Close()
return nil, "", model.ErrNotFound
}
if resp.StatusCode != http.StatusOK {
resp.Body.Close()
return nil, "", fmt.Errorf("error retrieving artwork from %s: %s", imageUrl, resp.Status)
}
return resp.Body, imageUrl.String(), nil
}