fix attachment file size limit in server backend (#35519) (#35720)

Backport #35519 by @a1012112796

fix #35512

Co-authored-by: a1012112796 <1012112796@qq.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

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))

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,
+		// FIXME: this size is used for both "issue attachment" and "release attachment"
-		Enabled:      true,
+		// The design is not right, these two should be different settings
+		MaxSize: 2048,
+
+		MaxFiles: 5,
+		Enabled:  true,
 	}
 	sec, _ := rootCfg.GetSection("attachment")
 	if sec == nil {

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

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)
 		}

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)
 		}

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
 	}

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,

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)

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
+type UploaderFile struct {
-func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, fileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) {
+	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.

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
+				if !ctx.ParseMultipartForm() {
-					ctx.APIErrorInternal(err)
 					return
 				}
 			}

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")

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
+				if !ctx.ParseMultipartForm() {
-					ctx.ServerError("ParseMultipartForm", err)
 					return
 				}
 			}

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)

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"
           }
         }
       }

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)

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)

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"
+	release := createNewReleaseUsingAPI(t, token, owner, repo, "release-tag", "", "Release Tag", "test")
-	buff := generateImg()
+	assetURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", owner.Name, repo.Name, release.ID)
-
-	assetURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", owner.Name, repo.Name, r.ID)
 
 	t.Run("multipart/form-data", func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
+		const filename = "image.png"
+
+		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)
+
+			req := NewRequestWithBody(t, http.MethodPost, uploadURL, bytes.NewReader(body.Bytes())).
+				AddTokenAuth(token).
+				SetHeader("Content-Type", writer.FormDataContentType())
+			return MakeRequest(t, req, http.StatusCreated)
+		}
 
-		body := &bytes.Buffer{}
+		performUpload(t, assetURL, bufLargeBytes, http.StatusRequestEntityTooLarge)
-
+
-		writer := multipart.NewWriter(body)
+		t.Run("UploadDefaultName", func(t *testing.T) {
-		part, err := writer.CreateFormFile("attachment", filename)
+			resp := performUpload(t, assetURL, bufImageBytes, http.StatusCreated)
-		assert.NoError(t, err)
+			var attachment api.Attachment
-		_, err = io.Copy(part, bytes.NewReader(buff.Bytes()))
+			DecodeJSON(t, resp, &attachment)
-		assert.NoError(t, err)
+			assert.Equal(t, filename, attachment.Name)
-		err = writer.Close()
+			assert.EqualValues(t, 104, attachment.Size)
-		assert.NoError(t, err)
+		})
-
+		t.Run("UploadWithName", func(t *testing.T) {
-		req := NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(body.Bytes())).
+			resp := performUpload(t, assetURL+"?name=test-asset", bufImageBytes, http.StatusCreated)
-			AddTokenAuth(token).
+			var attachment api.Attachment
-			SetHeader("Content-Type", writer.FormDataContentType())
+			DecodeJSON(t, resp, &attachment)
-		resp := MakeRequest(t, req, http.StatusCreated)
+			assert.Equal(t, "test-asset", attachment.Name)
-
+			assert.EqualValues(t, 104, attachment.Size)
-		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("application/octet-stream", func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 
-		req := NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(buff.Bytes())).
+		req := NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(bufImageBytes)).AddTokenAuth(token)
-			AddTokenAuth(token)
 		MakeRequest(t, req, http.StatusBadRequest)
 
-		req = NewRequestWithBody(t, http.MethodPost, assetURL+"?name=stream.bin", bytes.NewReader(buff.Bytes())).
+		req = NewRequestWithBody(t, http.MethodPost, assetURL+"?name=stream.bin", bytes.NewReader(bufLargeBytes)).AddTokenAuth(token)
-			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)

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 {
+func testGeneratePngBytes() []byte {
-	// Generate image
 	myImage := image.NewRGBA(image.Rect(0, 0, 32, 32))
 	var buff bytes.Buffer
-	png.Encode(&buff, myImage)
+	_ = png.Encode(&buff, myImage)
-	return buff
+	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)