diff --git a/server/nativeapi/albums.go b/server/nativeapi/albums.go index 7608d94cf..d170b6ddd 100644 --- a/server/nativeapi/albums.go +++ b/server/nativeapi/albums.go @@ -3,8 +3,10 @@ package nativeapi import ( "context" "errors" + "fmt" "io" "net/http" + "time" "github.com/deluan/rest" "github.com/go-chi/chi/v5" @@ -42,7 +44,13 @@ func (api *Router) uploadAlbumImage() http.HandlerFunc { if err != nil { return err } - filename, err := api.imgUpload.SetImage(ctx, consts.EntityAlbum, al.ID, al.Name, oldPath, reader, ext) + name := al.Name + if oldPath == "" && al.UploadedImage != "" { + // Current file is shared (post album-ID copy): write under a unique name so + // SetImage can't truncate the path the other album still references. + name = fmt.Sprintf("%s-%d", al.Name, time.Now().UnixMilli()) + } + filename, err := api.imgUpload.SetImage(ctx, consts.EntityAlbum, al.ID, name, oldPath, reader, ext) if err != nil { return err } diff --git a/server/nativeapi/albums_test.go b/server/nativeapi/albums_test.go index 9fe7ce542..79ffd3a04 100644 --- a/server/nativeapi/albums_test.go +++ b/server/nativeapi/albums_test.go @@ -1,9 +1,14 @@ package nativeapi import ( + "bytes" + "context" + "io" + "mime/multipart" "net/http" "net/http/httptest" + "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core" @@ -51,3 +56,76 @@ var _ = Describe("Album Image Endpoints", func() { Entry("disabled, regular user is forbidden", false, false, http.StatusForbidden), ) }) + +// tinyPNG is a valid 1x1 PNG, enough to pass handleImageUpload's image validation. +var tinyPNG = []byte{ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, + 0xde, 0x00, 0x00, 0x00, 0x0c, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x60, 0x64, 0x62, 0x06, + 0x00, 0x00, 0x0e, 0x00, 0x07, 0xd7, 0x6f, 0xe4, 0x78, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, + 0x44, 0xae, 0x42, 0x60, 0x82, +} + +type fakeImgUpload struct { + entityID, name, oldPath string +} + +func (f *fakeImgUpload) SetImage(_ context.Context, _ string, entityID string, name string, oldPath string, _ io.Reader, _ string) (string, error) { + f.entityID, f.name, f.oldPath = entityID, name, oldPath + return "stored.png", nil +} + +func (f *fakeImgUpload) RemoveImage(_ context.Context, path string) error { + f.oldPath = path + return nil +} + +var _ = Describe("uploadAlbumImage shared-file handling", func() { + var api *Router + var fake *fakeImgUpload + + upload := func(albumID string) { + body := &bytes.Buffer{} + w := multipart.NewWriter(body) + fw, err := w.CreateFormFile("image", "c.png") + Expect(err).ToNot(HaveOccurred()) + _, _ = fw.Write(tinyPNG) + Expect(w.Close()).To(Succeed()) + + rctx := chi.NewRouteContext() + rctx.URLParams.Add("id", albumID) + ctx := request.WithUser(GinkgoT().Context(), model.User{ID: "u", IsAdmin: true}) + ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx) + req := httptest.NewRequest("POST", "/album/"+albumID+"/image", body).WithContext(ctx) + req.Header.Set("Content-Type", w.FormDataContentType()) + + rec := httptest.NewRecorder() + api.uploadAlbumImage().ServeHTTP(rec, req) + Expect(rec.Code).To(Equal(http.StatusOK)) + } + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + ds := &tests.MockDataStore{} + ds.Album(GinkgoT().Context()).(*tests.MockAlbumRepo).SetData(model.Albums{ + {ID: "al-1", Name: "Album One", LibraryID: 1, UploadedImage: "shared.jpg"}, + {ID: "al-2", Name: "Album Two", LibraryID: 1, UploadedImage: "shared.jpg"}, + {ID: "al-solo", Name: "Album Solo", LibraryID: 1, UploadedImage: "solo.jpg"}, + }) + fake = &fakeImgUpload{} + api = &Router{ds: ds, imgUpload: fake} + }) + + It("replaces in place when the file is not shared", func() { + upload("al-solo") + Expect(fake.oldPath).ToNot(BeEmpty(), "sole reference: old file should be removed") + Expect(fake.name).To(Equal("Album Solo")) + }) + + It("keeps the shared file and writes under a unique name", func() { + upload("al-1") + Expect(fake.oldPath).To(BeEmpty(), "shared file must not be removed") + Expect(fake.name).ToNot(Equal("Album One"), "name must be de-duplicated so the derived filename is unique") + Expect(fake.name).To(HavePrefix("Album One-")) + }) +})