feat(playlist): support #EXTALBUMARTURL directive and sidecar images (#5131)

* feat(playlist): add migration for playlist image field rename and external URL * refactor(playlist): rename ImageFile to UploadedImage and ArtworkPath to UploadedImagePath Rename playlist model fields and methods for clarity in preparation for adding external image URL and sidecar image support. Add the new ExternalImageURL field to the Playlist model. * feat(playlist): parse #EXTALBUMARTURL directive in M3U imports * feat(playlist): always sync ExternalImageURL on re-scan, preserve UploadedImage * feat(artwork): add sidecar image discovery and cache invalidation for playlists Add playlist sidecar image support to the artwork reader fallback chain. A sidecar image (e.g., MyPlaylist.jpg next to MyPlaylist.m3u) is discovered via case-insensitive base name matching using model.IsImageFile(). Cache invalidation uses max(playlist.UpdatedAt, imageFile.ModTime()) to bust stale artwork when sidecar or ExternalImageURL local files change. * feat(artwork): add external image URL source to playlist artwork reader Add fromPlaylistExternalImage source function that resolves playlist cover art from ExternalImageURL, supporting both HTTP(S) URLs (via the existing fromURL helper) and local file paths (via os.Open). Insert it in the Reader() fallback chain between sidecar and tiled cover. * refactor(artwork): simplify playlist artwork source functions Extract shared fromLocalFile helper, use url.Parse for scheme check, and collapse sidecar directory scan conditions. * test(artwork): remove redundant fromPlaylistSidecar tests These tests duplicated scenarios already covered by findPlaylistSidecarPath tests combined with fromLocalFile (tested via fromPlaylistExternalImage). After refactoring fromPlaylistSidecar to a one-liner composing those two functions, the wrapper tests add no value. * fix(playlist): address security review comments from PR #5131: - Use url.PathUnescape instead of url.QueryUnescape for file:// URLs so that '+' in filenames is preserved (not decoded as space). - Validate all local image paths (file://, absolute, relative) against known library boundaries via libraryMatcher, rejecting paths outside any configured library. - Harden #EXTALBUMARTURL against path traversal and SSRF by adding EnableM3UExternalAlbumArt config flag (default false, also disabled by EnableExternalServices=false) to gate HTTP(S) URL storage at parse time and fetching at read time (defense in depth). - Log a warning when os.ReadDir fails in findPlaylistSidecarPath for diagnosability. - Extract resolveLocalPath helper to simplify resolveImageURL. Signed-off-by: Deluan <deluan@navidrome.org> * feat(playlist): implement human-friendly filename generation for uploaded playlist cover images Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-02 11:39:59 -05:00
parent c4fd8e3125
commit acd69f6a4f
15 changed files with 647 additions and 43 deletions
@@ -235,6 +235,113 @@ var _ = Describe("Artwork", func() {
 			})
 		})
 	})
+	Describe("playlistArtworkReader", func() {
+		Describe("findPlaylistSidecarPath", func() {
+			It("discovers sidecar image next to playlist file", func() {
+				tmpDir := GinkgoT().TempDir()
+				plsPath := filepath.Join(tmpDir, "MyPlaylist.m3u")
+				imgPath := filepath.Join(tmpDir, "MyPlaylist.jpg")
+				Expect(os.WriteFile(plsPath, []byte("#EXTM3U\n"), 0600)).To(Succeed())
+				Expect(os.WriteFile(imgPath, []byte("fake image"), 0600)).To(Succeed())
+
+				result := findPlaylistSidecarPath(GinkgoT().Context(), plsPath)
+				Expect(result).To(Equal(imgPath))
+			})
+
+			It("returns empty string when no sidecar image exists", func() {
+				tmpDir := GinkgoT().TempDir()
+				plsPath := filepath.Join(tmpDir, "MyPlaylist.m3u")
+				Expect(os.WriteFile(plsPath, []byte("#EXTM3U\n"), 0600)).To(Succeed())
+
+				result := findPlaylistSidecarPath(GinkgoT().Context(), plsPath)
+				Expect(result).To(BeEmpty())
+			})
+
+			It("returns empty string when playlist has no path", func() {
+				result := findPlaylistSidecarPath(GinkgoT().Context(), "")
+				Expect(result).To(BeEmpty())
+			})
+
+			It("finds sidecar with different case base name", func() {
+				tmpDir := GinkgoT().TempDir()
+				plsPath := filepath.Join(tmpDir, "myplaylist.m3u")
+				imgPath := filepath.Join(tmpDir, "MyPlaylist.jpg")
+				Expect(os.WriteFile(plsPath, []byte("#EXTM3U\n"), 0600)).To(Succeed())
+				Expect(os.WriteFile(imgPath, []byte("fake image"), 0600)).To(Succeed())
+
+				result := findPlaylistSidecarPath(GinkgoT().Context(), plsPath)
+				Expect(result).To(Equal(imgPath))
+			})
+		})
+
+		Describe("fromPlaylistExternalImage", func() {
+			It("opens local path from ExternalImageURL", func() {
+				tmpDir := GinkgoT().TempDir()
+				imgPath := filepath.Join(tmpDir, "cover.jpg")
+				Expect(os.WriteFile(imgPath, []byte("external image data"), 0600)).To(Succeed())
+
+				reader := &playlistArtworkReader{
+					pl: model.Playlist{ExternalImageURL: imgPath},
+				}
+				r, path, err := reader.fromPlaylistExternalImage(ctx)()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(r).ToNot(BeNil())
+				Expect(path).To(Equal(imgPath))
+				data, _ := io.ReadAll(r)
+				Expect(string(data)).To(Equal("external image data"))
+				r.Close()
+			})
+
+			It("returns nil when ExternalImageURL is empty", func() {
+				reader := &playlistArtworkReader{
+					pl: model.Playlist{ExternalImageURL: ""},
+				}
+				r, path, err := reader.fromPlaylistExternalImage(ctx)()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(r).To(BeNil())
+				Expect(path).To(BeEmpty())
+			})
+
+			It("returns error when local file does not exist", func() {
+				reader := &playlistArtworkReader{
+					pl: model.Playlist{ExternalImageURL: "/non/existent/path/cover.jpg"},
+				}
+				r, _, err := reader.fromPlaylistExternalImage(ctx)()
+				Expect(err).To(HaveOccurred())
+				Expect(r).To(BeNil())
+			})
+
+			It("skips HTTP URL when EnableM3UExternalAlbumArt is false", func() {
+				conf.Server.EnableM3UExternalAlbumArt = false
+
+				reader := &playlistArtworkReader{
+					pl: model.Playlist{ExternalImageURL: "https://example.com/cover.jpg"},
+				}
+				r, path, err := reader.fromPlaylistExternalImage(ctx)()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(r).To(BeNil())
+				Expect(path).To(BeEmpty())
+			})
+
+			It("still opens local path when EnableM3UExternalAlbumArt is false", func() {
+				conf.Server.EnableM3UExternalAlbumArt = false
+
+				tmpDir := GinkgoT().TempDir()
+				imgPath := filepath.Join(tmpDir, "cover.jpg")
+				Expect(os.WriteFile(imgPath, []byte("local image"), 0600)).To(Succeed())
+
+				reader := &playlistArtworkReader{
+					pl: model.Playlist{ExternalImageURL: imgPath},
+				}
+				r, path, err := reader.fromPlaylistExternalImage(ctx)()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(r).ToNot(BeNil())
+				Expect(path).To(Equal(imgPath))
+				r.Close()
+			})
+		})
+	})
+
 	Describe("resizedArtworkReader", func() {
 		BeforeEach(func() {
 			folderRepo.result = []model.Folder{{
@@ -8,10 +8,14 @@ import (
 	"image/draw"
 	"image/png"
 	"io"
+	"net/url"
 	"os"
+	"path/filepath"
+	"strings"
 	"time"

 	"github.com/disintegration/imaging"
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/utils/slice"
@@ -36,6 +40,24 @@ func newPlaylistArtworkReader(ctx context.Context, artwork *artwork, artID model
 	}
 	a.cacheKey.artID = artID
 	a.cacheKey.lastUpdate = pl.UpdatedAt
+
+	// Check sidecar and ExternalImageURL local file ModTimes for cache invalidation.
+	// If either is newer than the playlist's UpdatedAt, use that instead so the
+	// cache is busted when a user replaces a sidecar image or local file reference.
+	for _, path := range []string{
+		findPlaylistSidecarPath(ctx, pl.Path),
+		pl.ExternalImageURL,
+	} {
+		if path == "" || strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") {
+			continue
+		}
+		if info, err := os.Stat(path); err == nil {
+			if info.ModTime().After(a.cacheKey.lastUpdate) {
+				a.cacheKey.lastUpdate = info.ModTime()
+			}
+		}
+	}
+
 	return a, nil
 }

@@ -45,26 +67,82 @@ func (a *playlistArtworkReader) LastUpdated() time.Time {

 func (a *playlistArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) {
 	return selectImageReader(ctx, a.artID,
-		a.fromPlaylistImage(),
+		a.fromPlaylistUploadedImage(),
+		a.fromPlaylistSidecar(ctx),
+		a.fromPlaylistExternalImage(ctx),
 		a.fromGeneratedTiledCover(ctx),
 		fromAlbumPlaceholder(),
 	)
 }

-func (a *playlistArtworkReader) fromPlaylistImage() sourceFunc {
+func (a *playlistArtworkReader) fromPlaylistUploadedImage() sourceFunc {
+	return fromLocalFile(a.pl.UploadedImagePath())
+}
+
+func (a *playlistArtworkReader) fromPlaylistSidecar(ctx context.Context) sourceFunc {
+	return fromLocalFile(findPlaylistSidecarPath(ctx, a.pl.Path))
+}
+
+func (a *playlistArtworkReader) fromPlaylistExternalImage(ctx context.Context) sourceFunc {
 	return func() (io.ReadCloser, string, error) {
-		absPath := a.pl.ArtworkPath()
-		if absPath == "" {
+		imgURL := a.pl.ExternalImageURL
+		if imgURL == "" {
 			return nil, "", nil
 		}
-		f, err := os.Open(absPath)
+		parsed, err := url.Parse(imgURL)
 		if err != nil {
 			return nil, "", err
 		}
-		return f, absPath, nil
+		if parsed.Scheme == "http" || parsed.Scheme == "https" {
+			if !conf.Server.EnableM3UExternalAlbumArt {
+				return nil, "", nil
+			}
+			return fromURL(ctx, parsed)
+		}
+		return fromLocalFile(imgURL)()
 	}
 }

+// fromLocalFile returns a sourceFunc that opens the given local path.
+// Returns (nil, "", nil) if path is empty — signalling "not found, try next source".
+func fromLocalFile(path string) sourceFunc {
+	return func() (io.ReadCloser, string, error) {
+		if path == "" {
+			return nil, "", nil
+		}
+		f, err := os.Open(path)
+		if err != nil {
+			return nil, "", err
+		}
+		return f, path, nil
+	}
+}
+
+// findPlaylistSidecarPath scans the directory of the playlist file for a sidecar
+// image file with the same base name (case-insensitive). Returns empty string if
+// no matching image is found or if plsPath is empty.
+func findPlaylistSidecarPath(ctx context.Context, plsPath string) string {
+	if plsPath == "" {
+		return ""
+	}
+	dir := filepath.Dir(plsPath)
+	base := strings.TrimSuffix(filepath.Base(plsPath), filepath.Ext(plsPath))
+
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		log.Warn(ctx, "Could not read directory for playlist sidecar", "dir", dir, err)
+		return ""
+	}
+	for _, entry := range entries {
+		name := entry.Name()
+		nameBase := strings.TrimSuffix(name, filepath.Ext(name))
+		if !entry.IsDir() && strings.EqualFold(nameBase, base) && model.IsImageFile(name) {
+			return filepath.Join(dir, name)
+		}
+	}
+	return ""
+}
+
 func (a *playlistArtworkReader) fromGeneratedTiledCover(ctx context.Context) sourceFunc {
 	return func() (io.ReadCloser, string, error) {
 		tiles, err := a.loadTiles(ctx)