Merge 5925e8dbd8b1f423a3e3e6114f489459d4e6c67a into 4f7dc105b0414cd202fc7e560d51b8abc27ad7ef

core/playlists.go

@@ -1,6 +1,7 @@
 package core
 
 import (
+	"cmp"
 	"context"
 	"encoding/json"
 	"errors"
@@ -10,6 +11,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"strings"
 	"time"
 
@@ -233,7 +235,33 @@ func normalizePathForComparison(path string) string {
 	return strings.ToLower(norm.NFC.String(path))
 }
 
-// TODO This won't work for multiple libraries
+// pathResolution holds the result of resolving a playlist path to a library-relative path.
+type pathResolution struct {
+	absolutePath string
+	libraryPath  string
+	valid        bool
+}
+
+// toRelativePath converts the absolute path to a library-relative path.
+func (r pathResolution) toRelativePath() (string, error) {
+	if !r.valid {
+		return "", fmt.Errorf("invalid path resolution")
+	}
+	return filepath.Rel(r.libraryPath, r.absolutePath)
+}
+
+// newValidResolution creates a valid path resolution.
+func newValidResolution(absolutePath, libraryPath string) pathResolution {
+	return pathResolution{
+		absolutePath: absolutePath,
+		libraryPath:  libraryPath,
+		valid:        true,
+	}
+}
+
+// normalizePaths converts playlist file paths to library-relative paths.
+// For relative paths, it resolves them to absolute paths first, then determines which
+// library they belong to. This allows playlists to reference files across library boundaries.
 func (s *playlists) normalizePaths(ctx context.Context, pls *model.Playlist, folder *model.Folder, lines []string) ([]string, error) {
 	libRegex, err := s.compileLibraryPaths(ctx)
 	if err != nil {
@@ -242,39 +270,86 @@ func (s *playlists) normalizePaths(ctx context.Context, pls *model.Playlist, fol
 
 	res := make([]string, 0, len(lines))
 	for idx, line := range lines {
-		var libPath string
+		resolution := s.resolvePath(line, folder, libRegex)
-		var filePath string
 
-		if folder != nil && !filepath.IsAbs(line) {
+		if !resolution.valid {
-			libPath = folder.LibraryPath
-			filePath = filepath.Join(folder.AbsolutePath(), line)
-		} else {
-			cleanLine := filepath.Clean(line)
-			if libPath = libRegex.FindString(cleanLine); libPath != "" {
-				filePath = cleanLine
-			}
-		}
-
-		if libPath != "" {
-			if rel, err := filepath.Rel(libPath, filePath); err == nil {
-				res = append(res, rel)
-			} else {
-				log.Debug(ctx, "Error getting relative path", "playlist", pls.Name, "path", line, "libPath", libPath,
-					"filePath", filePath, err)
-			}
-		} else {
 			log.Warn(ctx, "Path in playlist not found in any library", "path", line, "line", idx)
+			continue
 		}
+
+		relativePath, err := resolution.toRelativePath()
+		if err != nil {
+			log.Debug(ctx, "Error getting relative path", "playlist", pls.Name, "path", line,
+				"libPath", resolution.libraryPath, "filePath", resolution.absolutePath, err)
+			continue
+		}
+
+		res = append(res, relativePath)
 	}
 	return slice.Map(res, filepath.ToSlash), nil
 }
 
+// resolvePath determines the absolute path and library path for a playlist entry.
+func (s *playlists) resolvePath(line string, folder *model.Folder, libRegex *regexp.Regexp) pathResolution {
+	if folder != nil && !filepath.IsAbs(line) {
+		return s.resolveRelativePath(line, folder, libRegex)
+	}
+	return s.resolveAbsolutePath(line, libRegex)
+}
+
+// resolveRelativePath handles relative paths by converting them to absolute paths
+// and finding their library location. This enables cross-library playlist references.
+func (s *playlists) resolveRelativePath(line string, folder *model.Folder, libRegex *regexp.Regexp) pathResolution {
+	// Step 1: Resolve relative path to absolute path based on playlist location
+	// Example: playlist at /music/playlists/test.m3u with line "../songs/abc.mp3"
+	//          resolves to /music/songs/abc.mp3
+	absolutePath := filepath.Clean(filepath.Join(folder.AbsolutePath(), line))
+
+	// Step 2: Determine which library this absolute path belongs to using regex matching
+	if libPath := libRegex.FindString(absolutePath); libPath != "" {
+		return newValidResolution(absolutePath, libPath)
+	}
+
+	// Fallback: Check if it's in the playlist's own library
+	return s.validatePathInLibrary(absolutePath, folder.LibraryPath)
+}
+
+// resolveAbsolutePath handles absolute paths by matching them against library paths.
+func (s *playlists) resolveAbsolutePath(line string, libRegex *regexp.Regexp) pathResolution {
+	cleanPath := filepath.Clean(line)
+	libPath := libRegex.FindString(cleanPath)
+
+	if libPath == "" {
+		return pathResolution{valid: false}
+	}
+	return newValidResolution(cleanPath, libPath)
+}
+
+// validatePathInLibrary verifies that an absolute path belongs to the specified library.
+// It rejects paths that escape the library using ".." segments.
+func (s *playlists) validatePathInLibrary(absolutePath, libraryPath string) pathResolution {
+	rel, err := filepath.Rel(libraryPath, absolutePath)
+	if err != nil || strings.HasPrefix(rel, "..") {
+		return pathResolution{valid: false}
+	}
+
+	return newValidResolution(absolutePath, libraryPath)
+}
+
 func (s *playlists) compileLibraryPaths(ctx context.Context) (*regexp.Regexp, error) {
 	libs, err := s.ds.Library(ctx).GetAll()
 	if err != nil {
 		return nil, err
 	}
 
+	// Sort libraries by path length (descending) to ensure longest paths match first.
+	// This prevents shorter paths that are prefixes from incorrectly matching.
+	// Example: /music-classical must be checked before /music
+	// Otherwise, /music-classical/track.mp3 would match /music instead of /music-classical
+	slices.SortFunc(libs, func(i, j model.Library) int {
+		return cmp.Compare(len(j.Path), len(i.Path)) // Reverse order for descending
+	})
+
 	// Create regex patterns for each library path
 	patterns := make([]string, len(libs))
 	for i, lib := range libs {
@@ -282,7 +357,7 @@ func (s *playlists) compileLibraryPaths(ctx context.Context) (*regexp.Regexp, er
 		escapedPath := regexp.QuoteMeta(cleanPath)
 		patterns[i] = fmt.Sprintf("^%s(?:/|$)", escapedPath)
 	}
-	// Combine all patterns into a single regex
+	// Combine all patterns into a single regex - order matters due to alternation
 	combinedPattern := strings.Join(patterns, "|")
 	re, err := regexp.Compile(combinedPattern)
 	if err != nil {

core/playlists_internal_test.go

@@ -0,0 +1,186 @@
+package core
+
+import (
+	"context"
+
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/tests"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("compileLibraryPaths", func() {
+	var ds *tests.MockDataStore
+	var mockLibRepo *tests.MockLibraryRepo
+	var ps *playlists
+	ctx := context.Background()
+
+	BeforeEach(func() {
+		mockLibRepo = &tests.MockLibraryRepo{}
+		ds = &tests.MockDataStore{
+			MockedLibrary: mockLibRepo,
+		}
+		ps = &playlists{ds: ds}
+	})
+
+	Describe("Longest library path matching", func() {
+		It("matches the longest library path when multiple libraries share a prefix", func() {
+			// Setup libraries with prefix conflicts
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/music"},
+				{ID: 2, Path: "/music-classical"},
+				{ID: 3, Path: "/music-classical/opera"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Test that longest path matches first
+			// Note: The regex pattern ^path(?:/|$) will match the path plus the trailing /
+			testCases := []struct {
+				path     string
+				expected string
+			}{
+				{"/music-classical/opera/track.mp3", "/music-classical/opera/"},
+				{"/music-classical/track.mp3", "/music-classical/"},
+				{"/music/track.mp3", "/music/"},
+				{"/music-classical/opera/", "/music-classical/opera/"}, // Trailing slash
+				{"/music-classical/opera", "/music-classical/opera"},   // Exact match (no trailing /)
+			}
+
+			for _, tc := range testCases {
+				matched := libRegex.FindString(tc.path)
+				Expect(matched).To(Equal(tc.expected), "Path %s should match %s, but got %s", tc.path, tc.expected, matched)
+			}
+		})
+
+		It("handles libraries with similar prefixes but different structures", func() {
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/home/user/music"},
+				{ID: 2, Path: "/home/user/music-backup"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Test that music-backup library is matched correctly
+			matched := libRegex.FindString("/home/user/music-backup/track.mp3")
+			Expect(matched).To(Equal("/home/user/music-backup/"))
+
+			// Test that music library is still matched correctly
+			matched = libRegex.FindString("/home/user/music/track.mp3")
+			Expect(matched).To(Equal("/home/user/music/"))
+		})
+
+		It("matches path that is exactly the library root", func() {
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/music"},
+				{ID: 2, Path: "/music-classical"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Exact library path should match (no trailing /)
+			matched := libRegex.FindString("/music-classical")
+			Expect(matched).To(Equal("/music-classical"))
+		})
+
+		It("handles complex nested library structures", func() {
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/media"},
+				{ID: 2, Path: "/media/audio"},
+				{ID: 3, Path: "/media/audio/classical"},
+				{ID: 4, Path: "/media/audio/classical/baroque"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+
+			testCases := []struct {
+				path     string
+				expected string
+			}{
+				{"/media/audio/classical/baroque/bach/track.mp3", "/media/audio/classical/baroque/"},
+				{"/media/audio/classical/mozart/track.mp3", "/media/audio/classical/"},
+				{"/media/audio/rock/track.mp3", "/media/audio/"},
+				{"/media/video/movie.mp4", "/media/"},
+			}
+
+			for _, tc := range testCases {
+				matched := libRegex.FindString(tc.path)
+				Expect(matched).To(Equal(tc.expected), "Path %s should match %s, but got %s", tc.path, tc.expected, matched)
+			}
+		})
+	})
+
+	Describe("Edge cases", func() {
+		It("handles empty library list", func() {
+			mockLibRepo.SetData([]model.Library{})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(libRegex).ToNot(BeNil())
+
+			// Should not match anything
+			matched := libRegex.FindString("/music/track.mp3")
+			Expect(matched).To(BeEmpty())
+		})
+
+		It("handles single library", func() {
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/music"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+
+			matched := libRegex.FindString("/music/track.mp3")
+			Expect(matched).To(Equal("/music/"))
+		})
+
+		It("handles libraries with special regex characters", func() {
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/music[test]"},
+				{ID: 2, Path: "/music(backup)"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(libRegex).ToNot(BeNil())
+
+			// Special characters should be escaped and match literally
+			matched := libRegex.FindString("/music[test]/track.mp3")
+			Expect(matched).To(Equal("/music[test]/"))
+		})
+	})
+
+	Describe("Regex pattern validation", func() {
+		It("ensures regex alternation respects order by testing actual matching behavior", func() {
+			mockLibRepo.SetData([]model.Library{
+				{ID: 1, Path: "/a"},
+				{ID: 2, Path: "/ab"},
+				{ID: 3, Path: "/abc"},
+			})
+
+			libRegex, err := ps.compileLibraryPaths(ctx)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Verify that longer paths match correctly (not cut off by shorter prefix)
+			// If ordering is wrong, /ab would match before /abc for path "/abc/file"
+			testCases := []struct {
+				path     string
+				expected string
+			}{
+				{"/abc/file.mp3", "/abc/"},
+				{"/ab/file.mp3", "/ab/"},
+				{"/a/file.mp3", "/a/"},
+			}
+
+			for _, tc := range testCases {
+				matched := libRegex.FindString(tc.path)
+				Expect(matched).To(Equal(tc.expected), "Path %s should match %s", tc.path, tc.expected)
+			}
+		})
+	})
+})

core/playlists_test.go

@@ -112,6 +112,159 @@ var _ = Describe("Playlists", func() {
 				Expect(err.Error()).To(ContainSubstring("line 19, column 1: invalid character '\\n'"))
 			})
 		})
+
+		Describe("Cross-library relative paths", func() {
+			var tmpDir, plsDir, songsDir string
+
+			BeforeEach(func() {
+				// Create temp directory structure
+				tmpDir = GinkgoT().TempDir()
+				plsDir = tmpDir + "/playlists"
+				songsDir = tmpDir + "/songs"
+				Expect(os.Mkdir(plsDir, 0755)).To(Succeed())
+				Expect(os.Mkdir(songsDir, 0755)).To(Succeed())
+
+				// Setup two different libraries with paths matching our temp structure
+				mockLibRepo.SetData([]model.Library{
+					{ID: 1, Path: songsDir},
+					{ID: 2, Path: plsDir},
+				})
+
+				// Create a mock media file repository that returns files for both libraries
+				// Note: The paths are relative to their respective library roots
+				ds.MockedMediaFile = &mockedMediaFileFromListRepo{
+					data: []string{
+						"abc.mp3", // This is songs/abc.mp3 relative to songsDir
+						"def.mp3", // This is playlists/def.mp3 relative to plsDir
+					},
+				}
+				ps = NewPlaylists(ds)
+			})
+
+			It("handles relative paths that reference files in other libraries", func() {
+				// Create a temporary playlist file with relative path
+				plsContent := "#PLAYLIST:Cross Library Test\n../songs/abc.mp3\ndef.mp3"
+				plsFile := plsDir + "/test.m3u"
+				Expect(os.WriteFile(plsFile, []byte(plsContent), 0600)).To(Succeed())
+
+				// Playlist is in the Playlists library folder
+				// Important: Path should be relative to LibraryPath, and Name is the folder name
+				plsFolder := &model.Folder{
+					ID:          "2",
+					LibraryID:   2,
+					LibraryPath: plsDir,
+					Path:        "",
+					Name:        "",
+				}
+
+				pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(pls.Tracks).To(HaveLen(2))
+				Expect(pls.Tracks[0].Path).To(Equal("abc.mp3")) // From songsDir library
+				Expect(pls.Tracks[1].Path).To(Equal("def.mp3")) // From plsDir library
+			})
+
+			It("ignores paths that point outside all libraries", func() {
+				// Create a temporary playlist file with path outside libraries
+				plsContent := "#PLAYLIST:Outside Test\n../../outside.mp3\nabc.mp3"
+				plsFile := plsDir + "/test.m3u"
+				Expect(os.WriteFile(plsFile, []byte(plsContent), 0600)).To(Succeed())
+
+				plsFolder := &model.Folder{
+					ID:          "2",
+					LibraryID:   2,
+					LibraryPath: plsDir,
+					Path:        "",
+					Name:        "",
+				}
+
+				pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u")
+				Expect(err).ToNot(HaveOccurred())
+				// Should only find abc.mp3, not outside.mp3
+				Expect(pls.Tracks).To(HaveLen(1))
+				Expect(pls.Tracks[0].Path).To(Equal("abc.mp3"))
+			})
+
+			It("handles relative paths with multiple '../' components", func() {
+				// Create a nested structure: tmpDir/playlists/subfolder/test.m3u
+				subFolder := plsDir + "/subfolder"
+				Expect(os.Mkdir(subFolder, 0755)).To(Succeed())
+
+				// Create the media file in the subfolder directory
+				// The mock will return it as "def.mp3" relative to plsDir
+				ds.MockedMediaFile = &mockedMediaFileFromListRepo{
+					data: []string{
+						"abc.mp3", // From songsDir library
+						"def.mp3", // From plsDir library root
+					},
+				}
+
+				// From subfolder, ../../songs/abc.mp3 should resolve to songs library
+				// ../def.mp3 should resolve to plsDir/def.mp3
+				plsContent := "#PLAYLIST:Nested Test\n../../songs/abc.mp3\n../def.mp3"
+				plsFile := subFolder + "/test.m3u"
+				Expect(os.WriteFile(plsFile, []byte(plsContent), 0600)).To(Succeed())
+
+				// The folder: AbsolutePath = LibraryPath + Path + Name
+				// So for /playlists/subfolder: LibraryPath=/playlists, Path="", Name="subfolder"
+				plsFolder := &model.Folder{
+					ID:          "2",
+					LibraryID:   2,
+					LibraryPath: plsDir,
+					Path:        "",          // Empty because subfolder is directly under library root
+					Name:        "subfolder", // The folder name
+				}
+
+				pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(pls.Tracks).To(HaveLen(2))
+				Expect(pls.Tracks[0].Path).To(Equal("abc.mp3")) // From songsDir library
+				Expect(pls.Tracks[1].Path).To(Equal("def.mp3")) // From plsDir library root
+			})
+
+			It("correctly resolves libraries when one path is a prefix of another", func() {
+				// This tests the bug where /music would match before /music-classical
+				// Create temp directory structure with prefix conflict
+				tmpDir := GinkgoT().TempDir()
+				musicDir := tmpDir + "/music"
+				musicClassicalDir := tmpDir + "/music-classical"
+				Expect(os.Mkdir(musicDir, 0755)).To(Succeed())
+				Expect(os.Mkdir(musicClassicalDir, 0755)).To(Succeed())
+
+				// Setup two libraries where one is a prefix of the other
+				mockLibRepo.SetData([]model.Library{
+					{ID: 1, Path: musicDir},          // /tmp/xxx/music
+					{ID: 2, Path: musicClassicalDir}, // /tmp/xxx/music-classical
+				})
+
+				// Mock will return tracks from both libraries
+				ds.MockedMediaFile = &mockedMediaFileFromListRepo{
+					data: []string{
+						"rock.mp3", // From music library
+						"bach.mp3", // From music-classical library
+					},
+				}
+
+				// Create playlist in music library that references music-classical
+				plsContent := "#PLAYLIST:Cross Prefix Test\nrock.mp3\n../music-classical/bach.mp3"
+				plsFile := musicDir + "/test.m3u"
+				Expect(os.WriteFile(plsFile, []byte(plsContent), 0600)).To(Succeed())
+
+				plsFolder := &model.Folder{
+					ID:          "1",
+					LibraryID:   1,
+					LibraryPath: musicDir,
+					Path:        "",
+					Name:        "",
+				}
+
+				pls, err := ps.ImportFile(ctx, plsFolder, "test.m3u")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(pls.Tracks).To(HaveLen(2))
+				Expect(pls.Tracks[0].Path).To(Equal("rock.mp3")) // From music library
+				Expect(pls.Tracks[1].Path).To(Equal("bach.mp3")) // From music-classical library (not music!)
+			})
+		})
 	})
 
 	Describe("ImportM3U", func() {