From 19e1830e9d285578f4bda91bba8d210dc955a644 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 27 Jun 2025 16:26:41 -0400 Subject: [PATCH] fix: return error when any musicFolderId is invalid or inaccessible Changed behavior from silently filtering invalid library IDs to returning ErrorDataNotFound (code 70) when any provided musicFolderId parameter is invalid or the user doesn't have access to it. The error message includes the specific library number for better debugging. This affects album/song list endpoints (getAlbumList, getRandomSongs, getSongsByGenre, getStarred) to provide consistent error handling across all Subsonic API endpoints. Updated corresponding tests to expect errors instead of silent filtering. --- server/subsonic/album_lists.go | 22 +++++++++++++++++----- server/subsonic/helpers.go | 11 +++++------ server/subsonic/helpers_test.go | 16 ++++------------ 3 files changed, 26 insertions(+), 23 deletions(-) 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()) }) })