diff --git a/server/imghttp/avatar.go b/server/imghttp/avatar.go index e6201249e..11434a8b2 100644 --- a/server/imghttp/avatar.go +++ b/server/imghttp/avatar.go @@ -1,6 +1,8 @@ package imghttp import ( + "errors" + "io" "net/http" "os" @@ -27,6 +29,19 @@ func ServeUserAvatar(w http.ResponseWriter, r *http.Request, u *model.User) bool return false } + // The stored extension can disagree with the bytes: the uploader re-encodes while keeping the + // caller's extension, so the type is sniffed from the content instead. + head := make([]byte, 512) + n, err := io.ReadFull(f, head) + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + log.Warn(r.Context(), "Could not read user avatar", "user", u.UserName, err) + return false + } + if _, err := f.Seek(0, io.SeekStart); err != nil { + log.Warn(r.Context(), "Could not rewind user avatar", "user", u.UserName, err) + return false + } + w.Header().Set("Content-Type", http.DetectContentType(head[:n])) w.Header().Set("ETag", `"`+u.AvatarTag()+`"`) w.Header().Set("Cache-Control", "private, no-cache") http.ServeContent(w, r, info.Name(), info.ModTime(), f) diff --git a/server/imghttp/avatar_test.go b/server/imghttp/avatar_test.go index 63a6c967a..81a5465d4 100644 --- a/server/imghttp/avatar_test.go +++ b/server/imghttp/avatar_test.go @@ -1,6 +1,11 @@ package imghttp_test import ( + "bytes" + "crypto/rand" + "image" + "image/jpeg" + "image/png" "net/http" "net/http/httptest" "os" @@ -45,6 +50,21 @@ var _ = Describe("ServeUserAvatar", func() { Expect(w.Header().Get("ETag")).To(Equal(`"` + usr.AvatarTag() + `"`)) }) + It("reports the content type of the bytes, not of the extension", func() { + usr.UploadedImage = writeAvatarBytes(usr, "gif", jpegBytes()) + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Header().Get("Content-Type")).To(Equal("image/jpeg")) + }) + + It("serves the whole file, not just what is left after sniffing", func() { + data := pngBytes() + Expect(len(data)).To(BeNumerically(">", 512)) + usr.UploadedImage = writeAvatarBytes(usr, "png", data) + Expect(imghttp.ServeUserAvatar(w, r, &usr)).To(BeTrue()) + Expect(w.Body.Bytes()).To(Equal(data)) + }) + It("answers 304 when the ETag matches", func() { usr.UploadedImage = writeAvatar(usr, "png") r.Header.Set("If-None-Match", `"`+usr.AvatarTag()+`"`) @@ -79,9 +99,33 @@ var _ = Describe("ServeUserAvatar", func() { }) func writeAvatar(u model.User, ext string) string { + return writeAvatarBytes(u, ext, pngBytes()) +} + +func writeAvatarBytes(u model.User, ext string, data []byte) 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()) + Expect(os.WriteFile(path, data, 0600)).To(Succeed()) return name } + +func pngBytes() []byte { + var buf bytes.Buffer + Expect(png.Encode(&buf, noiseImage())).To(Succeed()) + return buf.Bytes() +} + +func jpegBytes() []byte { + var buf bytes.Buffer + Expect(jpeg.Encode(&buf, noiseImage(), nil)).To(Succeed()) + return buf.Bytes() +} + +// noiseImage compresses poorly on purpose, so the encoded file is larger than the 512-byte sniff window. +func noiseImage() image.Image { + img := image.NewRGBA(image.Rect(0, 0, 64, 64)) + _, err := rand.Read(img.Pix) + Expect(err).ToNot(HaveOccurred()) + return img +}