From 61bb68fc6bfb9013eaa44c72a1f26ccc5a6ab600 Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 7 Sep 2026 19:24:05 -0400 Subject: [PATCH] feat(server): add shared user avatar serving helper ServeUserAvatar lives in server/imghttp so both server/subsonic and server/jellyfin can call it without importing each other. It delegates ETag/If-None-Match handling and Content-Type detection to http.ServeContent instead of hand-parsing the header, avoiding a false-304 on multi-valued or wildcard If-None-Match headers. --- server/imghttp/avatar.go | 34 ++++++++++++++ server/imghttp/avatar_test.go | 87 +++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 server/imghttp/avatar.go create mode 100644 server/imghttp/avatar_test.go diff --git a/server/imghttp/avatar.go b/server/imghttp/avatar.go new file mode 100644 index 000000000..e6201249e --- /dev/null +++ b/server/imghttp/avatar.go @@ -0,0 +1,34 @@ +package imghttp + +import ( + "net/http" + "os" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" +) + +// ServeUserAvatar writes the user's uploaded avatar, reporting whether it answered the request; +// when false the caller must fall back. http.ServeContent gives correct ETag/If-None-Match handling. +func ServeUserAvatar(w http.ResponseWriter, r *http.Request, u *model.User) bool { + path := u.UploadedImagePath() + if path == "" { + return false + } + f, err := os.Open(path) + if err != nil { + log.Warn(r.Context(), "Could not open user avatar", "user", u.UserName, err) + return false + } + defer f.Close() + info, err := f.Stat() + if err != nil { + log.Warn(r.Context(), "Could not stat user avatar", "user", u.UserName, err) + return false + } + + w.Header().Set("ETag", `"`+u.AvatarTag()+`"`) + w.Header().Set("Cache-Control", "private, no-cache") + http.ServeContent(w, r, info.Name(), info.ModTime(), f) + return true +} diff --git a/server/imghttp/avatar_test.go b/server/imghttp/avatar_test.go new file mode 100644 index 000000000..63a6c967a --- /dev/null +++ b/server/imghttp/avatar_test.go @@ -0,0 +1,87 @@ +package imghttp_test + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/imghttp" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ServeUserAvatar", func() { + var w *httptest.ResponseRecorder + var r *http.Request + var usr model.User + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DataFolder = conf.NewDir(GinkgoT().TempDir()) + w = httptest.NewRecorder() + r = httptest.NewRequest("GET", "/avatar", nil) + usr = model.User{ID: "u1", UserName: "deluan", UpdatedAt: time.Unix(1000, 0)} + }) + + It("returns false when the user has no avatar", func() { + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeFalse()) + }) + + It("returns false when the file is missing on disk", func() { + usr.UploadedImage = "u1_deluan.png" + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeFalse()) + }) + + It("serves the file with a content type and an ETag", func() { + usr.UploadedImage = writeAvatar(usr, "png") + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Header().Get("Content-Type")).To(Equal("image/png")) + Expect(w.Header().Get("ETag")).To(Equal(`"` + usr.AvatarTag() + `"`)) + }) + + It("answers 304 when the ETag matches", func() { + usr.UploadedImage = writeAvatar(usr, "png") + r.Header.Set("If-None-Match", `"`+usr.AvatarTag()+`"`) + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusNotModified)) + }) + + It("does not leak the absolute filesystem path in the response", func() { + usr.UploadedImage = writeAvatar(usr, "png") + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Body.String()).NotTo(ContainSubstring(conf.Server.DataFolder.String())) + for _, values := range w.Header() { + for _, v := range values { + Expect(v).NotTo(ContainSubstring(conf.Server.DataFolder.String())) + } + } + }) + + It("does not false-304 on an unrelated multi-value If-None-Match", func() { + usr.UploadedImage = writeAvatar(usr, "png") + r.Header.Set("If-None-Match", `"deadbeefdeadbeef", "cafecafecafecafe"`) + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + }) + + It("treats If-None-Match: * as matching the current representation", func() { + usr.UploadedImage = writeAvatar(usr, "png") + r.Header.Set("If-None-Match", "*") + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusNotModified)) + }) +}) + +func writeAvatar(u model.User, ext string) string { + name := u.ID + "_" + u.UserName + "." + ext + path := filepath.Join(conf.Server.DataFolder.String(), "avatar", name) + Expect(os.MkdirAll(filepath.Dir(path), 0755)).To(Succeed()) + Expect(os.WriteFile(path, []byte{0x89, 'P', 'N', 'G'}, 0600)).To(Succeed()) + return name +}