fix(subsonic): stop serving artwork for a deleted radio

artworkAccessible checked every kind except radio, which fell through to
the default "no per-user access control" branch and returned true without
a lookup. Radio deletion removes only the entity row -- item_artwork and
the uploaded file survive until the next prune -- so the old ra- id kept
serving the removed radio's image for up to a day.

This is a regression against master, where newRadioArtworkReader loaded
the radio first and returned its error, so deletion took effect at once.
Radios stay globally visible; only existence is checked.

CreateMockedRadioRepo left Data nil, so its own Put panicked on first
use; initialized it.

Reported by Codex on #5847.
This commit is contained in:
Deluan 2026-07-27 15:04:31 -04:00
parent e646ce5065
commit 0669f23471
3 changed files with 27 additions and 2 deletions

View File

@ -120,13 +120,15 @@ func (api *Router) artworkAccessible(ctx context.Context, id string) bool {
_, lookupErr = api.ds.MediaFile(ctx).Get(artID.ID)
case model.KindPlaylistArtwork:
_, lookupErr = api.ds.Playlist(ctx).Get(artID.ID)
case model.KindRadioArtwork:
_, lookupErr = api.ds.Radio(ctx).Get(artID.ID)
case model.KindDiscArtwork:
albumID, _, perr := model.ParseDiscArtworkID(artID.ID)
if perr != nil {
return false
}
_, lookupErr = api.ds.Album(ctx).Get(albumID)
default: // radio and anything else has no per-user artwork access control
default: // anything else has no per-user artwork access control
return true
}
return lookupErr == nil

View File

@ -32,9 +32,12 @@ var _ = Describe("MediaRetrievalController", func() {
BeforeEach(func() {
albumRepo := &tests.MockAlbumRepo{}
albumRepo.SetData(model.Albums{{ID: "34"}}) // the id the specs request, made accessible
radioRepo := tests.CreateMockedRadioRepo()
Expect(radioRepo.Put(&model.Radio{ID: "rd1", Name: "Radio"})).To(Succeed())
ds = &tests.MockDataStore{
MockedMediaFile: mockRepo,
MockedAlbum: albumRepo,
MockedRadio: radioRepo,
}
artwork = &fakeArtwork{data: "image data"}
router = New(ds, artwork, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, lyrics.NewLyrics(ds, nil), nil, nil)
@ -63,6 +66,26 @@ var _ = Describe("MediaRetrievalController", func() {
Expect(w.Body.String()).To(Equal(artwork.data))
})
It("serves radio artwork while the radio exists", func() {
r := newGetRequest("id=ra-rd1")
_, err := router.GetCoverArt(w, r)
Expect(err).ToNot(HaveOccurred())
Expect(w.Body.String()).To(Equal(artwork.data))
})
// A deleted radio keeps its item_artwork row and uploaded file until the next prune, so
// existence has to be re-checked or the old id keeps serving the removed radio's image.
It("serves a placeholder once the radio is gone", func() {
r := newGetRequest("id=ra-deleted")
_, err := router.GetCoverArt(w, r)
Expect(err).ToNot(HaveOccurred())
Expect(w.Code).To(Equal(200))
Expect(w.Body.String()).ToNot(Equal(artwork.data))
Expect(w.Header().Get("Cache-Control")).To(Equal("no-store"))
})
It("serves a placeholder for an entity the caller cannot access", func() {
// al-99 is not in the (filtered) album repo, so the caller must not get its bytes.
r := newGetRequest("id=al-99")

View File

@ -16,7 +16,7 @@ type MockedRadioRepo struct {
}
func CreateMockedRadioRepo() *MockedRadioRepo {
return &MockedRadioRepo{}
return &MockedRadioRepo{Data: map[string]*model.Radio{}}
}
func (m *MockedRadioRepo) SetError(err bool) {