From 2de2b4a143f801e046e9dc0caf971da01c3a9e1c Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 12 Jun 2026 10:00:42 +0200 Subject: [PATCH] refactor(background): share upload validation between v1 and v2 handlers Extract the MIME validation, file storage and project reload from the v1 UploadBackground handler into ValidateAndSaveBackgroundUpload so the upcoming v2 handler can reuse it instead of duplicating the logic. The v1 handler keeps its exact wire behaviour; the inline "not an image" check now returns a typed ErrFileIsNoImage that the handler maps to the same message. --- pkg/modules/background/handler/background.go | 68 +++++++++++--------- pkg/modules/background/handler/errors.go | 19 ++++++ 2 files changed, 57 insertions(+), 30 deletions(-) diff --git a/pkg/modules/background/handler/background.go b/pkg/modules/background/handler/background.go index a89784ee9..afe7901e5 100644 --- a/pkg/modules/background/handler/background.go +++ b/pkg/modules/background/handler/background.go @@ -204,44 +204,17 @@ func (bp *BackgroundProvider) UploadBackground(c *echo.Context) error { } defer srcf.Close() - // Validate we're dealing with an image - mime, err := mimetype.DetectReader(srcf) - if err != nil { + if err := ValidateAndSaveBackgroundUpload(s, auth, project, srcf, file.Filename, uint64(file.Size)); err != nil { _ = s.Rollback() - return err - } - if !strings.HasPrefix(mime.String(), "image") { - _ = s.Rollback() - return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."}) - } - supported := false - for _, m := range allowedImageMimes { - if mime.Is(m) { - supported = true - break + if IsErrFileIsNoImage(err) { + return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."}) } - } - if !supported { - _ = s.Rollback() - return c.JSON(http.StatusBadRequest, models.Message{Message: "Unsupported image format. Allowed: " + strings.Join(allowedImageMimes, ",")}) - } - - err = SaveBackgroundFile(s, auth, project, srcf, file.Filename, uint64(file.Size)) - if err != nil { - _ = s.Rollback() if files.IsErrFileIsTooLarge(err) { return echo.ErrBadRequest } if IsErrFileUnsupportedImageFormat(err) { return c.JSON(http.StatusBadRequest, models.Message{Message: "Unsupported image format. Allowed: " + strings.Join(allowedImageMimes, ",")}) } - - return err - } - - err = project.ReadOne(s, auth) - if err != nil { - _ = s.Rollback() return err } @@ -253,6 +226,41 @@ func (bp *BackgroundProvider) UploadBackground(c *echo.Context) error { return c.JSON(http.StatusOK, project) } +// ValidateAndSaveBackgroundUpload validates that srcf is a decodable image of an +// allowed type, stores it as the project's background and reloads the project so +// callers get the updated background metadata. It is the shared body of the v1 and +// v2 upload handlers; the multipart parsing and error-to-HTTP mapping stay in each +// handler. project must already be loaded and the caller must have verified write +// permission. On a non-image it returns ErrFileIsNoImage; on a recognized but +// undecodable format ErrFileUnsupportedImageFormat. +func ValidateAndSaveBackgroundUpload(s *xorm.Session, auth web.Auth, project *models.Project, srcf io.ReadSeeker, filename string, filesize uint64) error { + mime, err := mimetype.DetectReader(srcf) + if err != nil { + return err + } + if !strings.HasPrefix(mime.String(), "image") { + return ErrFileIsNoImage{Mime: mime.String()} + } + supported := false + for _, m := range allowedImageMimes { + if mime.Is(m) { + supported = true + break + } + } + if !supported { + return ErrFileUnsupportedImageFormat{Mime: mime.String()} + } + + // DetectReader consumed the head of the reader; SaveBackgroundFile seeks back to + // the start itself, so no rewind is needed here. + if err := SaveBackgroundFile(s, auth, project, srcf, filename, filesize); err != nil { + return err + } + + return project.ReadOne(s, auth) +} + func SaveBackgroundFile(s *xorm.Session, auth web.Auth, project *models.Project, srcf io.ReadSeeker, filename string, filesize uint64) (err error) { mime, _ := mimetype.DetectReader(srcf) _, _ = srcf.Seek(0, io.SeekStart) diff --git a/pkg/modules/background/handler/errors.go b/pkg/modules/background/handler/errors.go index beaf46657..dcddf1687 100644 --- a/pkg/modules/background/handler/errors.go +++ b/pkg/modules/background/handler/errors.go @@ -40,3 +40,22 @@ func IsErrFileUnsupportedImageFormat(err error) bool { ok := errors.As(err, &errFileUnsupportedImageFormat) return ok } + +// ErrFileIsNoImage is returned when an uploaded background does not sniff as an +// image at all (its detected mime type does not start with "image"). It is +// distinct from ErrFileUnsupportedImageFormat, which is a recognized image type +// the imaging library can't decode. +type ErrFileIsNoImage struct { + Mime string +} + +// Error is the error implementation of ErrFileIsNoImage +func (err ErrFileIsNoImage) Error() string { + return fmt.Sprintf("uploaded file is not an image [Mime: %s]", err.Mime) +} + +// IsErrFileIsNoImage checks if an error is ErrFileIsNoImage +func IsErrFileIsNoImage(err error) bool { + var errFileIsNoImage ErrFileIsNoImage + return errors.As(err, &errFileIsNoImage) +}