fix: ensure longest library path matches first to resolve prefix conflicts in playlists

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-11-07 00:08:48 -05:00
parent 3761503d7a
commit 5925e8dbd8
3 changed files with 240 additions and 1 deletions

View File

@ -1,6 +1,7 @@
package core
import (
"cmp"
"context"
"encoding/json"
"errors"
@ -10,6 +11,7 @@ import (
"os"
"path/filepath"
"regexp"
"slices"
"strings"
"time"
@ -340,6 +342,14 @@ func (s *playlists) compileLibraryPaths(ctx context.Context) (*regexp.Regexp, er
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 {
@ -347,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 {

View File

@ -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)
}
})
})
})

View File

@ -221,6 +221,49 @@ var _ = Describe("Playlists", func() {
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!)
})
})
})