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.
This commit is contained in:
Deluan 2025-06-27 16:26:41 -04:00
commit 19e1830e9d
3 changed files with 26 additions and 23 deletions

View file

@ -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)

View file

@ -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.

View file

@ -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())
})
})