mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
fix(windows): avoid filepath.Rel hang on UNC share roots
On Windows, filepath.Rel never returns when the base is a UNC share root (\\host\share) and the target is the same root with a trailing separator (golang/go#79784, fixed only in Go 1.28 by CL 786100). A user on Windows Server with MusicFolder='\\server\Music' (discussion #6232) sent CPU profiles showing 96% of CPU time in filepath.Rel, called from fromArtistFolder <- resolveArtistFolderPattern <- resolver.resolveArtist. For an album directly under the library root, loadArtistFolder's filepath.Dir returns '\\server\Music\', so the artwork worker spins forever and, because Worker.drain waits for the whole batch, the artist artwork pool stops. Add utils.RelPath, which normalizes a UNC volume root followed only by separators to the volume plus one separator before calling filepath.Rel. Both sides then compare equal and Rel returns '.' before reaching the loop. A bare '\\host\share' target under a deeper base now also gives '..' instead of an error. Drive-letter paths are left unchanged, and on non-Windows systems the helper behaves exactly like filepath.Rel. Use it at every production filepath.Rel call site: artist folder artwork, scanner library-relative paths, the watcher, playlist path resolution and PlaylistsPath matching, and the cache spread FS. Windows-only tests cover the UNC cases, with a timeout so a regression fails instead of hanging CI. Run under Wine, 6 of them fail with plain filepath.Rel (4 by timeout) and all pass with RelPath. The helper can be removed after moving to Go 1.28.
This commit is contained in:
parent
ce484083bf
commit
e42418db77
@ -30,7 +30,7 @@ func fromArtistFolder(ctx context.Context, libFS fs.FS, libPath, artistFolder, p
|
||||
if libFS == nil {
|
||||
return nil, "", fmt.Errorf("artist folder lookup unavailable")
|
||||
}
|
||||
rel, err := filepath.Rel(libPath, artistFolder)
|
||||
rel, err := utils.RelPath(libPath, artistFolder)
|
||||
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
|
||||
return nil, "", fmt.Errorf(`artist folder '%s' is outside library '%s'`, artistFolder, libPath)
|
||||
}
|
||||
|
||||
@ -15,6 +15,7 @@ import (
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
@ -148,7 +149,7 @@ func (r pathResolution) ToQualifiedString() (string, error) {
|
||||
if !r.valid {
|
||||
return "", fmt.Errorf("invalid path resolution")
|
||||
}
|
||||
relativePath, err := filepath.Rel(r.libraryPath, r.absolutePath)
|
||||
relativePath, err := utils.RelPath(r.libraryPath, r.absolutePath)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
@ -15,6 +15,7 @@ import (
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
)
|
||||
|
||||
type Playlists interface {
|
||||
@ -77,7 +78,7 @@ func InPath(folder model.Folder) bool {
|
||||
if conf.Server.PlaylistsPath == "" {
|
||||
return true
|
||||
}
|
||||
rel, _ := filepath.Rel(folder.LibraryPath, folder.AbsolutePath())
|
||||
rel, _ := utils.RelPath(folder.LibraryPath, folder.AbsolutePath())
|
||||
for path := range strings.SplitSeq(conf.Server.PlaylistsPath, string(filepath.ListSeparator)) {
|
||||
if match, _ := doublestar.Match(path, rel); match {
|
||||
return true
|
||||
|
||||
@ -15,6 +15,7 @@ import (
|
||||
"github.com/navidrome/navidrome/core/playlists"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
"github.com/navidrome/navidrome/utils/run"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
@ -65,7 +66,7 @@ func libraryRelativePath(libPath, folderPath string) string {
|
||||
if err != nil {
|
||||
return folderPath
|
||||
}
|
||||
rel, err := filepath.Rel(absLib, folderPath)
|
||||
rel, err := utils.RelPath(absLib, folderPath)
|
||||
if err != nil || !filepath.IsLocal(rel) {
|
||||
return folderPath
|
||||
}
|
||||
|
||||
@ -13,6 +13,7 @@ import (
|
||||
"github.com/navidrome/navidrome/core/storage"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
"github.com/navidrome/navidrome/utils/singleton"
|
||||
)
|
||||
|
||||
@ -245,7 +246,7 @@ func (w *watcher) processLibraryEvents(ctx context.Context, lib *model.Library,
|
||||
log.Debug(ctx, "Watcher stopped due to context cancellation", "libraryID", lib.ID, "name", lib.Name)
|
||||
return nil
|
||||
case path := <-events:
|
||||
path, err := filepath.Rel(absLibPath, path)
|
||||
path, err := utils.RelPath(absLibPath, path)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error getting relative path", "libraryID", lib.ID, "absolutePath", absLibPath, "path", path, err)
|
||||
continue
|
||||
|
||||
3
utils/cache/spread_fs.go
vendored
3
utils/cache/spread_fs.go
vendored
@ -12,6 +12,7 @@ import (
|
||||
"github.com/djherbis/fscache"
|
||||
"github.com/djherbis/stream"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
)
|
||||
|
||||
const completeMarkerSuffix = ".complete"
|
||||
@ -97,7 +98,7 @@ func (sfs *spreadFS) walkDataFiles(visit func(absoluteFilePath string)) error {
|
||||
log.Error("Error loading cache", "dir", sfs.root, err)
|
||||
return nil
|
||||
}
|
||||
path, err := filepath.Rel(sfs.root, absoluteFilePath)
|
||||
path, err := utils.RelPath(sfs.root, absoluteFilePath)
|
||||
if err != nil {
|
||||
return nil //nolint:nilerr
|
||||
}
|
||||
|
||||
@ -40,3 +40,16 @@ func FileExists(path string) bool {
|
||||
_, err := os.Stat(path)
|
||||
return err == nil || !os.IsNotExist(err)
|
||||
}
|
||||
|
||||
// RelPath is filepath.Rel without the UNC share root hang (golang/go#79784). Remove it after moving to Go 1.28.
|
||||
func RelPath(basePath, targPath string) (string, error) {
|
||||
return filepath.Rel(normalizeUNCRoot(basePath), normalizeUNCRoot(targPath))
|
||||
}
|
||||
|
||||
func normalizeUNCRoot(p string) string {
|
||||
vol := filepath.VolumeName(p)
|
||||
if len(vol) > 2 && strings.Trim(p[len(vol):], `\/`) == "" {
|
||||
return vol + string(filepath.Separator)
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
@ -223,3 +223,26 @@ var _ = Describe("FileExists", func() {
|
||||
Expect(result).To(Or(BeTrue(), BeFalse())) // Should not panic
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("RelPath", func() {
|
||||
DescribeTable("returns the same result as filepath.Rel",
|
||||
func(base, target string) {
|
||||
base, target = filepath.FromSlash(base), filepath.FromSlash(target)
|
||||
expected, expectedErr := filepath.Rel(base, target)
|
||||
rel, err := utils.RelPath(base, target)
|
||||
Expect(rel).To(Equal(expected))
|
||||
if expectedErr == nil {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
} else {
|
||||
Expect(err).To(MatchError(expectedErr.Error()))
|
||||
}
|
||||
},
|
||||
Entry("same path", "/music", "/music"),
|
||||
Entry("same path with trailing separator", "/music", "/music/"),
|
||||
Entry("child path", "/music", "/music/artist/album"),
|
||||
Entry("sibling path", "/music/a", "/music/b"),
|
||||
Entry("parent path", "/music/artist", "/music"),
|
||||
Entry("relative paths", "music", "music/artist"),
|
||||
Entry("absolute and relative paths", "/music", "music"),
|
||||
)
|
||||
})
|
||||
|
||||
40
utils/files_windows_test.go
Normal file
40
utils/files_windows_test.go
Normal file
@ -0,0 +1,40 @@
|
||||
package utils_test
|
||||
|
||||
import (
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("RelPath on Windows", func() {
|
||||
type result struct {
|
||||
rel string
|
||||
err error
|
||||
}
|
||||
|
||||
DescribeTable("handles UNC share roots without hanging",
|
||||
func(base, target, expected string) {
|
||||
done := make(chan result, 1)
|
||||
go func() {
|
||||
rel, err := utils.RelPath(base, target)
|
||||
done <- result{rel, err}
|
||||
}()
|
||||
var res result
|
||||
Eventually(done).WithTimeout(5 * time.Second).Should(Receive(&res))
|
||||
Expect(res.err).ToNot(HaveOccurred())
|
||||
Expect(res.rel).To(Equal(expected))
|
||||
},
|
||||
Entry("root and root with trailing separator", `\\server\Music`, `\\server\Music\`, "."),
|
||||
Entry("root with trailing separator and root", `\\server\Music\`, `\\server\Music`, "."),
|
||||
Entry("root and root with many separators", `\\server\Music`, `\\server\Music\\`, "."),
|
||||
Entry("root with forward slashes", `//server/Music`, `//server/Music/`, "."),
|
||||
Entry("root and child folder", `\\server\Music`, `\\server\Music\Artist`, "Artist"),
|
||||
Entry("root with trailing separator and child folder", `\\server\Music\`, `\\server\Music\Artist\Album`, `Artist\Album`),
|
||||
Entry("child folder and root", `\\server\Music\Artist`, `\\server\Music\`, ".."),
|
||||
Entry("child folder and root without trailing separator", `\\server\Music\Artist`, `\\server\Music`, ".."),
|
||||
Entry("root and root with different case", `\\server\Music`, `\\SERVER\music\`, "."),
|
||||
Entry("drive root and child folder", `C:\`, `C:\Music`, "Music"),
|
||||
)
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user