Merge f61d940d68799190e992fd817fc6666039d1075c into a45adbbb2eb5a620173b5c1710017368abfb6be6

This commit is contained in:
Junker der Provinz 2026-10-17 01:30:22 +03:00 committed by GitHub
commit 8ccefa3085
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 270 additions and 16 deletions

View File

@ -50,6 +50,15 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-
continue
}
// Likewise a full walk never enters a link back into the folders above it, so a target
// reached through one (e.g. a watcher event for playlists/music -> ..) is skipped.
dir := newDirRef(job.fs, folderPath)
ancestors, ok := ancestorsOf(job.fs, dir)
if !ok {
log.Debug(ctx, "Scanner: Skipping target folder reached through a symlink cycle", "path", folderPath)
continue
}
// Create checker and push patterns from root to this folder
checker := newIgnoreChecker(job.fs)
err = checker.PushAllParents(ctx, folderPath)
@ -59,7 +68,7 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-
}
// Recursively walk this folder and all its children
err = walkFolder(ctx, job, folderPath, checker, results)
err = walkFolder(ctx, job, dir, checker, results, ancestors)
if utils.IsCtxDone(ctx) {
return
}
@ -73,28 +82,40 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-
return results, nil
}
func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker *IgnoreChecker, results chan<- *folderEntry) error {
// dirRef is a folder to walk. realPath has all symlinks resolved and identifies the folder
// on disk, so a link back into one of its ancestors can be recognized as a cycle.
type dirRef struct {
path string
realPath string
}
func walkFolder(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreChecker, results chan<- *folderEntry, branch map[string]struct{}) error {
// Push patterns for this folder onto the stack
_ = checker.Push(ctx, currentFolder)
_ = checker.Push(ctx, dir.path)
defer checker.Pop() // Pop patterns when leaving this folder
folder, children, err := loadDir(ctx, job, currentFolder, checker)
// branch holds this folder and the ones above it. Only a link back into it is a cycle, two
// sibling links to the same folder are both walked.
branch[dir.realPath] = struct{}{}
defer delete(branch, dir.realPath)
folder, children, err := loadDir(ctx, job, dir, checker, branch)
if err != nil {
log.Warn(ctx, "Scanner: Error loading dir. Skipping", "path", currentFolder, err)
log.Warn(ctx, "Scanner: Error loading dir. Skipping", "path", dir.path, err)
return nil
}
for _, c := range children {
err := walkFolder(ctx, job, c, checker, results)
err := walkFolder(ctx, job, c, checker, results, branch)
if err != nil {
return err
}
}
dir := path.Clean(currentFolder)
log.Trace(ctx, "Scanner: Found directory", " path", dir, "audioFiles", maps.Keys(folder.audioFiles),
cleanPath := path.Clean(dir.path)
log.Trace(ctx, "Scanner: Found directory", " path", cleanPath, "audioFiles", maps.Keys(folder.audioFiles),
"images", maps.Keys(folder.imageFiles), "playlists", len(folder.playlistFiles), "imagesUpdatedAt", folder.imagesUpdatedAt,
"updTime", folder.updTime, "modTime", folder.modTime, "numChildren", len(children))
folder.path = dir
folder.path = cleanPath
folder.elapsed.Start()
select {
@ -105,7 +126,8 @@ func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker
}
}
func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreChecker) (folder *folderEntry, children []string, err error) {
func loadDir(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreChecker, branch map[string]struct{}) (folder *folderEntry, children []dirRef, err error) {
dirPath := dir.path
// Check if directory exists before creating the folder entry
// This is important to avoid removing the folder from lastUpdates if it doesn't exist
dirInfo, err := fs.Stat(job.fs, dirPath)
@ -118,20 +140,20 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC
folder = job.createFolderEntry(dirPath)
folder.modTime = dirInfo.ModTime()
dir, err := job.fs.Open(dirPath)
dirFile, err := job.fs.Open(dirPath)
if err != nil {
log.Warn(ctx, "Scanner: Error in Opening directory", "path", dirPath, err)
return folder, children, err
}
defer dir.Close()
dirFile, ok := dir.(fs.ReadDirFile)
defer dirFile.Close()
readDirFile, ok := dirFile.(fs.ReadDirFile)
if !ok {
log.Error(ctx, "Not a directory", "path", dirPath)
return folder, children, err
}
entries := fullReadDir(ctx, dirFile)
children = make([]string, 0, len(entries))
entries := fullReadDir(ctx, readDirFile)
children = make([]dirRef, 0, len(entries))
for _, entry := range entries {
entryPath := path.Join(dirPath, entry.Name())
if checker.ShouldIgnore(ctx, entryPath) {
@ -151,7 +173,13 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC
continue
}
if isDir && isDirReadable(ctx, job.fs, entryPath) {
children = append(children, entryPath)
child := newChildDirRef(job.fs, dir, entry)
if _, isCycle := branch[child.realPath]; isCycle {
log.Debug(ctx, "Scanner: Skipping symlink pointing back into a folder being scanned",
"path", entryPath, "target", child.realPath)
continue
}
children = append(children, child)
folder.numSubFolders++
} else {
fileInfo, err := entry.Info()
@ -246,6 +274,79 @@ func isSymlinkedPath(fsys fs.FS, folderPath string) bool {
const maxSymlinkHops = 40
// newDirRef returns the reference for a folder a walk starts at, which may itself be a symlink.
func newDirRef(fsys fs.FS, dirPath string) dirRef {
dir := dirRef{path: dirPath, realPath: path.Clean(dirPath)}
dir.realPath = resolveDirPath(fsys, dir)
return dir
}
// ancestorsOf returns the resolved paths of the folders above dir up to the library root, or
// false if dir is only reachable through a link back into one of them.
func ancestorsOf(fsys fs.FS, dir dirRef) (map[string]struct{}, bool) {
ancestors := map[string]struct{}{}
for p := path.Clean(dir.path); p != "." && p != "/"; {
p = path.Dir(p)
realPath := newDirRef(fsys, p).realPath
if _, seen := ancestors[realPath]; seen || realPath == dir.realPath {
return nil, false
}
ancestors[realPath] = struct{}{}
}
return ancestors, true
}
// newChildDirRef returns the reference for a subfolder of dir. Only symlinks need resolving,
// a real subfolder can never point back into its own ancestors.
func newChildDirRef(fsys fs.FS, dir dirRef, entry fs.DirEntry) dirRef {
child := dirRef{
path: path.Join(dir.path, entry.Name()),
realPath: path.Join(dir.realPath, entry.Name()),
}
if entry.Type()&fs.ModeSymlink != 0 {
child.realPath = resolveDirPath(fsys, child)
}
return child
}
// resolveDirPath returns dir's path with all symlinks resolved, or dir.realPath if it can't be
// resolved. Without OS-level resolution the chain is followed inside the FS from dir.realPath.
func resolveDirPath(fsys fs.FS, dir dirRef) string {
if resolver, ok := fsys.(storage.SymlinkResolverFS); ok {
target, err := resolver.ResolveSymlink(dir.path)
if err != nil {
return dir.realPath
}
return filepath.ToSlash(target)
}
target, hops := followSymlinkChain(fsys, dir.realPath)
if hops >= maxSymlinkHops {
return dir.realPath
}
return target
}
// followSymlinkChain follows linkPath with fs.ReadLink and returns the last path reached and
// the hops taken. maxSymlinkHops hops means the chain most likely loops.
func followSymlinkChain(fsys fs.FS, linkPath string) (string, int) {
cur := linkPath
hop := 0
for ; hop < maxSymlinkHops; hop++ {
target, err := fs.ReadLink(fsys, cur)
if err != nil {
break
}
if path.IsAbs(target) {
// Absolute targets are not valid fs.FS paths, so the next ReadLink fails and
// resolution stops here, leaving cur as the final target.
cur = target
} else {
cur = path.Join(path.Dir(cur), target)
}
}
return cur, hop
}
// resolveEntryName returns the name to classify the entry by, and whether to
// consider it at all. Symlinks are resolved to their final target so the caller
// classifies by the target's extension, not the link's name. Returns ok=false

View File

@ -9,6 +9,7 @@ import (
"path/filepath"
"slices"
"testing/fstest"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
@ -152,6 +153,158 @@ var _ = Describe("walk_dir_tree", func() {
)
})
// See #5334
Context("with symlink cycles", func() {
// An undetected cycle walks forever, so each walk gets a deadline and a folder cap
// well above what any of these layouts contains.
const walkTimeout = 3 * time.Second
const maxFolders = 25
walkFS := func(musicFS storage.MusicFS, libPath string, targetFolders ...string) map[string]*folderEntry {
job := &scanJob{fs: musicFS, lib: model.Library{Path: libPath}}
ctx, cancel := context.WithTimeout(GinkgoT().Context(), walkTimeout)
defer cancel()
results, err := walkDirTree(ctx, job, targetFolders...)
Expect(err).ToNot(HaveOccurred())
folders := map[string]*folderEntry{}
for folder := range results {
folders[folder.path] = folder
if len(folders) >= maxFolders {
cancel()
}
}
Expect(ctx.Err()).ToNot(Equal(context.DeadlineExceeded), "the walk never finished: symlink cycle not detected")
return folders
}
walk := func(mapFS fstest.MapFS, targetFolders ...string) map[string]*folderEntry {
return walkFS(&mockMusicFS{FS: mapFS}, "/music", targetFolders...)
}
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
conf.Server.Scanner.FollowSymlinks = true
})
It("skips a symlink pointing to the parent folder", func() {
folders := walk(fstest.MapFS{
"music/track1.mp3": {},
"music/tracks/track2.mp3": {},
"music/tracks/music": {Mode: fs.ModeSymlink, Data: []byte("..")},
})
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music", "music/tracks"))
Expect(folders["music/tracks"].audioFiles).To(HaveLen(1))
})
It("skips a symlink pointing to its own folder", func() {
folders := walk(fstest.MapFS{
"music/track1.mp3": {},
"music/music": {Mode: fs.ModeSymlink, Data: []byte(".")},
})
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music"))
Expect(folders["music"].audioFiles).To(HaveLen(1))
})
It("skips a symlink closing a cycle between two folders", func() {
folders := walk(fstest.MapFS{
"lib/a/track1.mp3": {},
"lib/a/toB": {Mode: fs.ModeSymlink, Data: []byte("../b")},
"lib/b/track2.mp3": {},
"lib/b/toA": {Mode: fs.ModeSymlink, Data: []byte("../a")},
})
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "lib", "lib/a", "lib/a/toB", "lib/b", "lib/b/toA"))
})
It("still follows a symlink to a folder outside the current branch", func() {
folders := walk(fstest.MapFS{
"lib/real/track1.mp3": {},
"lib/alias": {Mode: fs.ModeSymlink, Data: []byte("real")},
})
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "lib", "lib/real", "lib/alias"))
Expect(folders["lib/alias"].audioFiles).To(HaveKey("track1.mp3"))
})
It("does not follow the cycle when symlinks are disabled", func() {
conf.Server.Scanner.FollowSymlinks = false
folders := walk(fstest.MapFS{
"music/track1.mp3": {},
"music/tracks/track2.mp3": {},
"music/tracks/music": {Mode: fs.ModeSymlink, Data: []byte("..")},
})
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music", "music/tracks"))
})
It("skips a symlink back to the library root when walking only the folder holding it", func() {
folders := walk(fstest.MapFS{
"artist/album/track1.mp3": {},
"playlists/best.m3u": {},
"playlists/music": {Mode: fs.ModeSymlink, Data: []byte("..")},
}, "playlists")
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf("playlists"))
})
It("skips a target folder that is a symlink back to the library root", func() {
folders := walk(fstest.MapFS{
"artist/album/track1.mp3": {},
"playlists/music": {Mode: fs.ModeSymlink, Data: []byte("..")},
}, "playlists/music")
Expect(folders).To(BeEmpty())
})
// localFS resolves symlinks at the OS level instead of through fs.ReadLink
Context("production local storage FS", func() {
var musicFS storage.MusicFS
var libRoot string
BeforeEach(func() {
tests.SkipOnWindows("symlink semantics")
// The layout reported in #5334: playlists/music links back to the library root
libRoot = filepath.Join(GinkgoT().TempDir(), "music")
Expect(os.MkdirAll(filepath.Join(libRoot, "artist", "album"), 0755)).To(Succeed())
Expect(os.MkdirAll(filepath.Join(libRoot, "playlists"), 0755)).To(Succeed())
Expect(os.WriteFile(filepath.Join(libRoot, "artist", "album", "track1.mp3"), []byte("AUDIO"), 0600)).To(Succeed())
Expect(os.WriteFile(filepath.Join(libRoot, "playlists", "best.m3u"), []byte("#EXTM3U"), 0600)).To(Succeed())
Expect(os.Symlink("..", filepath.Join(libRoot, "playlists", "music"))).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())
})
It("skips a symlink pointing back into the library", func() {
folders := walkFS(musicFS, libRoot)
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "artist", "artist/album", "playlists"))
Expect(folders["artist/album"].audioFiles).To(HaveKey("track1.mp3"))
Expect(folders["playlists"].playlistFiles).To(HaveKey("best.m3u"))
})
It("skips the same symlink when walking only the folder holding it", func() {
folders := walkFS(musicFS, libRoot, "playlists")
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf("playlists"))
Expect(folders["playlists"].playlistFiles).To(HaveKey("best.m3u"))
})
It("skips target folders reached through the symlink", func() {
Expect(walkFS(musicFS, libRoot, "playlists/music", "playlists/music/artist")).To(BeEmpty())
})
})
})
Context("with target folders", func() {
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())