diff --git a/go.mod b/go.mod index 2ff2b4b..fcd58c5 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/google/uuid v1.6.0 github.com/jackc/pgx/v5 v5.10.0 github.com/joho/godotenv v1.5.1 + github.com/rwcarlsen/goexif v0.0.0-20190401172101-9e8deecbddbd golang.org/x/crypto v0.55.0 golang.org/x/image v0.45.0 ) diff --git a/go.sum b/go.sum index 479e538..984c104 100644 --- a/go.sum +++ b/go.sum @@ -45,6 +45,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/resend/resend-go/v3 v3.16.0 h1:t0Z/7k2sxnJGw8SjsCM9O8qkq3YRRHzTxWQNjhF2KhE= github.com/resend/resend-go/v3 v3.16.0/go.mod h1:iI7VA0NoGjWvsNii5iNC5Dy0llsI3HncXPejhniYzwE= +github.com/rwcarlsen/goexif v0.0.0-20190401172101-9e8deecbddbd h1:CmH9+J6ZSsIjUK3dcGsnCnO41eRBOnY12zwkn5qVwgc= +github.com/rwcarlsen/goexif v0.0.0-20190401172101-9e8deecbddbd/go.mod h1:hPqNNc0+uJM6H+SuU8sEs5K5IQeKccPqeSjfgcKGgPk= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= diff --git a/internal/web/post_images.go b/internal/web/post_images.go new file mode 100644 index 0000000..aeaa994 --- /dev/null +++ b/internal/web/post_images.go @@ -0,0 +1,395 @@ +package web + +import ( + "bytes" + "context" + "errors" + "image" + "image/jpeg" + "image/png" + "io" + "log" + "mime" + "mime/multipart" + "net/http" + "path" + "strings" + "time" + + "github.com/google/uuid" + "github.com/rwcarlsen/goexif/exif" + "golang.org/x/image/draw" + _ "golang.org/x/image/webp" + + "plumber/internal/blob" + "plumber/internal/store" +) + +const ( + defaultRequestBodyBytes = 3 << 20 + postImageMaxFileBytes = 5 << 20 + postImageMaxRequestBytes = 22 << 20 + postImageMultipartMemory = 2 << 20 + postImageMaxSourceDim = 6000 + postImageMaxSourcePixels = 16_000_000 + postImageMaxRenderedDim = 1600 + postImageCleanupTimeout = 10 * time.Second +) + +func requestBodyLimit(r *http.Request) int64 { + if r.Method != http.MethodPost { + return defaultRequestBodyBytes + } + switch { + case r.URL.Path == "/submit", r.URL.Path == "/posts": + return postImageMaxRequestBytes + case strings.HasPrefix(r.URL.Path, "/posts/") && strings.HasSuffix(r.URL.Path, "/edit"): + return postImageMaxRequestBytes + default: + return defaultRequestBodyBytes + } +} + +type postImageRequestError struct { + status int + message string + cause error +} + +func (e *postImageRequestError) Error() string { + if e.cause == nil { + return e.message + } + return e.message + ": " + e.cause.Error() +} + +func parsePostMutationForm(w http.ResponseWriter, r *http.Request) (func(), bool) { + contentType := r.Header.Get("Content-Type") + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil && strings.HasPrefix(strings.ToLower(contentType), "multipart/") { + http.Error(w, "Could not read image upload.", http.StatusBadRequest) + return func() {}, false + } + if mediaType != "multipart/form-data" { + if err := r.ParseForm(); err != nil { + writePostImageRequestError(w, err) + return func() {}, false + } + return func() {}, true + } + if err := r.ParseMultipartForm(postImageMultipartMemory); err != nil { + writePostImageRequestError(w, err) + return func() {}, false + } + cleanup := func() { + if r.MultipartForm != nil { + _ = r.MultipartForm.RemoveAll() + } + } + return cleanup, true +} + +func writePostImageRequestError(w http.ResponseWriter, err error) { + var requestErr *postImageRequestError + if errors.As(err, &requestErr) { + http.Error(w, requestErr.message, requestErr.status) + return + } + var maxErr *http.MaxBytesError + if errors.As(err, &maxErr) { + http.Error(w, "Image upload is too large.", http.StatusRequestEntityTooLarge) + return + } + http.Error(w, "Could not read image upload.", http.StatusBadRequest) +} + +func (s *Server) postImagesFromForm( + ctx context.Context, + r *http.Request, + postID string, + existing []store.PostImage, +) ([]store.PostImage, []string, error) { + if r.MultipartForm == nil { + return append([]store.PostImage(nil), existing...), nil, nil + } + retained, err := retainedPostImages(r.MultipartForm, existing) + if err != nil { + return nil, nil, err + } + files := r.MultipartForm.File["images"] + descriptions := r.MultipartForm.Value["image_description"] + if len(descriptions) > len(files) { + return nil, nil, invalidPostImage("Image descriptions do not match selected images.", nil) + } + if len(retained)+len(files) > store.MaxPostImages { + return nil, nil, invalidPostImage("You can attach up to 4 images.", nil) + } + if len(files) > 0 && !s.cfg.Blob.Enabled() { + return nil, nil, &postImageRequestError{ + status: http.StatusServiceUnavailable, + message: "Image uploads are not configured on this server.", + } + } + + images := append([]store.PostImage(nil), retained...) + newKeys := make([]string, 0, len(files)) + for i, header := range files { + description := "" + if i < len(descriptions) { + description = strings.TrimSpace(descriptions[i]) + } + if len([]rune(description)) > store.MaxImageDescriptionRunes { + s.deletePostImageObjects(newKeys) + return nil, nil, invalidPostImage("Image descriptions must be 500 characters or fewer.", nil) + } + prepared, err := preparePostImage(header) + if err != nil { + s.deletePostImageObjects(newKeys) + return nil, nil, err + } + imageID := uuid.NewString() + objectKey := path.Join("post-images", postID, imageID+prepared.extension) + publicURL, err := s.cfg.Blob.Upload(ctx, blob.FileUpload{ + Key: objectKey, + Body: bytes.NewReader(prepared.body), + ContentType: prepared.contentType, + Size: int64(len(prepared.body)), + }) + if err != nil { + s.deletePostImageObjects(newKeys) + return nil, nil, &postImageRequestError{ + status: http.StatusServiceUnavailable, + message: "Could not upload image. Try again later.", + cause: err, + } + } + newKeys = append(newKeys, objectKey) + images = append(images, store.PostImage{ + ID: imageID, + PostID: postID, + ObjectKey: objectKey, + PublicURL: publicURL, + Description: description, + Width: prepared.width, + Height: prepared.height, + }) + } + return images, newKeys, nil +} + +func retainedPostImages(form *multipart.Form, existing []store.PostImage) ([]store.PostImage, error) { + byID := make(map[string]store.PostImage, len(existing)) + for _, image := range existing { + byID[image.ID] = image + } + ids := form.Value["existing_image_id"] + descriptions := form.Value["existing_image_description"] + if len(descriptions) > len(ids) { + return nil, invalidPostImage("Existing image descriptions do not match the images.", nil) + } + seen := make(map[string]bool, len(ids)) + retained := make([]store.PostImage, 0, len(ids)) + for i, id := range ids { + id = strings.TrimSpace(id) + image, ok := byID[id] + if !ok || seen[id] { + return nil, invalidPostImage("An existing image selection is invalid.", nil) + } + seen[id] = true + if i < len(descriptions) { + image.Description = strings.TrimSpace(descriptions[i]) + } + if len([]rune(image.Description)) > store.MaxImageDescriptionRunes { + return nil, invalidPostImage("Image descriptions must be 500 characters or fewer.", nil) + } + retained = append(retained, image) + } + return retained, nil +} + +func invalidPostImage(message string, cause error) error { + return &postImageRequestError{status: http.StatusBadRequest, message: message, cause: cause} +} + +type preparedPostImage struct { + body []byte + extension string + contentType string + width int + height int +} + +func preparePostImage(header *multipart.FileHeader) (preparedPostImage, error) { + if header == nil { + return preparedPostImage{}, invalidPostImage("Select a valid image.", nil) + } + if header.Size > postImageMaxFileBytes { + return preparedPostImage{}, &postImageRequestError{ + status: http.StatusRequestEntityTooLarge, + message: "Each image must be 5 MB or smaller.", + } + } + file, err := header.Open() + if err != nil { + return preparedPostImage{}, invalidPostImage("Could not read image.", err) + } + defer file.Close() + raw, err := io.ReadAll(io.LimitReader(file, postImageMaxFileBytes+1)) + if err != nil { + return preparedPostImage{}, invalidPostImage("Could not read image.", err) + } + if len(raw) == 0 { + return preparedPostImage{}, invalidPostImage("Images cannot be empty.", nil) + } + if int64(len(raw)) > postImageMaxFileBytes { + return preparedPostImage{}, &postImageRequestError{ + status: http.StatusRequestEntityTooLarge, + message: "Each image must be 5 MB or smaller.", + } + } + sniffed := http.DetectContentType(raw) + switch sniffed { + case "image/jpeg", "image/png", "image/webp": + default: + return preparedPostImage{}, invalidPostImage("Images must be JPEG, PNG, or WebP.", nil) + } + cfg, format, err := image.DecodeConfig(bytes.NewReader(raw)) + if err != nil { + return preparedPostImage{}, invalidPostImage("Could not decode image.", err) + } + if cfg.Width <= 0 || cfg.Height <= 0 || + cfg.Width > postImageMaxSourceDim || cfg.Height > postImageMaxSourceDim || + int64(cfg.Width)*int64(cfg.Height) > postImageMaxSourcePixels { + return preparedPostImage{}, invalidPostImage("Image dimensions are too large.", nil) + } + decoded, decodedFormat, err := image.Decode(bytes.NewReader(raw)) + if err != nil { + return preparedPostImage{}, invalidPostImage("Could not decode image.", err) + } + if format != "" { + decodedFormat = format + } + if sniffed == "image/jpeg" { + decoded = orientPostImage(decoded, jpegOrientation(raw)) + } + decoded = fitPostImage(decoded, postImageMaxRenderedDim) + + var out bytes.Buffer + result := preparedPostImage{} + switch decodedFormat { + case "jpeg": + if err := jpeg.Encode(&out, decoded, &jpeg.Options{Quality: 85}); err != nil { + return preparedPostImage{}, invalidPostImage("Could not encode image.", err) + } + result.extension = ".jpg" + result.contentType = "image/jpeg" + case "png", "webp": + if err := png.Encode(&out, decoded); err != nil { + return preparedPostImage{}, invalidPostImage("Could not encode image.", err) + } + result.extension = ".png" + result.contentType = "image/png" + default: + return preparedPostImage{}, invalidPostImage("Images must be JPEG, PNG, or WebP.", nil) + } + result.body = out.Bytes() + result.width = decoded.Bounds().Dx() + result.height = decoded.Bounds().Dy() + return result, nil +} + +func jpegOrientation(raw []byte) int { + metadata, err := exif.Decode(bytes.NewReader(raw)) + if err != nil { + return 1 + } + tag, err := metadata.Get(exif.Orientation) + if err != nil { + return 1 + } + orientation, err := tag.Int(0) + if err != nil || orientation < 1 || orientation > 8 { + return 1 + } + return orientation +} + +func orientPostImage(source image.Image, orientation int) image.Image { + if orientation <= 1 || orientation > 8 { + return source + } + bounds := source.Bounds() + width, height := bounds.Dx(), bounds.Dy() + targetWidth, targetHeight := width, height + if orientation >= 5 { + targetWidth, targetHeight = height, width + } + target := image.NewNRGBA(image.Rect(0, 0, targetWidth, targetHeight)) + for y := 0; y < targetHeight; y++ { + for x := 0; x < targetWidth; x++ { + sourceX, sourceY := x, y + switch orientation { + case 2: + sourceX = width - 1 - x + case 3: + sourceX, sourceY = width-1-x, height-1-y + case 4: + sourceY = height - 1 - y + case 5: + sourceX, sourceY = y, x + case 6: + sourceX, sourceY = y, height-1-x + case 7: + sourceX, sourceY = width-1-y, height-1-x + case 8: + sourceX, sourceY = width-1-y, x + } + target.Set(x, y, source.At(bounds.Min.X+sourceX, bounds.Min.Y+sourceY)) + } + } + return target +} + +func fitPostImage(source image.Image, maxDimension int) image.Image { + bounds := source.Bounds() + width, height := bounds.Dx(), bounds.Dy() + if width <= maxDimension && height <= maxDimension { + return source + } + scale := float64(maxDimension) / float64(width) + if float64(height)*scale > float64(maxDimension) { + scale = float64(maxDimension) / float64(height) + } + targetWidth := max(1, int(float64(width)*scale)) + targetHeight := max(1, int(float64(height)*scale)) + target := image.NewNRGBA(image.Rect(0, 0, targetWidth, targetHeight)) + draw.CatmullRom.Scale(target, target.Bounds(), source, bounds, draw.Over, nil) + return target +} + +func (s *Server) deletePostImageObjects(keys []string) { + if len(keys) == 0 { + return + } + ctx, cancel := context.WithTimeout(context.Background(), postImageCleanupTimeout) + defer cancel() + for _, key := range keys { + if err := s.cfg.Blob.Delete(ctx, key); err != nil { + log.Printf("delete post image %s: %v", key, err) + } + } +} + +func removedPostImageKeys(before, after []store.PostImage) []string { + retained := make(map[string]bool, len(after)) + for _, image := range after { + retained[image.ObjectKey] = true + } + var removed []string + for _, image := range before { + if !retained[image.ObjectKey] { + removed = append(removed, image.ObjectKey) + } + } + return removed +} diff --git a/internal/web/post_images_test.go b/internal/web/post_images_test.go new file mode 100644 index 0000000..e5046de --- /dev/null +++ b/internal/web/post_images_test.go @@ -0,0 +1,443 @@ +package web + +import ( + "bytes" + "context" + "errors" + "image" + "image/color" + "image/jpeg" + "image/png" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "plumber/internal/blob" + "plumber/internal/pacific" + "plumber/internal/store" +) + +func TestPreparePostImage(t *testing.T) { + t.Parallel() + + wide := solidPNG(t, 2000, 1000) + prepared, err := preparePostImageHeader(t, "wide.png", wide) + if err != nil { + t.Fatal(err) + } + if prepared.width != 1600 || prepared.height != 800 || + prepared.extension != ".png" || prepared.contentType != "image/png" { + t.Fatalf("prepared PNG = %+v", prepared) + } + + jpegBody := solidJPEG(t, 40, 20) + prepared, err = preparePostImageHeader(t, "photo.jpg", jpegBody) + if err != nil { + t.Fatal(err) + } + if prepared.width != 40 || prepared.height != 20 || + prepared.extension != ".jpg" || prepared.contentType != "image/jpeg" { + t.Fatalf("prepared JPEG = %+v", prepared) + } + + if _, err := preparePostImageHeader(t, "notes.txt", []byte("not an image")); err == nil { + t.Fatal("text upload unexpectedly succeeded") + } + _, err = preparePostImageHeader(t, "too-large.jpg", make([]byte, postImageMaxFileBytes+1)) + var requestErr *postImageRequestError + if !errors.As(err, &requestErr) || requestErr.status != http.StatusRequestEntityTooLarge { + t.Fatalf("oversized file error = %v, want 413 request error", err) + } + if _, err := preparePostImageHeader(t, "too-wide.png", solidPNG(t, 6001, 1)); err == nil { + t.Fatal("oversized dimensions unexpectedly succeeded") + } +} + +func TestOrientPostImage(t *testing.T) { + t.Parallel() + + source := image.NewNRGBA(image.Rect(0, 0, 2, 1)) + source.Set(0, 0, color.NRGBA{R: 255, A: 255}) + source.Set(1, 0, color.NRGBA{B: 255, A: 255}) + rotated := orientPostImage(source, 6) + if rotated.Bounds().Dx() != 1 || rotated.Bounds().Dy() != 2 { + t.Fatalf("rotated bounds = %v", rotated.Bounds()) + } + top := color.NRGBAModel.Convert(rotated.At(0, 0)).(color.NRGBA) + bottom := color.NRGBAModel.Convert(rotated.At(0, 1)).(color.NRGBA) + if top.R != 255 || bottom.B != 255 { + t.Fatalf("rotation colors top=%v bottom=%v", top, bottom) + } +} + +func TestPostImageMultipartLifecycle(t *testing.T) { + t.Parallel() + + images := &recordingImageBlob{} + srv, mem := newTestServer(t, Config{Blob: images}) + handler := srv.Handler() + homeowner := seedUser(t, mem, uniq("images"), "hunter22", store.RoleUser) + cookies := loginUser(t, handler, homeowner.Username, "hunter22") + csrf := csrfForCookies(t, handler, cookies) + + rec := multipartPost(t, handler, "/submit", map[string][]string{ + "_csrf": {csrf}, + "title": {"Leaky valve"}, + "body": {"Two views of the leak."}, + "city": {"Oakland"}, + "image_description": {"Front view", "Under the sink"}, + }, []multipartTestFile{ + {name: "front.png", body: solidPNG(t, 80, 40)}, + {name: "under.jpg", body: solidJPEG(t, 40, 80)}, + }, cookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("root image upload status = %d: %s", rec.Code, rec.Body.String()) + } + roots, err := mem.ListRootPosts(context.Background(), pacific.Today(), homeowner.ID) + if err != nil || len(roots) != 1 { + t.Fatalf("roots = %+v, %v", roots, err) + } + root, err := mem.GetPost(context.Background(), roots[0].ID) + if err != nil { + t.Fatal(err) + } + if len(root.Images) != 2 || + root.Images[0].Description != "Front view" || + root.Images[1].Description != "Under the sink" { + t.Fatalf("root images = %+v", root.Images) + } + + rec = multipartPost(t, handler, "/posts", map[string][]string{ + "_csrf": {csrf}, + "parent_id": {root.ID}, + "body": {"Here is the model label."}, + "image_description": {"Model label"}, + }, []multipartTestFile{{name: "label.png", body: solidPNG(t, 60, 30)}}, cookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("reply image upload status = %d: %s", rec.Code, rec.Body.String()) + } + thread, err := mem.GetPostThread(context.Background(), root.ID) + if err != nil || len(thread.Replies) != 1 { + t.Fatalf("thread = %+v, %v", thread, err) + } + reply := thread.Replies[0] + if len(reply.Images) != 1 || reply.Images[0].Description != "Model label" { + t.Fatalf("reply images = %+v", reply.Images) + } + originalKey := reply.Images[0].ObjectKey + + rec = multipartPost(t, handler, "/posts/"+reply.ID+"/edit", map[string][]string{ + "_csrf": {csrf}, + "body": {"Updated label photos."}, + "existing_image_id": {reply.Images[0].ID}, + "existing_image_description": {"Existing label"}, + "image_description": {"Serial number"}, + }, []multipartTestFile{{name: "serial.jpg", body: solidJPEG(t, 50, 25)}}, cookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("image edit status = %d: %s", rec.Code, rec.Body.String()) + } + edited, err := mem.GetPost(context.Background(), reply.ID) + if err != nil { + t.Fatal(err) + } + if len(edited.Images) != 2 || + edited.Images[0].Description != "Existing label" || + edited.Images[1].Description != "Serial number" { + t.Fatalf("edited images = %+v", edited.Images) + } + + rec = multipartPost(t, handler, "/posts/"+reply.ID+"/edit", map[string][]string{ + "_csrf": {csrf}, + "body": {"Keep only the serial number."}, + "existing_image_id": {edited.Images[1].ID}, + "existing_image_description": {"Serial number"}, + }, nil, cookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("image removal status = %d: %s", rec.Code, rec.Body.String()) + } + edited, err = mem.GetPost(context.Background(), reply.ID) + if err != nil { + t.Fatal(err) + } + if len(edited.Images) != 1 || edited.Images[0].Description != "Serial number" { + t.Fatalf("images after removal = %+v", edited.Images) + } + if !images.wasDeleted(originalKey) { + t.Fatalf("removed object %q was not deleted: %+v", originalKey, images.deletedKeys()) + } + + uploadsBefore := images.uploadCount() + rec = multipartPost(t, handler, "/posts/"+reply.ID+"/edit", map[string][]string{ + "_csrf": {csrf}, + "body": {"Invalid retained image."}, + "existing_image_id": {"not-owned"}, + }, nil, cookies) + if rec.Code != http.StatusBadRequest { + t.Fatalf("invalid retained image status = %d, want 400", rec.Code) + } + unchanged, err := mem.GetPost(context.Background(), reply.ID) + if err != nil { + t.Fatal(err) + } + if unchanged.Body != "Keep only the serial number." || len(unchanged.Images) != 1 { + t.Fatalf("invalid retained image changed post: %+v", unchanged) + } + + files := make([]multipartTestFile, store.MaxPostImages+1) + for i := range files { + files[i] = multipartTestFile{name: "extra.png", body: solidPNG(t, 10, 10)} + } + rec = multipartPost(t, handler, "/posts", map[string][]string{ + "_csrf": {csrf}, + "parent_id": {root.ID}, + "body": {"Too many images."}, + }, files, cookies) + if rec.Code != http.StatusBadRequest { + t.Fatalf("five-image status = %d, want 400", rec.Code) + } + if images.uploadCount() != uploadsBefore { + t.Fatal("five-image request uploaded objects before rejecting count") + } +} + +func TestPostImageUploadCompensation(t *testing.T) { + t.Parallel() + + t.Run("blob failure deletes earlier upload", func(t *testing.T) { + images := &recordingImageBlob{failAt: 2} + srv, mem := newTestServer(t, Config{Blob: images}) + handler := srv.Handler() + user := seedUser(t, mem, uniq("blob-fail"), "hunter22", store.RoleUser) + cookies := loginUser(t, handler, user.Username, "hunter22") + csrf := csrfForCookies(t, handler, cookies) + rec := multipartPost(t, handler, "/submit", map[string][]string{ + "_csrf": {csrf}, + "title": {"Upload failure"}, + "body": {"Should not persist."}, + }, []multipartTestFile{ + {name: "one.png", body: solidPNG(t, 10, 10)}, + {name: "two.png", body: solidPNG(t, 10, 10)}, + }, cookies) + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("blob failure status = %d: %s", rec.Code, rec.Body.String()) + } + if images.uploadCount() != 1 || len(images.deletedKeys()) != 1 { + t.Fatalf("blob compensation uploads=%d deletes=%v", images.uploadCount(), images.deletedKeys()) + } + }) + + t.Run("store failure deletes uploaded object", func(t *testing.T) { + images := &recordingImageBlob{} + mem := store.NewMemory() + failing := &failingCreatePostStore{Store: mem} + srv := newTestServerStore(t, failing, Config{Blob: images}) + handler := srv.Handler() + user := seedUser(t, mem, uniq("store-fail"), "hunter22", store.RoleUser) + cookies := loginUser(t, handler, user.Username, "hunter22") + csrf := csrfForCookies(t, handler, cookies) + rec := multipartPost(t, handler, "/submit", map[string][]string{ + "_csrf": {csrf}, + "title": {"Store failure"}, + "body": {"Should clean up."}, + }, []multipartTestFile{{name: "one.png", body: solidPNG(t, 10, 10)}}, cookies) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("store failure status = %d: %s", rec.Code, rec.Body.String()) + } + if images.uploadCount() != 1 || len(images.deletedKeys()) != 1 { + t.Fatalf("store compensation uploads=%d deletes=%v", images.uploadCount(), images.deletedKeys()) + } + }) +} + +func TestPostImageRequestLimits(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + method string + path string + want int64 + }{ + {http.MethodPost, "/submit", postImageMaxRequestBytes}, + {http.MethodPost, "/posts", postImageMaxRequestBytes}, + {http.MethodPost, "/posts/id/edit", postImageMaxRequestBytes}, + {http.MethodPost, "/login", defaultRequestBodyBytes}, + {http.MethodGet, "/posts", defaultRequestBodyBytes}, + } { + req := httptest.NewRequest(test.method, test.path, nil) + if got := requestBodyLimit(req); got != test.want { + t.Errorf("%s %s limit = %d, want %d", test.method, test.path, got, test.want) + } + } +} + +type multipartTestFile struct { + name string + body []byte +} + +func multipartPost( + t *testing.T, + handler http.Handler, + requestPath string, + fields map[string][]string, + files []multipartTestFile, + cookies []*http.Cookie, +) *httptest.ResponseRecorder { + t.Helper() + var body bytes.Buffer + writer := multipart.NewWriter(&body) + for name, values := range fields { + for _, value := range values { + if err := writer.WriteField(name, value); err != nil { + t.Fatal(err) + } + } + } + for _, file := range files { + part, err := writer.CreateFormFile("images", file.name) + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(file.body); err != nil { + t.Fatal(err) + } + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, requestPath, &body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + for _, cookie := range cookies { + req.AddCookie(cookie) + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec +} + +func preparePostImageHeader(t *testing.T, name string, body []byte) (preparedPostImage, error) { + t.Helper() + var requestBody bytes.Buffer + writer := multipart.NewWriter(&requestBody) + part, err := writer.CreateFormFile("images", name) + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(body); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/posts", &requestBody) + req.Header.Set("Content-Type", writer.FormDataContentType()) + if err := req.ParseMultipartForm(postImageMultipartMemory); err != nil { + t.Fatal(err) + } + defer req.MultipartForm.RemoveAll() + return preparePostImage(req.MultipartForm.File["images"][0]) +} + +func solidPNG(t *testing.T, width, height int) []byte { + t.Helper() + img := image.NewNRGBA(image.Rect(0, 0, width, height)) + for y := 0; y < height; y++ { + for x := 0; x < width; x++ { + img.Set(x, y, color.NRGBA{R: 30, G: 90, B: 140, A: 255}) + } + } + var out bytes.Buffer + if err := png.Encode(&out, img); err != nil { + t.Fatal(err) + } + return out.Bytes() +} + +func solidJPEG(t *testing.T, width, height int) []byte { + t.Helper() + img := image.NewNRGBA(image.Rect(0, 0, width, height)) + for y := 0; y < height; y++ { + for x := 0; x < width; x++ { + img.Set(x, y, color.NRGBA{R: 140, G: 90, B: 30, A: 255}) + } + } + var out bytes.Buffer + if err := jpeg.Encode(&out, img, &jpeg.Options{Quality: 90}); err != nil { + t.Fatal(err) + } + return out.Bytes() +} + +type recordedImageUpload struct { + key string + contentType string + body []byte +} + +type recordingImageBlob struct { + mu sync.Mutex + calls int + failAt int + uploads []recordedImageUpload + deletes []string +} + +func (b *recordingImageBlob) Enabled() bool { return true } + +func (b *recordingImageBlob) Upload(_ context.Context, object blob.FileUpload) (string, error) { + b.mu.Lock() + defer b.mu.Unlock() + b.calls++ + if b.failAt > 0 && b.calls == b.failAt { + return "", errors.New("injected upload failure") + } + body, err := io.ReadAll(object.Body) + if err != nil { + return "", err + } + b.uploads = append(b.uploads, recordedImageUpload{ + key: object.Key, + contentType: object.ContentType, + body: body, + }) + return "https://cdn.example/" + object.Key, nil +} + +func (b *recordingImageBlob) Delete(_ context.Context, key string) error { + b.mu.Lock() + defer b.mu.Unlock() + b.deletes = append(b.deletes, key) + return nil +} + +func (b *recordingImageBlob) uploadCount() int { + b.mu.Lock() + defer b.mu.Unlock() + return len(b.uploads) +} + +func (b *recordingImageBlob) deletedKeys() []string { + b.mu.Lock() + defer b.mu.Unlock() + return append([]string(nil), b.deletes...) +} + +func (b *recordingImageBlob) wasDeleted(key string) bool { + for _, deleted := range b.deletedKeys() { + if deleted == key { + return true + } + } + return false +} + +type failingCreatePostStore struct { + store.Store +} + +func (f *failingCreatePostStore) CreatePost(context.Context, *store.Post) error { + return errors.New("injected store failure") +} diff --git a/internal/web/posts.go b/internal/web/posts.go index 35d2f2e..fe4ad5c 100644 --- a/internal/web/posts.go +++ b/internal/web/posts.go @@ -12,6 +12,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/google/uuid" "plumber/internal/mail" "plumber/internal/store" @@ -20,6 +21,11 @@ import ( // handleCreatePost creates either a root question or a reply. Replies are // limited to the root author and admins, and cannot be added to hidden threads. func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { + cleanup, ok := parsePostMutationForm(w, r) + if !ok { + return + } + defer cleanup() if !s.requireCSRF(w, r) { return } @@ -37,6 +43,7 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { } post := &store.Post{ + ID: uuid.NewString(), AuthorID: user.ID, Body: truncateRunes(body, 12000), } @@ -71,7 +78,14 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { root = threadRoot } + images, newKeys, err := s.postImagesFromForm(r.Context(), r, post.ID, nil) + if err != nil { + writePostImageRequestError(w, err) + return + } + post.Images = images if err := s.store.CreatePost(r.Context(), post); err != nil { + s.deletePostImageObjects(newKeys) if errors.Is(err, store.ErrInvalidPost) { http.Error(w, "invalid post", http.StatusBadRequest) return @@ -149,6 +163,11 @@ func (s *Server) notifyPostReply( // handleEditPost updates only a post's body after verifying that the current // homeowner owns it or that an admin is editing an admin-authored post. func (s *Server) handleEditPost(w http.ResponseWriter, r *http.Request) { + cleanup, ok := parsePostMutationForm(w, r) + if !ok { + return + } + defer cleanup() if !s.requireCSRF(w, r) { return } @@ -177,8 +196,16 @@ func (s *Server) handleEditPost(w http.ResponseWriter, r *http.Request) { http.Error(w, "post body required", http.StatusBadRequest) return } + previousImages := append([]store.PostImage(nil), post.Images...) + images, newKeys, err := s.postImagesFromForm(r.Context(), r, post.ID, previousImages) + if err != nil { + writePostImageRequestError(w, err) + return + } post.Body = truncateRunes(body, 12000) + post.Images = images if err := s.store.UpdatePost(r.Context(), post); err != nil { + s.deletePostImageObjects(newKeys) if errors.Is(err, store.ErrInvalidPost) { http.Error(w, "invalid post", http.StatusBadRequest) return @@ -190,6 +217,7 @@ func (s *Server) handleEditPost(w http.ResponseWriter, r *http.Request) { http.Error(w, "could not save post", http.StatusInternalServerError) return } + s.deletePostImageObjects(removedPostImageKeys(previousImages, images)) http.Redirect( w, r, diff --git a/internal/web/server.go b/internal/web/server.go index 0998575..e4372b1 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -18,6 +18,7 @@ import ( "github.com/alexedwards/scs/v2" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" + "github.com/google/uuid" "plumber/internal/blob" "plumber/internal/geo" @@ -182,7 +183,7 @@ func (s *Server) Handler() http.Handler { r.Use(middleware.Recoverer) r.Use(func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - r.Body = http.MaxBytesReader(w, r.Body, 3<<20) + r.Body = http.MaxBytesReader(w, r.Body, requestBodyLimit(r)) next.ServeHTTP(w, r) }) }) @@ -331,6 +332,11 @@ func (s *Server) handleSubmitForm(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleSubmit(w http.ResponseWriter, r *http.Request) { + cleanup, ok := parsePostMutationForm(w, r) + if !ok { + return + } + defer cleanup() if !s.requireCSRF(w, r) { return } @@ -362,12 +368,20 @@ func (s *Server) handleSubmit(w http.ResponseWriter, r *http.Request) { city = truncateRunes(city, 80) } post := &store.Post{ + ID: uuid.NewString(), AuthorID: u.ID, Title: title, Body: body, City: city, } + images, newKeys, err := s.postImagesFromForm(r.Context(), r, post.ID, nil) + if err != nil { + writePostImageRequestError(w, err) + return + } + post.Images = images if err := s.store.CreatePost(r.Context(), post); err != nil { + s.deletePostImageObjects(newKeys) http.Error(w, "could not save question", http.StatusInternalServerError) return }