diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index d74468940..44e36e7d2 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -236,7 +236,7 @@ func (api *Router) GetAlbumInfo(r *http.Request) (*responses.Subsonic, error) { response.AlbumInfo.LargeImageUrl = publicurl.ImageURL(r.Context(), album.CoverArtID(), 1200) } - response.AlbumInfo.LastFmUrl = album.ExternalUrl + response.AlbumInfo.LastFmUrl = lastFmURLOrEmpty(album.ExternalUrl) response.AlbumInfo.MusicBrainzID = album.MbzAlbumID return response, nil @@ -302,7 +302,7 @@ func (api *Router) getArtistInfo(r *http.Request) (*responses.ArtistInfoBase, *m base.MediumImageUrl = publicurl.ImageURL(r.Context(), artist.CoverArtID(), 600) base.LargeImageUrl = publicurl.ImageURL(r.Context(), artist.CoverArtID(), 1200) } - base.LastFmUrl = artist.ExternalUrl + base.LastFmUrl = lastFmURLOrEmpty(artist.ExternalUrl) base.MusicBrainzID = artist.MbzArtistID return &base, &artist.SimilarArtists, nil diff --git a/server/subsonic/browsing_test.go b/server/subsonic/browsing_test.go index 71d758cfd..099addae4 100644 --- a/server/subsonic/browsing_test.go +++ b/server/subsonic/browsing_test.go @@ -168,6 +168,18 @@ var _ = Describe("Browsing", func() { Expect(resp.AlbumInfo.SmallImageUrl).ToNot(BeEmpty()) Expect(resp.AlbumInfo.LargeImageUrl).ToNot(BeEmpty()) }) + It("only reports Last.fm pages as lastFmUrl", func() { + api.provider = &fakeInfoProvider{album: &model.Album{ID: "al-1", ExternalUrl: "https://www.last.fm/music/Radiohead/OK+Computer"}} + r := httptest.NewRequest("GET", "/rest/getAlbumInfo?id=al-1", nil) + resp, err := api.GetAlbumInfo(r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.AlbumInfo.LastFmUrl).To(Equal("https://www.last.fm/music/Radiohead/OK+Computer")) + + api.provider = &fakeInfoProvider{album: &model.Album{ID: "al-1", ExternalUrl: "https://www.deezer.com/album/12345"}} + resp, err = api.GetAlbumInfo(r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.AlbumInfo.LastFmUrl).To(BeEmpty()) + }) It("omits image URLs when the album artwork is known absent", func() { api.provider = &fakeInfoProvider{album: &model.Album{ID: "al-1", ItemImage: model.ItemImage{ImageAbsent: true}}} r := httptest.NewRequest("GET", "/rest/getAlbumInfo?id=al-1", nil) @@ -187,6 +199,18 @@ var _ = Describe("Browsing", func() { Expect(err).ToNot(HaveOccurred()) Expect(resp.ArtistInfo.SmallImageUrl).ToNot(BeEmpty()) }) + It("only reports Last.fm pages as lastFmUrl", func() { + api.provider = &fakeInfoProvider{artist: &model.Artist{ID: "ar-1", ExternalUrl: "https://www.last.fm/music/Radiohead"}} + r := httptest.NewRequest("GET", "/rest/getArtistInfo?id=ar-1", nil) + resp, err := api.GetArtistInfo(r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.ArtistInfo.LastFmUrl).To(Equal("https://www.last.fm/music/Radiohead")) + + api.provider = &fakeInfoProvider{artist: &model.Artist{ID: "ar-1", ExternalUrl: "https://www.radiohead.com/"}} + resp, err = api.GetArtistInfo(r) + Expect(err).ToNot(HaveOccurred()) + Expect(resp.ArtistInfo.LastFmUrl).To(BeEmpty()) + }) It("omits image URLs when the artist artwork is known absent", func() { api.provider = &fakeInfoProvider{artist: &model.Artist{ID: "ar-1", ItemImage: model.ItemImage{ImageAbsent: true}}} r := httptest.NewRequest("GET", "/rest/getArtistInfo?id=ar-1", nil) diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index 55b1b213e..e5dc8bdfd 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -7,6 +7,7 @@ import ( "fmt" "mime" "net/http" + "net/url" "slices" "sort" "strings" @@ -104,6 +105,23 @@ func coverArtOrEmpty(id model.ArtworkID, absent bool) string { return id.String() } +// lastFmURLOrEmpty returns the URL only when it points to a Last.fm music page, +// as other agents may store an unrelated site in ExternalUrl. +func lastFmURLOrEmpty(externalURL string) string { + u, err := url.Parse(externalURL) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") { + return "" + } + host := strings.ToLower(u.Hostname()) + if host != "last.fm" && !strings.HasSuffix(host, ".last.fm") { + return "" + } + if !strings.HasPrefix(u.Path, "/music/") { + return "" + } + return externalURL +} + func toArtist(r *http.Request, a model.Artist) responses.Artist { artist := responses.Artist{ Id: a.ID, diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 69fda4681..4df452c78 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -53,6 +53,22 @@ var _ = Describe("helpers", func() { }) }) + Describe("lastFmURLOrEmpty", func() { + It("keeps Last.fm artist and album pages", func() { + Expect(lastFmURLOrEmpty("https://www.last.fm/music/Radiohead")).To(Equal("https://www.last.fm/music/Radiohead")) + Expect(lastFmURLOrEmpty("http://last.fm/music/Radiohead/OK+Computer")).To(Equal("http://last.fm/music/Radiohead/OK+Computer")) + }) + It("omits URLs from other sites", func() { + Expect(lastFmURLOrEmpty("https://www.radiohead.com/")).To(BeEmpty()) + Expect(lastFmURLOrEmpty("https://www.deezer.com/artist/399")).To(BeEmpty()) + Expect(lastFmURLOrEmpty("https://evil.example/last.fm/music/Radiohead")).To(BeEmpty()) + Expect(lastFmURLOrEmpty("https://notlast.fm/music/Radiohead")).To(BeEmpty()) + Expect(lastFmURLOrEmpty("https://www.last.fm/user/someone")).To(BeEmpty()) + Expect(lastFmURLOrEmpty("javascript:alert(1)//last.fm/music/")).To(BeEmpty()) + Expect(lastFmURLOrEmpty("")).To(BeEmpty()) + }) + }) + Describe("sanitizeSlashes", func() { It("maps / to _", func() { Expect(sanitizeSlashes("AC/DC")).To(Equal("AC_DC"))