nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-18 07:30:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(scanner): skip symlinks back into the ancestors of a targeted folder

Browse Source 
A scan of a single folder started its branch at that folder, so a link
like playlists/music -> .. walked the whole library again. Seed the
branch with the resolved paths of the folders above the target.

Also shortens the comments around the cycle check.

Signed-off-by: junkerderprovinz <jdp@braethoria.com>
This commit is contained in:
junkerderprovinz 2026-10-09 20:55:05 +02:00
parent 582e333400
commit 09249ed1c6
2 changed files with 57 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
scanner/walk_dir_tree.go
View File

@ -59,7 +59,7 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-
}
// Recursively walk this folder and all its children
err = walkFolder(ctx, job, newDirRef(job.fs, folderPath), checker, results, map[string]struct{}{})
err = walkFolder(ctx, job, newDirRef(job.fs, folderPath), checker, results, ancestorsOf(job.fs, folderPath))
if utils.IsCtxDone(ctx) {
return
}
@ -73,10 +73,8 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-
return results, nil
}
// dirRef is a folder to be walked: its path in the library's filesystem and its resolved
// path, with all symlinks followed. The resolved path is the folder's identity: two paths
// that resolve to the same one are the same folder on disk, which is how symlink cycles
// are detected while walking (see #5334).
// 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
@ -87,10 +85,8 @@ func walkFolder(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreCh
_ = checker.Push(ctx, dir.path)
defer checker.Pop() // Pop patterns when leaving this folder
// Keep track of the folders in the current branch, so any symlink pointing back into
// one of them is recognized as a cycle and skipped. Removed again on the way out, so
// two sibling folders linking to the same target are both walked - that is not a
// cycle, just the same folder reached twice.
// 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)
@ -135,8 +131,6 @@ func loadDir(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreCheck
folder = job.createFolderEntry(dirPath)
folder.modTime = dirInfo.ModTime()
// Named dirFile rather than dir: dir is now the folder reference this function
// received, and shadowing it here would silently walk the wrong path.
dirFile, err := job.fs.Open(dirPath)
if err != nil {
log.Warn(ctx, "Scanner: Error in Opening directory", "path", dirPath, err)
@ -171,9 +165,6 @@ func loadDir(ctx context.Context, job *scanJob, dir dirRef, checker *IgnoreCheck
}
if isDir && isDirReadable(ctx, job.fs, entryPath) {
child := newChildDirRef(job.fs, dir, entry)
// A symlink whose target is already in the current branch points back into a
// folder being walked right now: following it would walk the same folders
// forever. Everything else is walked normally.
if _, isCycle := branch[child.realPath]; isCycle {
log.Debug(ctx, "Scanner: Skipping symlink pointing back into a folder being scanned",
"path", entryPath, "target", child.realPath)
@ -274,17 +265,26 @@ func isSymlinkedPath(fsys fs.FS, folderPath string) bool {
const maxSymlinkHops = 40
// newDirRef returns the reference for the folder a walk starts at, resolving it in case
// the folder itself is (or is reached through) a symlink.
// 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
}
// newChildDirRef returns the reference for a subfolder of dir. Only symlinks need to be
// resolved: a real subfolder is always a new folder, it can never point back into one of
// its own ancestors.
// ancestorsOf returns the resolved paths of the folders above folderPath up to the library
// root, so walking only folderPath catches links back into them as a full walk would.
func ancestorsOf(fsys fs.FS, folderPath string) map[string]struct{} {
ancestors := map[string]struct{}{}
for p := path.Clean(folderPath); p != "." && p != "/"; {
p = path.Dir(p)
ancestors[newDirRef(fsys, p).realPath] = struct{}{}
}
return ancestors
}
// 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()),
@ -296,12 +296,8 @@ func newChildDirRef(fsys fs.FS, dir dirRef, entry fs.DirEntry) dirRef {
return child
}
// resolveDirPath returns the path of the folder dir points to, with all symlinks
// resolved. Storages backed by a real filesystem resolve the whole path at the OS level.
// For any other filesystem the symlink chain is followed with fs.ReadLink, starting from
// the parent's already resolved path, so the walk keeps comparing paths in the same
// (FS-relative) space. If the path cannot be resolved it is returned as is, and the
// folder is walked as any other.
// 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)
@ -317,10 +313,8 @@ func resolveDirPath(fsys fs.FS, dir dirRef) string {
return target
}
// followSymlinkChain follows the symlink chain starting at linkPath using fs.ReadLink,
// and returns the last path it could reach, plus the number of hops it took to get there.
// Zero hops means linkPath is not a symlink this filesystem can read; maxSymlinkHops means
// the chain is too long to be resolved and is most likely a loop.
// 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

54
scanner/walk_dir_tree_test.go
View File

@ -153,22 +153,19 @@ var _ = Describe("walk_dir_tree", func() {
)
})
// Regression tests for #5334: a symlink pointing back into a folder that is already
// being walked made the scanner walk the same folders over and over, re-adding the
// same files until the OS hit its recursion/path limits.
// See #5334
Context("with symlink cycles", func() {
// A cycle that is not detected makes the walk run forever, so it gets a deadline
// and a hard cap on the number of folders. All layouts below are small enough to
// be walked instantly, and none of them has more folders than the cap.
// 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) map[string]*folderEntry {
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)
results, err := walkDirTree(ctx, job, targetFolders...)
Expect(err).ToNot(HaveOccurred())
folders := map[string]*folderEntry{}
@ -182,8 +179,8 @@ var _ = Describe("walk_dir_tree", func() {
return folders
}
walk := func(mapFS fstest.MapFS) map[string]*folderEntry {
return walkFS(&mockMusicFS{FS: mapFS}, "/music")
walk := func(mapFS fstest.MapFS, targetFolders ...string) map[string]*folderEntry {
return walkFS(&mockMusicFS{FS: mapFS}, "/music", targetFolders...)
}
BeforeEach(func() {
@ -244,8 +241,17 @@ var _ = Describe("walk_dir_tree", func() {
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "music", "music/tracks"))
})
// The production localFS resolves symlinks at the OS level, a different code path
// than the in-memory filesystem used by the specs above.
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"))
})
// 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
@ -253,12 +259,13 @@ var _ = Describe("walk_dir_tree", func() {
BeforeEach(func() {
tests.SkipOnWindows("symlink semantics")
// The layout reported in #5334: a subfolder linking back to the library root
// 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, "tracks"), 0755)).To(Succeed())
Expect(os.WriteFile(filepath.Join(libRoot, "track1.mp3"), []byte("AUDIO"), 0600)).To(Succeed())
Expect(os.WriteFile(filepath.Join(libRoot, "tracks", "track2.mp3"), []byte("AUDIO"), 0600)).To(Succeed())
Expect(os.Symlink("..", filepath.Join(libRoot, "tracks", "music"))).To(Succeed())
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())
@ -271,9 +278,16 @@ var _ = Describe("walk_dir_tree", func() {
It("skips a symlink pointing back into the library", func() {
folders := walkFS(musicFS, libRoot)
Expect(slices.Collect(maps.Keys(folders))).To(ConsistOf(".", "tracks"))
Expect(folders["."].audioFiles).To(HaveKey("track1.mp3"))
Expect(folders["tracks"].audioFiles).To(HaveKey("track2.mp3"))
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"))
})
})
})