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., ABCD) 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