From 1703b59d2450feeefa7f3423fc37bf71cdf31130 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 5 Jul 2026 22:07:32 -0400 Subject: [PATCH] fix(jellyfin): hex-encode item ids at the API boundary for Jellyfin client compatibility Finamp (and presumably other clients) parses ids as radix-16, but Navidrome's base62 nanoids aren't valid hex and crash its queue packing. Hex-encode every id emitted by the Jellyfin API and decode every id received, keeping the transform reversible and stateless at the boundary rather than touching any model id. --- server/jellyfin/annotations.go | 10 +++-- server/jellyfin/annotations_test.go | 16 ++++---- server/jellyfin/browsing.go | 3 +- server/jellyfin/dto/ids.go | 26 +++++++++++++ server/jellyfin/dto/ids_test.go | 35 +++++++++++++++++ server/jellyfin/dto/mappers.go | 27 +++++++------ server/jellyfin/dto/mappers_test.go | 21 ++++++---- server/jellyfin/images.go | 3 +- server/jellyfin/images_test.go | 5 ++- server/jellyfin/items.go | 4 +- server/jellyfin/items_test.go | 60 ++++++++++++++--------------- server/jellyfin/library.go | 2 +- server/jellyfin/playlists.go | 17 ++++---- server/jellyfin/playlists_test.go | 10 ++--- server/jellyfin/sessions.go | 3 ++ server/jellyfin/stream.go | 2 +- server/jellyfin/stream_test.go | 6 +-- server/jellyfin/users_test.go | 6 +-- 18 files changed, 168 insertions(+), 88 deletions(-) create mode 100644 server/jellyfin/dto/ids.go create mode 100644 server/jellyfin/dto/ids_test.go diff --git a/server/jellyfin/annotations.go b/server/jellyfin/annotations.go index f5e904e17..54844ab22 100644 --- a/server/jellyfin/annotations.go +++ b/server/jellyfin/annotations.go @@ -43,7 +43,7 @@ func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id s } func (api *Router) setFavorite(w http.ResponseWriter, r *http.Request, starred bool) { - id := chi.URLParam(r, "itemId") + id := dto.DecodeID(chi.URLParam(r, "itemId")) repo, ok := api.resolveAnnotated(w, r, id) if !ok { return @@ -52,7 +52,8 @@ func (api *Router) setFavorite(w http.ResponseWriter, r *http.Request, starred b api.internalError(w, r, err) return } - api.ok(w, r, &dto.UserItemDataDto{IsFavorite: starred, Key: id, ItemId: id}) + encodedID := dto.EncodeID(id) + api.ok(w, r, &dto.UserItemDataDto{IsFavorite: starred, Key: encodedID, ItemId: encodedID}) } func (api *Router) markFavorite(w http.ResponseWriter, r *http.Request) { api.setFavorite(w, r, true) } @@ -61,7 +62,7 @@ func (api *Router) unmarkFavorite(w http.ResponseWriter, r *http.Request) { } func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating int) { - id := chi.URLParam(r, "itemId") + id := dto.DecodeID(chi.URLParam(r, "itemId")) repo, ok := api.resolveAnnotated(w, r, id) if !ok { return @@ -70,7 +71,8 @@ func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating api.internalError(w, r, err) return } - d := &dto.UserItemDataDto{Key: id, ItemId: id} + encodedID := dto.EncodeID(id) + d := &dto.UserItemDataDto{Key: encodedID, ItemId: encodedID} if rating > 0 { jfRating := float64(rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10, mirrors dto.UserData d.Rating = &jfRating diff --git a/server/jellyfin/annotations_test.go b/server/jellyfin/annotations_test.go index a42033212..d11b5a094 100644 --- a/server/jellyfin/annotations_test.go +++ b/server/jellyfin/annotations_test.go @@ -46,8 +46,8 @@ var _ = Describe("Annotations", func() { albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo) albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/a1", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", "a1") + r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) api.markFavorite(w, r) Expect(w.Code).To(Equal(http.StatusOK)) var d dto.UserItemDataDto @@ -89,8 +89,8 @@ var _ = Describe("Annotations", func() { albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo) albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}}) w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/a1", nil).WithContext(ctxUser()) // only has access to library 1 - r = withChiURLParam(r, "itemId", "a1") + 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) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(albumRepo.Data["a1"].Starred).To(BeFalse()) @@ -136,8 +136,8 @@ var _ = Describe("Annotations", func() { albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo) albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/a1/Rating?Rating=10", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", "a1") + 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) Expect(w.Code).To(Equal(http.StatusOK)) Expect(albumRepo.Data["a1"].Rating).To(Equal(5)) @@ -161,8 +161,8 @@ var _ = Describe("Annotations", func() { albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo) albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}}) w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/a1/Rating?Rating=10", nil).WithContext(ctxUser()) // only has access to library 1 - r = withChiURLParam(r, "itemId", "a1") + 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) Expect(w.Code).To(Equal(http.StatusNotFound)) Expect(albumRepo.Data["a1"].Rating).To(Equal(0)) diff --git a/server/jellyfin/browsing.go b/server/jellyfin/browsing.go index 911f13703..835b02498 100644 --- a/server/jellyfin/browsing.go +++ b/server/jellyfin/browsing.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/utils/req" ) @@ -16,7 +17,7 @@ func (api *Router) getArtists(w http.ResponseWriter, r *http.Request) { 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, p.StringOr("ParentId", "")) + scopeIDs, _ := resolveLibraryScope(ctx, dto.DecodeID(p.StringOr("ParentId", ""))) res, err := api.listArtists(ctx, opts, scopeIDs, p.StringOr("SearchTerm", ""), false) if err != nil { diff --git a/server/jellyfin/dto/ids.go b/server/jellyfin/dto/ids.go new file mode 100644 index 000000000..6ac0596dd --- /dev/null +++ b/server/jellyfin/dto/ids.go @@ -0,0 +1,26 @@ +package dto + +import "encoding/hex" + +// EncodeID renders a Navidrome id as lowercase hex so Jellyfin clients (which parse ids as +// radix-16, e.g. Finamp's queue packing) accept it. Navidrome's base62 nanoids aren't valid +// hex and crash such clients if emitted as-is. +func EncodeID(id string) string { + if id == "" { + return "" + } + return hex.EncodeToString([]byte(id)) +} + +// DecodeID reverses EncodeID. Non-hex input (a raw id, or one that's already been decoded) is +// returned unchanged, so it's safe to call on any inbound id even though every id Navidrome +// itself emits is hex-encoded. +func DecodeID(id string) string { + if id == "" { + return "" + } + if b, err := hex.DecodeString(id); err == nil && len(b) > 0 { + return string(b) + } + return id +} diff --git a/server/jellyfin/dto/ids_test.go b/server/jellyfin/dto/ids_test.go new file mode 100644 index 000000000..26a957604 --- /dev/null +++ b/server/jellyfin/dto/ids_test.go @@ -0,0 +1,35 @@ +package dto + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("id codec", func() { + It("round-trips a base62 nanoid through Encode/Decode", func() { + id := "5QFKvMsJrd57QE2Le2dKKo" + Expect(DecodeID(EncodeID(id))).To(Equal(id)) + }) + + It("passes a raw (non-hex) id through DecodeID unchanged", func() { + Expect(DecodeID("5QFKvMsJrd57QE2Le2dKKo")).To(Equal("5QFKvMsJrd57QE2Le2dKKo")) + }) + + It("produces valid lowercase hex", func() { + encoded := EncodeID("song-1") + Expect(encoded).To(MatchRegexp("^[0-9a-f]+$")) + Expect(encoded).To(HaveLen(len("song-1") * 2)) + }) + + It("round-trips the empty string", func() { + Expect(EncodeID("")).To(Equal("")) + Expect(DecodeID("")).To(Equal("")) + }) + + It("decodes a hex-looking raw id incorrectly only when re-encoded consistently (encode/decode is always internally consistent)", func() { + // "a1" happens to be valid hex on its own; DecodeID can't tell a coincidental hex + // string apart from one we encoded. Callers must always encode ids on emission and + // decode them on receipt so this ambiguity never surfaces in practice. + Expect(DecodeID(EncodeID("a1"))).To(Equal("a1")) + }) +}) diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index 4157b5c2c..0a166ab92 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -14,7 +14,7 @@ func TicksFromSeconds(sec float32) int64 { return int64(float64(sec) * 1e7) } // download dialog reads MediaSources[0].Size from the former. func MediaSourceFromMediaFile(mf model.MediaFile) MediaSourceInfo { return MediaSourceInfo{ - Id: mf.ID, + Id: EncodeID(mf.ID), Protocol: "Http", Container: mf.Suffix, Size: mf.Size, @@ -34,12 +34,15 @@ func MediaSourceFromMediaFile(mf model.MediaFile) MediaSourceInfo { } func UserData(a model.Annotations, itemID string) *UserItemDataDto { + // Callers pass the raw model id; encode here so Key/ItemId match the encoded Id on the + // BaseItemDto this UserData is attached to. + encodedID := EncodeID(itemID) d := &UserItemDataDto{ PlayCount: int(a.PlayCount), IsFavorite: a.Starred, Played: a.PlayCount > 0, - Key: itemID, - ItemId: itemID, + Key: encodedID, + ItemId: encodedID, } if a.Rating > 0 { r := float64(a.Rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10 @@ -55,13 +58,13 @@ func UserData(a model.Annotations, itemID string) *UserItemDataDto { func SongToBaseItem(mf model.MediaFile) BaseItemDto { item := BaseItemDto{ Name: mf.Title, - Id: mf.ID, + Id: EncodeID(mf.ID), Type: "Audio", MediaType: "Audio", IsFolder: false, - ParentId: mf.AlbumID, + ParentId: EncodeID(mf.AlbumID), Album: mf.Album, - AlbumId: mf.AlbumID, + AlbumId: EncodeID(mf.AlbumID), AlbumArtist: mf.AlbumArtist, Artists: []string{mf.Artist}, RunTimeTicks: TicksFromSeconds(mf.Duration), @@ -97,10 +100,10 @@ func SongToBaseItem(mf model.MediaFile) BaseItemDto { func AlbumToBaseItem(al model.Album) BaseItemDto { item := BaseItemDto{ Name: al.Name, - Id: al.ID, + Id: EncodeID(al.ID), Type: "MusicAlbum", IsFolder: true, - ParentId: al.AlbumArtistID, + ParentId: EncodeID(al.AlbumArtistID), AlbumArtist: al.AlbumArtist, Album: al.Name, ChildCount: new(al.SongCount), @@ -111,7 +114,7 @@ func AlbumToBaseItem(al model.Album) BaseItemDto { UserData: UserData(al.Annotations, al.ID), } if al.AlbumArtistID != "" { - item.AlbumArtists = []NameGuidPair{{Name: al.AlbumArtist, Id: al.AlbumArtistID}} + item.AlbumArtists = []NameGuidPair{{Name: al.AlbumArtist, Id: EncodeID(al.AlbumArtistID)}} item.ArtistItems = item.AlbumArtists } if al.MaxYear > 0 { @@ -128,7 +131,7 @@ func AlbumToBaseItem(al model.Album) BaseItemDto { func ArtistToBaseItem(ar model.Artist) BaseItemDto { return BaseItemDto{ Name: ar.Name, - Id: ar.ID, + Id: EncodeID(ar.ID), Type: "MusicArtist", IsFolder: true, AlbumCount: new(ar.AlbumCount), @@ -142,7 +145,7 @@ func ArtistToBaseItem(ar model.Artist) BaseItemDto { func GenreToBaseItem(g model.Genre) BaseItemDto { return BaseItemDto{ Name: g.Name, - Id: g.ID, + Id: EncodeID(g.ID), Type: "MusicGenre", IsFolder: true, BackdropImageTags: []string{}, @@ -155,7 +158,7 @@ func GenreToBaseItem(g model.Genre) BaseItemDto { func PlaylistToBaseItem(p model.Playlist) BaseItemDto { return BaseItemDto{ Name: p.Name, - Id: p.ID, + Id: EncodeID(p.ID), Type: "Playlist", IsFolder: true, MediaType: "Audio", diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index 53ac024e5..ec37351d0 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -21,14 +21,16 @@ var _ = Describe("mappers", func() { item := SongToBaseItem(mf) Expect(item.Type).To(Equal("Audio")) Expect(item.IsFolder).To(BeFalse()) - Expect(item.Id).To(Equal("song-1")) - Expect(item.AlbumId).To(Equal("alb-1")) - Expect(item.ParentId).To(Equal("alb-1")) + Expect(item.Id).To(Equal(EncodeID("song-1"))) + Expect(item.AlbumId).To(Equal(EncodeID("alb-1"))) + Expect(item.ParentId).To(Equal(EncodeID("alb-1"))) Expect(item.RunTimeTicks).To(Equal(int64(600_000_000))) Expect(*item.IndexNumber).To(Equal(3)) Expect(item.UserData.IsFavorite).To(BeTrue()) Expect(item.UserData.PlayCount).To(Equal(2)) Expect(item.UserData.Played).To(BeTrue()) + Expect(item.UserData.Key).To(Equal(EncodeID("song-1"))) + Expect(item.UserData.ItemId).To(Equal(EncodeID("song-1"))) Expect(item.MediaSources).To(HaveLen(1)) Expect(item.MediaSources[0].Size).To(Equal(int64(2_500_000))) }) @@ -36,7 +38,7 @@ var _ = Describe("mappers", func() { It("builds a MediaSourceInfo from a media file", func() { mf := model.MediaFile{ID: "s1", Size: 5242880, Suffix: "mp3", BitRate: 320, Duration: 100} src := MediaSourceFromMediaFile(mf) - Expect(src.Id).To(Equal("s1")) + Expect(src.Id).To(Equal(EncodeID("s1"))) Expect(src.Size).To(Equal(int64(5242880))) Expect(src.Container).To(Equal("mp3")) Expect(src.Bitrate).To(Equal(320_000)) @@ -87,7 +89,11 @@ var _ = Describe("mappers", func() { item := AlbumToBaseItem(al) Expect(item.Type).To(Equal("MusicAlbum")) Expect(item.IsFolder).To(BeTrue()) - Expect(item.ParentId).To(Equal("art-1")) + Expect(item.Id).To(Equal(EncodeID("alb-1"))) + Expect(item.ParentId).To(Equal(EncodeID("art-1"))) + Expect(item.AlbumArtists).To(HaveLen(1)) + Expect(item.AlbumArtists[0].Id).To(Equal(EncodeID("art-1"))) + Expect(item.ArtistItems).To(Equal(item.AlbumArtists)) Expect(*item.ProductionYear).To(Equal(1999)) Expect(*item.ChildCount).To(Equal(10)) }) @@ -97,6 +103,7 @@ var _ = Describe("mappers", func() { item := ArtistToBaseItem(ar) Expect(item.Type).To(Equal("MusicArtist")) Expect(item.IsFolder).To(BeTrue()) + Expect(item.Id).To(Equal(EncodeID("art-1"))) Expect(*item.AlbumCount).To(Equal(2)) }) @@ -105,7 +112,7 @@ var _ = Describe("mappers", func() { item := GenreToBaseItem(g) Expect(item.Type).To(Equal("MusicGenre")) Expect(item.IsFolder).To(BeTrue()) - Expect(item.Id).To(Equal("genre-1")) + Expect(item.Id).To(Equal(EncodeID("genre-1"))) Expect(item.Name).To(Equal("Rock")) }) @@ -114,7 +121,7 @@ var _ = Describe("mappers", func() { item := PlaylistToBaseItem(p) Expect(item.Type).To(Equal("Playlist")) Expect(item.IsFolder).To(BeTrue()) - Expect(item.Id).To(Equal("pl-1")) + Expect(item.Id).To(Equal(EncodeID("pl-1"))) Expect(item.Name).To(Equal("Chill")) Expect(item.MediaType).To(Equal("Audio")) Expect(*item.ChildCount).To(Equal(7)) diff --git a/server/jellyfin/images.go b/server/jellyfin/images.go index 8548bd557..1750e74d2 100644 --- a/server/jellyfin/images.go +++ b/server/jellyfin/images.go @@ -10,11 +10,12 @@ import ( "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" ) func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - itemId := chi.URLParam(r, "itemId") + itemId := dto.DecodeID(chi.URLParam(r, "itemId")) size, _ := strconv.Atoi(r.URL.Query().Get("maxWidth")) artID := api.resolveArtworkID(ctx, itemId) diff --git a/server/jellyfin/images_test.go b/server/jellyfin/images_test.go index cfd9ec6e3..cc89abe04 100644 --- a/server/jellyfin/images_test.go +++ b/server/jellyfin/images_test.go @@ -11,6 +11,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -48,7 +49,7 @@ var _ = Describe("Images", func() { fa := &fakeArtwork{} api := &Router{ds: ds, artwork: fa} - w, r := newImageRequest("a1") + w, r := newImageRequest(dto.EncodeID("a1")) api.getItemImage(w, r) Expect(w.Code).To(Equal(http.StatusOK)) @@ -64,7 +65,7 @@ var _ = Describe("Images", func() { fa := &fakeArtwork{data: png} api := &Router{ds: ds, artwork: fa} - w, r := newImageRequest("a1") + w, r := newImageRequest(dto.EncodeID("a1")) api.getItemImage(w, r) Expect(w.Code).To(Equal(http.StatusOK)) diff --git a/server/jellyfin/items.go b/server/jellyfin/items.go index 7406581b5..34900c983 100644 --- a/server/jellyfin/items.go +++ b/server/jellyfin/items.go @@ -37,7 +37,7 @@ 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) - parentId := p.StringOr("ParentId", "") + parentId := dto.DecodeID(p.StringOr("ParentId", "")) search := p.StringOr("SearchTerm", "") favOnly := strings.Contains(p.StringOr("Filters", ""), "IsFavorite") sortBy := p.StringOr("SortBy", "") @@ -240,7 +240,7 @@ func (api *Router) listPlaylists(ctx context.Context, opts model.QueryOptions, f // libraries. func (api *Router) getItem(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - id := chi.URLParam(r, "itemId") + id := dto.DecodeID(chi.URLParam(r, "itemId")) u, _ := request.UserFrom(ctx) // Finamp resolves a /UserViews entry (Id=library id) by fetching it as a plain item; without // this, the home screen and every library tab 404 trying to probe it as an album/artist/song. diff --git a/server/jellyfin/items_test.go b/server/jellyfin/items_test.go index 025f9af14..7aed12363 100644 --- a/server/jellyfin/items_test.go +++ b/server/jellyfin/items_test.go @@ -62,19 +62,19 @@ var _ = Describe("Items", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}}) ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", AlbumID: "a1"}}) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items?ParentId=a1&IncludeItemTypes=Audio", nil).WithContext(ctxUser()) + r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("a1")+"&IncludeItemTypes=Audio", nil).WithContext(ctxUser()) api.getItems(w, r) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.Items).To(HaveLen(1)) Expect(res.Items[0].Type).To(Equal("Audio")) - Expect(res.Items[0].Id).To(Equal("s1")) + Expect(res.Items[0].Id).To(Equal(dto.EncodeID("s1"))) }) It("lists an artist's albums when ParentId is an artist and type is MusicAlbum", 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=ar1&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUser()) + r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("ar1")+"&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUser()) api.getItems(w, r) var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) @@ -116,7 +116,7 @@ var _ = Describe("Items", func() { Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.Items).To(HaveLen(1)) Expect(res.Items[0].Type).To(Equal("Playlist")) - Expect(res.Items[0].Id).To(Equal("p1")) + Expect(res.Items[0].Id).To(Equal(dto.EncodeID("p1"))) Expect(res.TotalRecordCount).To(Equal(1)) }) @@ -304,7 +304,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}, {ID: 2}} - r := httptest.NewRequest("GET", "/Items?ParentId=2&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) + r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("2")+"&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) api.getItems(w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := albumRepo.Options.Filters.ToSql() @@ -319,7 +319,7 @@ var _ = Describe("Items", func() { albumRepo.SetData(model.Albums{{ID: "a1", Name: "One"}}) w := httptest.NewRecorder() libs := model.Libraries{{ID: 1}} // no access to library 99 - r := httptest.NewRequest("GET", "/Items?ParentId=99&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) + r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID("99")+"&IncludeItemTypes=MusicAlbum", nil).WithContext(ctxUserWithLibraries(libs)) api.getItems(w, r) Expect(w.Code).To(Equal(http.StatusOK)) sql, args, err := albumRepo.Options.Filters.ToSql() @@ -355,13 +355,13 @@ var _ = Describe("Items", func() { It("returns an album by id", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/a1", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", "a1") + r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) - Expect(item.Id).To(Equal("a1")) + Expect(item.Id).To(Equal(dto.EncodeID("a1"))) Expect(item.Type).To(Equal("MusicAlbum")) }) @@ -376,8 +376,8 @@ var _ = Describe("Items", func() { It("returns 404 for an album in a library the user can't access", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}}) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/a1", nil).WithContext(ctxUser()) // only has access to library 1 - r = withChiURLParam(r, "itemId", "a1") + 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) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -385,8 +385,8 @@ var _ = Describe("Items", func() { It("returns 404 for a song in a library the user can't access", func() { ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 2}}) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/s1", nil).WithContext(ctxUser()) // only has access to library 1 - r = withChiURLParam(r, "itemId", "s1") + 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) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -394,13 +394,13 @@ var _ = Describe("Items", func() { It("returns an album to an admin even when it's outside their (empty) Libraries", func() { ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}}) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/a1", nil).WithContext(ctxAdmin()) // admin, Libraries: nil - r = withChiURLParam(r, "itemId", "a1") + 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) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) - Expect(item.Id).To(Equal("a1")) + Expect(item.Id).To(Equal(dto.EncodeID("a1"))) }) // Finamp fetches a /UserViews entry (Id=library id) as a plain item to resolve the @@ -408,13 +408,13 @@ var _ = Describe("Items", func() { It("resolves a library-view id (from /UserViews) as a CollectionFolder item", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1, Name: "Music Library"}} - r := httptest.NewRequest("GET", "/Items/1", nil).WithContext(ctxUserWithLibraries(libs)) - r = withChiURLParam(r, "itemId", "1") + r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("1"), nil).WithContext(ctxUserWithLibraries(libs)) + r = withChiURLParam(r, "itemId", dto.EncodeID("1")) api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) - Expect(item.Id).To(Equal("1")) + Expect(item.Id).To(Equal(dto.EncodeID("1"))) Expect(item.Name).To(Equal("Music Library")) Expect(item.Type).To(Equal("CollectionFolder")) Expect(item.CollectionType).To(Equal("music")) @@ -424,8 +424,8 @@ var _ = Describe("Items", func() { It("does not resolve a library-view id the user has no access to", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 2, Name: "Other"}} // no access to library 1 - r := httptest.NewRequest("GET", "/Items/1", nil).WithContext(ctxUserWithLibraries(libs)) - r = withChiURLParam(r, "itemId", "1") + r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("1"), nil).WithContext(ctxUserWithLibraries(libs)) + r = withChiURLParam(r, "itemId", dto.EncodeID("1")) api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -435,13 +435,13 @@ var _ = Describe("Items", func() { It("resolves a playlist id via the playlists service", func() { fp.getByIDPls = &model.Playlist{ID: "p1", Name: "My Mix", SongCount: 5} w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/p1", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", "p1") + r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("p1"), nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("p1")) api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) - Expect(item.Id).To(Equal("p1")) + Expect(item.Id).To(Equal(dto.EncodeID("p1"))) Expect(item.Name).To(Equal("My Mix")) Expect(item.Type).To(Equal("Playlist")) }) @@ -449,8 +449,8 @@ var _ = Describe("Items", func() { It("returns 404 for a non-owned or absent playlist id", func() { fp.getByIDErr = model.ErrNotFound w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/p1", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", "p1") + r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("p1"), nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("p1")) api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusNotFound)) }) @@ -458,13 +458,13 @@ var _ = Describe("Items", func() { It("resolves a library-view id for an admin even though their Libraries slice is empty", func() { ds.Library(context.Background()).(*tests.MockLibraryRepo).SetData(model.Libraries{{ID: 1, Name: "Music Library"}}) w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/Items/1", nil).WithContext(ctxAdmin()) - r = withChiURLParam(r, "itemId", "1") + r := httptest.NewRequest("GET", "/Items/"+dto.EncodeID("1"), nil).WithContext(ctxAdmin()) + r = withChiURLParam(r, "itemId", dto.EncodeID("1")) api.getItem(w, r) Expect(w.Code).To(Equal(http.StatusOK)) var item dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &item)).To(Succeed()) - Expect(item.Id).To(Equal("1")) + Expect(item.Id).To(Equal(dto.EncodeID("1"))) Expect(item.Name).To(Equal("Music Library")) Expect(item.Type).To(Equal("CollectionFolder")) }) @@ -480,7 +480,7 @@ var _ = Describe("Items", func() { var items []dto.BaseItemDto Expect(json.Unmarshal(w.Body.Bytes(), &items)).To(Succeed()) Expect(items).To(HaveLen(1)) - Expect(items[0].Id).To(Equal("a1")) + Expect(items[0].Id).To(Equal(dto.EncodeID("a1"))) }) It("scopes to the user's accessible libraries", func() { diff --git a/server/jellyfin/library.go b/server/jellyfin/library.go index 170f5e3d0..f4e982f0a 100644 --- a/server/jellyfin/library.go +++ b/server/jellyfin/library.go @@ -39,7 +39,7 @@ func resolveLibraryScope(ctx context.Context, parentId string) (scopeIDs []int, // UserView's id as a plain item to load the library. func libraryView(lib model.Library) dto.BaseItemDto { return dto.BaseItemDto{ - Id: strconv.Itoa(lib.ID), + Id: dto.EncodeID(strconv.Itoa(lib.ID)), Name: lib.Name, Type: "CollectionFolder", CollectionType: "music", diff --git a/server/jellyfin/playlists.go b/server/jellyfin/playlists.go index 3fac0655b..9ff4afef9 100644 --- a/server/jellyfin/playlists.go +++ b/server/jellyfin/playlists.go @@ -26,12 +26,13 @@ func (api *Router) createPlaylist(w http.ResponseWriter, r *http.Request) { http.Error(w, "Bad Request", http.StatusBadRequest) return } - id, err := api.playlists.Create(r.Context(), "", body.Name, body.Ids) + ids := slice.Map(body.Ids, dto.DecodeID) + id, err := api.playlists.Create(r.Context(), "", body.Name, ids) if err != nil { api.internalError(w, r, err) return } - api.ok(w, r, map[string]string{"Id": id}) + api.ok(w, r, map[string]string{"Id": dto.EncodeID(id)}) } // trackToBaseItem maps a playlist entry to a BaseItemDto, tagging it with PlaylistItemId — the @@ -42,7 +43,7 @@ func (api *Router) createPlaylist(w http.ResponseWriter, r *http.Request) { // the same song appears more than once in the playlist. func trackToBaseItem(t model.PlaylistTrack) dto.BaseItemDto { item := dto.SongToBaseItem(t.MediaFile) - item.PlaylistItemId = t.ID + item.PlaylistItemId = dto.EncodeID(t.ID) return item } @@ -51,7 +52,7 @@ func trackToBaseItem(t model.PlaylistTrack) dto.BaseItemDto { // a generic 404 so a playlist id can't be used to probe for the existence of private playlists. func (api *Router) getPlaylistItems(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - id := chi.URLParam(r, "playlistId") + id := dto.DecodeID(chi.URLParam(r, "playlistId")) pls, err := api.playlists.GetWithTracks(ctx, id) if err != nil { http.Error(w, "Not Found", http.StatusNotFound) @@ -83,8 +84,8 @@ func queryParam(r *http.Request, lower, pascal string) string { // ids). Ownership/editability is enforced by AddTracks itself; any error maps to 404. func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - id := chi.URLParam(r, "playlistId") - ids := splitIds(queryParam(r, "ids", "Ids")) + id := dto.DecodeID(chi.URLParam(r, "playlistId")) + ids := slice.Map(splitIds(queryParam(r, "ids", "Ids")), dto.DecodeID) if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil { http.Error(w, "Not Found", http.StatusNotFound) return @@ -99,8 +100,8 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { // any error maps to 404. func (api *Router) removeFromPlaylist(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - id := chi.URLParam(r, "playlistId") - ids := splitIds(queryParam(r, "entryIds", "EntryIds")) + id := dto.DecodeID(chi.URLParam(r, "playlistId")) + ids := slice.Map(splitIds(queryParam(r, "entryIds", "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 a6b6064a3..601dbb0f2 100644 --- a/server/jellyfin/playlists_test.go +++ b/server/jellyfin/playlists_test.go @@ -100,7 +100,7 @@ var _ = Describe("Playlists", func() { Expect(w.Code).To(Equal(http.StatusOK)) var res map[string]string Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) - Expect(res["Id"]).To(Equal("pl-new")) + Expect(res["Id"]).To(Equal(dto.EncodeID("pl-new"))) Expect(fp.createdName).To(Equal("Mix")) Expect(fp.createdIds).To(Equal([]string{"s1", "s2"})) }) @@ -141,11 +141,11 @@ var _ = Describe("Playlists", func() { Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.TotalRecordCount).To(Equal(2)) Expect(res.Items).To(HaveLen(2)) - Expect(res.Items[0].Id).To(Equal("s1")) + Expect(res.Items[0].Id).To(Equal(dto.EncodeID("s1"))) Expect(res.Items[0].Type).To(Equal("Audio")) - Expect(res.Items[0].PlaylistItemId).To(Equal("1")) - Expect(res.Items[1].Id).To(Equal("s2")) - Expect(res.Items[1].PlaylistItemId).To(Equal("2")) + Expect(res.Items[0].PlaylistItemId).To(Equal(dto.EncodeID("1"))) + Expect(res.Items[1].Id).To(Equal(dto.EncodeID("s2"))) + Expect(res.Items[1].PlaylistItemId).To(Equal(dto.EncodeID("2"))) }) It("returns 404 for a non-owned or absent playlist", func() { diff --git a/server/jellyfin/sessions.go b/server/jellyfin/sessions.go index 6ae63b6c9..0275468c2 100644 --- a/server/jellyfin/sessions.go +++ b/server/jellyfin/sessions.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/core/scrobbler" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/server/jellyfin/dto" ) // playbackReport is the subset of Jellyfin's PlaybackStartInfo/PlaybackProgressInfo @@ -21,12 +22,14 @@ type playbackReport struct { // decodeReport reads the playback report body. ItemId falls back to a query // param, as some clients send it there instead of (or in addition to) the JSON body. +// ItemId is decoded here since it flows straight into scrobbler lookups by raw media file id. 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 = dto.DecodeID(body.ItemId) return body } diff --git a/server/jellyfin/stream.go b/server/jellyfin/stream.go index 3cc3cbed9..a71495210 100644 --- a/server/jellyfin/stream.go +++ b/server/jellyfin/stream.go @@ -18,7 +18,7 @@ import ( // content, mirroring the same gate applied to getItem in items.go. func (api *Router) mediaFileForRequest(w http.ResponseWriter, r *http.Request) (*model.MediaFile, bool) { ctx := r.Context() - id := chi.URLParam(r, "itemId") + id := dto.DecodeID(chi.URLParam(r, "itemId")) mf, err := api.ds.MediaFile(ctx).Get(id) if err != nil { http.Error(w, "Not Found", http.StatusNotFound) diff --git a/server/jellyfin/stream_test.go b/server/jellyfin/stream_test.go index d5ddb3842..7e6c8b868 100644 --- a/server/jellyfin/stream_test.go +++ b/server/jellyfin/stream_test.go @@ -42,15 +42,15 @@ var _ = Describe("Stream", func() { {ID: "s1", Title: "Song", Suffix: "mp3", Duration: 100, Size: 1000, LibraryID: 1}, }) w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Items/s1/PlaybackInfo", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", "s1") + r := httptest.NewRequest("POST", "/Items/"+dto.EncodeID("s1")+"/PlaybackInfo", nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("s1")) api.getPlaybackInfo(w, r) Expect(w.Code).To(Equal(http.StatusOK)) var res dto.PlaybackInfoResponse Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.MediaSources).To(HaveLen(1)) - Expect(res.MediaSources[0].Id).To(Equal("s1")) + Expect(res.MediaSources[0].Id).To(Equal(dto.EncodeID("s1"))) Expect(res.MediaSources[0].Container).To(Equal("mp3")) Expect(res.MediaSources[0].Size).To(Equal(int64(1000))) Expect(res.PlaySessionId).ToNot(BeEmpty()) diff --git a/server/jellyfin/users_test.go b/server/jellyfin/users_test.go index 76a0758a3..daf8e0c74 100644 --- a/server/jellyfin/users_test.go +++ b/server/jellyfin/users_test.go @@ -33,13 +33,13 @@ var _ = Describe("Users", func() { Expect(res.Items).To(HaveLen(2)) Expect(res.TotalRecordCount).To(Equal(2)) - Expect(res.Items[0].Id).To(Equal("1")) + Expect(res.Items[0].Id).To(Equal(dto.EncodeID("1"))) Expect(res.Items[0].Name).To(Equal("Music")) Expect(res.Items[0].Type).To(Equal("CollectionFolder")) Expect(res.Items[0].CollectionType).To(Equal("music")) Expect(res.Items[0].IsFolder).To(BeTrue()) - Expect(res.Items[1].Id).To(Equal("2")) + Expect(res.Items[1].Id).To(Equal(dto.EncodeID("2"))) Expect(res.Items[1].Name).To(Equal("Podcasts")) }) @@ -51,7 +51,7 @@ var _ = Describe("Users", func() { var res dto.QueryResult Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) Expect(res.Items).To(HaveLen(1)) - Expect(res.Items[0].Id).To(Equal("1")) + Expect(res.Items[0].Id).To(Equal(dto.EncodeID("1"))) }) It("returns no views for a user with no library access", func() {