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

a1012112796 · wxiaoguang · GiteaBot · commit f9dfd00b4545 · 2025-10-21T15:07:25.000Z
fix #35512 --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
diff --git 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
@@ -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
@@ -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
@@ -4,13 +4,15 @@
 package repo
 
 import (
+	"errors"
 	"net/http"
 
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"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
@@ -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
@@ -4,14 +4,15 @@
 package repo
 
 import (
-	"io"
+	"errors"
 	"net/http"
 	"strings"
 
 	repo_model "code.gitea.io/gitea/models/repo"
 	"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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
diff --git a/tests/integration/api_comment_attachment_test.go b/tests/integration/api_comment_attachment_test.go
diff --git a/tests/integration/api_issue_attachment_test.go b/tests/integration/api_issue_attachment_test.go
diff --git a/tests/integration/api_releases_test.go b/tests/integration/api_releases_test.go
diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,8 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) {`
`45`	`45`	`}`
`46`	`46`	`defer file.Close()`
`47`	`47`
`48`		`- attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, &repo_model.Attachment{`
	`48`	`+ uploaderFile := attachment.NewLimitedUploaderKnownSize(file, header.Size)`
	`49`	`+ attach, err := attachment.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, allowedTypes, &repo_model.Attachment{`
`49`	`50`	`Name: header.Filename,`
`50`	`51`	`UploaderID: ctx.Doer.ID,`
`51`	`52`	`RepoID: repoID,`
Original file line number	Diff line number	Diff line change
`@@ -41,6 +41,8 @@ func UploadFileToServer(ctx *context.Context) {`
`41`	`41`	`return`
`42`	`42`	`}`
`43`	`43`
	`44`	`+ // FIXME: need to check the file size according to setting.Repository.Upload.FileMaxSize`
	`45`	`+`
`44`	`46`	`uploaded, err := repo_model.NewUpload(ctx, name, buf, file)`
`45`	`47`	`if err != nil {`
`46`	`48`	`ctx.ServerError("NewUpload", err)`
Original file line number	Diff line number	Diff line change
`@@ -229,8 +229,7 @@ func APIContexter() func(http.Handler) http.Handler {`
`229`	`229`
`230`	`230`	`// If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid.`
`231`	`231`	`if ctx.Req.Method == http.MethodPost && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") {`
`232`		`- if err := ctx.Req.ParseMultipartForm(setting.Attachment.MaxSize << 20); err != nil && !strings.Contains(err.Error(), "EOF") { // 32MB max size`
`233`		`- ctx.APIErrorInternal(err)`
	`232`	`+ if !ctx.ParseMultipartForm() {`
`234`	`233`	`return`
`235`	`234`	`}`
`236`	`235`	`}`