diff --git a/.gitignore b/.gitignore index 25d23cac0..469ce5eb9 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,7 @@ docs/resources/ pkg/static/templates_vfsdata.go files/ !pkg/files/ +!pkg/web/files/ vikunja-dump* vendor/ os-packages/ diff --git a/pkg/models/task_attachment.go b/pkg/models/task_attachment.go index 59d8d7594..846cd0903 100644 --- a/pkg/models/task_attachment.go +++ b/pkg/models/task_attachment.go @@ -23,6 +23,7 @@ import ( "image/png" "io" "strconv" + "strings" "time" "code.vikunja.io/api/pkg/events" @@ -106,6 +107,74 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadSeeker, realna return nil } +// AttachmentToUpload is a transport-neutral file to attach, so the upload logic +// can be shared by the multipart v1 handler and the Huma v2 handler. +type AttachmentToUpload struct { + Reader io.ReadSeeker + Filename string + Size uint64 +} + +// UploadTaskAttachments checks create access to the task, then stores each file, +// collecting per-file failures rather than aborting. The caller owns the session +// and the commit. A returned err means the request as a whole failed (e.g. +// forbidden); per-file failures come back in failures instead. +func UploadTaskAttachments(s *xorm.Session, a web.Auth, taskID int64, uploads []*AttachmentToUpload) (success []*TaskAttachment, failures []error, err error) { + ta := &TaskAttachment{TaskID: taskID} + can, err := ta.CanCreate(s, a) + if err != nil { + return nil, nil, err + } + if !can { + return nil, nil, ErrGenericForbidden{} + } + + for _, upload := range uploads { + attachment := &TaskAttachment{TaskID: taskID} + if err := attachment.NewAttachment(s, upload.Reader, upload.Filename, upload.Size, a); err != nil { + failures = append(failures, err) + continue + } + success = append(success, attachment) + } + return success, failures, nil +} + +// LoadTaskAttachmentForDownload checks read access, loads the attachment with its +// open file, and resolves a preview if previewSize is set and the file is an image. +// It returns the loaded attachment and, when applicable, the preview bytes (the +// caller serves those instead of the file). The caller owns the session, the +// commit, and writing the response. Returns ErrGenericForbidden on denied access. +func LoadTaskAttachmentForDownload(s *xorm.Session, a web.Auth, taskID, attachmentID int64, previewSize PreviewSize) (ta *TaskAttachment, preview []byte, err error) { + ta = &TaskAttachment{ID: attachmentID, TaskID: taskID} + can, _, err := ta.CanRead(s, a) + if err != nil { + return nil, nil, err + } + if !can { + return nil, nil, ErrGenericForbidden{} + } + + if err := ta.ReadOne(s, a); err != nil { + return nil, nil, err + } + if err := ta.File.LoadFileByID(); err != nil { + return nil, nil, err + } + + if previewSize != PreviewSizeUnknown && strings.HasPrefix(ta.File.Mime, "image") { + preview = ta.GetPreview(previewSize) + // GetPreview consumes the file reader; re-open it for the non-preview fallback. + if preview == nil { + if err := ta.File.LoadFileByID(); err != nil { + return nil, nil, err + } + } + } + + return ta, preview, nil +} + // ReadOne returns a task attachment func (ta *TaskAttachment) ReadOne(s *xorm.Session, _ web.Auth) (err error) { query := s.Where("id = ?", ta.ID).NoAutoCondition() diff --git a/pkg/routes/api/v1/task_attachment.go b/pkg/routes/api/v1/task_attachment.go index dd6478703..68197a3bf 100644 --- a/pkg/routes/api/v1/task_attachment.go +++ b/pkg/routes/api/v1/task_attachment.go @@ -18,43 +18,16 @@ package v1 import ( "errors" - "io" - "mime" "net/http" - "strconv" - "strings" - "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/models" auth2 "code.vikunja.io/api/pkg/modules/auth" - "code.vikunja.io/api/pkg/web" + webfiles "code.vikunja.io/api/pkg/web/files" "github.com/labstack/echo/v5" ) -// attachmentUploadError represents a structured error for attachment upload failures -type attachmentUploadError struct { - Code int `json:"code,omitempty"` - Message string `json:"message"` -} - -// toAttachmentUploadError converts an error to a structured attachmentUploadError -func toAttachmentUploadError(err error) attachmentUploadError { - // Try to get structured error info from HTTPErrorProcessor - if httpErr, ok := err.(web.HTTPErrorProcessor); ok { - errDetails := httpErr.HTTPError() - return attachmentUploadError{ - Code: errDetails.Code, - Message: errDetails.Message, - } - } - // Fall back to just the error message - return attachmentUploadError{ - Message: err.Error(), - } -} - // UploadTaskAttachment handles everything needed for the upload of a task attachment // @Summary Upload a task attachment // @Description Upload a task attachment. You can pass multiple files with the files form param. @@ -76,7 +49,6 @@ func UploadTaskAttachment(c *echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "No task ID provided").Wrap(err) } - // Permissions check auth, err := auth2.GetAuthFromClaims(c) if err != nil { return err @@ -85,15 +57,6 @@ func UploadTaskAttachment(c *echo.Context) error { s := db.NewSession() defer s.Close() - can, err := taskAttachment.CanCreate(s, auth) - if err != nil { - _ = s.Rollback() - return err - } - if !can { - return echo.ErrForbidden - } - // Multipart form form, err := c.MultipartForm() if err != nil { @@ -104,31 +67,23 @@ func UploadTaskAttachment(c *echo.Context) error { return err } - type result struct { - Errors []attachmentUploadError `json:"errors"` - Success []*models.TaskAttachment `json:"success"` - } - r := &result{} fileHeaders := form.File["files"] + uploads := make([]*models.AttachmentToUpload, 0, len(fileHeaders)) + var openErrors []error for _, file := range fileHeaders { - // We create a new attachment object here to have a clean start - ta := &models.TaskAttachment{ - TaskID: taskAttachment.TaskID, - } - f, err := file.Open() if err != nil { - r.Errors = append(r.Errors, toAttachmentUploadError(err)) + openErrors = append(openErrors, err) continue } defer f.Close() + uploads = append(uploads, &models.AttachmentToUpload{Reader: f, Filename: file.Filename, Size: uint64(file.Size)}) + } - err = ta.NewAttachment(s, f, file.Filename, uint64(file.Size), auth) - if err != nil { - r.Errors = append(r.Errors, toAttachmentUploadError(err)) - continue - } - r.Success = append(r.Success, ta) + success, failures, err := models.UploadTaskAttachments(s, auth, taskAttachment.TaskID, uploads) + if err != nil { + _ = s.Rollback() + return err } if err := s.Commit(); err != nil { @@ -136,7 +91,7 @@ func UploadTaskAttachment(c *echo.Context) error { return err } - return c.JSON(http.StatusOK, r) + return c.JSON(http.StatusOK, webfiles.BuildUploadResult(success, append(openErrors, failures...))) } // GetTaskAttachment returns a task attachment to download for the user @@ -160,7 +115,6 @@ func GetTaskAttachment(c *echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "No task ID provided").Wrap(err) } - // Permissions check auth, err := auth2.GetAuthFromClaims(c) if err != nil { return err @@ -169,36 +123,11 @@ func GetTaskAttachment(c *echo.Context) error { s := db.NewSession() defer s.Close() - can, _, err := taskAttachment.CanRead(s, auth) - if err != nil { - _ = s.Rollback() - return err - } - if !can { - return echo.ErrForbidden - } - - // Get the attachment incl file - err = taskAttachment.ReadOne(s, auth) - if err != nil { - _ = s.Rollback() - return err - } - - // Open the file so its content is available for preview generation and download - err = taskAttachment.File.LoadFileByID() - if err != nil { - _ = s.Rollback() - return err - } - - // If the preview query parameter is set, get the preview (cached or generate) previewSize := models.GetPreviewSizeFromString(c.QueryParam("preview_size")) - if previewSize != models.PreviewSizeUnknown && strings.HasPrefix(taskAttachment.File.Mime, "image") { - previewFileBytes := taskAttachment.GetPreview(previewSize) - if previewFileBytes != nil { - return c.Blob(http.StatusOK, "image/png", previewFileBytes) - } + attachment, preview, err := models.LoadTaskAttachmentForDownload(s, auth, taskAttachment.TaskID, taskAttachment.ID, previewSize) + if err != nil { + _ = s.Rollback() + return err } if err := s.Commit(); err != nil { @@ -206,36 +135,6 @@ func GetTaskAttachment(c *echo.Context) error { return err } - mimeToReturn := taskAttachment.File.Mime - if mimeToReturn == "" { - mimeToReturn = "application/octet-stream" - } - - c.Response().Header().Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{ - "filename": taskAttachment.File.Name, - })) - c.Response().Header().Set("Content-Type", mimeToReturn) - c.Response().Header().Set("Content-Length", strconv.FormatUint(taskAttachment.File.Size, 10)) - c.Response().Header().Set("Last-Modified", taskAttachment.File.Created.UTC().Format(http.TimeFormat)) - // Override the global no-store directive so browsers can cache attachments. - // no-cache allows caching but requires revalidation via If-Modified-Since. - c.Response().Header().Set("Cache-Control", "no-cache") - - if config.FilesType.GetString() == "s3" { - // Check If-Modified-Since and return 304 if the file hasn't changed. - // http.ServeContent handles this automatically for local files. - if ifModSince := c.Request().Header.Get("If-Modified-Since"); ifModSince != "" { - if t, parseErr := http.ParseTime(ifModSince); parseErr == nil && !taskAttachment.File.Created.UTC().After(t) { - return c.NoContent(http.StatusNotModified) - } - } - - // s3 files cannot use http.ServeContent as it requires a Seekable file - // so we stream the file content directly to the response - _, err = io.Copy(c.Response(), taskAttachment.File.File) - return err - } - - http.ServeContent(c.Response(), c.Request(), taskAttachment.File.Name, taskAttachment.File.Created, taskAttachment.File.File.(io.ReadSeeker)) + webfiles.WriteAttachmentDownload(c.Response(), c.Request(), attachment, preview) return nil } diff --git a/pkg/web/files/task_attachment.go b/pkg/web/files/task_attachment.go new file mode 100644 index 000000000..3db78e62f --- /dev/null +++ b/pkg/web/files/task_attachment.go @@ -0,0 +1,104 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// Package files holds the HTTP-layer glue for serving task attachments — +// the upload-result DTOs and the download response writer — shared by the +// v1 and v2 handlers. The domain logic stays in pkg/models; this package +// only translates it to and from the wire. +package files + +import ( + "io" + "mime" + "net/http" + "strconv" + + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/web" +) + +// AttachmentUploadError is a per-file upload failure. +type AttachmentUploadError struct { + Code int `json:"code,omitempty" doc:"Vikunja numeric error code, when the failure carries one."` + Message string `json:"message" doc:"A human-readable description of why this file failed."` +} + +// AttachmentUploadResult is the outcome of an attachment upload: files are +// processed independently, so a per-file failure lands in Errors while the +// rest still succeed. +type AttachmentUploadResult struct { + Errors []AttachmentUploadError `json:"errors" doc:"Per-file failures. A file that fails here does not fail the whole request; the others still upload."` + Success []*models.TaskAttachment `json:"success" doc:"The attachments that were created successfully."` +} + +// BuildUploadResult turns the domain function's plain return values into the +// wire DTO, mapping each failure to its numeric code when it carries one. +func BuildUploadResult(success []*models.TaskAttachment, failures []error) *AttachmentUploadResult { + r := &AttachmentUploadResult{Success: success} + for _, err := range failures { + r.Errors = append(r.Errors, toAttachmentUploadError(err)) + } + return r +} + +func toAttachmentUploadError(err error) AttachmentUploadError { + if httpErr, ok := err.(web.HTTPErrorProcessor); ok { + details := httpErr.HTTPError() + return AttachmentUploadError{Code: details.Code, Message: details.Message} + } + return AttachmentUploadError{Message: err.Error()} +} + +// WriteAttachmentDownload streams the attachment (or its preview) to the response: +// http.ServeContent for seekable local files (Range + If-Modified-Since for free), +// a manual 304 + io.Copy otherwise. It closes the file reader. +func WriteAttachmentDownload(w http.ResponseWriter, r *http.Request, ta *models.TaskAttachment, preview []byte) { + defer func() { _ = ta.File.File.Close() }() + + if preview != nil { + w.Header().Set("Content-Type", "image/png") + w.Header().Set("Content-Length", strconv.Itoa(len(preview))) + _, _ = w.Write(preview) + return + } + + mimeToReturn := ta.File.Mime + if mimeToReturn == "" { + mimeToReturn = "application/octet-stream" + } + w.Header().Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{"filename": ta.File.Name})) + w.Header().Set("Content-Type", mimeToReturn) + w.Header().Set("Content-Length", strconv.FormatUint(ta.File.Size, 10)) + w.Header().Set("Last-Modified", ta.File.Created.UTC().Format(http.TimeFormat)) + // Override the global no-store directive so browsers can cache attachments. + w.Header().Set("Cache-Control", "no-cache") + + // Local files are *os.File (seekable), so ServeContent gives Range + + // If-Modified-Since for free; s3 (and the in-memory test storage) return a + // non-seekable reader, so check If-Modified-Since manually and io.Copy. + if seeker, ok := ta.File.File.(io.ReadSeeker); ok { + http.ServeContent(w, r, ta.File.Name, ta.File.Created, seeker) + return + } + + if ifModSince := r.Header.Get("If-Modified-Since"); ifModSince != "" { + if t, parseErr := http.ParseTime(ifModSince); parseErr == nil && !ta.File.Created.UTC().After(t) { + w.WriteHeader(http.StatusNotModified) + return + } + } + _, _ = io.Copy(w, ta.File.File) +} diff --git a/pkg/web/files/task_attachment_test.go b/pkg/web/files/task_attachment_test.go new file mode 100644 index 000000000..5b282b6b6 --- /dev/null +++ b/pkg/web/files/task_attachment_test.go @@ -0,0 +1,56 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package files + +import ( + "errors" + "testing" + + "code.vikunja.io/api/pkg/models" + + "github.com/stretchr/testify/assert" +) + +func TestBuildUploadResult(t *testing.T) { + t.Run("maps a domain error to its numeric code", func(t *testing.T) { + // ErrTaskAttachmentIsTooLarge is an HTTPErrorProcessor, so its Code must surface. + r := BuildUploadResult(nil, []error{models.ErrTaskAttachmentIsTooLarge{Size: 99}}) + assert.Empty(t, r.Success) + if assert.Len(t, r.Errors, 1) { + assert.Equal(t, models.ErrCodeTaskAttachmentIsTooLarge, r.Errors[0].Code) + assert.NotEmpty(t, r.Errors[0].Message) + } + }) + + t.Run("plain error has no code, just the message", func(t *testing.T) { + r := BuildUploadResult(nil, []error{errors.New("boom")}) + if assert.Len(t, r.Errors, 1) { + assert.Zero(t, r.Errors[0].Code) + assert.Equal(t, "boom", r.Errors[0].Message) + } + }) + + t.Run("preserves success and failure order", func(t *testing.T) { + success := []*models.TaskAttachment{{ID: 1}, {ID: 2}} + r := BuildUploadResult(success, []error{errors.New("first"), errors.New("second")}) + assert.Equal(t, success, r.Success) + if assert.Len(t, r.Errors, 2) { + assert.Equal(t, "first", r.Errors[0].Message) + assert.Equal(t, "second", r.Errors[1].Message) + } + }) +}