From dfc69d8db06a6a32450c7ae70de7925df133a80a Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 7 Nov 2025 17:11:07 -0500 Subject: [PATCH] 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. --- core/playlists.go | 36 +++++++++------------------------ core/playlists_internal_test.go | 28 ------------------------- core/playlists_test.go | 4 ++-- 3 files changed, 12 insertions(+), 56 deletions(-) diff --git a/core/playlists.go b/core/playlists.go index e42dd15d2..164221697 100644 --- a/core/playlists.go +++ b/core/playlists.go @@ -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. diff --git a/core/playlists_internal_test.go b/core/playlists_internal_test.go index 922187531..b06a004d2 100644 --- a/core/playlists_internal_test.go +++ b/core/playlists_internal_test.go @@ -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 diff --git a/core/playlists_test.go b/core/playlists_test.go index fb2082e2f..59b027690 100644 --- a/core/playlists_test.go +++ b/core/playlists_test.go @@ -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,