mirror of
https://github.com/navidrome/navidrome.git
synced 2026-04-03 06:41:01 +00:00
refactor: remove unnecessary path validation fallback
Remove validatePathInLibrary function and its fallback logic in resolveRelativePath. The library matcher should always find the correct library, including the playlist's own library. If this fails, we now return an invalid resolution instead of attempting a fallback validation. This simplifies the code by removing redundant validation logic that was masking test setup issues. Also fixes test mock configuration to properly set up library paths that match folder LibraryPath values.
This commit is contained in:
parent
4d55c1b709
commit
dfc69d8db0
@ -341,17 +341,17 @@ func (r *pathResolver) resolveRelativePath(line string, folder *model.Folder) pa
|
||||
|
||||
// Step 2: Determine which library this absolute path belongs to
|
||||
libID, libPath := r.matcher.findLibraryForPath(absolutePath)
|
||||
if libID != 0 {
|
||||
return pathResolution{
|
||||
absolutePath: absolutePath,
|
||||
libraryPath: libPath,
|
||||
libraryID: libID,
|
||||
valid: true,
|
||||
}
|
||||
if libID == 0 {
|
||||
// Path not found in any library - this should not happen as the playlist's
|
||||
// own library should have been matched above
|
||||
return pathResolution{valid: false}
|
||||
}
|
||||
return pathResolution{
|
||||
absolutePath: absolutePath,
|
||||
libraryPath: libPath,
|
||||
libraryID: libID,
|
||||
valid: true,
|
||||
}
|
||||
|
||||
// Fallback: Check if it's in the playlist's own library
|
||||
return r.validatePathInLibrary(absolutePath, folder.LibraryPath, folder.LibraryID)
|
||||
}
|
||||
|
||||
// resolveAbsolutePath handles absolute paths by matching them against library paths.
|
||||
@ -370,22 +370,6 @@ func (r *pathResolver) resolveAbsolutePath(line string) pathResolution {
|
||||
}
|
||||
}
|
||||
|
||||
// validatePathInLibrary verifies that an absolute path belongs to the specified library.
|
||||
// It rejects paths that escape the library using ".." segments.
|
||||
func (r *pathResolver) validatePathInLibrary(absolutePath, libraryPath string, libraryID int) pathResolution {
|
||||
rel, err := filepath.Rel(libraryPath, absolutePath)
|
||||
if err != nil || strings.HasPrefix(rel, "..") {
|
||||
return pathResolution{valid: false}
|
||||
}
|
||||
|
||||
return pathResolution{
|
||||
absolutePath: absolutePath,
|
||||
libraryPath: libraryPath,
|
||||
libraryID: libraryID,
|
||||
valid: true,
|
||||
}
|
||||
}
|
||||
|
||||
// resolvePaths converts playlist file paths to library-qualified paths (format: "libraryID:relativePath").
|
||||
// For relative paths, it resolves them to absolute paths first, then determines which
|
||||
// library they belong to. This allows playlists to reference files across library boundaries.
|
||||
|
||||
@ -335,34 +335,6 @@ var _ = Describe("pathResolver", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("validatePathInLibrary", func() {
|
||||
It("validates path within library", func() {
|
||||
resolution := resolver.validatePathInLibrary("/music/artist/track.mp3", "/music", 1)
|
||||
|
||||
Expect(resolution.valid).To(BeTrue())
|
||||
Expect(resolution.libraryID).To(Equal(1))
|
||||
Expect(resolution.libraryPath).To(Equal("/music"))
|
||||
})
|
||||
|
||||
It("rejects path that escapes library using ..", func() {
|
||||
resolution := resolver.validatePathInLibrary("/music/../etc/passwd", "/music", 1)
|
||||
|
||||
Expect(resolution.valid).To(BeFalse())
|
||||
})
|
||||
|
||||
It("rejects path outside library", func() {
|
||||
resolution := resolver.validatePathInLibrary("/podcasts/track.mp3", "/music", 1)
|
||||
|
||||
Expect(resolution.valid).To(BeFalse())
|
||||
})
|
||||
|
||||
It("accepts path exactly at library root", func() {
|
||||
resolution := resolver.validatePathInLibrary("/music", "/music", 1)
|
||||
|
||||
Expect(resolution.valid).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Cross-library resolution scenarios", func() {
|
||||
It("handles playlist in library A referencing file in library B", func() {
|
||||
// Playlist is in /music/playlists
|
||||
|
||||
@ -33,8 +33,6 @@ var _ = Describe("Playlists", func() {
|
||||
MockedLibrary: mockLibRepo,
|
||||
}
|
||||
ctx = request.WithUser(ctx, model.User{ID: "123"})
|
||||
// Path should be libPath, but we want to match the root folder referenced in the m3u, which is `/`
|
||||
mockLibRepo.SetData([]model.Library{{ID: 1, Path: "/"}})
|
||||
})
|
||||
|
||||
Describe("ImportFile", func() {
|
||||
@ -43,6 +41,8 @@ var _ = Describe("Playlists", func() {
|
||||
ps = core.NewPlaylists(ds)
|
||||
ds.MockedMediaFile = &mockedMediaFileRepo{}
|
||||
libPath, _ := os.Getwd()
|
||||
// Set up library with the actual library path that matches the folder
|
||||
mockLibRepo.SetData([]model.Library{{ID: 1, Path: libPath}})
|
||||
folder = &model.Folder{
|
||||
ID: "1",
|
||||
LibraryID: 1,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user