diff --git a/core/playlists.go b/core/playlists.go index f98179f88..3d1cce32f 100644 --- a/core/playlists.go +++ b/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 - var filePath string + resolution := s.resolvePath(line, folder, libRegex) - if folder != nil && !filepath.IsAbs(line) { - 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 { + if !resolution.valid { 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 { diff --git a/core/playlists_internal_test.go b/core/playlists_internal_test.go new file mode 100644 index 000000000..2fda603b3 --- /dev/null +++ b/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) + } + }) + }) +}) diff --git a/core/playlists_test.go b/core/playlists_test.go index fb42f9c9f..d5e33ad0f 100644 --- a/core/playlists_test.go +++ b/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() {