diff --git a/server/jellyfin/annotations.go b/server/jellyfin/annotations.go index 54844ab22..ef79a6868 100644 --- a/server/jellyfin/annotations.go +++ b/server/jellyfin/annotations.go @@ -81,7 +81,7 @@ func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating } func (api *Router) setRating(w http.ResponseWriter, r *http.Request) { - jfRating := req.Params(r).IntOr("Rating", 0) + jfRating := req.Params(r).IntOr("rating", 0) jfRating = min(max(jfRating, 0), 10) // clamp: a client sending e.g. Rating=100 must not write an out-of-domain Navidrome rating api.setItemRating(w, r, jfRating/2) // Jellyfin 0-10 -> Navidrome 0-5 } diff --git a/server/jellyfin/annotations_test.go b/server/jellyfin/annotations_test.go index d11b5a094..cf04e349b 100644 --- a/server/jellyfin/annotations_test.go +++ b/server/jellyfin/annotations_test.go @@ -34,7 +34,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.markFavorite(w, r) + invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var d dto.UserItemDataDto Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed()) @@ -48,7 +48,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.markFavorite(w, r) + invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var d dto.UserItemDataDto Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed()) @@ -63,7 +63,7 @@ var _ = Describe("Annotations", func() { // alice only has access to library 1, but artists aren't gated per-library. r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/ar1", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "ar1") - api.markFavorite(w, r) + invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var d dto.UserItemDataDto Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed()) @@ -77,7 +77,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("DELETE", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.unmarkFavorite(w, r) + invoke(api.unmarkFavorite, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var d dto.UserItemDataDto Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed()) @@ -91,7 +91,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) // only has access to library 1 r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.markFavorite(w, r) + invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(albumRepo.Data["a1"].Starred).To(BeFalse()) }) @@ -102,7 +102,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser()) // only has access to library 1 r = withChiURLParam(r, "itemId", "s1") - api.markFavorite(w, r) + invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(mfRepo.Data["s1"].Starred).To(BeFalse()) }) @@ -111,7 +111,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/missing", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "missing") - api.markFavorite(w, r) + invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) }) @@ -123,7 +123,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=8", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.setRating(w, r) + invoke(api.setRating, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Data["s1"].Rating).To(Equal(4)) var d dto.UserItemDataDto @@ -138,7 +138,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/Items/"+dto.EncodeID("a1")+"/Rating?Rating=10", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.setRating(w, r) + invoke(api.setRating, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(albumRepo.Data["a1"].Rating).To(Equal(5)) }) @@ -149,7 +149,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("DELETE", "/Users/u1/Items/s1/Rating", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.removeRating(w, r) + invoke(api.removeRating, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Data["s1"].Rating).To(Equal(0)) var d dto.UserItemDataDto @@ -163,7 +163,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/Items/"+dto.EncodeID("a1")+"/Rating?Rating=10", nil).WithContext(ctxUser()) // only has access to library 1 r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.setRating(w, r) + invoke(api.setRating, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(albumRepo.Data["a1"].Rating).To(Equal(0)) }) @@ -174,7 +174,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=100", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.setRating(w, r) + invoke(api.setRating, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Data["s1"].Rating).To(Equal(5)) }) @@ -185,7 +185,7 @@ var _ = Describe("Annotations", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=-5", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.setRating(w, r) + invoke(api.setRating, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Data["s1"].Rating).To(Equal(0)) }) diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index affab5fc3..3293fcba3 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -43,6 +43,10 @@ func New(ds model.DataStore, artwork artwork.Artwork, streamer stream.MediaStrea func (api *Router) routes() http.Handler { inner := chi.NewRouter() + // Read every query parameter case-insensitively, like real Jellyfin. Must precede all routes + // so both public and authenticated handlers (and the api_key auth check) see folded keys. + inner.Use(normalizeQueryKeys) + // Public (no auth): handshake + login. inner.Get("/System/Info/Public", api.getPublicSystemInfo) inner.Get("/System/Ping", api.ping) diff --git a/server/jellyfin/browsing.go b/server/jellyfin/browsing.go index a2194c2c4..9bdd28e24 100644 --- a/server/jellyfin/browsing.go +++ b/server/jellyfin/browsing.go @@ -25,12 +25,12 @@ func (api *Router) getAlbumArtists(w http.ResponseWriter, r *http.Request) { func (api *Router) listArtistsByRole(w http.ResponseWriter, r *http.Request, role model.Role) { ctx := r.Context() p := req.Params(r) - opts := model.QueryOptions{Offset: p.IntOr("StartIndex", 0), Max: p.IntOr("Limit", 0)} - applySort(&opts, "MusicArtist", p.StringOr("SortBy", ""), p.StringOr("SortOrder", "")) + opts := model.QueryOptions{Offset: p.IntOr("startindex", 0), Max: p.IntOr("limit", 0)} + applySort(&opts, "MusicArtist", p.StringOr("sortby", ""), p.StringOr("sortorder", "")) - scopeIDs, _ := resolveLibraryScope(ctx, dto.DecodeID(p.StringOr("ParentId", ""))) + scopeIDs, _ := resolveLibraryScope(ctx, dto.DecodeID(p.StringOr("parentid", ""))) - res, err := api.listArtists(ctx, opts, scopeIDs, p.StringOr("SearchTerm", ""), false, role) + res, err := api.listArtists(ctx, opts, scopeIDs, p.StringOr("searchterm", ""), false, role) if err != nil { api.internalError(w, r, err) return diff --git a/server/jellyfin/browsing_test.go b/server/jellyfin/browsing_test.go index c037d363e..7f50355e1 100644 --- a/server/jellyfin/browsing_test.go +++ b/server/jellyfin/browsing_test.go @@ -36,7 +36,7 @@ var _ = Describe("Browsing", func() { ds.Artist(context.Background()).(*tests.MockArtistRepo).SetData(model.Artists{{ID: "ar1", Name: "A"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Artists", nil).WithContext(ctxUser(model.Libraries{{ID: 1}})) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -48,7 +48,7 @@ var _ = Describe("Browsing", func() { ds.Artist(context.Background()).(*tests.MockArtistRepo).SetData(model.Artists{{ID: "ar1", Name: "A"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Artists/AlbumArtists", nil).WithContext(ctxUser(model.Libraries{{ID: 1}})) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -61,7 +61,7 @@ var _ = Describe("Browsing", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Artists", nil).WithContext(ctxUser(libs)) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := artistRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -75,7 +75,7 @@ var _ = Describe("Browsing", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Artists?ParentId=2", nil).WithContext(ctxUser(libs)) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := artistRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -90,7 +90,7 @@ var _ = Describe("Browsing", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}} // no access to library 99 r := httptest.NewRequest("GET", "/Artists?ParentId=99", nil).WithContext(ctxUser(libs)) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := artistRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -104,7 +104,7 @@ var _ = Describe("Browsing", func() { artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Artists?SearchTerm=art", nil).WithContext(ctxUser(model.Libraries{{ID: 1}})) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -116,7 +116,7 @@ var _ = Describe("Browsing", func() { artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Artists?StartIndex=5&Limit=10", nil).WithContext(ctxUser(model.Libraries{{ID: 1}})) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(artistRepo.Options.Offset).To(Equal(5)) Expect(artistRepo.Options.Max).To(Equal(10)) @@ -127,7 +127,7 @@ var _ = Describe("Browsing", func() { artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Artists", nil).WithContext(ctxAdmin()) - api.getArtists(w, r) + invoke(api.getArtists, w, r) Expect(w.Code).To(Equal(http.StatusOK)) // accessibleLibraryIDs is empty for an admin (Libraries is nil), so // ApplyArtistLibraryFilter([]) is a no-op: no library_id restriction is added. @@ -144,7 +144,7 @@ var _ = Describe("Browsing", func() { It("lists genres via /Genres", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Genres", nil).WithContext(ctxUser(model.Libraries{{ID: 1}})) - api.getGenres(w, r) + invoke(api.getGenres, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -154,7 +154,7 @@ var _ = Describe("Browsing", func() { It("handles /MusicGenres the same way", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/MusicGenres", nil).WithContext(ctxUser(model.Libraries{{ID: 1}})) - api.getGenres(w, r) + invoke(api.getGenres, w, r) Expect(w.Code).To(Equal(http.StatusOK)) }) }) diff --git a/server/jellyfin/e2e/browsing_test.go b/server/jellyfin/e2e/browsing_test.go index de4219498..0f8f27e9d 100644 --- a/server/jellyfin/e2e/browsing_test.go +++ b/server/jellyfin/e2e/browsing_test.go @@ -119,6 +119,35 @@ var _ = Describe("Browsing", func() { }) }) + // Jellify (and the official Jellyfin TypeScript SDK) send query params in camelCase + // (parentId, includeItemTypes, albumArtistIds), where Finamp sends PascalCase. Real Jellyfin + // binds them case-insensitively; these guard that our dispatcher does too, and that browsing an + // album with only parentId (no IncludeItemTypes, as Jellify does) returns its tracks. + Describe("camelCase query params (Jellify / JS SDK)", func() { + lib1 := enc("1") + + It("filters albums by camelCase albumArtistIds", func() { + q := queryResult(get("/Items?includeItemTypes=MusicAlbum&recursive=true&parentId=" + lib1 + "&albumArtistIds=" + enc(artistID("The Beatles")))) + Expect(names(q.Items)).To(ConsistOf("Abbey Road", "Help!")) + }) + + It("filters songs by camelCase artistIds", func() { + q := queryResult(get("/Items?includeItemTypes=Audio&recursive=true&parentId=" + lib1 + "&artistIds=" + enc(artistID("The Beatles")))) + Expect(names(q.Items)).To(ConsistOf("Come Together", "Something", "Help!")) + }) + + It("browses an album's tracks with only camelCase parentId (no IncludeItemTypes)", func() { + q := queryResult(get("/Items?parentId=" + enc(albumID("Abbey Road")) + "&sortBy=ParentIndexNumber&sortBy=IndexNumber&sortBy=SortName")) + Expect(q.TotalRecordCount).To(Equal(2)) + Expect(names(q.Items)).To(Equal([]string{"Something", "Come Together"})) + }) + + It("browses an artist's albums with only camelCase parentId (no IncludeItemTypes)", func() { + q := queryResult(get("/Items?parentId=" + enc(artistID("The Beatles")))) + Expect(names(q.Items)).To(ConsistOf("Abbey Road", "Help!")) + }) + }) + Describe("search, batch and pagination", func() { It("searches albums by term", func() { q := queryResult(get("/Items?IncludeItemTypes=MusicAlbum&Recursive=true&SearchTerm=Abbey")) diff --git a/server/jellyfin/images.go b/server/jellyfin/images.go index 7f758f42a..175e1490f 100644 --- a/server/jellyfin/images.go +++ b/server/jellyfin/images.go @@ -24,7 +24,7 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) { // approach core/artwork's cache warmer uses to pre-generate every item's cover. ctx := request.WithUser(r.Context(), model.User{IsAdmin: true}) itemId := dto.DecodeID(chi.URLParam(r, "itemId")) - size, _ := strconv.Atoi(r.URL.Query().Get("maxWidth")) + size, _ := strconv.Atoi(r.URL.Query().Get("maxwidth")) artID := api.resolveArtworkID(ctx, itemId) reader, _, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false) diff --git a/server/jellyfin/items.go b/server/jellyfin/items.go index 278fc3f48..ddc449b34 100644 --- a/server/jellyfin/items.go +++ b/server/jellyfin/items.go @@ -37,39 +37,50 @@ func (api *Router) getItems(w http.ResponseWriter, r *http.Request) { // (IncludeItemTypes=Audio,MusicAlbum,Playlist&Filters=IsFavorite). func (api *Router) queryItems(ctx context.Context, r *http.Request) (dto.QueryResult, error) { p := req.Params(r) + // normalizeQueryKeys has folded every query key to lowercase, so params are read by their + // lowercase name here (matching real Jellyfin's case-insensitive binding). + // // Jellyfin's /Items?ids= is a batch-fetch-by-id that cuts across the type dispatch below - // entirely; Finamp's download/sync uses it to fetch a single track's BaseItemDto. Query - // params are case-sensitive, and clients send both "Ids" and "ids". - if ids := p.StringOr("Ids", p.StringOr("ids", "")); ids != "" { + // entirely; Finamp's download/sync uses it to fetch a single track's BaseItemDto. + if ids := p.StringOr("ids", ""); ids != "" { var items []dto.BaseItemDto - for _, rawID := range strings.Split(ids, ",") { + for rawID := range strings.SplitSeq(ids, ",") { if item, ok := api.resolveItemByID(ctx, dto.DecodeID(strings.TrimSpace(rawID))); ok { items = append(items, item) } } return result(items, len(items), 0), nil } - parentId := dto.DecodeID(p.StringOr("ParentId", "")) - search := p.StringOr("SearchTerm", "") + parentId := dto.DecodeID(p.StringOr("parentid", "")) + search := p.StringOr("searchterm", "") // Clients express "favorites only" two ways: Jellyfin's Filters=IsFavorite, and the standalone // isFavorite=true query param (Finamp's artist "Favourite tracks" widget uses the latter). - favOnly := strings.Contains(p.StringOr("Filters", ""), "IsFavorite") || - p.BoolOr("IsFavorite", false) || p.BoolOr("isFavorite", false) - sortBy := p.StringOr("SortBy", "") - sortOrder := p.StringOr("SortOrder", "") - offset := p.IntOr("StartIndex", 0) - limit := p.IntOr("Limit", 0) - types := parseTypes(p.StringOr("IncludeItemTypes", "")) + favOnly := strings.Contains(p.StringOr("filters", ""), "IsFavorite") || p.BoolOr("isfavorite", false) + sortBy := p.StringOr("sortby", "") + sortOrder := p.StringOr("sortorder", "") + offset := p.IntOr("startindex", 0) + limit := p.IntOr("limit", 0) + rawTypes := p.StringOr("includeitemtypes", "") + types := parseTypes(rawTypes) // An artist's page filters by artist, not by ParentId: Finamp sends ParentId= for // scoping plus AlbumArtistIds/ArtistIds/contributingArtistIds for the artist itself. Without // this, an artist's albums/tracks come back unfiltered (all artists). artistId := firstDecodedID(firstNonEmpty( - p.StringOr("AlbumArtistIds", ""), - p.StringOr("ArtistIds", ""), - p.StringOr("ContributingArtistIds", p.StringOr("contributingArtistIds", "")), + p.StringOr("albumartistids", ""), + p.StringOr("artistids", ""), + p.StringOr("contributingartistids", ""), )) scopeIDs, isLibraryParent := resolveLibraryScope(ctx, parentId) + // When no item type is requested and ParentId is an album, browse into that album's tracks: + // Jellify opens an album with just parentId= (no IncludeItemTypes), and Jellyfin infers + // the child type from the parent. Without this we'd fall back to parseTypes' MusicAlbum default + // and list every album instead. An artist parent keeps that default (browse its albums). + if rawTypes == "" && parentId != "" && !isLibraryParent { + if _, err := api.ds.Album(ctx).Get(parentId); err == nil { + types = []string{"Audio"} + } + } entityParent := parentId // ParentId-as-entity-id (an artist for a MusicAlbum query, an album for an Audio query) only // makes sense when browsing a single type; a multi-type query has no single natural parent @@ -137,9 +148,9 @@ func firstDecodedID(s string) string { // parseTypes returns every recognized entry in the (possibly comma-separated) IncludeItemTypes, // in the order they appear, defaulting to []string{"MusicAlbum"} when nothing is recognized. // The MusicAlbum default makes ParentId= (no explicit type) browse into that artist's -// albums, matching the UserViews -> artists -> albums -> songs hierarchy; callers browsing into -// an album are expected to pass IncludeItemTypes=Audio explicitly, as Finamp and Jellyfin's own -// clients do. +// albums, matching the UserViews -> artists -> albums -> songs hierarchy. Browsing into an album +// with no explicit type (as Jellify does) is handled by queryItems, which infers Audio when +// ParentId names an album. func parseTypes(types string) []string { var recognized []string for t := range strings.SplitSeq(types, ",") { @@ -378,7 +389,7 @@ func (api *Router) deleteItem(w http.ResponseWriter, r *http.Request) { func (api *Router) getLatest(w http.ResponseWriter, r *http.Request) { ctx := r.Context() opts := filter.AlbumsByNewest() - opts.Max = req.Params(r).IntOr("Limit", 20) + opts.Max = req.Params(r).IntOr("limit", 20) opts = filter.ApplyLibraryFilter(opts, accessibleLibraryIDs(ctx)) albums, err := api.ds.Album(ctx).GetAll(opts) if err != nil { diff --git a/server/jellyfin/items_test.go b/server/jellyfin/items_test.go index 0768a1bbf..d5a86a126 100644 --- a/server/jellyfin/items_test.go +++ b/server/jellyfin/items_test.go @@ -49,7 +49,7 @@ var _ = Describe("Items", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}, {ID: "a2", Name: "Two"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&Recursive=true", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -63,7 +63,7 @@ var _ = Describe("Items", func() { ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", AlbumID: "a1"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("a1")+"&IncludeItemTypes=Audio", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.Items).To(HaveLen(1)) @@ -75,7 +75,7 @@ var _ = Describe("Items", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", AlbumArtistID: "ar1"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("ar1")+"&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.Items).To(HaveLen(1)) @@ -89,7 +89,7 @@ var _ = Describe("Items", func() { ds.Artist(context.Background()).(*tests.MockArtistRepo).SetData(model.Artists{{ID: "ar1", Name: "Artist"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicArtist", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.Items).To(HaveLen(1)) @@ -99,7 +99,7 @@ var _ = Describe("Items", func() { It("lists genres when IncludeItemTypes=MusicGenre", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicGenre", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -110,7 +110,7 @@ var _ = Describe("Items", func() { ds.Playlist(context.Background()).(*tests.MockPlaylistRepo).SetData(model.Playlists{{ID: "p1", Name: "My Mix", SongCount: 5}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Playlist", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -125,7 +125,7 @@ var _ = Describe("Items", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio,MusicAlbum", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -143,7 +143,7 @@ var _ = Describe("Items", func() { ds.Playlist(context.Background()).(*tests.MockPlaylistRepo).SetData(model.Playlists{{ID: "p1", Name: "My Mix"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio,MusicAlbum,Playlist&Filters=IsFavorite", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -160,7 +160,7 @@ var _ = Describe("Items", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}, {ID: "a2", Name: "Two"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio,MusicAlbum&StartIndex=1&Limit=2", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -174,7 +174,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&Filters=IsFavorite", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, _, err := albumRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -186,7 +186,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SearchTerm=one", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -198,7 +198,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&StartIndex=5&Limit=10", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(albumRepo.Options.Offset).To(Equal(5)) Expect(albumRepo.Options.Max).To(Equal(10)) @@ -211,7 +211,7 @@ var _ = Describe("Items", func() { ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?Ids="+dto.EncodeID("s1"), nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -226,7 +226,7 @@ var _ = Describe("Items", func() { ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?ids="+dto.EncodeID("a1")+","+dto.EncodeID("s1"), nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -243,7 +243,7 @@ var _ = Describe("Items", func() { ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 2}}) // alice only has access to library 1 w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?Ids="+dto.EncodeID("a1")+","+dto.EncodeID("s1"), nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -259,7 +259,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=PlayCount", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(albumRepo.Options.Sort).To(Equal("play_count")) }) @@ -269,7 +269,7 @@ var _ = Describe("Items", func() { mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=DatePlayed", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Options.Sort).To(Equal("play_date")) }) @@ -279,7 +279,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=DateCreated,SortName", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(albumRepo.Options.Sort).To(Equal("recently_added")) }) @@ -289,7 +289,7 @@ var _ = Describe("Items", func() { mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=Unknown1,Unknown2,SortName", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Options.Sort).To(Equal("title")) }) @@ -299,7 +299,7 @@ var _ = Describe("Items", func() { mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=ParentIndexNumber,IndexNumber,SortName", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Options.Sort).To(Equal("album")) }) @@ -309,7 +309,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=SeriesSortName", nil).WithContext(ctxUser()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(albumRepo.Options.Sort).To(Equal("")) }) @@ -322,7 +322,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := albumRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -336,7 +336,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio", nil).WithContext(ctxUserWithLibraries(libs)) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := mfRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -350,7 +350,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicArtist", nil).WithContext(ctxUserWithLibraries(libs)) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := artistRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -364,7 +364,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("2")+"&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := albumRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -379,7 +379,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}} // no access to library 99 r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("99")+"&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := albumRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) @@ -396,7 +396,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}, {ID: "a2", Name: "Two", LibraryID: 2}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum", nil).WithContext(ctxAdmin()) - api.getItems(w, r) + invoke(api.getItems, w, r) Expect(w.Code).To(Equal(http.StatusOK)) // accessibleLibraryIDs is empty for an admin (Libraries is nil), so // ApplyLibraryFilter([]) is a no-op: no library_id restriction is added. @@ -416,7 +416,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) @@ -428,7 +428,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/missing", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "missing") - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -437,7 +437,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) // only has access to library 1 r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -446,7 +446,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("s1"), nil).WithContext(ctxUser()) // only has access to library 1 r = withChiURLParam(r, "itemId", dto.EncodeID("s1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -455,7 +455,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("a1"), nil).WithContext(ctxAdmin()) // admin, Libraries: nil r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) @@ -469,7 +469,7 @@ var _ = Describe("Items", func() { libs := model.Libraries{{ID: 1, Name: "Music Library"}} r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("1"), nil).WithContext(ctxUserWithLibraries(libs)) r = withChiURLParam(r, "itemId", dto.EncodeID("1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) @@ -485,7 +485,7 @@ var _ = Describe("Items", func() { libs := model.Libraries{{ID: 2, Name: "Other"}} // no access to library 1 r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("1"), nil).WithContext(ctxUserWithLibraries(libs)) r = withChiURLParam(r, "itemId", dto.EncodeID("1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -496,7 +496,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("p1"), nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", dto.EncodeID("p1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) @@ -510,7 +510,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("p1"), nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", dto.EncodeID("p1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -519,7 +519,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("1"), nil).WithContext(ctxAdmin()) r = withChiURLParam(r, "itemId", dto.EncodeID("1")) - api.getItem(w, r) + invoke(api.getItem, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) @@ -534,7 +534,7 @@ var _ = Describe("Items", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Users/u1/Items/Latest", nil).WithContext(ctxUser()) - api.getLatest(w, r) + invoke(api.getLatest, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var items []dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &items)).To(Succeed()) @@ -548,7 +548,7 @@ var _ = Describe("Items", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} r := httptest.NewRequest("GET", "/Users/u1/Items/Latest", nil).WithContext(ctxUserWithLibraries(libs)) - api.getLatest(w, r) + invoke(api.getLatest, w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := albumRepo.Options.Filters.ToSql() Expect(err).NotTo(HaveOccurred()) diff --git a/server/jellyfin/jellyfin_suite_test.go b/server/jellyfin/jellyfin_suite_test.go index 1cf3c5b80..aab9628a0 100644 --- a/server/jellyfin/jellyfin_suite_test.go +++ b/server/jellyfin/jellyfin_suite_test.go @@ -1,6 +1,7 @@ package jellyfin import ( + "net/http" "testing" "github.com/navidrome/navidrome/log" @@ -15,3 +16,10 @@ func TestJellyfinApi(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Jellyfin API Suite") } + +// invoke runs a handler through normalizeQueryKeys, mirroring the router. These unit tests call +// handlers directly (with withChiURLParam for path params) instead of routing, so without this the +// case-insensitive query folding real requests get would be skipped and PascalCase params dropped. +func invoke(h http.HandlerFunc, w http.ResponseWriter, r *http.Request) { + normalizeQueryKeys(h).ServeHTTP(w, r) +} diff --git a/server/jellyfin/middlewares.go b/server/jellyfin/middlewares.go index f3568ae03..81a503c5b 100644 --- a/server/jellyfin/middlewares.go +++ b/server/jellyfin/middlewares.go @@ -3,13 +3,46 @@ package jellyfin import ( "net" "net/http" + "net/url" "regexp" + "strings" "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/request" ) +// normalizeQueryKeys folds every query-parameter key to lowercase so handlers can read params +// case-insensitively, matching real Jellyfin (whose ASP.NET model binding ignores case). Clients +// disagree on casing: Finamp sends PascalCase (ParentId, IncludeItemTypes), while Jellify and the +// official Jellyfin TypeScript SDK send camelCase (parentId, includeItemTypes). A case-sensitive +// read would silently drop one client's filters, sort and paging. The original request is left +// untouched so request logging still shows the casing the client sent; a shallow copy with a +// rewritten query is passed downstream. Handlers therefore read query params by their lowercase +// name. Only keys are folded — values keep their case (e.g. IncludeItemTypes=MusicAlbum). +func normalizeQueryKeys(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + folded := make(url.Values, len(q)) + changed := false + for k, vs := range q { + lk := strings.ToLower(k) + folded[lk] = vs + if lk != k { + changed = true + } + } + if changed { + r2 := *r + u := *r.URL + u.RawQuery = folded.Encode() + r2.URL = &u + r = &r2 + } + next.ServeHTTP(w, r) + }) +} + type embyAuth struct { Client, Device, DeviceId, Version, Token string } @@ -51,11 +84,13 @@ func tokenFromRequest(r *http.Request) string { if t := parseEmbyAuth(r).Token; t != "" { return t } + // Both api_key and ApiKey are used in the wild (Finamp's just_audio engine fetches direct-file + // URLs with ?ApiKey=); normalizeQueryKeys has already folded key case, but the two spellings + // differ by an underscore, not case, so both are still checked. if t := r.URL.Query().Get("api_key"); t != "" { return t } - // Finamp's just_audio engine fetches direct-file URLs with ?ApiKey= (PascalCase). - return r.URL.Query().Get("ApiKey") + return r.URL.Query().Get("apikey") } func (api *Router) authenticate(next http.Handler) http.Handler { diff --git a/server/jellyfin/middlewares_test.go b/server/jellyfin/middlewares_test.go index 1dcb5fd6e..7c4a61976 100644 --- a/server/jellyfin/middlewares_test.go +++ b/server/jellyfin/middlewares_test.go @@ -85,8 +85,34 @@ var _ = Describe("tokenFromRequest", func() { Expect(tokenFromRequest(r)).To(Equal("tok123")) }) - It("accepts the PascalCase ApiKey query param, as sent by Finamp/just_audio", func() { + It("accepts a PascalCase ApiKey query param once normalizeQueryKeys has folded it", func() { r := httptest.NewRequest("GET", "/Items/s1/File?ApiKey=tok123", nil) - Expect(tokenFromRequest(r)).To(Equal("tok123")) + var got string + invoke(func(_ http.ResponseWriter, r *http.Request) { got = tokenFromRequest(r) }, httptest.NewRecorder(), r) + Expect(got).To(Equal("tok123")) + }) +}) + +var _ = Describe("normalizeQueryKeys", func() { + // keyFor runs a request through normalizeQueryKeys and reports the value the handler sees for + // the given (lowercase) key — i.e. what a case-insensitive read would find. + keyFor := func(rawQuery, key string) string { + r := httptest.NewRequest("GET", "/Items?"+rawQuery, nil) + var got string + invoke(func(_ http.ResponseWriter, r *http.Request) { got = r.URL.Query().Get(key) }, httptest.NewRecorder(), r) + return got + } + + It("folds PascalCase (Finamp) and camelCase (Jellify) keys to lowercase", func() { + Expect(keyFor("ParentId=abc", "parentid")).To(Equal("abc")) + Expect(keyFor("parentId=abc", "parentid")).To(Equal("abc")) + }) + + It("leaves values untouched", func() { + Expect(keyFor("IncludeItemTypes=MusicAlbum,Audio", "includeitemtypes")).To(Equal("MusicAlbum,Audio")) + }) + + It("passes already-lowercase keys through unchanged", func() { + Expect(keyFor("container=mp3", "container")).To(Equal("mp3")) }) }) diff --git a/server/jellyfin/playlists.go b/server/jellyfin/playlists.go index e34bd56a8..a749cea75 100644 --- a/server/jellyfin/playlists.go +++ b/server/jellyfin/playlists.go @@ -148,15 +148,6 @@ func splitIds(s string) []string { return strings.Split(s, ",") } -// queryParam reads lower first, falling back to pascal. Real Jellyfin clients (e.g. Finamp) send -// lowercase query params (ids, entryIds); PascalCase is also accepted for robustness. -func queryParam(r *http.Request, lower, pascal string) string { - if v := r.URL.Query().Get(lower); v != "" { - return v - } - return r.URL.Query().Get(pascal) -} - // expandContainerIDs turns the ids a Jellyfin client sends when building a playlist into the // underlying media file ids. Clients populate the id list with containers — albums, artists, // playlists — not just songs, and expect the server to expand each into its tracks, in order. @@ -195,7 +186,7 @@ func (api *Router) songIDs(ctx context.Context, opts model.QueryOptions) []strin func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { ctx := r.Context() id := dto.DecodeID(chi.URLParam(r, "playlistId")) - ids := api.expandContainerIDs(ctx, slice.Map(splitIds(queryParam(r, "ids", "Ids")), dto.DecodeID)) + ids := api.expandContainerIDs(ctx, slice.Map(splitIds(r.URL.Query().Get("ids")), dto.DecodeID)) if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil { http.Error(w, "Not Found", http.StatusNotFound) return @@ -211,7 +202,7 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { func (api *Router) removeFromPlaylist(w http.ResponseWriter, r *http.Request) { ctx := r.Context() id := dto.DecodeID(chi.URLParam(r, "playlistId")) - ids := slice.Map(splitIds(queryParam(r, "entryIds", "EntryIds")), dto.DecodeID) + ids := slice.Map(splitIds(r.URL.Query().Get("entryids")), dto.DecodeID) if err := api.playlists.RemoveTracks(ctx, id, ids); err != nil { http.Error(w, "Not Found", http.StatusNotFound) return diff --git a/server/jellyfin/playlists_test.go b/server/jellyfin/playlists_test.go index df12ed48e..804123003 100644 --- a/server/jellyfin/playlists_test.go +++ b/server/jellyfin/playlists_test.go @@ -132,7 +132,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`{"Name":"Mix","Ids":["s1","s2"]}`)). WithContext(context.Background()) - api.createPlaylist(w, r) + invoke(api.createPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res map[string]string Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -145,7 +145,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`not json`)). WithContext(context.Background()) - api.createPlaylist(w, r) + invoke(api.createPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusBadRequest)) }) @@ -154,7 +154,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`{"Name":"Mix"}`)). WithContext(context.Background()) - api.createPlaylist(w, r) + invoke(api.createPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusInternalServerError)) }) }) @@ -208,7 +208,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists", strings.NewReader(`{"Name":"Mix","Ids":["`+id+`"]}`)). WithContext(ctx) - api.createPlaylist(w, r) + invoke(api.createPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusOK)) } @@ -258,7 +258,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Playlists/pl1", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.getPlaylist(w, r) + invoke(api.getPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.PlaylistInfo Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -272,7 +272,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Playlists/missing", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "missing") - api.getPlaylist(w, r) + invoke(api.getPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) }) @@ -317,17 +317,17 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists/pl1/Items?ids=s1,s2", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.addToPlaylist(w, r) + invoke(api.addToPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(fp.addPlaylistID).To(Equal("pl1")) Expect(fp.addIds).To(Equal([]string{"s1", "s2"})) }) - It("falls back to the PascalCase Ids param", func() { + It("accepts a PascalCase Ids param (case-folded by the middleware)", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists/pl1/Items?Ids=s1,s2", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.addToPlaylist(w, r) + invoke(api.addToPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(fp.addIds).To(Equal([]string{"s1", "s2"})) }) @@ -337,7 +337,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists/pl1/Items?ids=s1", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.addToPlaylist(w, r) + invoke(api.addToPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -345,7 +345,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("POST", "/Playlists/pl1/Items", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.addToPlaylist(w, r) + invoke(api.addToPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(fp.addPlaylistID).To(Equal("pl1")) Expect(fp.addIds).To(BeEmpty()) @@ -357,17 +357,17 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items?entryIds=1,2", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.removeFromPlaylist(w, r) + invoke(api.removeFromPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(fp.removePlaylistID).To(Equal("pl1")) Expect(fp.removeIds).To(Equal([]string{"1", "2"})) }) - It("falls back to the PascalCase EntryIds param", func() { + It("accepts a PascalCase EntryIds param (case-folded by the middleware)", func() { w := httptest.NewRecorder() r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items?EntryIds=1,2", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.removeFromPlaylist(w, r) + invoke(api.removeFromPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(fp.removeIds).To(Equal([]string{"1", "2"})) }) @@ -377,7 +377,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items?entryIds=1", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.removeFromPlaylist(w, r) + invoke(api.removeFromPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -385,7 +385,7 @@ var _ = Describe("Playlists", func() { w := httptest.NewRecorder() r := httptest.NewRequest("DELETE", "/Playlists/pl1/Items", nil).WithContext(context.Background()) r = withChiURLParam(r, "playlistId", "pl1") - api.removeFromPlaylist(w, r) + invoke(api.removeFromPlaylist, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(fp.removePlaylistID).To(Equal("pl1")) Expect(fp.removeIds).To(BeEmpty()) diff --git a/server/jellyfin/sessions.go b/server/jellyfin/sessions.go index 4ed45222a..2e3c51246 100644 --- a/server/jellyfin/sessions.go +++ b/server/jellyfin/sessions.go @@ -26,7 +26,7 @@ func decodeReport(r *http.Request) playbackReport { var body playbackReport _ = json.NewDecoder(r.Body).Decode(&body) if body.ItemId == "" { - body.ItemId = r.URL.Query().Get("ItemId") + body.ItemId = r.URL.Query().Get("itemid") } body.ItemId = dto.DecodeID(body.ItemId) return body diff --git a/server/jellyfin/sessions_test.go b/server/jellyfin/sessions_test.go index a08443e62..35cafe138 100644 --- a/server/jellyfin/sessions_test.go +++ b/server/jellyfin/sessions_test.go @@ -67,7 +67,7 @@ var _ = Describe("Sessions", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Playing", strings.NewReader(`{"ItemId":"s1","PositionTicks":10000000}`))) - api.reportPlaybackStart(w, r) + invoke(api.reportPlaybackStart, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(pt.reported).To(HaveLen(1)) @@ -82,7 +82,7 @@ var _ = Describe("Sessions", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Playing?ItemId=s2", nil)) - api.reportPlaybackStart(w, r) + invoke(api.reportPlaybackStart, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(pt.reported).To(HaveLen(1)) @@ -95,7 +95,7 @@ var _ = Describe("Sessions", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Playing/Progress", strings.NewReader(`{"ItemId":"s1","PositionTicks":20000000,"IsPaused":false}`))) - api.reportPlaybackProgress(w, r) + invoke(api.reportPlaybackProgress, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(pt.reported).To(HaveLen(1)) @@ -107,7 +107,7 @@ var _ = Describe("Sessions", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Playing/Progress", strings.NewReader(`{"ItemId":"s1","PositionTicks":20000000,"IsPaused":true}`))) - api.reportPlaybackProgress(w, r) + invoke(api.reportPlaybackProgress, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(pt.reported).To(HaveLen(1)) @@ -120,7 +120,7 @@ var _ = Describe("Sessions", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Playing/Stopped", strings.NewReader(`{"ItemId":"s1","PositionTicks":600000000}`))) - api.reportPlaybackStopped(w, r) + invoke(api.reportPlaybackStopped, w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) diff --git a/server/jellyfin/stream.go b/server/jellyfin/stream.go index a71495210..43069cd0d 100644 --- a/server/jellyfin/stream.go +++ b/server/jellyfin/stream.go @@ -64,10 +64,10 @@ func (api *Router) streamAudio(w http.ResponseWriter, r *http.Request) { format = "raw" } - bitRate := p.IntOr("audioBitRate", 0) + bitRate := p.IntOr("audiobitrate", 0) if bitRate == 0 { // maxStreamingBitrate is bits/sec by Jellyfin convention; ResolveRequest expects kbps. - bitRate = p.IntOr("maxStreamingBitrate", 0) / 1000 + bitRate = p.IntOr("maxstreamingbitrate", 0) / 1000 } streamReq := api.transcodeDecider.ResolveRequest(ctx, mf, format, bitRate, 0) diff --git a/server/jellyfin/stream_test.go b/server/jellyfin/stream_test.go index 7e6c8b868..8b5f19f6e 100644 --- a/server/jellyfin/stream_test.go +++ b/server/jellyfin/stream_test.go @@ -87,7 +87,7 @@ var _ = Describe("Stream", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Audio/s1/stream", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.streamAudio(w, r) + invoke(api.streamAudio, w, r) Expect(w.Code).To(Equal(http.StatusOK)) Expect(decider.invoked).To(BeTrue()) @@ -102,7 +102,7 @@ var _ = Describe("Stream", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Audio/s1/stream", nil).WithContext(ctxUser()) // only has access to library 1 r = withChiURLParam(r, "itemId", "s1") - api.streamAudio(w, r) + invoke(api.streamAudio, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(decider.invoked).To(BeFalse()) @@ -113,7 +113,7 @@ var _ = Describe("Stream", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Audio/missing/stream", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "missing") - api.streamAudio(w, r) + invoke(api.streamAudio, w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(decider.invoked).To(BeFalse()) @@ -128,7 +128,7 @@ var _ = Describe("Stream", func() { w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Audio/s1/stream", nil).WithContext(ctxUser()) r = withChiURLParam(r, "itemId", "s1") - api.streamAudio(w, r) + invoke(api.streamAudio, w, r) Expect(w.Code).To(Equal(http.StatusInternalServerError)) })