From a4e23b81d34837b4c0edb73862639e1c250573b9 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 21 Oct 2025 23:07:11 +0800 Subject: [PATCH] fix attachment file size limit in server backend (#35519) fix #35512 --------- Co-authored-by: wxiaoguang --- modules/git/utils.go | 27 ------ modules/setting/attachment.go | 10 ++- modules/util/error.go | 1 + routers/api/v1/repo/issue_attachment.go | 9 +- .../api/v1/repo/issue_comment_attachment.go | 8 +- routers/api/v1/repo/release_attachment.go | 22 +++-- routers/web/repo/attachment.go | 3 +- routers/web/repo/editor_uploader.go | 2 + services/attachment/attachment.go | 44 ++++++++-- services/context/api.go | 3 +- services/context/base.go | 15 ++++ services/context/context.go | 3 +- services/mailer/incoming/incoming_handler.go | 10 ++- templates/swagger/v1_json.tmpl | 9 ++ .../api_comment_attachment_test.go | 7 +- .../integration/api_issue_attachment_test.go | 7 +- tests/integration/api_releases_test.go | 82 ++++++++++--------- tests/integration/attachment_test.go | 16 ++-- 18 files changed, 169 insertions(+), 109 deletions(-) diff --git a/modules/git/utils.go b/modules/git/utils.go index b5f188904a..e7d30ce9ee 100644 --- a/modules/git/utils.go +++ b/modules/git/utils.go @@ -6,7 +6,6 @@ package git import ( "crypto/sha1" "encoding/hex" - "io" "strconv" "strings" "sync" @@ -68,32 +67,6 @@ func ParseBool(value string) (result, valid bool) { return intValue != 0, true } -// LimitedReaderCloser is a limited reader closer -type LimitedReaderCloser struct { - R io.Reader - C io.Closer - N int64 -} - -// Read implements io.Reader -func (l *LimitedReaderCloser) Read(p []byte) (n int, err error) { - if l.N <= 0 { - _ = l.C.Close() - return 0, io.EOF - } - if int64(len(p)) > l.N { - p = p[0:l.N] - } - n, err = l.R.Read(p) - l.N -= int64(n) - return n, err -} - -// Close implements io.Closer -func (l *LimitedReaderCloser) Close() error { - return l.C.Close() -} - func HashFilePathForWebUI(s string) string { h := sha1.New() _, _ = h.Write([]byte(s)) diff --git a/modules/setting/attachment.go b/modules/setting/attachment.go index c11b0c478a..5d420c987c 100644 --- a/modules/setting/attachment.go +++ b/modules/setting/attachment.go @@ -16,9 +16,13 @@ var Attachment AttachmentSettingType func loadAttachmentFrom(rootCfg ConfigProvider) (err error) { Attachment = AttachmentSettingType{ AllowedTypes: ".avif,.cpuprofile,.csv,.dmp,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.json,.jsonc,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.webp,.xls,.xlsx,.zip", - MaxSize: 2048, - MaxFiles: 5, - Enabled: true, + + // FIXME: this size is used for both "issue attachment" and "release attachment" + // The design is not right, these two should be different settings + MaxSize: 2048, + + MaxFiles: 5, + Enabled: true, } sec, _ := rootCfg.GetSection("attachment") if sec == nil { diff --git a/modules/util/error.go b/modules/util/error.go index 6b2721618e..24fa1ba151 100644 --- a/modules/util/error.go +++ b/modules/util/error.go @@ -16,6 +16,7 @@ var ( ErrPermissionDenied = errors.New("permission denied") // also implies HTTP 403 ErrNotExist = errors.New("resource does not exist") // also implies HTTP 404 ErrAlreadyExist = errors.New("resource already exists") // also implies HTTP 409 + ErrContentTooLarge = errors.New("content exceeds limit") // also implies HTTP 413 // ErrUnprocessableContent implies HTTP 422, the syntax of the request content is correct, // but the server is unable to process the contained instructions diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 3f751a295c..bfe9c92f1c 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -4,6 +4,7 @@ package repo import ( + "errors" "net/http" issues_model "code.gitea.io/gitea/models/issues" @@ -11,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" @@ -154,6 +156,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/error" // "404": // "$ref": "#/responses/error" + // "413": + // "$ref": "#/responses/error" // "422": // "$ref": "#/responses/validationError" // "423": @@ -181,7 +185,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + uploaderFile := attachment_service.NewLimitedUploaderKnownSize(file, header.Size) + attachment, err := attachment_service.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -190,6 +195,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { if err != nil { if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusUnprocessableEntity, err) + } else if errors.Is(err, util.ErrContentTooLarge) { + ctx.APIError(http.StatusRequestEntityTooLarge, err) } else { ctx.APIErrorInternal(err) } diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 5f660c5750..3227f5ddee 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" @@ -161,6 +162,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { // "$ref": "#/responses/forbidden" // "404": // "$ref": "#/responses/error" + // "413": + // "$ref": "#/responses/error" // "422": // "$ref": "#/responses/validationError" // "423": @@ -189,7 +192,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + uploaderFile := attachment_service.NewLimitedUploaderKnownSize(file, header.Size) + attachment, err := attachment_service.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -199,6 +203,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { if err != nil { if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusUnprocessableEntity, err) + } else if errors.Is(err, util.ErrContentTooLarge) { + ctx.APIError(http.StatusRequestEntityTooLarge, err) } else { ctx.APIErrorInternal(err) } diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index defde81a1d..43e97beb27 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -4,7 +4,7 @@ package repo import ( - "io" + "errors" "net/http" "strings" @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" @@ -191,6 +192,8 @@ func CreateReleaseAttachment(ctx *context.APIContext) { // "$ref": "#/responses/error" // "404": // "$ref": "#/responses/notFound" + // "413": + // "$ref": "#/responses/error" // Check if attachments are enabled if !setting.Attachment.Enabled { @@ -205,10 +208,8 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } // Get uploaded file from request - var content io.ReadCloser var filename string - var size int64 = -1 - + var uploaderFile *attachment_service.UploaderFile if strings.HasPrefix(strings.ToLower(ctx.Req.Header.Get("Content-Type")), "multipart/form-data") { file, header, err := ctx.Req.FormFile("attachment") if err != nil { @@ -217,15 +218,14 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } defer file.Close() - content = file - size = header.Size filename = header.Filename if name := ctx.FormString("name"); name != "" { filename = name } + uploaderFile = attachment_service.NewLimitedUploaderKnownSize(file, header.Size) } else { - content = ctx.Req.Body filename = ctx.FormString("name") + uploaderFile = attachment_service.NewLimitedUploaderMaxBytesReader(ctx.Req.Body, ctx.Resp) } if filename == "" { @@ -234,7 +234,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } // Create a new attachment and save the file - attach, err := attachment_service.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{ + attach, err := attachment_service.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, setting.Repository.Release.AllowedTypes, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -245,6 +245,12 @@ func CreateReleaseAttachment(ctx *context.APIContext) { ctx.APIError(http.StatusBadRequest, err) return } + + if errors.Is(err, util.ErrContentTooLarge) { + ctx.APIError(http.StatusRequestEntityTooLarge, err) + return + } + ctx.APIErrorInternal(err) return } diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index f696669196..54200d8de8 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -45,7 +45,8 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { } defer file.Close() - attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, &repo_model.Attachment{ + uploaderFile := attachment.NewLimitedUploaderKnownSize(file, header.Size) + attach, err := attachment.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, allowedTypes, &repo_model.Attachment{ Name: header.Filename, UploaderID: ctx.Doer.ID, RepoID: repoID, diff --git a/routers/web/repo/editor_uploader.go b/routers/web/repo/editor_uploader.go index 1ce9a1aca4..5ad07cda74 100644 --- a/routers/web/repo/editor_uploader.go +++ b/routers/web/repo/editor_uploader.go @@ -41,6 +41,8 @@ func UploadFileToServer(ctx *context.Context) { return } + // FIXME: need to check the file size according to setting.Repository.Upload.FileMaxSize + uploaded, err := repo_model.NewUpload(ctx, name, buf, file) if err != nil { ctx.ServerError("NewUpload", err) diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index ccb97c66c8..eb208a141c 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -6,11 +6,14 @@ package attachment import ( "bytes" "context" + "errors" "fmt" "io" + "net/http" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/context/upload" @@ -28,27 +31,56 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R attach.UUID = uuid.New().String() size, err := storage.Attachments.Save(attach.RelativePath(), file, size) if err != nil { - return fmt.Errorf("Create: %w", err) + return fmt.Errorf("Attachments.Save: %w", err) } attach.Size = size - return db.Insert(ctx, attach) }) return attach, err } -// UploadAttachment upload new attachment into storage and update database -func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, fileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { +type UploaderFile struct { + rd io.ReadCloser + size int64 + respWriter http.ResponseWriter +} + +func NewLimitedUploaderKnownSize(r io.Reader, size int64) *UploaderFile { + return &UploaderFile{rd: io.NopCloser(r), size: size} +} + +func NewLimitedUploaderMaxBytesReader(r io.ReadCloser, w http.ResponseWriter) *UploaderFile { + return &UploaderFile{rd: r, size: -1, respWriter: w} +} + +func UploadAttachmentGeneralSizeLimit(ctx context.Context, file *UploaderFile, allowedTypes string, attach *repo_model.Attachment) (*repo_model.Attachment, error) { + return uploadAttachment(ctx, file, allowedTypes, setting.Attachment.MaxSize<<20, attach) +} + +func uploadAttachment(ctx context.Context, file *UploaderFile, allowedTypes string, maxFileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { + src := file.rd + if file.size < 0 { + src = http.MaxBytesReader(file.respWriter, src, maxFileSize) + } buf := make([]byte, 1024) - n, _ := util.ReadAtMost(file, buf) + n, _ := util.ReadAtMost(src, buf) buf = buf[:n] if err := upload.Verify(buf, attach.Name, allowedTypes); err != nil { return nil, err } - return NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), file), fileSize) + if maxFileSize >= 0 && file.size > maxFileSize { + return nil, util.ErrorWrap(util.ErrContentTooLarge, "attachment exceeds limit %d", maxFileSize) + } + + attach, err := NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), src), file.size) + var maxBytesError *http.MaxBytesError + if errors.As(err, &maxBytesError) { + return nil, util.ErrorWrap(util.ErrContentTooLarge, "attachment exceeds limit %d", maxFileSize) + } + return attach, err } // UpdateAttachment updates an attachment, verifying that its name is among the allowed types. diff --git a/services/context/api.go b/services/context/api.go index ab50a360f4..d698b91163 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -229,8 +229,7 @@ func APIContexter() func(http.Handler) http.Handler { // If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid. if ctx.Req.Method == http.MethodPost && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") { - if err := ctx.Req.ParseMultipartForm(setting.Attachment.MaxSize << 20); err != nil && !strings.Contains(err.Error(), "EOF") { // 32MB max size - ctx.APIErrorInternal(err) + if !ctx.ParseMultipartForm() { return } } diff --git a/services/context/base.go b/services/context/base.go index 28d6656fd1..de839ede81 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -4,6 +4,7 @@ package context import ( + "errors" "fmt" "html/template" "io" @@ -42,6 +43,20 @@ type Base struct { Locale translation.Locale } +func (b *Base) ParseMultipartForm() bool { + err := b.Req.ParseMultipartForm(32 << 20) + if err != nil { + // TODO: all errors caused by client side should be ignored (connection closed). + if !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + // Errors caused by server side (disk full) should be logged. + log.Error("Failed to parse request multipart form for %s: %v", b.Req.RequestURI, err) + } + b.HTTPError(http.StatusInternalServerError, "failed to parse request multipart form") + return false + } + return true +} + // AppendAccessControlExposeHeaders append headers by name to "Access-Control-Expose-Headers" header func (b *Base) AppendAccessControlExposeHeaders(names ...string) { val := b.RespHeader().Get("Access-Control-Expose-Headers") diff --git a/services/context/context.go b/services/context/context.go index 32ec260aab..4e83dee807 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -186,8 +186,7 @@ func Contexter() func(next http.Handler) http.Handler { // If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid. if ctx.Req.Method == http.MethodPost && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") { - if err := ctx.Req.ParseMultipartForm(setting.Attachment.MaxSize << 20); err != nil && !strings.Contains(err.Error(), "EOF") { // 32MB max size - ctx.ServerError("ParseMultipartForm", err) + if !ctx.ParseMultipartForm() { return } } diff --git a/services/mailer/incoming/incoming_handler.go b/services/mailer/incoming/incoming_handler.go index 38a234eac1..440b3a6b59 100644 --- a/services/mailer/incoming/incoming_handler.go +++ b/services/mailer/incoming/incoming_handler.go @@ -6,6 +6,7 @@ package incoming import ( "bytes" "context" + "errors" "fmt" issues_model "code.gitea.io/gitea/models/issues" @@ -85,7 +86,9 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u attachmentIDs := make([]string, 0, len(content.Attachments)) if setting.Attachment.Enabled { for _, attachment := range content.Attachments { - a, err := attachment_service.UploadAttachment(ctx, bytes.NewReader(attachment.Content), setting.Attachment.AllowedTypes, int64(len(attachment.Content)), &repo_model.Attachment{ + attachmentBuf := bytes.NewReader(attachment.Content) + uploaderFile := attachment_service.NewLimitedUploaderKnownSize(attachmentBuf, attachmentBuf.Size()) + a, err := attachment_service.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, setting.Attachment.AllowedTypes, &repo_model.Attachment{ Name: attachment.Name, UploaderID: doer.ID, RepoID: issue.Repo.ID, @@ -95,6 +98,11 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u log.Info("Skipping disallowed attachment type: %s", attachment.Name) continue } + if errors.Is(err, util.ErrContentTooLarge) { + log.Info("Skipping attachment exceeding size limit: %s", attachment.Name) + continue + } + return err } attachmentIDs = append(attachmentIDs, a.UUID) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 325f0b78a0..966aff12f8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -9569,6 +9569,9 @@ "404": { "$ref": "#/responses/error" }, + "413": { + "$ref": "#/responses/error" + }, "422": { "$ref": "#/responses/validationError" }, @@ -10194,6 +10197,9 @@ "404": { "$ref": "#/responses/error" }, + "413": { + "$ref": "#/responses/error" + }, "422": { "$ref": "#/responses/validationError" }, @@ -15510,6 +15516,9 @@ }, "404": { "$ref": "#/responses/notFound" + }, + "413": { + "$ref": "#/responses/error" } } } diff --git a/tests/integration/api_comment_attachment_test.go b/tests/integration/api_comment_attachment_test.go index cddae27d94..ae90331962 100644 --- a/tests/integration/api_comment_attachment_test.go +++ b/tests/integration/api_comment_attachment_test.go @@ -6,7 +6,6 @@ package integration import ( "bytes" "fmt" - "io" "mime/multipart" "net/http" "testing" @@ -95,15 +94,13 @@ func TestAPICreateCommentAttachment(t *testing.T) { session := loginUser(t, repoOwner.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) - filename := "image.png" - buff := generateImg() body := &bytes.Buffer{} // Setup multi-part writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("attachment", filename) + part, err := writer.CreateFormFile("attachment", "image.png") assert.NoError(t, err) - _, err = io.Copy(part, &buff) + _, err = part.Write(testGeneratePngBytes()) assert.NoError(t, err) err = writer.Close() assert.NoError(t, err) diff --git a/tests/integration/api_issue_attachment_test.go b/tests/integration/api_issue_attachment_test.go index 6806d27df2..12acaeae5d 100644 --- a/tests/integration/api_issue_attachment_test.go +++ b/tests/integration/api_issue_attachment_test.go @@ -6,7 +6,6 @@ package integration import ( "bytes" "fmt" - "io" "mime/multipart" "net/http" "testing" @@ -72,15 +71,13 @@ func TestAPICreateIssueAttachment(t *testing.T) { session := loginUser(t, repoOwner.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) - filename := "image.png" - buff := generateImg() body := &bytes.Buffer{} // Setup multi-part writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("attachment", filename) + part, err := writer.CreateFormFile("attachment", "image.png") assert.NoError(t, err) - _, err = io.Copy(part, &buff) + _, err = part.Write(testGeneratePngBytes()) assert.NoError(t, err) err = writer.Close() assert.NoError(t, err) diff --git a/tests/integration/api_releases_test.go b/tests/integration/api_releases_test.go index 1a04bec64e..53d37df8e0 100644 --- a/tests/integration/api_releases_test.go +++ b/tests/integration/api_releases_test.go @@ -9,6 +9,7 @@ import ( "io" "mime/multipart" "net/http" + "net/http/httptest" "net/url" "strings" "testing" @@ -18,7 +19,9 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -294,67 +297,70 @@ func TestAPIDeleteReleaseByTagName(t *testing.T) { func TestAPIUploadAssetRelease(t *testing.T) { defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Attachment.MaxSize, 1)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) session := loginUser(t, owner.LowerName) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - r := createNewReleaseUsingAPI(t, token, owner, repo, "release-tag", "", "Release Tag", "test") + bufImageBytes := testGeneratePngBytes() + bufLargeBytes := bytes.Repeat([]byte{' '}, 2*1024*1024) - filename := "image.png" - buff := generateImg() - - assetURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", owner.Name, repo.Name, r.ID) + release := createNewReleaseUsingAPI(t, token, owner, repo, "release-tag", "", "Release Tag", "test") + assetURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", owner.Name, repo.Name, release.ID) t.Run("multipart/form-data", func(t *testing.T) { defer tests.PrintCurrentTest(t)() + const filename = "image.png" - body := &bytes.Buffer{} + performUpload := func(t *testing.T, uploadURL string, buf []byte, expectedStatus int) *httptest.ResponseRecorder { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + part, err := writer.CreateFormFile("attachment", filename) + assert.NoError(t, err) + _, err = io.Copy(part, bytes.NewReader(bufImageBytes)) + assert.NoError(t, err) + err = writer.Close() + assert.NoError(t, err) - writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("attachment", filename) - assert.NoError(t, err) - _, err = io.Copy(part, bytes.NewReader(buff.Bytes())) - assert.NoError(t, err) - err = writer.Close() - assert.NoError(t, err) + req := NewRequestWithBody(t, http.MethodPost, uploadURL, bytes.NewReader(body.Bytes())). + AddTokenAuth(token). + SetHeader("Content-Type", writer.FormDataContentType()) + return MakeRequest(t, req, http.StatusCreated) + } - req := NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(body.Bytes())). - AddTokenAuth(token). - SetHeader("Content-Type", writer.FormDataContentType()) - resp := MakeRequest(t, req, http.StatusCreated) + performUpload(t, assetURL, bufLargeBytes, http.StatusRequestEntityTooLarge) - var attachment *api.Attachment - DecodeJSON(t, resp, &attachment) - - assert.Equal(t, filename, attachment.Name) - assert.EqualValues(t, 104, attachment.Size) - - req = NewRequestWithBody(t, http.MethodPost, assetURL+"?name=test-asset", bytes.NewReader(body.Bytes())). - AddTokenAuth(token). - SetHeader("Content-Type", writer.FormDataContentType()) - resp = MakeRequest(t, req, http.StatusCreated) - - var attachment2 *api.Attachment - DecodeJSON(t, resp, &attachment2) - - assert.Equal(t, "test-asset", attachment2.Name) - assert.EqualValues(t, 104, attachment2.Size) + t.Run("UploadDefaultName", func(t *testing.T) { + resp := performUpload(t, assetURL, bufImageBytes, http.StatusCreated) + var attachment api.Attachment + DecodeJSON(t, resp, &attachment) + assert.Equal(t, filename, attachment.Name) + assert.EqualValues(t, 104, attachment.Size) + }) + t.Run("UploadWithName", func(t *testing.T) { + resp := performUpload(t, assetURL+"?name=test-asset", bufImageBytes, http.StatusCreated) + var attachment api.Attachment + DecodeJSON(t, resp, &attachment) + assert.Equal(t, "test-asset", attachment.Name) + assert.EqualValues(t, 104, attachment.Size) + }) }) t.Run("application/octet-stream", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(buff.Bytes())). - AddTokenAuth(token) + req := NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(bufImageBytes)).AddTokenAuth(token) MakeRequest(t, req, http.StatusBadRequest) - req = NewRequestWithBody(t, http.MethodPost, assetURL+"?name=stream.bin", bytes.NewReader(buff.Bytes())). - AddTokenAuth(token) + req = NewRequestWithBody(t, http.MethodPost, assetURL+"?name=stream.bin", bytes.NewReader(bufLargeBytes)).AddTokenAuth(token) + MakeRequest(t, req, http.StatusRequestEntityTooLarge) + + req = NewRequestWithBody(t, http.MethodPost, assetURL+"?name=stream.bin", bytes.NewReader(bufImageBytes)).AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusCreated) - var attachment *api.Attachment + var attachment api.Attachment DecodeJSON(t, resp, &attachment) assert.Equal(t, "stream.bin", attachment.Name) diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index 30c394e9b0..44aaee09f8 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -7,7 +7,6 @@ import ( "bytes" "image" "image/png" - "io" "mime/multipart" "net/http" "strings" @@ -21,22 +20,21 @@ import ( "github.com/stretchr/testify/assert" ) -func generateImg() bytes.Buffer { - // Generate image +func testGeneratePngBytes() []byte { myImage := image.NewRGBA(image.Rect(0, 0, 32, 32)) var buff bytes.Buffer - png.Encode(&buff, myImage) - return buff + _ = png.Encode(&buff, myImage) + return buff.Bytes() } -func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filename string, buff bytes.Buffer, expectedStatus int) string { +func testCreateIssueAttachment(t *testing.T, session *TestSession, csrf, repoURL, filename string, content []byte, expectedStatus int) string { body := &bytes.Buffer{} // Setup multi-part writer := multipart.NewWriter(body) part, err := writer.CreateFormFile("file", filename) assert.NoError(t, err) - _, err = io.Copy(part, &buff) + _, err = part.Write(content) assert.NoError(t, err) err = writer.Close() assert.NoError(t, err) @@ -57,14 +55,14 @@ func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filenam func TestCreateAnonymousAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() session := emptyTestSession(t) - createAttachment(t, session, GetAnonymousCSRFToken(t, session), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther) + testCreateIssueAttachment(t, session, GetAnonymousCSRFToken(t, session), "user2/repo1", "image.png", testGeneratePngBytes(), http.StatusSeeOther) } func TestCreateIssueAttachment(t *testing.T) { defer tests.PrepareTestEnv(t)() const repoURL = "user2/repo1" session := loginUser(t, "user2") - uuid := createAttachment(t, session, GetUserCSRFToken(t, session), repoURL, "image.png", generateImg(), http.StatusOK) + uuid := testCreateIssueAttachment(t, session, GetUserCSRFToken(t, session), repoURL, "image.png", testGeneratePngBytes(), http.StatusOK) req := NewRequest(t, "GET", repoURL+"/issues/new") resp := session.MakeRequest(t, req, http.StatusOK)