diff --git a/.github/fix-issue-6276/check-report.ps1 b/.github/fix-issue-6276/check-report.ps1 deleted file mode 100644 index e5f371891..000000000 --- a/.github/fix-issue-6276/check-report.ps1 +++ /dev/null @@ -1,59 +0,0 @@ -# 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 deleted file mode 100644 index 651ba0863..000000000 --- a/.github/workflows/fix-issue-6276-windows.yml +++ /dev/null @@ -1,65 +0,0 @@ -# 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 diff --git a/conf/configuration.go b/conf/configuration.go index 3032cabd6..efa9cbf9a 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -816,54 +816,11 @@ 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 _, pattern := range PlaylistsPathPatterns() { - if _, err := doublestar.Match(pattern, ""); err != nil { - return fmt.Errorf("invalid PlaylistsPath %q: %w", pattern, err) + 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) } } return nil diff --git a/conf/configuration_test.go b/conf/configuration_test.go index e62ca3d1b..8c4c8ab86 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -379,78 +379,6 @@ 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("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() diff --git a/conf/export_test.go b/conf/export_test.go index cf074b92b..d1e1a6f99 100644 --- a/conf/export_test.go +++ b/conf/export_test.go @@ -36,5 +36,3 @@ 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 6ed7b8020..25960f0fe 100644 --- a/core/playlists/import_test.go +++ b/core/playlists/import_test.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" "strconv" "strings" "time" @@ -1141,53 +1140,16 @@ var _ = Describe("Playlists - Import", func() { }) It("returns true if folder is in PlaylistsPath", func() { - conf.Server.PlaylistsPath = strings.Join([]string{"other/**", "playlists/**"}, string(filepath.ListSeparator)) + tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)") + conf.Server.PlaylistsPath = "other/**:playlists/**" 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()) @@ -1200,43 +1162,6 @@ 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 7d3b5bca8..c9bc03b97 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -4,8 +4,9 @@ import ( "context" "io" "os" - "path" + "path/filepath" "strconv" + "strings" "github.com/bmatcuk/doublestar/v4" "github.com/deluan/rest" @@ -76,10 +77,9 @@ func InPath(folder model.Folder) bool { if conf.Server.PlaylistsPath == "" { return true } - // 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 { + rel, _ := filepath.Rel(folder.LibraryPath, folder.AbsolutePath()) + for path := range strings.SplitSeq(conf.Server.PlaylistsPath, string(filepath.ListSeparator)) { + if match, _ := doublestar.Match(path, rel); match { return true } } diff --git a/core/storage/local/local.go b/core/storage/local/local.go index 686838565..e2ce00a1b 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -76,6 +76,17 @@ func (lfs *localFS) ResolveSymlink(name string) (string, error) { return filepath.EvalSymlinks(filepath.Join(lfs.root, filepath.FromSlash(name))) } +// ReadLink and Lstat implement fs.ReadLinkFS, so callers can detect symlinks without following them. +var _ fs.ReadLinkFS = (*localFS)(nil) + +func (lfs *localFS) ReadLink(name string) (string, error) { + return fs.ReadLink(lfs.FS, name) +} + +func (lfs *localFS) Lstat(name string) (fs.FileInfo, error) { + return fs.Lstat(lfs.FS, name) +} + func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) { res, err := lfs.extractor.Parse(path...) if err != nil { diff --git a/scanner/folder_entry.go b/scanner/folder_entry.go index a3f51790c..e7eef223c 100644 --- a/scanner/folder_entry.go +++ b/scanner/folder_entry.go @@ -113,10 +113,6 @@ 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 9590430cd..4493f9309 100644 --- a/scanner/folder_entry_test.go +++ b/scanner/folder_entry_test.go @@ -231,24 +231,6 @@ 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 deleted file mode 100644 index 5524ca2de..000000000 --- a/scanner/scanner_playlists_path_test.go +++ /dev/null @@ -1,283 +0,0 @@ -package scanner_test - -import ( - "context" - "os" - "path" - "path/filepath" - "runtime" - "slices" - "time" - - "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"}), - ) - }) - - // 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"}) - }) - }) -}) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 1864a6a44..d39cad5f6 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -43,6 +43,13 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- continue } + // A full walk never descends into symlinked folders when following is disabled, so a + // target reached through one (e.g. a watcher event for a new link) is skipped too. + if !conf.Server.Scanner.FollowSymlinks && isSymlinkedPath(job.fs, folderPath) { + log.Debug(ctx, "Scanner: Skipping symlinked target folder, following is disabled", "path", folderPath) + continue + } + // Create checker and push patterns from root to this folder checker := newIgnoreChecker(job.fs) err = checker.PushAllParents(ctx, folderPath) @@ -225,6 +232,18 @@ func isDirOrSymlinkToDir(fsys fs.FS, baseDir string, dirEnt fs.DirEntry) (bool, return fileInfo.IsDir(), nil } +// isSymlinkedPath returns true if folderPath, or any of its parent folders, is a symbolic link. +// It needs fsys to implement fs.ReadLinkFS, otherwise links are followed and never detected. +func isSymlinkedPath(fsys fs.FS, folderPath string) bool { + for p := path.Clean(folderPath); p != "." && p != "/"; p = path.Dir(p) { + info, err := fs.Lstat(fsys, p) + if err == nil && info.Mode()&fs.ModeSymlink != 0 { + return true + } + } + return false +} + const maxSymlinkHops = 40 // resolveEntryName returns the name to classify the entry by, and whether to diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 43939e5c2..0c2aba3e0 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -4,8 +4,10 @@ import ( "context" "fmt" "io/fs" + "maps" "os" "path/filepath" + "slices" "testing/fstest" "github.com/navidrome/navidrome/conf" @@ -260,6 +262,44 @@ var _ = Describe("walk_dir_tree", func() { // Folders not in targets should remain in lastUpdates Expect(job.lastUpdates).To(HaveKey(model.FolderID(job.lib, "OtherArtist/Album3"))) }) + + // #6292: a watcher event for a new folder symlink makes the link itself a scan target + Context("symlinked target folders (production local storage FS)", func() { + BeforeEach(func() { + libRoot := GinkgoT().TempDir() + Expect(os.MkdirAll(filepath.Join(libRoot, "Mozart", "Album1"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "Mozart", "Album1", "track.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.Symlink("Mozart", filepath.Join(libRoot, "Wolfgang Amadeus Mozart"))).To(Succeed()) + job = &scanJob{fs: newLocalMusicFS(libRoot), lib: model.Library{Path: libRoot}} + }) + + walkTargets := func(targets ...string) map[string]*folderEntry { + results, err := walkDirTree(ctx, job, targets...) + Expect(err).ToNot(HaveOccurred()) + folders := map[string]*folderEntry{} + for folder := range results { + folders[folder.path] = folder + } + return folders + } + + DescribeTable("with FollowSymlinks disabled", + func(target string, expected ...string) { + conf.Server.Scanner.FollowSymlinks = false + Expect(slices.Collect(maps.Keys(walkTargets(target)))).To(ConsistOf(expected)) + }, + Entry("skips a target that is a symlink", "Wolfgang Amadeus Mozart"), + Entry("skips a target under a symlinked folder", "Wolfgang Amadeus Mozart/Album1"), + Entry("walks a regular target", "Mozart", "Mozart", "Mozart/Album1"), + ) + + It("walks a symlinked target when FollowSymlinks is enabled", func() { + conf.Server.Scanner.FollowSymlinks = true + folders := walkTargets("Wolfgang Amadeus Mozart") + Expect(folders).To(HaveKey("Wolfgang Amadeus Mozart/Album1")) + Expect(folders["Wolfgang Amadeus Mozart/Album1"].audioFiles).To(HaveKey("track.mp3")) + }) + }) }) }) @@ -433,8 +473,8 @@ var _ = Describe("walk_dir_tree", func() { }) // Regression for #5752: the production localFS must resolve file symlinks. - // It wraps os.DirFS behind the fs.FS interface, so fs.ReadLink-based - // resolution is not available and full OS-level resolution is required. + // fs.ReadLink-based resolution can't follow targets outside the library + // root, so full OS-level resolution is required. Context("production local storage FS", func() { var libRoot string var musicFS storage.MusicFS @@ -460,12 +500,7 @@ var _ = Describe("walk_dir_tree", func() { Expect(os.Symlink(filepath.Join(pool, "mid.wav"), filepath.Join(libRoot, "evil.wav"))).To(Succeed()) Expect(os.Symlink(filepath.Join(pool, "missing.mp3"), filepath.Join(libRoot, "broken.mp3"))).To(Succeed()) - u, err := storage.LocalPathToURL(libRoot) - Expect(err).ToNot(HaveOccurred()) - s, err := storage.For(u.String()) - Expect(err).ToNot(HaveOccurred()) - musicFS, err = s.FS() - Expect(err).ToNot(HaveOccurred()) + musicFS = newLocalMusicFS(libRoot) }) walkRoot := func() *folderEntry { @@ -700,6 +735,17 @@ func getDirEntry(baseDir, name string) os.DirEntry { panic(fmt.Sprintf("Could not find %s in %s", name, baseDir)) } +// newLocalMusicFS returns the production local storage MusicFS rooted at libRoot +func newLocalMusicFS(libRoot string) storage.MusicFS { + u, err := storage.LocalPathToURL(libRoot) + Expect(err).ToNot(HaveOccurred()) + s, err := storage.For(u.String()) + Expect(err).ToNot(HaveOccurred()) + musicFS, err := s.FS() + Expect(err).ToNot(HaveOccurred()) + return musicFS +} + // mockMusicFS is a mock implementation of the MusicFS interface that supports symlinks type mockMusicFS struct { storage.MusicFS