From f8674115d27ddb6ab5f8fbf33bd0e05f102bede0 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:01:52 -0700 Subject: [PATCH 1/5] fix(playlists): import smart playlists when PlaylistsPath uses Windows separators Both the relative folder path and each PlaylistsPath pattern are converted with filepath.ToSlash before doublestar.Match. The existing in-path example has its Windows skip removed and joins its patterns with filepath.ListSeparator. New cases cover a path from filepath.Join, the same folder written with forward slashes, and neighboring directories that stay outside the configured path after normalization. Fixes #6276 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> --- core/playlists/import_test.go | 41 +++++++++++++++++++++++++++++++++-- core/playlists/playlists.go | 4 +++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/core/playlists/import_test.go b/core/playlists/import_test.go index 25960f0fe..050bbf48b 100644 --- a/core/playlists/import_test.go +++ b/core/playlists/import_test.go @@ -1140,16 +1140,53 @@ var _ = Describe("Playlists - Import", func() { }) It("returns true if folder is in PlaylistsPath", func() { - tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)") - conf.Server.PlaylistsPath = "other/**:playlists/**" + conf.Server.PlaylistsPath = strings.Join([]string{"other/**", "playlists/**"}, string(filepath.ListSeparator)) Expect(playlists.InPath(folder)).To(BeTrue()) }) + It("returns true if folder matches a PlaylistsPath joined with the OS separator", func() { + nsp := model.Folder{ + LibraryPath: "/music", + Path: "Playlists", + Name: "navidrome", + } + conf.Server.PlaylistsPath = filepath.Join("Playlists", "navidrome") + Expect(playlists.InPath(nsp)).To(BeTrue()) + }) + + It("returns true if folder matches a forward-slash PlaylistsPath", func() { + nsp := model.Folder{ + LibraryPath: "/music", + Path: "Playlists", + Name: "navidrome", + } + conf.Server.PlaylistsPath = "Playlists/navidrome" + Expect(playlists.InPath(nsp)).To(BeTrue()) + }) + It("returns false if folder is not in PlaylistsPath", func() { conf.Server.PlaylistsPath = "other" Expect(playlists.InPath(folder)).To(BeFalse()) }) + It("returns false for a different directory after normalization", func() { + nsp := model.Folder{ + LibraryPath: "/music", + Path: "Playlists", + Name: "navidrome", + } + sibling := model.Folder{ + LibraryPath: "/music", + Path: "Playlists", + Name: "other", + } + conf.Server.PlaylistsPath = filepath.Join("Other", "dir") + Expect(playlists.InPath(nsp)).To(BeFalse()) + + conf.Server.PlaylistsPath = filepath.Join("Playlists", "navidrome", "**") + Expect(playlists.InPath(sibling)).To(BeFalse()) + }) + It("returns true if for a playlist in root of MusicFolder if PlaylistsPath is '.'", func() { conf.Server.PlaylistsPath = "." Expect(playlists.InPath(folder)).To(BeFalse()) diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index c9bc03b97..9bf03a726 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -78,8 +78,10 @@ func InPath(folder model.Folder) bool { return true } rel, _ := filepath.Rel(folder.LibraryPath, folder.AbsolutePath()) + // doublestar splits only on / and treats \ as an escape, so normalize OS separators first. + rel = filepath.ToSlash(rel) for path := range strings.SplitSeq(conf.Server.PlaylistsPath, string(filepath.ListSeparator)) { - if match, _ := doublestar.Match(path, rel); match { + if match, _ := doublestar.Match(filepath.ToSlash(path), rel); match { return true } } From cbf55d932d579095c206e429c2ba7905fcf19496 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 7 Oct 2026 10:10:32 -0400 Subject: [PATCH 2/5] fix(playlists): keep escaped glob brackets in PlaylistsPath on Windows (#6276) Converting every '\' to '/' broke escaped literals such as '\[Mix\]', which matched a real "[Mix]" folder before. On Windows, '\' is now a path separator except for an escaped pair ('\[...\]', '\{...\}') or a lone '\]' / '\}', so 'Playlists\navidrome', 'Playlists\{rock,jazz}', 'Playlists\[ab]' and '\[Mix\]' all work. Other OSes keep the pattern as is. A separator right before an escaped pair must be written as '/' or '\\' ('Playlists/\[Mix\]'). InPath now matches the folder's slash-separated library path (path.Join(Path, Name)) instead of a filepath.Rel/ToSlash round trip, and gets its patterns from conf.PlaylistsPathPatterns, which the config validation can share. Tests cover the conversion, InPath with scanner-built folders, and scans of real folders (nested paths, brackets, braces, controls). --- conf/configuration.go | 44 ++++++ conf/configuration_test.go | 38 ++++++ conf/export_test.go | 2 + core/playlists/import_test.go | 38 ++++++ core/playlists/playlists.go | 12 +- scanner/scanner_playlists_path_test.go | 180 +++++++++++++++++++++++++ 6 files changed, 307 insertions(+), 7 deletions(-) create mode 100644 scanner/scanner_playlists_path_test.go diff --git a/conf/configuration.go b/conf/configuration.go index efa9cbf9a..cd5ea8d3a 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -816,6 +816,50 @@ func disableExternalServices() { } } +// PlaylistsPathPatterns returns the PlaylistsPath globs in the slash-separated form doublestar matches. +func PlaylistsPathPatterns() []string { + var patterns []string + for pattern := range strings.SplitSeq(Server.PlaylistsPath, string(filepath.ListSeparator)) { + patterns = append(patterns, toGlobPattern(pattern, filepath.Separator == '\\')) + } + return patterns +} + +// toGlobPattern turns Windows '\' separators into '/'. Windows names can contain brackets and +// braces, so an escaped pair ('\[...\]', '\{...\}') and a lone '\]' or '\}' stay escapes. +func toGlobPattern(pattern string, windows bool) string { + if !windows { + return pattern + } + var sb strings.Builder + for i := 0; i < len(pattern); i++ { + if pattern[i] != '\\' { + sb.WriteByte(pattern[i]) + continue + } + if i+1 < len(pattern) && isEscape(pattern[i+1], pattern[i+2:]) { + sb.WriteString(pattern[i : i+2]) + i++ + continue + } + sb.WriteByte('/') + } + return sb.String() +} + +// isEscape reports whether a '\' followed by c (and then rest) escapes c rather than separating. +func isEscape(c byte, rest string) bool { + closer := map[byte]byte{'[': ']', '{': '}'}[c] + switch { + case c == ']' || c == '}': + return true + case closer == 0: + return false + } + next := strings.IndexByte(rest, '\\') + return next >= 0 && next+1 < len(rest) && rest[next+1] == closer +} + func validatePlaylistsPath() error { for path := range strings.SplitSeq(Server.PlaylistsPath, string(filepath.ListSeparator)) { _, err := doublestar.Match(path, "") diff --git a/conf/configuration_test.go b/conf/configuration_test.go index 8c4c8ab86..e51d3084c 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -379,6 +379,44 @@ var _ = Describe("Configuration", func() { ) }) + Describe("PlaylistsPathPatterns", func() { + It("splits the list with the OS list separator", func() { + conf.Server.PlaylistsPath = "." + string(filepath.ListSeparator) + "Playlists/**" + Expect(conf.PlaylistsPathPatterns()).To(Equal([]string{".", "Playlists/**"})) + }) + + DescribeTable("converts a Windows pattern to slash form", + func(pattern, expected string) { + Expect(conf.ToGlobPattern(pattern, true)).To(Equal(expected)) + }, + Entry("separator", `Playlists\navidrome`, "Playlists/navidrome"), + Entry("separator before **", `Playlists\**`, "Playlists/**"), + Entry("separator before braces", `Playlists\{rock,jazz}`, "Playlists/{rock,jazz}"), + Entry("trailing separator", `Playlists\`, "Playlists/"), + Entry("escaped brackets", `\[Mix\]`, `\[Mix\]`), + Entry("escaped brackets after a slash", `Playlists/\[Mix\]`, `Playlists/\[Mix\]`), + Entry("escaped brackets after a separator", `Playlists\\[Mix\]`, `Playlists/\[Mix\]`), + Entry("character class", `[[]Mix]`, `[[]Mix]`), + Entry("separator before a character class", `Playlists\[[]Mix]`, "Playlists/[[]Mix]"), + Entry("separator before a bracket range", `Playlists\[ab]`, "Playlists/[ab]"), + Entry("escaped braces", `\{Mix\}`, `\{Mix\}`), + Entry("escaped braces after a separator", `Playlists\\{Mix\}`, `Playlists/\{Mix\}`), + Entry("lone escaped closing bracket", `Mix\]`, `Mix\]`), + // Ambiguous: an escaped pair wins, so a separator right before it must be '/' or '\\' + Entry("escaped pair right after a name", `Playlists\[Mix\]`, `Playlists\[Mix\]`), + Entry("forward slashes", "Playlists/navidrome", "Playlists/navidrome"), + ) + + DescribeTable("keeps a non-Windows pattern as is", + func(pattern string) { + Expect(conf.ToGlobPattern(pattern, false)).To(Equal(pattern)) + }, + Entry("backslash escape", `Playlists\navidrome`), + Entry("escaped brackets", `\[Mix\]`), + Entry("escaped star", `\*`), + ) + }) + Describe("MaxImageSize floor", func() { BeforeEach(func() { viper.Reset() diff --git a/conf/export_test.go b/conf/export_test.go index d1e1a6f99..cf074b92b 100644 --- a/conf/export_test.go +++ b/conf/export_test.go @@ -36,3 +36,5 @@ func SetLogFatal(f func(...any)) func() { var UnknownConfigKeys = unknownConfigKeys var SuggestOptions = suggestOptions + +var ToGlobPattern = toGlobPattern diff --git a/core/playlists/import_test.go b/core/playlists/import_test.go index 050bbf48b..6ed7b8020 100644 --- a/core/playlists/import_test.go +++ b/core/playlists/import_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strconv" "strings" "time" @@ -1199,6 +1200,43 @@ var _ = Describe("Playlists - Import", func() { Expect(playlists.InPath(folder2)).To(BeTrue()) }) + + // Folders built like the scanner does (no LibraryPath), on the native OS + DescribeTable("matches scanner folders", + func(pattern, folderPath string, expected bool) { + conf.Server.PlaylistsPath = pattern + f := model.NewFolder(model.Library{ID: 1, Path: GinkgoT().TempDir()}, folderPath) + Expect(playlists.InPath(*f)).To(Equal(expected)) + }, + Entry("nested folder, exact pattern", "Playlists/navidrome", "Playlists/navidrome", true), + Entry("nested folder, ** pattern", "Playlists/**", "Playlists/navidrome/Deep", true), + Entry("nested folder, second item of a list", "."+string(filepath.ListSeparator)+"Playlists/navidrome", "Playlists/navidrome", true), + Entry("top-level folder", "Playlists", "Playlists", true), + Entry("root folder, '.' in a list", "."+string(filepath.ListSeparator)+"Playlists/navidrome", ".", true), + Entry("sibling folder is excluded", "Playlists/navidrome", "Playlists/other", false), + Entry("child folder is excluded by an exact pattern", "Playlists/navidrome", "Playlists/navidrome/Deep", false), + Entry("root folder is excluded by a nested pattern", "Playlists/navidrome", ".", false), + Entry("escaped brackets, top-level", `\[Mix\]`, "[Mix]", true), + Entry("escaped brackets, nested", `Playlists/\[Mix\]`, "Playlists/[Mix]", true), + Entry("escaped brackets exclude a plain folder", `\[Mix\]`, "Mix", false), + Entry("escaped brackets, nested, exclude a plain folder", `Playlists/\[Mix\]`, "Playlists/Mix", false), + Entry("character class literal, top-level", `[[]Mix]`, "[Mix]", true), + Entry("character class literal, nested", `Playlists/[[]Mix]`, "Playlists/[Mix]", true), + Entry("unescaped brackets are a character class", `[Mix]`, "[Mix]", false), + Entry("brace alternatives", "Playlists/{rock,jazz}", "Playlists/jazz", true), + Entry("brace alternatives exclude others", "Playlists/{rock,jazz}", "Playlists/pop", false), + // Backslash is a path separator on Windows (except in escaped brackets or braces), an escape elsewhere + Entry("backslash separator", `Playlists\navidrome`, "Playlists/navidrome", runtime.GOOS == "windows"), + Entry("backslash separator before **", `Playlists\**`, "Playlists/navidrome/Deep", runtime.GOOS == "windows"), + Entry("backslash separator before braces", `Playlists\{rock,jazz}`, "Playlists/rock", runtime.GOOS == "windows"), + Entry("backslash separator, sibling folder is excluded", `Playlists\navidrome`, "Playlists/other", false), + Entry("backslash before escaped brackets, nested", `Playlists\\[Mix\]`, "Playlists/[Mix]", runtime.GOOS == "windows"), + Entry("backslash separator before a character class", `Playlists\[[]Mix]`, "Playlists/[Mix]", runtime.GOOS == "windows"), + Entry("backslash separator before a bracket range", `Playlists\[ab]`, "Playlists/a", runtime.GOOS == "windows"), + Entry("backslash separator before a bracket range excludes others", `Playlists\[ab]`, "Playlists/c", false), + Entry("escaped braces", `\{Mix\}`, "{Mix}", true), + Entry("escaped braces exclude a plain folder", `\{Mix\}`, "Mix", false), + ) }) }) diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index 9bf03a726..7d3b5bca8 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -4,9 +4,8 @@ import ( "context" "io" "os" - "path/filepath" + "path" "strconv" - "strings" "github.com/bmatcuk/doublestar/v4" "github.com/deluan/rest" @@ -77,11 +76,10 @@ func InPath(folder model.Folder) bool { if conf.Server.PlaylistsPath == "" { return true } - rel, _ := filepath.Rel(folder.LibraryPath, folder.AbsolutePath()) - // doublestar splits only on / and treats \ as an escape, so normalize OS separators first. - rel = filepath.ToSlash(rel) - for path := range strings.SplitSeq(conf.Server.PlaylistsPath, string(filepath.ListSeparator)) { - if match, _ := doublestar.Match(filepath.ToSlash(path), rel); match { + // Folder paths are already slash-separated and relative to the library, as doublestar expects + rel := path.Join(folder.Path, folder.Name) + for _, pattern := range conf.PlaylistsPathPatterns() { + if match, _ := doublestar.Match(pattern, rel); match { return true } } diff --git a/scanner/scanner_playlists_path_test.go b/scanner/scanner_playlists_path_test.go new file mode 100644 index 000000000..0985f4654 --- /dev/null +++ b/scanner/scanner_playlists_path_test.go @@ -0,0 +1,180 @@ +package scanner_test + +import ( + "context" + "os" + "path" + "path/filepath" + "runtime" + "slices" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/core/metrics" + "github.com/navidrome/navidrome/core/playlists" + "github.com/navidrome/navidrome/db" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/persistence" + "github.com/navidrome/navidrome/scanner" + "github.com/navidrome/navidrome/server/events" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Scans a real library folder on the native filesystem (so Windows paths go through the OS path +// handling) and checks what reaches the DB in phase 1 (num_playlists) and phase 4 (playlists). +var _ = Describe("Scanner - PlaylistsPath", Ordered, ContinueOnFailure, func() { + var ctx context.Context + var ds model.DataStore + var s model.Scanner + var libPath string + + BeforeAll(func() { + ctx = request.WithUser(GinkgoT().Context(), model.User{ID: "123", IsAdmin: true}) + // The DB stays open until the suite ends, and Windows can't delete an open file + tmpDir, err := os.MkdirTemp("", "scanner-playlists-path-test") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { _ = os.RemoveAll(tmpDir) }) + conf.Server.DbPath = filepath.Join(tmpDir, "test-scanner.db?_journal_mode=WAL") + db.Db().SetMaxOpenConns(1) + }) + + writeNSP := func(relPath, name string) { + GinkgoHelper() + full := filepath.Join(libPath, filepath.FromSlash(relPath)) + Expect(os.MkdirAll(filepath.Dir(full), 0755)).To(Succeed()) + nsp := `{"name": "` + name + `", "all": [{"is": {"loved": true}}]}` + Expect(os.WriteFile(full, []byte(nsp), 0600)).To(Succeed()) + } + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + libPath = GinkgoT().TempDir() + conf.Server.MusicFolder = libPath + conf.Server.DevExternalScanner = false + conf.Server.AutoImportPlaylists = true + + db.Init(ctx) + DeferCleanup(func() { + Expect(tests.ClearDB()).To(Succeed()) + }) + ds = persistence.New(db.Db()) + + adminUser := model.User{ID: "123", UserName: "admin", Name: "Admin User", IsAdmin: true, NewPassword: "password"} + Expect(ds.User().Put(ctx, &adminUser)).To(Succeed()) + + lib := model.Library{ID: 1, Name: "Native Library", Path: libPath} + Expect(ds.Library().Put(ctx, &lib)).To(Succeed()) + + s = scanner.New(ctx, ds, events.NoopBroker(), + playlists.NewPlaylists(ds, artwork.NewUploader(ds)), metrics.NewNoopInstance()) + }) + + scan := func(fullScan bool) { + GinkgoHelper() + _, err := s.ScanAll(ctx, fullScan) + Expect(err).ToNot(HaveOccurred()) + } + + // One map, so a failure shows both the phase 1 and the phase 4 results + results := func() map[string][]string { + GinkgoHelper() + folders, err := ds.Folder().GetAll(ctx) + Expect(err).ToNot(HaveOccurred()) + var withPlaylists []string + for _, f := range folders { + if f.NumPlaylists > 0 { + withPlaylists = append(withPlaylists, path.Join(f.Path, f.Name)) + } + } + all, err := ds.Playlist().GetAll(ctx) + Expect(err).ToNot(HaveOccurred()) + var names []string + for _, p := range all { + Expect(p.Path).To(HavePrefix(libPath)) + names = append(names, p.Name) + } + return map[string][]string{ + "phase 1: folders with num_playlists > 0": slices.Sorted(slices.Values(withPlaylists)), + "phase 4: imported playlists": slices.Sorted(slices.Values(names)), + } + } + + expectResults := func(folders, names []string) { + GinkgoHelper() + Expect(results()).To(Equal(map[string][]string{ + "phase 1: folders with num_playlists > 0": slices.Sorted(slices.Values(folders)), + "phase 4: imported playlists": slices.Sorted(slices.Values(names)), + })) + } + + onWindows := func(values ...string) []string { + if runtime.GOOS == "windows" { + return values + } + return nil + } + + // Phase 4 keeps its folder cursor open while importing, so with the suite's single DB connection + // a scan stalls past ~6 playlist folders. Each library below stays under that. + Describe("selecting nested folders", func() { + BeforeEach(func() { + writeNSP("Root.nsp", "Root") + writeNSP("Playlists/navidrome/Rock.nsp", "Rock") + writeNSP("Playlists/navidrome/Deep/Nested.nsp", "Nested") + writeNSP("Playlists/other/Other.nsp", "Other") + }) + + DescribeTable("imports only playlists inside PlaylistsPath", + func(pattern string, folders, names []string) { + conf.Server.PlaylistsPath = pattern + scan(true) + expectResults(folders, names) + }, + Entry("empty (default) imports everything", "", + []string{".", "Playlists/navidrome", "Playlists/navidrome/Deep", "Playlists/other"}, + []string{"Root", "Rock", "Nested", "Other"}), + Entry("nested folder", "Playlists/navidrome", + []string{"Playlists/navidrome"}, []string{"Rock"}), + Entry("root and a nested ** pattern", "."+string(filepath.ListSeparator)+"Playlists/navidrome/**", + []string{".", "Playlists/navidrome", "Playlists/navidrome/Deep"}, []string{"Root", "Rock", "Nested"}), + Entry("non-matching pattern imports nothing", "Music/**", nil, nil), + // Backslash is a path separator on Windows (except in escaped brackets or braces), an escape elsewhere + Entry("backslash nested folder (issue #6276 config)", `Playlists\navidrome`, + onWindows("Playlists/navidrome"), onWindows("Rock")), + Entry("backslash separator before braces", `Playlists\{navidrome,other}`, + onWindows("Playlists/navidrome", "Playlists/other"), onWindows("Rock", "Other")), + ) + }) + + Describe("selecting folders with brackets in their names", func() { + BeforeEach(func() { + writeNSP("[Mix]/Mix.nsp", "Mix") + writeNSP("Mix/Plain.nsp", "Plain") + writeNSP("Playlists/[Mix]/NestedMix.nsp", "NestedMix") + writeNSP("{Mix}/Braces.nsp", "Braces") + }) + + DescribeTable("imports only playlists inside PlaylistsPath", + func(pattern string, folders, names []string) { + conf.Server.PlaylistsPath = pattern + scan(true) + expectResults(folders, names) + }, + Entry("brackets: empty (default) imports everything", "", + []string{"[Mix]", "Mix", "Playlists/[Mix]", "{Mix}"}, []string{"Mix", "Plain", "NestedMix", "Braces"}), + Entry("brackets: escaped, top-level", `\[Mix\]`, []string{"[Mix]"}, []string{"Mix"}), + Entry("brackets: escaped, nested", `Playlists/\[Mix\]`, []string{"Playlists/[Mix]"}, []string{"NestedMix"}), + Entry("brackets: character class literal", `[[]Mix]`, []string{"[Mix]"}, []string{"Mix"}), + Entry("brackets: unescaped brackets are a character class", `[Mix]`, nil, nil), + Entry("brackets: backslash before escaped brackets, nested", `Playlists\\[Mix\]`, + onWindows("Playlists/[Mix]"), onWindows("NestedMix")), + Entry("brackets: backslash separator before a character class", `Playlists\[[]Mix]`, + onWindows("Playlists/[Mix]"), onWindows("NestedMix")), + Entry("brackets: escaped braces", `\{Mix\}`, []string{"{Mix}"}, []string{"Braces"}), + ) + }) +}) From c892862c12cc4afefc3cf787b3821ff52ae23e0d Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 7 Oct 2026 10:11:00 -0400 Subject: [PATCH 3/5] fix(conf): validate PlaylistsPath the same way it is matched (#6276) validatePlaylistsPath checked the raw pattern, while InPath matches the converted one. On Windows, 'Playlists\{rock,jazz}' failed validation and stopped the server, although InPath matches it, and 'Playlists\[Mix' passed validation but is invalid once converted. Validation now uses conf.PlaylistsPathPatterns, like InPath. Tests load real TOML files through conf.LoadFromFile. --- conf/configuration.go | 7 +++---- conf/configuration_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index cd5ea8d3a..3032cabd6 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -861,10 +861,9 @@ func isEscape(c byte, rest string) bool { } func validatePlaylistsPath() error { - for path := range strings.SplitSeq(Server.PlaylistsPath, string(filepath.ListSeparator)) { - _, err := doublestar.Match(path, "") - if err != nil { - return fmt.Errorf("invalid PlaylistsPath %q: %w", path, err) + for _, pattern := range PlaylistsPathPatterns() { + if _, err := doublestar.Match(pattern, ""); err != nil { + return fmt.Errorf("invalid PlaylistsPath %q: %w", pattern, err) } } return nil diff --git a/conf/configuration_test.go b/conf/configuration_test.go index e51d3084c..e62ca3d1b 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -417,6 +417,40 @@ var _ = Describe("Configuration", func() { ) }) + Describe("PlaylistsPath validation", func() { + // A real TOML file with literal (single-quoted) strings, like Windows users write paths + loadConfig := func(pattern string) (err error) { + file := filepath.Join(GinkgoT().TempDir(), "navidrome.toml") + Expect(os.WriteFile(file, []byte("PlaylistsPath = '"+pattern+"'\n"), 0600)).To(Succeed()) + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%v", r) + } + }() + conf.LoadFromFile(file) + return nil + } + + DescribeTable("accepts exactly the patterns InPath can use", + func(pattern string, accepted bool) { + err := loadConfig(pattern) + if accepted { + Expect(err).ToNot(HaveOccurred()) + Expect(conf.Server.PlaylistsPath).To(Equal(pattern)) + } else { + Expect(err).To(MatchError(ContainSubstring("invalid PlaylistsPath"))) + } + }, + Entry("backslash path (issue #6276)", `Playlists\navidrome`, true), + Entry("escaped brackets", `\[Mix\]`, true), + Entry("backslash before a bracket range", `Playlists\[ab]`, true), + Entry("unterminated character class", `[Mix`, false), + Entry("unterminated brace alternatives", `{rock,jazz`, false), + Entry("backslash before brace alternatives", `Playlists\{rock,jazz}`, runtime.GOOS == "windows"), + Entry("backslash before an unterminated class", `Playlists\[Mix`, runtime.GOOS != "windows"), + ) + }) + Describe("MaxImageSize floor", func() { BeforeEach(func() { viper.Reset() From 55986a4daa9d94acb671e97ca7a2c4abe5ee0195 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 7 Oct 2026 10:14:32 -0400 Subject: [PATCH 4/5] fix(scanner): let quick scans update num_playlists when PlaylistsPath changes (#6276) A quick scan only rewrites a folder when its hash changes, and the hash did not depend on PlaylistsPath. Folders indexed while their playlists were ignored (the Windows bug in #6276, or an older PlaylistsPath) kept num_playlists = 0 when a cover image or a subfolder kept their row, so their playlists were never imported without a full scan. The hash now includes whether PlaylistsPath includes the folder, only for folders with playlist files, so other folders keep their hash. Each folder with playlists is rescanned once after upgrading. Tests scan real folders: recovery on an unchanged quick scan (playlist only, cover image, subfolder), full scan, touched playlist, a folder that stays excluded, one that becomes excluded, and a folder without playlists that is not rescanned. --- scanner/folder_entry.go | 4 + scanner/folder_entry_test.go | 18 +++++ scanner/scanner_playlists_path_test.go | 103 +++++++++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/scanner/folder_entry.go b/scanner/folder_entry.go index e7eef223c..a3f51790c 100644 --- a/scanner/folder_entry.go +++ b/scanner/folder_entry.go @@ -113,6 +113,10 @@ func (f *folderEntry) hash() string { f.numSubFolders, f.imagesUpdatedAt.UTC(), ) + // Lets a quick scan update num_playlists when PlaylistsPath starts or stops including the folder + if len(f.playlistFiles) > 0 { + _, _ = fmt.Fprintf(h, ":%t", playlists.InPath(*model.NewFolder(f.job.lib, f.path))) + } // Sort the keys of audio, image and playlist files to ensure consistent hashing audioKeys := slices.Collect(maps.Keys(f.audioFiles)) diff --git a/scanner/folder_entry_test.go b/scanner/folder_entry_test.go index 4493f9309..9590430cd 100644 --- a/scanner/folder_entry_test.go +++ b/scanner/folder_entry_test.go @@ -231,6 +231,24 @@ var _ = Describe("folder_entry", func() { Expect(hash1).To(Equal(hash2)) }) + It("produces different hash when PlaylistsPath starts including the folder's playlists", func() { + entry.playlistFiles = map[string]fs.DirEntry{"list.nsp": &fakeDirEntry{name: "list.nsp"}} + conf.Server.PlaylistsPath = "other" + excluded := entry.hash() + + conf.Server.PlaylistsPath = "test/folder" + Expect(entry.hash()).ToNot(Equal(excluded)) + }) + + It("keeps the hash of a folder without playlists when PlaylistsPath changes", func() { + entry.audioFiles = map[string]fs.DirEntry{"song.mp3": &fakeDirEntry{name: "song.mp3"}} + conf.Server.PlaylistsPath = "other" + excluded := entry.hash() + + conf.Server.PlaylistsPath = "test/folder" + Expect(entry.hash()).To(Equal(excluded)) + }) + It("produces different hash when audio files change", func() { entry.audioFiles = map[string]fs.DirEntry{ "song1.mp3": &fakeDirEntry{name: "song1.mp3"}, diff --git a/scanner/scanner_playlists_path_test.go b/scanner/scanner_playlists_path_test.go index 0985f4654..5524ca2de 100644 --- a/scanner/scanner_playlists_path_test.go +++ b/scanner/scanner_playlists_path_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "runtime" "slices" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -177,4 +178,106 @@ var _ = Describe("Scanner - PlaylistsPath", Ordered, ContinueOnFailure, func() { Entry("brackets: escaped braces", `\{Mix\}`, []string{"{Mix}"}, []string{"Braces"}), ) }) + + // Folders indexed while their playlists were ignored keep num_playlists = 0. GC purges a folder + // left with nothing else, but a cover image or a subfolder keeps its row. + Describe("recovering folders indexed while their playlists were ignored", func() { + indexIgnoringPlaylists := func() map[string]model.Folder { + GinkgoHelper() + conf.Server.PlaylistsPath = "Music/**" + scan(true) + expectResults(nil, nil) + folders, err := ds.Folder().GetAll(ctx) + Expect(err).ToNot(HaveOccurred()) + stored := map[string]model.Folder{} + for _, f := range folders { + Expect(f.NumPlaylists).To(BeZero()) + stored[path.Join(f.Path, f.Name)] = f + } + return stored + } + + It("recovery: playlist-only folder, unchanged quick scan", func() { + writeNSP("Playlists/navidrome/Rock.nsp", "Rock") + Expect(indexIgnoringPlaylists()).ToNot(HaveKey("Playlists/navidrome"), "GC purged the folder") + + conf.Server.PlaylistsPath = "Playlists/navidrome" + scan(false) + expectResults([]string{"Playlists/navidrome"}, []string{"Rock"}) + }) + + Describe("folder kept by a playlist cover image", func() { + var stored map[string]model.Folder + + BeforeEach(func() { + writeNSP("Playlists/navidrome/Rock.nsp", "Rock") + Expect(os.WriteFile(filepath.Join(libPath, "Playlists", "navidrome", "Rock.jpg"), []byte("synthetic"), 0600)).To(Succeed()) + Expect(os.MkdirAll(filepath.Join(libPath, "Art"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libPath, "Art", "cover.jpg"), []byte("synthetic"), 0600)).To(Succeed()) + stored = indexIgnoringPlaylists() + Expect(stored).To(HaveKey("Playlists/navidrome")) + Expect(stored["Playlists/navidrome"].Path).To(Equal("Playlists")) + Expect(stored["Playlists/navidrome"].ImageFiles).To(ConsistOf("Rock.jpg")) + Expect(stored).To(HaveKey("Art")) + }) + + It("recovery: cover image folder, unchanged quick scan", func() { + conf.Server.PlaylistsPath = "Playlists/navidrome" + scan(false) + expectResults([]string{"Playlists/navidrome"}, []string{"Rock"}) + }) + + It("recovery: cover image folder, full scan", func() { + conf.Server.PlaylistsPath = "Playlists/navidrome" + scan(true) + expectResults([]string{"Playlists/navidrome"}, []string{"Rock"}) + }) + + It("recovery: cover image folder, quick scan after touching the playlist", func() { + conf.Server.PlaylistsPath = "Playlists/navidrome" + later := time.Now().Add(time.Minute) + Expect(os.Chtimes(filepath.Join(libPath, "Playlists", "navidrome", "Rock.nsp"), later, later)).To(Succeed()) + scan(false) + expectResults([]string{"Playlists/navidrome"}, []string{"Rock"}) + }) + + It("recovery: still-excluded folder stays ignored on a quick scan", func() { + conf.Server.PlaylistsPath = "Other/**" + scan(false) + expectResults(nil, nil) + }) + + It("recovery: folder without playlist files is not rescanned", func() { + conf.Server.PlaylistsPath = "Playlists/navidrome" + scan(false) + art, err := ds.Folder().Get(ctx, stored["Art"].ID) + Expect(err).ToNot(HaveOccurred()) + Expect(art.Hash).To(Equal(stored["Art"].Hash)) + Expect(art.UpdateAt).To(BeTemporally("==", stored["Art"].UpdateAt)) + }) + + It("recovery: quick scan stops counting playlists PlaylistsPath no longer includes", func() { + conf.Server.PlaylistsPath = "Playlists/navidrome" + scan(false) + expectResults([]string{"Playlists/navidrome"}, []string{"Rock"}) + + conf.Server.PlaylistsPath = "Other/**" + scan(false) + // Already imported playlists stay; only the folder count changes + expectResults(nil, []string{"Rock"}) + }) + }) + + It("recovery: parent folder with a subfolder, unchanged quick scan", func() { + writeNSP("Playlists/navidrome/Rock.nsp", "Rock") + writeNSP("Playlists/navidrome/Deep/Nested.nsp", "Nested") + stored := indexIgnoringPlaylists() + Expect(stored).To(HaveKey("Playlists/navidrome"), "kept as the parent of Deep") + Expect(stored).ToNot(HaveKey("Playlists/navidrome/Deep"), "GC purged the leaf") + + conf.Server.PlaylistsPath = "Playlists/navidrome/**" + scan(false) + expectResults([]string{"Playlists/navidrome", "Playlists/navidrome/Deep"}, []string{"Rock", "Nested"}) + }) + }) }) From 7db352f7daf8c4dc0907e9c4642c9c4e3940118d Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 7 Oct 2026 10:15:51 -0400 Subject: [PATCH 5/5] ci: validate the #6276 follow-ups on Windows (branch-only, not for merge) --- .github/fix-issue-6276/check-report.ps1 | 59 ++++++++++++++++++ .github/workflows/fix-issue-6276-windows.yml | 65 ++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 .github/fix-issue-6276/check-report.ps1 create mode 100644 .github/workflows/fix-issue-6276-windows.yml diff --git a/.github/fix-issue-6276/check-report.ps1 b/.github/fix-issue-6276/check-report.ps1 new file mode 100644 index 000000000..e5f371891 --- /dev/null +++ b/.github/fix-issue-6276/check-report.ps1 @@ -0,0 +1,59 @@ +# CI-only check for the #6276 follow-ups. Every #6276 spec must have run and passed: it fails on +# any missing, skipped or failed spec in the groups below, and writes a summary to the job. +param( + [Parameter(Mandatory)][string]$TestOutcome, + [Parameter(Mandatory)][string[]]$Reports +) +$ErrorActionPreference = 'Stop' + +# group (container text, or a spec name) = expected number of specs +$groups = [ordered]@{ + 'PlaylistsPathPatterns' = 19 # conf: Windows/Unix pattern conversion + 'PlaylistsPath validation' = 7 # conf: conf.LoadFromFile + 'InPath' = 35 # core/playlists: PR #6281 specs + scanner-built folders + 'Scanner - PlaylistsPath' = 22 # scanner: real folders, phase 1 + phase 4, recovery + "produces different hash when PlaylistsPath starts including the folder's playlists" = 1 + 'keeps the hash of a folder without playlists when PlaylistsPath changes' = 1 +} + +$specs = @() +$skipped = @() +foreach ($r in $Reports) { + if (-not (Test-Path $r)) { throw "Missing Ginkgo report: $r" } + foreach ($suite in (Get-Content $r -Raw | ConvertFrom-Json)) { + foreach ($spec in $suite.SpecReports) { + if ($spec.LeafNodeType -ne 'It') { continue } + $specs += $spec + if ($spec.State -eq 'skipped') { $skipped += "$($spec.ContainerHierarchyTexts -join ' > ') > $($spec.LeafNodeText)" } + } + } +} + +$problems = @() +$rows = @('| Group | Expected | Ran | Passed |', '|---|---|---|---|') +foreach ($g in $groups.Keys) { + $inGroup = @($specs | Where-Object { $_.ContainerHierarchyTexts -contains $g -or $_.LeafNodeText -eq $g }) + $passed = @($inGroup | Where-Object { $_.State -eq 'passed' }) + foreach ($s in ($inGroup | Where-Object { $_.State -ne 'passed' })) { + $problems += "$g > $($s.LeafNodeText): $($s.State)" + } + if ($inGroup.Count -ne $groups[$g]) { $problems += "$g : expected $($groups[$g]) specs, found $($inGroup.Count)" } + $rows += "| $g | $($groups[$g]) | $($inGroup.Count) | $($passed.Count) |" +} +if ($TestOutcome -ne 'success') { $problems += "Test step outcome: $TestOutcome" } + +$total = @($specs).Count +$summary = @("## #6276 follow-ups on $([System.Environment]::OSVersion.VersionString)", '', + "All specs in conf, core/playlists, scanner: $total ($(@($specs | Where-Object State -eq 'passed').Count) passed, $($skipped.Count) skipped). Test step: **$TestOutcome**", '') + $rows +if ($skipped.Count -gt 0) { + $summary += '', 'Skipped (pre-existing Windows skips, outside the #6276 groups):' + $summary += @($skipped | ForEach-Object { "- $_" }) +} +if ($problems.Count -eq 0) { + $summary += '', '**Result: every #6276 spec ran and passed.**' +} else { + $summary += '', '**Result: FAILED**' + $summary += @($problems | ForEach-Object { "- $_" }) +} +$summary -join "`n" | Tee-Object -Append -FilePath $env:GITHUB_STEP_SUMMARY +if ($problems.Count -gt 0) { exit 1 } diff --git a/.github/workflows/fix-issue-6276-windows.yml b/.github/workflows/fix-issue-6276-windows.yml new file mode 100644 index 000000000..651ba0863 --- /dev/null +++ b/.github/workflows/fix-issue-6276-windows.yml @@ -0,0 +1,65 @@ +# CI-only validation of the #6276 follow-ups to PR #6281 on a native Windows runner. Runs the full +# conf, core/playlists and scanner packages, then checks every #6276 spec ran and passed. +name: "Fix #6276: Windows PlaylistsPath" + +on: + push: + branches: + - t3code/fix-issue-6276-pr6281-followups + workflow_dispatch: + +permissions: + contents: read + +jobs: + windows: + name: Windows + runs-on: windows-2022 + timeout-minutes: 45 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version-file: go.mod + + - uses: msys2/setup-msys2@ec48f7c5447b3140e2b088413ae3a55687bccb6e # v2 + with: + msystem: MINGW64 + install: mingw-w64-x86_64-gcc + update: false + + - name: Add mingw64 to PATH + shell: bash + run: echo "C:/msys64/mingw64/bin" >> $GITHUB_PATH + + - name: Build test binaries + shell: bash + env: + CGO_ENABLED: "1" + run: | + go version + go test -count=1 -tags netgo,sqlite_fts5 -run '^$' ./conf/ ./core/playlists/ ./scanner/ + + - name: Test conf, core/playlists, scanner + id: test + shell: bash + env: + CGO_ENABLED: "1" + # Some suites change to the repo root, so use absolute report paths. All packages always run. + run: | + rc=0 + for pkg in conf:./conf/ playlists:./core/playlists/ scanner:./scanner/; do + go test -v -count=1 -timeout 15m -tags netgo,sqlite_fts5 "${pkg#*:}" -ginkgo.no-color \ + -ginkgo.json-report="$GITHUB_WORKSPACE/report-${pkg%%:*}.json" || rc=1 + done + exit $rc + + - name: Check every #6276 spec ran and passed + if: always() && steps.test.outcome != 'skipped' + shell: pwsh + run: > + ./.github/fix-issue-6276/check-report.ps1 -TestOutcome ${{ steps.test.outcome }} + -Reports report-conf.json, report-playlists.json, report-scanner.json