fix: improve path resolution for cross-library playlists and enhance error handling

Signed-off-by: Deluan <deluan@navidrome.org>

core/playlists.go

@@ -195,12 +195,13 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m
 			}
 			filteredLines = append(filteredLines, line)
 		}
-		pathsWithLibrary, err := s.normalizePaths(ctx, pls, folder, filteredLines)
+		resolvedPaths, err := s.resolvePaths(ctx, folder, filteredLines)
 		if err != nil {
-			log.Warn(ctx, "Error normalizing paths in playlist", "playlist", pls.Name, err)
+			log.Warn(ctx, "Error resolving paths in playlist", "playlist", pls.Name, err)
 			continue
 		}
-		found, err := mediaFileRepository.FindByPaths(pathsWithLibrary)
+
+		found, err := mediaFileRepository.FindByPaths(resolvedPaths)
 		if err != nil {
 			log.Warn(ctx, "Error reading files from DB", "playlist", pls.Name, err)
 			continue
@@ -212,7 +213,7 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m
 			key := fmt.Sprintf("%d:%s", found[idx].LibraryID, normalizePathForComparison(found[idx].Path))
 			existing[key] = idx
 		}
-		for _, path := range pathsWithLibrary {
+		for _, path := range resolvedPaths {
 			// Parse the library-qualified path
 			parts := strings.SplitN(path, ":", 2)
 			// Path is already qualified: "libraryID:path"
@@ -250,8 +251,9 @@ type pathResolution struct {
 	valid        bool
 }
 
-// toLibraryQualifiedPath converts the absolute path to a library-qualified path format: "libraryID:relativePath".
+// ToQualifiedString converts the path resolution to a library-qualified string with forward slashes.
-func (r pathResolution) toLibraryQualifiedPath() (string, error) {
+// Format: "libraryID:relativePath" with forward slashes for path separators.
+func (r pathResolution) ToQualifiedString() (string, error) {
 	if !r.valid {
 		return "", fmt.Errorf("invalid path resolution")
 	}
@@ -259,17 +261,8 @@ func (r pathResolution) toLibraryQualifiedPath() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return fmt.Sprintf("%d:%s", r.libraryID, relativePath), nil
+	// Convert path separators to forward slashes
-}
+	return fmt.Sprintf("%d:%s", r.libraryID, filepath.ToSlash(relativePath)), nil
-
-// newValidResolution creates a valid path resolution.
-func newValidResolution(absolutePath, libraryPath string, libraryID int) pathResolution {
-	return pathResolution{
-		absolutePath: absolutePath,
-		libraryPath:  libraryPath,
-		libraryID:    libraryID,
-		valid:        true,
-	}
 }
 
 // libraryMatcher holds sorted libraries with cleaned paths for efficient path matching.
@@ -315,99 +308,121 @@ func newLibraryMatcher(libs model.Libraries) *libraryMatcher {
 	}
 }
 
-// normalizePaths converts playlist file paths to library-qualified paths (format: "libraryID:relativePath").
+// pathResolver handles path resolution logic for playlist imports.
-// For relative paths, it resolves them to absolute paths first, then determines which
+type pathResolver struct {
-// library they belong to. This allows playlists to reference files across library boundaries.
+	matcher *libraryMatcher
-func (s *playlists) normalizePaths(ctx context.Context, pls *model.Playlist, folder *model.Folder, lines []string) ([]string, error) {
+}
-	matcher, err := s.buildLibraryMatcher(ctx)
+
+// newPathResolver creates a pathResolver with libraries loaded from the datastore.
+func newPathResolver(ctx context.Context, ds model.DataStore) (*pathResolver, error) {
+	matcher, err := buildLibraryMatcher(ctx, ds)
 	if err != nil {
 		return nil, err
 	}
+	return &pathResolver{matcher: matcher}, nil
+}
 
-	res := make([]string, 0, len(lines))
+// buildLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first).
-	for idx, line := range lines {
+func buildLibraryMatcher(ctx context.Context, ds model.DataStore) (*libraryMatcher, error) {
-		resolution := s.resolvePath(line, folder, matcher)
+	libs, err := ds.Library(ctx).GetAll()
-
+	if err != nil {
-		if !resolution.valid {
+		return nil, err
-			log.Warn(ctx, "Path in playlist not found in any library", "path", line, "line", idx)
-			continue
-		}
-
-		qualifiedPath, err := resolution.toLibraryQualifiedPath()
-		if err != nil {
-			log.Debug(ctx, "Error getting library-qualified path", "playlist", pls.Name, "path", line,
-				"libPath", resolution.libraryPath, "filePath", resolution.absolutePath, err)
-			continue
-		}
-
-		res = append(res, qualifiedPath)
 	}
-	// Convert path separators to forward slashes (only the path part, not the library ID prefix)
+	return newLibraryMatcher(libs), nil
-	return slice.Map(res, func(qp string) string {
-		parts := strings.SplitN(qp, ":", 2)
-		if len(parts) == 2 {
-			return parts[0] + ":" + filepath.ToSlash(parts[1])
-		}
-		return filepath.ToSlash(qp)
-	}), nil
 }
 
 // resolvePath determines the absolute path and library path for a playlist entry.
-func (s *playlists) resolvePath(line string, folder *model.Folder, matcher *libraryMatcher) pathResolution {
+func (r *pathResolver) resolvePath(line string, folder *model.Folder) pathResolution {
 	if folder != nil && !filepath.IsAbs(line) {
-		return s.resolveRelativePath(line, folder, matcher)
+		return r.resolveRelativePath(line, folder)
 	}
-	return s.resolveAbsolutePath(line, matcher)
+	return r.resolveAbsolutePath(line)
 }
 
 // resolveRelativePath handles relative paths by converting them to absolute paths
 // and finding their library location. This enables cross-library playlist references.
-func (s *playlists) resolveRelativePath(line string, folder *model.Folder, matcher *libraryMatcher) pathResolution {
+func (r *pathResolver) resolveRelativePath(line string, folder *model.Folder) pathResolution {
 	// Step 1: Resolve relative path to absolute path based on playlist location
 	// Example: playlist at /music/playlists/test.m3u with line "../songs/abc.mp3"
 	//          resolves to /music/songs/abc.mp3
 	absolutePath := filepath.Clean(filepath.Join(folder.AbsolutePath(), line))
 
 	// Step 2: Determine which library this absolute path belongs to
-	libID, libPath := matcher.findLibraryForPath(absolutePath)
+	libID, libPath := r.matcher.findLibraryForPath(absolutePath)
 	if libID != 0 {
-		return newValidResolution(absolutePath, libPath, libID)
+		return pathResolution{
+			absolutePath: absolutePath,
+			libraryPath:  libPath,
+			libraryID:    libID,
+			valid:        true,
+		}
 	}
 
 	// Fallback: Check if it's in the playlist's own library
-	return s.validatePathInLibrary(absolutePath, folder.LibraryPath, folder.LibraryID)
+	return r.validatePathInLibrary(absolutePath, folder.LibraryPath, folder.LibraryID)
 }
 
 // resolveAbsolutePath handles absolute paths by matching them against library paths.
-func (s *playlists) resolveAbsolutePath(line string, matcher *libraryMatcher) pathResolution {
+func (r *pathResolver) resolveAbsolutePath(line string) pathResolution {
 	cleanPath := filepath.Clean(line)
-	libID, libPath := matcher.findLibraryForPath(cleanPath)
+	libID, libPath := r.matcher.findLibraryForPath(cleanPath)
 
 	if libID == 0 {
 		return pathResolution{valid: false}
 	}
-	return newValidResolution(cleanPath, libPath, libID)
+	return pathResolution{
+		absolutePath: cleanPath,
+		libraryPath:  libPath,
+		libraryID:    libID,
+		valid:        true,
+	}
 }
 
 // validatePathInLibrary verifies that an absolute path belongs to the specified library.
 // It rejects paths that escape the library using ".." segments.
-func (s *playlists) validatePathInLibrary(absolutePath, libraryPath string, libraryID int) pathResolution {
+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 newValidResolution(absolutePath, libraryPath, libraryID)
+	return pathResolution{
+		absolutePath: absolutePath,
+		libraryPath:  libraryPath,
+		libraryID:    libraryID,
+		valid:        true,
+	}
 }
 
-// buildLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first).
+// resolvePaths converts playlist file paths to library-qualified paths (format: "libraryID:relativePath").
-func (s *playlists) buildLibraryMatcher(ctx context.Context) (*libraryMatcher, error) {
+// For relative paths, it resolves them to absolute paths first, then determines which
-	libs, err := s.ds.Library(ctx).GetAll()
+// library they belong to. This allows playlists to reference files across library boundaries.
+func (s *playlists) resolvePaths(ctx context.Context, folder *model.Folder, lines []string) ([]string, error) {
+	resolver, err := newPathResolver(ctx, s.ds)
 	if err != nil {
 		return nil, err
 	}
 
-	return newLibraryMatcher(libs), nil
+	results := make([]string, 0, len(lines))
+	for idx, line := range lines {
+		resolution := resolver.resolvePath(line, folder)
+
+		if !resolution.valid {
+			log.Warn(ctx, "Path in playlist not found in any library", "path", line, "line", idx)
+			continue
+		}
+
+		qualifiedPath, err := resolution.ToQualifiedString()
+		if err != nil {
+			log.Debug(ctx, "Error getting library-qualified path", "path", line,
+				"libPath", resolution.libraryPath, "filePath", resolution.absolutePath, err)
+			continue
+		}
+
+		results = append(results, qualifiedPath)
+	}
+
+	return results, nil
 }
 
 func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist) error {

core/playlists_internal_test.go

@@ -13,7 +13,6 @@ import (
 var _ = Describe("buildLibraryMatcher", func() {
 	var ds *tests.MockDataStore
 	var mockLibRepo *tests.MockLibraryRepo
-	var ps *playlists
 	ctx := context.Background()
 
 	BeforeEach(func() {
@@ -21,7 +20,6 @@ var _ = Describe("buildLibraryMatcher", func() {
 		ds = &tests.MockDataStore{
 			MockedLibrary: mockLibRepo,
 		}
-		ps = &playlists{ds: ds}
 	})
 
 	Describe("Longest library path matching", func() {
@@ -33,7 +31,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 3, Path: "/music-classical/opera"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 
 			// Test that longest path matches first and returns correct library ID
@@ -61,7 +59,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 2, Path: "/home/user/music-backup"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 
 			// Test that music-backup library is matched correctly
@@ -81,7 +79,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 2, Path: "/music-classical"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 
 			// Exact library path should match
@@ -98,7 +96,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 4, Path: "/media/audio/classical/baroque"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 
 			testCases := []struct {
@@ -124,7 +122,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 		It("handles empty library list", func() {
 			mockLibRepo.SetData([]model.Library{})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(matcher).ToNot(BeNil())
 
@@ -139,7 +137,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 1, Path: "/music"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 
 			libID, libPath := matcher.findLibraryForPath("/music/track.mp3")
@@ -153,7 +151,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 2, Path: "/music(backup)"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(matcher).ToNot(BeNil())
 
@@ -172,7 +170,7 @@ var _ = Describe("buildLibraryMatcher", func() {
 				{ID: 3, Path: "/abc"},
 			})
 
-			matcher, err := ps.buildLibraryMatcher(ctx)
+			matcher, err := buildLibraryMatcher(ctx, ds)
 			Expect(err).ToNot(HaveOccurred())
 
 			// Verify that longer paths match correctly (not cut off by shorter prefix)
@@ -193,6 +191,248 @@ var _ = Describe("buildLibraryMatcher", func() {
 	})
 })
 
+var _ = Describe("pathResolver", func() {
+	var ds *tests.MockDataStore
+	var mockLibRepo *tests.MockLibraryRepo
+	var resolver *pathResolver
+	ctx := context.Background()
+
+	BeforeEach(func() {
+		mockLibRepo = &tests.MockLibraryRepo{}
+		ds = &tests.MockDataStore{
+			MockedLibrary: mockLibRepo,
+		}
+
+		// Setup test libraries
+		mockLibRepo.SetData([]model.Library{
+			{ID: 1, Path: "/music"},
+			{ID: 2, Path: "/music-classical"},
+			{ID: 3, Path: "/podcasts"},
+		})
+
+		var err error
+		resolver, err = newPathResolver(ctx, ds)
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	Describe("resolvePath", func() {
+		It("resolves absolute paths", func() {
+			resolution := resolver.resolvePath("/music/artist/album/track.mp3", nil)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(1))
+			Expect(resolution.libraryPath).To(Equal("/music"))
+			Expect(resolution.absolutePath).To(Equal("/music/artist/album/track.mp3"))
+		})
+
+		It("resolves relative paths when folder is provided", func() {
+			folder := &model.Folder{
+				Path:        "playlists",
+				LibraryPath: "/music",
+				LibraryID:   1,
+			}
+
+			resolution := resolver.resolvePath("../artist/album/track.mp3", folder)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(1))
+			Expect(resolution.absolutePath).To(Equal("/music/artist/album/track.mp3"))
+		})
+
+		It("returns invalid resolution for paths outside any library", func() {
+			resolution := resolver.resolvePath("/outside/library/track.mp3", nil)
+
+			Expect(resolution.valid).To(BeFalse())
+		})
+	})
+
+	Describe("resolveAbsolutePath", func() {
+		It("resolves path within a library", func() {
+			resolution := resolver.resolveAbsolutePath("/music/track.mp3")
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(1))
+			Expect(resolution.libraryPath).To(Equal("/music"))
+			Expect(resolution.absolutePath).To(Equal("/music/track.mp3"))
+		})
+
+		It("resolves path to the longest matching library", func() {
+			resolution := resolver.resolveAbsolutePath("/music-classical/track.mp3")
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(2))
+			Expect(resolution.libraryPath).To(Equal("/music-classical"))
+		})
+
+		It("returns invalid resolution for path outside libraries", func() {
+			resolution := resolver.resolveAbsolutePath("/videos/movie.mp4")
+
+			Expect(resolution.valid).To(BeFalse())
+		})
+
+		It("cleans the path before matching", func() {
+			resolution := resolver.resolveAbsolutePath("/music//artist/../artist/track.mp3")
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.absolutePath).To(Equal("/music/artist/track.mp3"))
+		})
+	})
+
+	Describe("resolveRelativePath", func() {
+		It("resolves relative path within same library", func() {
+			folder := &model.Folder{
+				Path:        "playlists",
+				LibraryPath: "/music",
+				LibraryID:   1,
+			}
+
+			resolution := resolver.resolveRelativePath("../songs/track.mp3", folder)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(1))
+			Expect(resolution.absolutePath).To(Equal("/music/songs/track.mp3"))
+		})
+
+		It("resolves relative path to different library", func() {
+			folder := &model.Folder{
+				Path:        "playlists",
+				LibraryPath: "/music",
+				LibraryID:   1,
+			}
+
+			// Path goes up and into a different library
+			resolution := resolver.resolveRelativePath("../../podcasts/episode.mp3", folder)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(3))
+			Expect(resolution.libraryPath).To(Equal("/podcasts"))
+		})
+
+		It("uses matcher to find correct library for resolved path", func() {
+			folder := &model.Folder{
+				Path:        "playlists",
+				LibraryPath: "/music",
+				LibraryID:   1,
+			}
+
+			// This relative path resolves to music-classical library
+			resolution := resolver.resolveRelativePath("../../music-classical/track.mp3", folder)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(2))
+			Expect(resolution.libraryPath).To(Equal("/music-classical"))
+		})
+
+		It("returns invalid for relative paths escaping all libraries", func() {
+			folder := &model.Folder{
+				Path:        "playlists",
+				LibraryPath: "/music",
+				LibraryID:   1,
+			}
+
+			resolution := resolver.resolveRelativePath("../../../../etc/passwd", folder)
+
+			Expect(resolution.valid).To(BeFalse())
+		})
+	})
+
+	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
+			folder := &model.Folder{
+				Path:        "playlists",
+				LibraryPath: "/music",
+				LibraryID:   1,
+			}
+
+			// Relative path that goes to /podcasts library
+			resolution := resolver.resolvePath("../../podcasts/show/episode.mp3", folder)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(3), "Should resolve to podcasts library")
+			Expect(resolution.libraryPath).To(Equal("/podcasts"))
+		})
+
+		It("prefers longer library paths when resolving", func() {
+			// Ensure /music-classical is matched instead of /music
+			resolution := resolver.resolvePath("/music-classical/baroque/track.mp3", nil)
+
+			Expect(resolution.valid).To(BeTrue())
+			Expect(resolution.libraryID).To(Equal(2), "Should match /music-classical, not /music")
+		})
+	})
+})
+
+var _ = Describe("pathResolution", func() {
+	Describe("ToQualifiedString", func() {
+		It("converts valid resolution to qualified string with forward slashes", func() {
+			resolution := pathResolution{
+				absolutePath: "/music/artist/album/track.mp3",
+				libraryPath:  "/music",
+				libraryID:    1,
+				valid:        true,
+			}
+
+			qualifiedStr, err := resolution.ToQualifiedString()
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(qualifiedStr).To(Equal("1:artist/album/track.mp3"))
+		})
+
+		It("handles Windows-style paths by converting to forward slashes", func() {
+			resolution := pathResolution{
+				absolutePath: "/music/artist/album/track.mp3",
+				libraryPath:  "/music",
+				libraryID:    2,
+				valid:        true,
+			}
+
+			qualifiedStr, err := resolution.ToQualifiedString()
+
+			Expect(err).ToNot(HaveOccurred())
+			// Should always use forward slashes regardless of OS
+			Expect(qualifiedStr).To(ContainSubstring("2:"))
+			Expect(qualifiedStr).ToNot(ContainSubstring("\\"))
+		})
+
+		It("returns error for invalid resolution", func() {
+			resolution := pathResolution{valid: false}
+
+			_, err := resolution.ToQualifiedString()
+
+			Expect(err).To(HaveOccurred())
+		})
+	})
+})
+
 var _ = Describe("normalizePathForComparison", func() {
 	It("normalizes Unicode characters to NFC form and converts to lowercase", func() {
 		// Test with NFD (decomposed) input - as would come from macOS filesystem