From acd69f6a4fbeb68a0bf86ae65a393531bb4d7d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Mon, 2 Mar 2026 11:39:59 -0500 Subject: [PATCH] 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 * feat(playlist): implement human-friendly filename generation for uploaded playlist cover images Signed-off-by: Deluan --------- Signed-off-by: Deluan --- conf/configuration.go | 3 + core/artwork/artwork_internal_test.go | 107 ++++++++ core/artwork/reader_playlist.go | 90 ++++++- core/playlists/import.go | 1 + core/playlists/import_test.go | 229 +++++++++++++++++- core/playlists/parse_m3u.go | 57 ++++- core/playlists/playlists.go | 14 +- core/playlists/playlists_test.go | 18 +- ...0302021413_rename_playlist_image_fields.go | 30 +++ model/playlist.go | 52 ++-- model/playlist_test.go | 22 ++ tests/fixtures/playlists/pls-with-art-url.m3u | 5 + ui/src/playlist/PlaylistDetails.jsx | 2 +- utils/files.go | 17 ++ utils/files_test.go | 43 ++++ 15 files changed, 647 insertions(+), 43 deletions(-) create mode 100644 db/migrations/20260302021413_rename_playlist_image_fields.go create mode 100644 tests/fixtures/playlists/pls-with-art-url.m3u diff --git a/conf/configuration.go b/conf/configuration.go index 3e3b42355..0d32815ee 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -46,6 +46,7 @@ type configOptions struct { EnableTranscodingCancellation bool EnableDownloads bool EnableExternalServices bool + EnableM3UExternalAlbumArt bool EnableInsightsCollector bool EnableMediaFileCoverArt bool TranscodingCacheSize string @@ -474,6 +475,7 @@ func parseIniFileConfiguration() { func disableExternalServices() { log.Info("All external integrations are DISABLED!") Server.EnableInsightsCollector = false + Server.EnableM3UExternalAlbumArt = false Server.LastFM.Enabled = false Server.Spotify.ID = "" Server.Deezer.Enabled = false @@ -638,6 +640,7 @@ func setViperDefaults() { viper.SetDefault("smartPlaylistRefreshDelay", 5*time.Second) viper.SetDefault("enabledownloads", true) viper.SetDefault("enableexternalservices", true) + viper.SetDefault("enablem3uexternalalbumart", false) viper.SetDefault("enablemediafilecoverart", true) viper.SetDefault("autotranscodedownload", false) viper.SetDefault("defaultdownsamplingformat", consts.DefaultDownsamplingFormat) diff --git a/core/artwork/artwork_internal_test.go b/core/artwork/artwork_internal_test.go index c18caf737..e2ea7adb0 100644 --- a/core/artwork/artwork_internal_test.go +++ b/core/artwork/artwork_internal_test.go @@ -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{{ diff --git a/core/artwork/reader_playlist.go b/core/artwork/reader_playlist.go index 09bfe221b..91d47b0b0 100644 --- a/core/artwork/reader_playlist.go +++ b/core/artwork/reader_playlist.go @@ -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) diff --git a/core/playlists/import.go b/core/playlists/import.go index 40e230527..4462554c7 100644 --- a/core/playlists/import.go +++ b/core/playlists/import.go @@ -106,6 +106,7 @@ func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist) newPls.Comment = pls.Comment newPls.OwnerID = pls.OwnerID newPls.Public = pls.Public + newPls.UploadedImage = pls.UploadedImage // Preserve manual upload newPls.EvaluatedAt = &time.Time{} } else { log.Info(ctx, "Adding synced playlist", "playlist", newPls.Name, "path", newPls.Path, "owner", owner.UserName) diff --git a/core/playlists/import_test.go b/core/playlists/import_test.go index a42c3f3eb..5312df95d 100644 --- a/core/playlists/import_test.go +++ b/core/playlists/import_test.go @@ -2,7 +2,9 @@ package playlists_test import ( "context" + "fmt" "os" + "path/filepath" "strconv" "strings" "time" @@ -39,6 +41,7 @@ var _ = Describe("Playlists - Import", func() { Describe("ImportFile", func() { var folder *model.Folder BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) ps = playlists.NewPlaylists(ds) ds.MockedMediaFile = &mockedMediaFileRepo{} libPath, _ := os.Getwd() @@ -93,6 +96,213 @@ var _ = Describe("Playlists - Import", func() { Expect(pls.Tracks).To(HaveLen(1)) Expect(pls.Tracks[0].Path).To(Equal("tests/fixtures/playlists/test.mp3")) }) + + It("parses #EXTALBUMARTURL with HTTP URL", func() { + conf.Server.EnableM3UExternalAlbumArt = true + + pls, err := ps.ImportFile(ctx, folder, "pls-with-art-url.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(Equal("https://example.com/cover.jpg")) + Expect(pls.Tracks).To(HaveLen(2)) + }) + + It("parses #EXTALBUMARTURL with absolute local path", func() { + tmpDir := GinkgoT().TempDir() + imgPath := filepath.Join(tmpDir, "cover.jpg") + Expect(os.WriteFile(imgPath, []byte("fake image"), 0600)).To(Succeed()) + + m3u := fmt.Sprintf("#EXTALBUMARTURL:%s\ntest.mp3\ntest.ogg\n", imgPath) + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3", "test.ogg"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(Equal(imgPath)) + }) + + It("parses #EXTALBUMARTURL with relative local path", func() { + tmpDir := GinkgoT().TempDir() + Expect(os.WriteFile(filepath.Join(tmpDir, "cover.jpg"), []byte("fake image"), 0600)).To(Succeed()) + + m3u := "#EXTALBUMARTURL:cover.jpg\ntest.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(Equal(filepath.Join(tmpDir, "cover.jpg"))) + }) + + It("parses #EXTALBUMARTURL with file:// URL", func() { + tmpDir := GinkgoT().TempDir() + imgPath := filepath.Join(tmpDir, "my cover.jpg") + Expect(os.WriteFile(imgPath, []byte("fake image"), 0600)).To(Succeed()) + + m3u := fmt.Sprintf("#EXTALBUMARTURL:file://%s\ntest.mp3\n", strings.ReplaceAll(imgPath, " ", "%20")) + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(Equal(imgPath)) + }) + + It("preserves + in file:// URLs (PathUnescape, not QueryUnescape)", func() { + tmpDir := GinkgoT().TempDir() + imgPath := filepath.Join(tmpDir, "A+B.jpg") + Expect(os.WriteFile(imgPath, []byte("fake image"), 0600)).To(Succeed()) + + m3u := fmt.Sprintf("#EXTALBUMARTURL:file://%s\ntest.mp3\n", imgPath) + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(Equal(imgPath)) + }) + + It("rejects #EXTALBUMARTURL with absolute path outside library boundaries", func() { + tmpDir := GinkgoT().TempDir() + + m3u := "#EXTALBUMARTURL:/etc/passwd\ntest.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(BeEmpty()) + }) + + It("rejects #EXTALBUMARTURL with file:// URL outside library boundaries", func() { + tmpDir := GinkgoT().TempDir() + + m3u := "#EXTALBUMARTURL:file:///etc/passwd\ntest.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(BeEmpty()) + }) + + It("rejects #EXTALBUMARTURL with relative path escaping library", func() { + tmpDir := GinkgoT().TempDir() + + m3u := "#EXTALBUMARTURL:../../etc/passwd\ntest.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(BeEmpty()) + }) + + It("ignores HTTP #EXTALBUMARTURL when EnableM3UExternalAlbumArt is false", func() { + conf.Server.EnableM3UExternalAlbumArt = false + + tmpDir := GinkgoT().TempDir() + m3u := "#EXTALBUMARTURL:https://example.com/cover.jpg\ntest.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(BeEmpty()) + }) + + It("updates ExternalImageURL on re-scan even when UploadedImage is set", func() { + conf.Server.EnableM3UExternalAlbumArt = true + + tmpDir := GinkgoT().TempDir() + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + m3u := "#EXTALBUMARTURL:https://example.com/new-cover.jpg\ntest.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + existingPls := &model.Playlist{ + ID: "existing-id", + Name: "Existing Playlist", + Path: plsFile, + Sync: true, + UploadedImage: "existing-id.jpg", + ExternalImageURL: "https://example.com/old-cover.jpg", + } + mockPlsRepo.PathMap = map[string]*model.Playlist{plsFile: existingPls} + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.UploadedImage).To(Equal("existing-id.jpg")) + Expect(pls.ExternalImageURL).To(Equal("https://example.com/new-cover.jpg")) + }) + + It("clears ExternalImageURL on re-scan when directive is removed", func() { + tmpDir := GinkgoT().TempDir() + mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) + ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} + ps = playlists.NewPlaylists(ds) + + m3u := "test.mp3\n" + plsFile := filepath.Join(tmpDir, "test.m3u") + Expect(os.WriteFile(plsFile, []byte(m3u), 0600)).To(Succeed()) + + existingPls := &model.Playlist{ + ID: "existing-id", + Name: "Existing Playlist", + Path: plsFile, + Sync: true, + ExternalImageURL: "https://example.com/old-cover.jpg", + } + mockPlsRepo.PathMap = map[string]*model.Playlist{plsFile: existingPls} + + plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} + pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u") + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(BeEmpty()) + }) }) Describe("NSP", func() { @@ -125,7 +335,6 @@ var _ = Describe("Playlists - Import", func() { Expect(pls.Public).To(BeFalse()) }) It("uses server default when public field is absent", func() { - DeferCleanup(configtest.SetupConfig()) conf.Server.DefaultPlaylistPublicVisibility = true pls, err := ps.ImportFile(ctx, folder, "recently_played.nsp") @@ -495,6 +704,24 @@ var _ = Describe("Playlists - Import", func() { Expect(pls.Tracks[0].Path).To(Equal("abc/tEsT1.Mp3")) }) + It("parses #EXTALBUMARTURL with HTTP URL via ImportM3U", func() { + conf.Server.EnableM3UExternalAlbumArt = true + + repo.data = []string{"tests/test.mp3"} + m3u := "#EXTALBUMARTURL:https://example.com/cover.jpg\n/music/tests/test.mp3\n" + pls, err := ps.ImportM3U(ctx, strings.NewReader(m3u)) + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(Equal("https://example.com/cover.jpg")) + }) + + It("ignores relative #EXTALBUMARTURL when imported via API (no folder context)", func() { + repo.data = []string{"tests/test.mp3"} + m3u := "#EXTALBUMARTURL:cover.jpg\n/music/tests/test.mp3\n" + pls, err := ps.ImportM3U(ctx, strings.NewReader(m3u)) + Expect(err).ToNot(HaveOccurred()) + Expect(pls.ExternalImageURL).To(BeEmpty()) + }) + // Fullwidth characters (e.g., ＡＢＣＤ) are not handled by SQLite's NOCASE collation, // so we need exact matching for non-ASCII characters. It("matches fullwidth characters exactly (SQLite NOCASE limitation)", func() { diff --git a/core/playlists/parse_m3u.go b/core/playlists/parse_m3u.go index 4e79d15c6..97ed7df6f 100644 --- a/core/playlists/parse_m3u.go +++ b/core/playlists/parse_m3u.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/slice" @@ -34,13 +35,17 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m pls.Name = line[len("#PLAYLIST:"):] continue } + if after, ok := strings.CutPrefix(line, "#EXTALBUMARTURL:"); ok { + pls.ExternalImageURL = resolveImageURL(after, folder, resolver.matcher) + continue + } // Skip empty lines and extended info if line == "" || strings.HasPrefix(line, "#") { continue } if after, ok := strings.CutPrefix(line, "file://"); ok { line = after - line, _ = url.QueryUnescape(line) + line, _ = url.PathUnescape(line) } if !model.IsAudioFile(line) { continue @@ -267,3 +272,53 @@ func (r *pathResolver) resolvePaths(ctx context.Context, folder *model.Folder, l return results, nil } + +// resolveImageURL resolves an #EXTALBUMARTURL value to a storable string. +// HTTP(S) URLs are stored as-is (gated by EnableM3UExternalAlbumArt). +// Local paths (file://, absolute, or relative) are resolved to an absolute path +// and validated against known library boundaries via matcher. +func resolveImageURL(value string, folder *model.Folder, matcher *libraryMatcher) string { + value = strings.TrimSpace(value) + if value == "" { + return "" + } + + // HTTP(S) URLs — store as-is, but only if external album art is enabled + if strings.HasPrefix(value, "http://") || strings.HasPrefix(value, "https://") { + if !conf.Server.EnableM3UExternalAlbumArt { + return "" + } + return value + } + + // Resolve to local absolute path + localPath, ok := resolveLocalPath(value, folder) + if !ok { + return "" + } + + // Validate path is within a known library + if libID, _ := matcher.findLibraryForPath(localPath); libID == 0 { + return "" + } + return localPath +} + +// resolveLocalPath converts a file://, absolute, or relative path to a clean absolute path. +// Returns ("", false) if the path cannot be resolved. +func resolveLocalPath(value string, folder *model.Folder) (string, bool) { + if after, ok := strings.CutPrefix(value, "file://"); ok { + decoded, err := url.PathUnescape(after) + if err != nil { + return "", false + } + return filepath.Clean(decoded), true + } + if filepath.IsAbs(value) { + return filepath.Clean(value), true + } + if folder == nil { + return "", false + } + return filepath.Clean(filepath.Join(folder.AbsolutePath(), value)), true +} diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index 7e881b730..0649b16a9 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -131,7 +131,7 @@ func (s *playlists) Delete(ctx context.Context, id string) error { } // Clean up custom cover image file if one exists - if path := pls.ArtworkPath(); path != "" { + if path := pls.UploadedImagePath(); path != "" { if err := os.Remove(path); err != nil && !os.IsNotExist(err) { log.Warn(ctx, "Failed to remove playlist image on delete", "path", path, err) } @@ -288,10 +288,10 @@ func (s *playlists) SetImage(ctx context.Context, playlistID string, reader io.R return err } - filename := playlistID + ext - oldPath := pls.ArtworkPath() - pls.ImageFile = filename - absPath := pls.ArtworkPath() + filename := pls.ImageFilename(ext) + oldPath := pls.UploadedImagePath() + pls.UploadedImage = filename + absPath := pls.UploadedImagePath() if err := os.MkdirAll(filepath.Dir(absPath), 0755); err != nil { return fmt.Errorf("creating playlist images directory: %w", err) @@ -324,12 +324,12 @@ func (s *playlists) RemoveImage(ctx context.Context, playlistID string) error { return err } - if path := pls.ArtworkPath(); path != "" { + if path := pls.UploadedImagePath(); path != "" { if err := os.Remove(path); err != nil && !os.IsNotExist(err) { log.Warn(ctx, "Failed to remove playlist image", "path", path, err) } } - pls.ImageFile = "" + pls.UploadedImage = "" return s.ds.Playlist(ctx).Put(pls) } diff --git a/core/playlists/playlists_test.go b/core/playlists/playlists_test.go index 4d42bbeb9..ec73c329a 100644 --- a/core/playlists/playlists_test.go +++ b/core/playlists/playlists_test.go @@ -315,14 +315,14 @@ var _ = Describe("Playlists", func() { ps = playlists.NewPlaylists(ds) }) - It("saves image file and updates ImageFile", func() { + It("saves image file and updates UploadedImage", func() { ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) reader := strings.NewReader("fake image data") err := ps.SetImage(ctx, "pls-1", reader, ".jpg") Expect(err).ToNot(HaveOccurred()) - Expect(mockPlsRepo.Last.ImageFile).To(Equal("pls-1.jpg")) - absPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1.jpg") + Expect(mockPlsRepo.Last.UploadedImage).To(Equal("pls-1_my_playlist.jpg")) + absPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1_my_playlist.jpg") data, err := os.ReadFile(absPath) Expect(err).ToNot(HaveOccurred()) Expect(string(data)).To(Equal("fake image data")) @@ -334,14 +334,14 @@ var _ = Describe("Playlists", func() { // Upload first image err := ps.SetImage(ctx, "pls-1", strings.NewReader("first"), ".png") Expect(err).ToNot(HaveOccurred()) - oldPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1.png") + oldPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1_my_playlist.png") Expect(oldPath).To(BeAnExistingFile()) // Upload replacement image err = ps.SetImage(ctx, "pls-1", strings.NewReader("second"), ".jpg") Expect(err).ToNot(HaveOccurred()) Expect(oldPath).ToNot(BeAnExistingFile()) - newPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1.jpg") + newPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1_my_playlist.jpg") Expect(newPath).To(BeAnExistingFile()) }) @@ -378,19 +378,19 @@ var _ = Describe("Playlists", func() { Expect(os.WriteFile(filepath.Join(imgDir, "pls-1.jpg"), []byte("img data"), 0600)).To(Succeed()) mockPlsRepo.Data = map[string]*model.Playlist{ - "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1", ImageFile: "pls-1.jpg"}, + "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1", UploadedImage: "pls-1.jpg"}, "pls-empty": {ID: "pls-empty", Name: "No Cover", OwnerID: "user-1"}, "pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"}, } ps = playlists.NewPlaylists(ds) }) - It("removes file and clears ImageFile", func() { + It("removes file and clears UploadedImage", func() { ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) err := ps.RemoveImage(ctx, "pls-1") Expect(err).ToNot(HaveOccurred()) - Expect(mockPlsRepo.Last.ImageFile).To(BeEmpty()) + Expect(mockPlsRepo.Last.UploadedImage).To(BeEmpty()) absPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1.jpg") Expect(absPath).ToNot(BeAnExistingFile()) }) @@ -399,7 +399,7 @@ var _ = Describe("Playlists", func() { ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) err := ps.RemoveImage(ctx, "pls-empty") Expect(err).ToNot(HaveOccurred()) - Expect(mockPlsRepo.Last.ImageFile).To(BeEmpty()) + Expect(mockPlsRepo.Last.UploadedImage).To(BeEmpty()) }) It("denies non-owner", func() { diff --git a/db/migrations/20260302021413_rename_playlist_image_fields.go b/db/migrations/20260302021413_rename_playlist_image_fields.go new file mode 100644 index 000000000..1e9754637 --- /dev/null +++ b/db/migrations/20260302021413_rename_playlist_image_fields.go @@ -0,0 +1,30 @@ +package migrations + +import ( + "context" + "database/sql" + + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upRenamePlaylistImageFields, downRenamePlaylistImageFields) +} + +func upRenamePlaylistImageFields(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, `ALTER TABLE playlist RENAME COLUMN image_file TO uploaded_image;`) + if err != nil { + return err + } + _, err = tx.ExecContext(ctx, `ALTER TABLE playlist ADD COLUMN external_image_url VARCHAR(255) DEFAULT '';`) + return err +} + +func downRenamePlaylistImageFields(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, `ALTER TABLE playlist DROP COLUMN external_image_url;`) + if err != nil { + return err + } + _, err = tx.ExecContext(ctx, `ALTER TABLE playlist RENAME COLUMN uploaded_image TO image_file;`) + return err +} diff --git a/model/playlist.go b/model/playlist.go index b6a52dcf2..5c9052eb7 100644 --- a/model/playlist.go +++ b/model/playlist.go @@ -9,24 +9,26 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model/criteria" + "github.com/navidrome/navidrome/utils" ) type Playlist struct { - ID string `structs:"id" json:"id"` - Name string `structs:"name" json:"name"` - Comment string `structs:"comment" json:"comment"` - Duration float32 `structs:"duration" json:"duration"` - Size int64 `structs:"size" json:"size"` - SongCount int `structs:"song_count" json:"songCount"` - OwnerName string `structs:"-" json:"ownerName"` - OwnerID string `structs:"owner_id" json:"ownerId"` - Public bool `structs:"public" json:"public"` - Tracks PlaylistTracks `structs:"-" json:"tracks,omitempty"` - Path string `structs:"path" json:"path"` - Sync bool `structs:"sync" json:"sync"` - ImageFile string `structs:"image_file" json:"imageFile"` - CreatedAt time.Time `structs:"created_at" json:"createdAt"` - UpdatedAt time.Time `structs:"updated_at" json:"updatedAt"` + ID string `structs:"id" json:"id"` + Name string `structs:"name" json:"name"` + Comment string `structs:"comment" json:"comment"` + Duration float32 `structs:"duration" json:"duration"` + Size int64 `structs:"size" json:"size"` + SongCount int `structs:"song_count" json:"songCount"` + OwnerName string `structs:"-" json:"ownerName"` + OwnerID string `structs:"owner_id" json:"ownerId"` + Public bool `structs:"public" json:"public"` + Tracks PlaylistTracks `structs:"-" json:"tracks,omitempty"` + Path string `structs:"path" json:"path"` + Sync bool `structs:"sync" json:"sync"` + UploadedImage string `structs:"uploaded_image" json:"uploadedImage"` + ExternalImageURL string `structs:"external_image_url" json:"externalImageUrl,omitempty"` + CreatedAt time.Time `structs:"created_at" json:"createdAt"` + UpdatedAt time.Time `structs:"updated_at" json:"updatedAt"` // SmartPlaylist attributes Rules *criteria.Criteria `structs:"rules" json:"rules"` @@ -106,15 +108,29 @@ func (pls *Playlist) AddMediaFiles(mfs MediaFiles) { pls.refreshStats() } +// ImageFilename returns a human-friendly filename for an uploaded playlist cover image. +// Format: _, falling back to if the name cleans to empty. +func (pls Playlist) ImageFilename(ext string) string { + clean := utils.CleanFileName(pls.Name) + if clean == "" { + return pls.ID + ext + } + return pls.ID + "_" + clean + ext +} + func (pls Playlist) CoverArtID() ArtworkID { return artworkIDFromPlaylist(pls) } -func (pls Playlist) ArtworkPath() string { - if pls.ImageFile == "" { +// UploadedImagePath returns the absolute filesystem path for a manually uploaded +// playlist cover image. Returns empty string if no image has been uploaded. +// This does NOT cover sidecar images or external URLs — those are resolved +// by the artwork reader's fallback chain. +func (pls Playlist) UploadedImagePath() string { + if pls.UploadedImage == "" { return "" } - return filepath.Join(conf.Server.DataFolder, consts.ArtworkFolder, "playlist", pls.ImageFile) + return filepath.Join(conf.Server.DataFolder, consts.ArtworkFolder, "playlist", pls.UploadedImage) } type Playlists []Playlist diff --git a/model/playlist_test.go b/model/playlist_test.go index a54cecd53..98dd4e978 100644 --- a/model/playlist_test.go +++ b/model/playlist_test.go @@ -7,6 +7,28 @@ import ( ) var _ = Describe("Playlist", func() { + Describe("ImageFilename", func() { + It("returns ID_cleanname.ext for a normal name", func() { + pls := model.Playlist{ID: "abc123", Name: "My Cool Playlist"} + Expect(pls.ImageFilename(".jpg")).To(Equal("abc123_my_cool_playlist.jpg")) + }) + + It("falls back to ID.ext when name cleans to empty", func() { + pls := model.Playlist{ID: "abc123", Name: "!!!"} + Expect(pls.ImageFilename(".png")).To(Equal("abc123.png")) + }) + + It("falls back to ID.ext for empty name", func() { + pls := model.Playlist{ID: "abc123", Name: ""} + Expect(pls.ImageFilename(".jpg")).To(Equal("abc123.jpg")) + }) + + It("handles names with special characters", func() { + pls := model.Playlist{ID: "x1", Name: "Rock & Roll! (2024)"} + Expect(pls.ImageFilename(".webp")).To(Equal("x1_rock__roll_2024.webp")) + }) + }) + Describe("ToM3U8()", func() { var pls model.Playlist BeforeEach(func() { diff --git a/tests/fixtures/playlists/pls-with-art-url.m3u b/tests/fixtures/playlists/pls-with-art-url.m3u new file mode 100644 index 000000000..9dbf180f8 --- /dev/null +++ b/tests/fixtures/playlists/pls-with-art-url.m3u @@ -0,0 +1,5 @@ +#EXTM3U +#PLAYLIST:Playlist With Art +#EXTALBUMARTURL:https://example.com/cover.jpg +test.mp3 +test.ogg diff --git a/ui/src/playlist/PlaylistDetails.jsx b/ui/src/playlist/PlaylistDetails.jsx index 4c242dd2a..b24446cb9 100644 --- a/ui/src/playlist/PlaylistDetails.jsx +++ b/ui/src/playlist/PlaylistDetails.jsx @@ -246,7 +246,7 @@ const PlaylistDetails = (props) => { - {record.imageFile && ( + {record.uploadedImage && ( diff --git a/utils/files.go b/utils/files.go index 9bdc262c5..2fce307ea 100644 --- a/utils/files.go +++ b/utils/files.go @@ -4,11 +4,14 @@ import ( "os" "path" "path/filepath" + "regexp" "strings" "github.com/navidrome/navidrome/model/id" ) +var cleanFileNameRe = regexp.MustCompile(`[^a-z0-9_-]`) + func TempFileName(prefix, suffix string) string { return filepath.Join(os.TempDir(), prefix+id.NewRandom()+suffix) } @@ -18,6 +21,20 @@ func BaseName(filePath string) string { return strings.TrimSuffix(p, path.Ext(p)) } +// CleanFileName produces a filesystem-safe, human-readable version of a name. +// It lowercases, replaces spaces with underscores, strips non-alphanumeric +// characters (except underscore and hyphen), and truncates to 50 characters. +func CleanFileName(name string) string { + s := strings.ToLower(strings.TrimSpace(name)) + s = strings.ReplaceAll(s, " ", "_") + s = cleanFileNameRe.ReplaceAllString(s, "") + if len(s) > 50 { + s = s[:50] + } + s = strings.TrimRight(s, "_-") + return s +} + // FileExists checks if a file or directory exists func FileExists(path string) bool { _, err := os.Stat(path) diff --git a/utils/files_test.go b/utils/files_test.go index dcb28aafb..72fc4f96f 100644 --- a/utils/files_test.go +++ b/utils/files_test.go @@ -99,6 +99,49 @@ var _ = Describe("BaseName", func() { }) }) +var _ = Describe("CleanFileName", func() { + It("lowercases and replaces spaces with underscores", func() { + Expect(utils.CleanFileName("My Cool Playlist")).To(Equal("my_cool_playlist")) + }) + + It("strips special characters", func() { + Expect(utils.CleanFileName("Rock & Roll! (2024)")).To(Equal("rock__roll_2024")) + }) + + It("handles unicode characters", func() { + Expect(utils.CleanFileName("Música Favorita")).To(Equal("msica_favorita")) + }) + + It("preserves hyphens", func() { + Expect(utils.CleanFileName("lo-fi beats")).To(Equal("lo-fi_beats")) + }) + + It("returns empty string for empty input", func() { + Expect(utils.CleanFileName("")).To(BeEmpty()) + }) + + It("returns empty string for whitespace-only input", func() { + Expect(utils.CleanFileName(" ")).To(BeEmpty()) + }) + + It("returns empty string when all characters are stripped", func() { + Expect(utils.CleanFileName("!!!@@@###")).To(BeEmpty()) + }) + + It("truncates to 50 characters", func() { + long := strings.Repeat("abcdefghij", 10) // 100 chars + result := utils.CleanFileName(long) + Expect(len(result)).To(Equal(50)) + }) + + It("trims trailing underscores and hyphens after truncation", func() { + // 49 a's + space + "b" = after clean: 49 a's + "_b" = 51 chars, truncated to 50 = 49 a's + "_" + name := strings.Repeat("a", 49) + " b" + result := utils.CleanFileName(name) + Expect(result).To(Equal(strings.Repeat("a", 49))) + }) +}) + var _ = Describe("FileExists", func() { var tempFile *os.File var tempDir string