diff --git a/server/subsonic/album_lists.go b/server/subsonic/album_lists.go index 18c664b6f..56cf469c5 100644 --- a/server/subsonic/album_lists.go +++ b/server/subsonic/album_lists.go @@ -63,7 +63,10 @@ func (api *Router) getAlbumList(r *http.Request) (model.Albums, int64, error) { } // Get optional library IDs from musicFolderId parameter - musicFolderIds, _ := selectedMusicFolderIds(r, false) + musicFolderIds, err := selectedMusicFolderIds(r, false) + if err != nil { + return nil, 0, err + } opts = filter.ApplyLibraryFilter(opts, musicFolderIds) opts.Offset = p.IntOr("offset", 0) @@ -118,7 +121,10 @@ func (api *Router) getStarredItems(r *http.Request) (model.Artists, model.Albums ctx := r.Context() // Get optional library IDs from musicFolderId parameter - musicFolderIds, _ := selectedMusicFolderIds(r, false) + musicFolderIds, err := selectedMusicFolderIds(r, false) + if err != nil { + return nil, nil, nil, err + } // Prepare variables to capture results from parallel execution var artists model.Artists @@ -126,7 +132,7 @@ func (api *Router) getStarredItems(r *http.Request) (model.Artists, model.Albums var mediaFiles model.MediaFiles // Execute all three queries in parallel for better performance - err := run.Parallel( + err = run.Parallel( // Query starred artists func() error { artistOpts := filter.ApplyArtistLibraryFilter(filter.ArtistsByStarred(), musicFolderIds) @@ -226,7 +232,10 @@ func (api *Router) GetRandomSongs(r *http.Request) (*responses.Subsonic, error) toYear := p.IntOr("toYear", 0) // Get optional library IDs from musicFolderId parameter - musicFolderIds, _ := selectedMusicFolderIds(r, false) + musicFolderIds, err := selectedMusicFolderIds(r, false) + if err != nil { + return nil, err + } opts := filter.SongsByRandom(genre, fromYear, toYear) opts = filter.ApplyLibraryFilter(opts, musicFolderIds) @@ -249,7 +258,10 @@ func (api *Router) GetSongsByGenre(r *http.Request) (*responses.Subsonic, error) genre, _ := p.String("genre") // Get optional library IDs from musicFolderId parameter - musicFolderIds, _ := selectedMusicFolderIds(r, false) + musicFolderIds, err := selectedMusicFolderIds(r, false) + if err != nil { + return nil, err + } opts := filter.ByGenre(genre) opts = filter.ApplyLibraryFilter(opts, musicFolderIds) diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index 70a85bc57..f9733bb3f 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -486,7 +486,7 @@ func getUserAccessibleLibraries(ctx context.Context) []model.Library { // selectedMusicFolderIds retrieves the music folder IDs from the request parameters. // If no IDs are provided, it returns all libraries the user has access to (based on the user found in the context). // If the parameter is required and not present, it returns an error. -// Invalid library IDs (those the user doesn't have access to) are silently filtered out. +// If any of the provided library IDs are invalid (don't exist or user doesn't have access), returns ErrorDataNotFound. func selectedMusicFolderIds(r *http.Request, required bool) ([]int, error) { p := req.Params(r) musicFolderIds, err := p.Ints("musicFolderId") @@ -501,14 +501,13 @@ func selectedMusicFolderIds(r *http.Request, required bool) ([]int, error) { accessibleLibraryIds := slice.Map(libraries, func(lib model.Library) int { return lib.ID }) if len(musicFolderIds) > 0 { - // Validate and filter out invalid library IDs - validIds := make([]int, 0, len(musicFolderIds)) + // Validate all provided library IDs - if any are invalid, return an error for _, id := range musicFolderIds { - if slices.Contains(accessibleLibraryIds, id) { - validIds = append(validIds, id) + if !slices.Contains(accessibleLibraryIds, id) { + return nil, newError(responses.ErrorDataNotFound, "Library %d not found or not accessible", id) } } - return validIds, nil + return musicFolderIds, nil } // If no musicFolderId is provided, return all libraries the user has access to. diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 2f388217a..a6508d4bb 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -204,22 +204,14 @@ var _ = Describe("helpers", func() { Expect(ids).To(Equal([]int{2})) // Only valid ID is returned }) - It("should filter out library IDs the user doesn't have access to", func() { + It("should return error when any library ID is not accessible", func() { r := httptest.NewRequest("GET", "/test?musicFolderId=1&musicFolderId=5&musicFolderId=2&musicFolderId=99", nil) r = r.WithContext(ctx) ids, err := selectedMusicFolderIds(r, false) - Expect(err).ToNot(HaveOccurred()) - Expect(ids).To(Equal([]int{1, 2})) // Only accessible libraries (1, 2) are returned, 5 and 99 are filtered out - }) - - It("should return empty slice when user has no access to any of the requested libraries", func() { - r := httptest.NewRequest("GET", "/test?musicFolderId=99&musicFolderId=100", nil) - r = r.WithContext(ctx) - - ids, err := selectedMusicFolderIds(r, false) - Expect(err).ToNot(HaveOccurred()) - Expect(ids).To(Equal([]int{})) // No accessible libraries in the request + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Library 5 not found or not accessible")) + Expect(ids).To(BeNil()) }) })