From 734230480927d9e0fd5bdcde76d96a48f05165d5 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Mon, 22 Sep 2025 17:29:49 +0800 Subject: [PATCH 01/10] fix attachment file size limit in server backend fix #35512 Signed-off-by: a1012112796 <1012112796@qq.com> --- routers/api/v1/repo/issue_attachment.go | 4 ++-- .../api/v1/repo/issue_comment_attachment.go | 4 ++-- routers/api/v1/repo/release_attachment.go | 4 ++-- routers/web/repo/attachment.go | 2 +- services/attachment/attachment.go | 20 ++++++++++++++++++- services/mailer/incoming/incoming_handler.go | 7 ++++++- 6 files changed, 32 insertions(+), 9 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 3f751a295c37e..921965aa0d6a3 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -181,14 +181,14 @@ func CreateIssueAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, setting.Attachment.MaxSize<<20, header.Size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, IssueID: issue.ID, }) if err != nil { - if upload.IsErrFileTypeForbidden(err) { + if upload.IsErrFileTypeForbidden(err) || attachment_service.IsErrAttachmentSizeExceed(err) { ctx.APIError(http.StatusUnprocessableEntity, 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 5f660c57504dd..3dbdb325a285a 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -189,7 +189,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { filename = query } - attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, setting.Attachment.MaxSize<<20, header.Size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, @@ -197,7 +197,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { CommentID: comment.ID, }) if err != nil { - if upload.IsErrFileTypeForbidden(err) { + if upload.IsErrFileTypeForbidden(err) || attachment_service.IsErrAttachmentSizeExceed(err) { ctx.APIError(http.StatusUnprocessableEntity, err) } else { ctx.APIErrorInternal(err) diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index defde81a1d2ae..9fb5a4ea9c780 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -234,14 +234,14 @@ 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.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, setting.Attachment.MaxSize<<20, size, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, ReleaseID: releaseID, }) if err != nil { - if upload.IsErrFileTypeForbidden(err) { + if upload.IsErrFileTypeForbidden(err) || attachment_service.IsErrAttachmentSizeExceed(err) { ctx.APIError(http.StatusBadRequest, err) return } diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index f696669196100..ff46c2894bac2 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -45,7 +45,7 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { } defer file.Close() - attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, &repo_model.Attachment{ + attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, setting.Attachment.MaxSize<<20, &repo_model.Attachment{ Name: header.Filename, UploaderID: ctx.Doer.ID, RepoID: repoID, diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index ccb97c66c82b1..c12395387c9b0 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -38,8 +38,22 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R return attach, err } +type ErrAttachmentSizeExceed struct { + MaxSize int64 + Size int64 +} + +func (e *ErrAttachmentSizeExceed) Error() string { + return fmt.Sprintf("attachment size %d exceed limit %d", e.Size, e.MaxSize) +} + +func IsErrAttachmentSizeExceed(err error) bool { + _, ok := err.(*ErrAttachmentSizeExceed) + return ok +} + // 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) { +func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, maxFileSize, fileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { buf := make([]byte, 1024) n, _ := util.ReadAtMost(file, buf) buf = buf[:n] @@ -48,6 +62,10 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, return nil, err } + if maxFileSize >= 0 && fileSize > (maxFileSize) { + return nil, &ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} + } + return NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), file), fileSize) } diff --git a/services/mailer/incoming/incoming_handler.go b/services/mailer/incoming/incoming_handler.go index 38a234eac1fd2..c2e6535935af2 100644 --- a/services/mailer/incoming/incoming_handler.go +++ b/services/mailer/incoming/incoming_handler.go @@ -85,7 +85,7 @@ 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{ + a, err := attachment_service.UploadAttachment(ctx, bytes.NewReader(attachment.Content), setting.Attachment.AllowedTypes, setting.Attachment.MaxSize<<20, int64(len(attachment.Content)), &repo_model.Attachment{ Name: attachment.Name, UploaderID: doer.ID, RepoID: issue.Repo.ID, @@ -95,6 +95,11 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u log.Info("Skipping disallowed attachment type: %s", attachment.Name) continue } + if attachment_service.IsErrAttachmentSizeExceed(err) { + log.Info("Skipping attachment exceeding size limit: %s", attachment.Name) + continue + } + return err } attachmentIDs = append(attachmentIDs, a.UUID) From 08035a7af260dd474a0de8b69c594c280b4ab7f4 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 23 Sep 2025 10:45:01 +0800 Subject: [PATCH 02/10] fix test and apply suggestions Signed-off-by: a1012112796 <1012112796@qq.com> --- routers/api/v1/repo/issue_attachment.go | 6 +++++- routers/api/v1/repo/issue_comment_attachment.go | 6 +++++- routers/api/v1/repo/release_attachment.go | 10 +++++++++- routers/web/repo/attachment.go | 2 +- templates/swagger/v1_json.tmpl | 9 +++++++++ 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 921965aa0d6a3..a1c4500e89e4b 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -154,6 +154,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { // "$ref": "#/responses/error" // "404": // "$ref": "#/responses/error" + // "413": + // "$ref": "#/responses/error" // "422": // "$ref": "#/responses/validationError" // "423": @@ -188,8 +190,10 @@ func CreateIssueAttachment(ctx *context.APIContext) { IssueID: issue.ID, }) if err != nil { - if upload.IsErrFileTypeForbidden(err) || attachment_service.IsErrAttachmentSizeExceed(err) { + if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusUnprocessableEntity, err) + } else if attachment_service.IsErrAttachmentSizeExceed(err) { + 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 3dbdb325a285a..62f0963ba23df 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -161,6 +161,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { // "$ref": "#/responses/forbidden" // "404": // "$ref": "#/responses/error" + // "413": + // "$ref": "#/responses/error" // "422": // "$ref": "#/responses/validationError" // "423": @@ -197,8 +199,10 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { CommentID: comment.ID, }) if err != nil { - if upload.IsErrFileTypeForbidden(err) || attachment_service.IsErrAttachmentSizeExceed(err) { + if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusUnprocessableEntity, err) + } else if attachment_service.IsErrAttachmentSizeExceed(err) { + 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 9fb5a4ea9c780..685ad87585fd0 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -191,6 +191,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 { @@ -241,10 +243,16 @@ func CreateReleaseAttachment(ctx *context.APIContext) { ReleaseID: releaseID, }) if err != nil { - if upload.IsErrFileTypeForbidden(err) || attachment_service.IsErrAttachmentSizeExceed(err) { + if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusBadRequest, err) return } + + if attachment_service.IsErrAttachmentSizeExceed(err) { + 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 ff46c2894bac2..e6ba598c2b638 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -45,7 +45,7 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { } defer file.Close() - attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, setting.Attachment.MaxSize<<20, &repo_model.Attachment{ + attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, setting.Attachment.MaxSize<<20, header.Size, &repo_model.Attachment{ Name: header.Filename, UploaderID: ctx.Doer.ID, RepoID: repoID, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6dbc7e2a0e817..a0e8470b194d5 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" } } } From e19a4b17a580a309eb6de885a8900426cc599a3a Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 24 Sep 2025 23:00:49 +0800 Subject: [PATCH 03/10] fix nits --- services/attachment/attachment.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index c12395387c9b0..17990a3fa1b38 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -43,15 +43,19 @@ type ErrAttachmentSizeExceed struct { Size int64 } -func (e *ErrAttachmentSizeExceed) Error() string { +func (e ErrAttachmentSizeExceed) Error() string { return fmt.Sprintf("attachment size %d exceed limit %d", e.Size, e.MaxSize) } func IsErrAttachmentSizeExceed(err error) bool { - _, ok := err.(*ErrAttachmentSizeExceed) + _, ok := err.(ErrAttachmentSizeExceed) return ok } +func (err ErrAttachmentSizeExceed) Unwrap() error { + return util.ErrInvalidArgument +} + // UploadAttachment upload new attachment into storage and update database func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, maxFileSize, fileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { buf := make([]byte, 1024) @@ -63,7 +67,7 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, } if maxFileSize >= 0 && fileSize > (maxFileSize) { - return nil, &ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} + return nil, ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} } return NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), file), fileSize) From 5f200eea5bd324302125ba8c32db94b6cfdb33db Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 24 Sep 2025 23:21:57 +0800 Subject: [PATCH 04/10] user modern golang error system --- modules/util/error.go | 1 + routers/api/v1/repo/issue_attachment.go | 4 +++- routers/api/v1/repo/issue_comment_attachment.go | 3 ++- routers/api/v1/repo/release_attachment.go | 4 +++- services/attachment/attachment.go | 11 +++-------- services/mailer/incoming/incoming_handler.go | 3 ++- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/modules/util/error.go b/modules/util/error.go index 6b2721618ec60..24fa1ba151dd3 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 a1c4500e89e4b..841b048ed59c6 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" @@ -192,7 +194,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { if err != nil { if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusUnprocessableEntity, err) - } else if attachment_service.IsErrAttachmentSizeExceed(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 62f0963ba23df..e5ec120464397 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" @@ -201,7 +202,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { if err != nil { if upload.IsErrFileTypeForbidden(err) { ctx.APIError(http.StatusUnprocessableEntity, err) - } else if attachment_service.IsErrAttachmentSizeExceed(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 685ad87585fd0..1e483af3e68f6 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -4,6 +4,7 @@ package repo import ( + "errors" "io" "net/http" "strings" @@ -12,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" @@ -248,7 +250,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { return } - if attachment_service.IsErrAttachmentSizeExceed(err) { + if errors.Is(err, util.ErrContentTooLarge) { ctx.APIError(http.StatusRequestEntityTooLarge, err) return } diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 17990a3fa1b38..99bd2f1f31a56 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -44,16 +44,11 @@ type ErrAttachmentSizeExceed struct { } func (e ErrAttachmentSizeExceed) Error() string { - return fmt.Sprintf("attachment size %d exceed limit %d", e.Size, e.MaxSize) + return fmt.Sprintf("attachment size %d exceeds limit %d", e.Size, e.MaxSize) } -func IsErrAttachmentSizeExceed(err error) bool { - _, ok := err.(ErrAttachmentSizeExceed) - return ok -} - -func (err ErrAttachmentSizeExceed) Unwrap() error { - return util.ErrInvalidArgument +func (e ErrAttachmentSizeExceed) Unwrap() error { + return util.ErrContentTooLarge } // UploadAttachment upload new attachment into storage and update database diff --git a/services/mailer/incoming/incoming_handler.go b/services/mailer/incoming/incoming_handler.go index c2e6535935af2..77ebc8d0cc42b 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" @@ -95,7 +96,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u log.Info("Skipping disallowed attachment type: %s", attachment.Name) continue } - if attachment_service.IsErrAttachmentSizeExceed(err) { + if errors.Is(err, util.ErrContentTooLarge) { log.Info("Skipping attachment exceeding size limit: %s", attachment.Name) continue } From f0408982cb63d042b4acce800cbca85faafcc061 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 6 Oct 2025 10:43:26 -0700 Subject: [PATCH 05/10] Update services/attachment/attachment.go Co-authored-by: 6543 <6543@obermui.de> Signed-off-by: Lunny Xiao --- services/attachment/attachment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 99bd2f1f31a56..9bdf37f03eeeb 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -61,7 +61,7 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, return nil, err } - if maxFileSize >= 0 && fileSize > (maxFileSize) { + if maxFileSize >= 0 && fileSize > maxFileSize { return nil, ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} } From c90df6d9ed7c09d3ac5f8217c512c328313e87cb Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 7 Oct 2025 09:30:30 +0200 Subject: [PATCH 06/10] enhance test TestUploadAttachment --- services/attachment/attachment_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/attachment/attachment_test.go b/services/attachment/attachment_test.go index 8ecac8d7a33ec..affc2b0914c9d 100644 --- a/services/attachment/attachment_test.go +++ b/services/attachment/attachment_test.go @@ -15,6 +15,7 @@ import ( _ "code.gitea.io/gitea/models/actions" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMain(m *testing.M) { @@ -28,15 +29,18 @@ func TestUploadAttachment(t *testing.T) { fPath := "./attachment_test.go" f, err := os.Open(fPath) - assert.NoError(t, err) + require.NoError(t, err) defer f.Close() + fs, err := f.Stat() + require.NoError(t, err) attach, err := NewAttachment(t.Context(), &repo_model.Attachment{ RepoID: 1, UploaderID: user.ID, Name: filepath.Base(fPath), - }, f, -1) + }, f, fs.Size()) assert.NoError(t, err) + assert.Equal(t, fs.Size(), attach.Size) attachment, err := repo_model.GetAttachmentByUUID(t.Context(), attach.UUID) assert.NoError(t, err) From a70dc88fb1d355619243800c08198d1228c4f4ca Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 7 Oct 2025 10:23:17 +0200 Subject: [PATCH 07/10] introduce attachmentLimitedReader to make sure io stream dont bypass size limits --- services/attachment/attachment.go | 35 +++++++++++++++----- services/attachment/attachment_test.go | 46 ++++++++++++++++++++++++-- services/attachment/reader.go | 32 ++++++++++++++++++ 3 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 services/attachment/reader.go diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 9bdf37f03eeeb..49b10850ce891 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -24,7 +24,7 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R return nil, fmt.Errorf("attachment %s should belong to a repository", attach.Name) } - err := db.WithTx(ctx, func(ctx context.Context) error { + if err := db.WithTx(ctx, func(ctx context.Context) error { attach.UUID = uuid.New().String() size, err := storage.Attachments.Save(attach.RelativePath(), file, size) if err != nil { @@ -33,9 +33,11 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R attach.Size = size return db.Insert(ctx, attach) - }) + }); err != nil { + return nil, err + } - return attach, err + return attach, nil } type ErrAttachmentSizeExceed struct { @@ -43,14 +45,22 @@ type ErrAttachmentSizeExceed struct { Size int64 } -func (e ErrAttachmentSizeExceed) Error() string { +func (e *ErrAttachmentSizeExceed) Error() string { + if e.Size == 0 { + return fmt.Sprintf("attachment size exceeds limit %d", e.MaxSize) + } return fmt.Sprintf("attachment size %d exceeds limit %d", e.Size, e.MaxSize) } -func (e ErrAttachmentSizeExceed) Unwrap() error { +func (e *ErrAttachmentSizeExceed) Unwrap() error { return util.ErrContentTooLarge } +func (e *ErrAttachmentSizeExceed) Is(target error) bool { + _, ok := target.(*ErrAttachmentSizeExceed) + return ok +} + // UploadAttachment upload new attachment into storage and update database func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, maxFileSize, fileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { buf := make([]byte, 1024) @@ -61,11 +71,20 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, return nil, err } - if maxFileSize >= 0 && fileSize > maxFileSize { - return nil, ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} + reader := io.MultiReader(bytes.NewReader(buf), file) + + // enforce file size limit + if maxFileSize >= 0 { + if fileSize > maxFileSize { + return nil, &ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} + } + // limit reader to max file size with additional 1k more, + // to allow side-cases where encoding tells us its exactly maxFileSize but the actual created file is bit more, + // while still make sure the limit is enforced + reader = attachmentLimitedReader(reader, maxFileSize+1024) } - return NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), file), fileSize) + return NewAttachment(ctx, attach, reader, fileSize) } // UpdateAttachment updates an attachment, verifying that its name is among the allowed types. diff --git a/services/attachment/attachment_test.go b/services/attachment/attachment_test.go index affc2b0914c9d..522bf10476e63 100644 --- a/services/attachment/attachment_test.go +++ b/services/attachment/attachment_test.go @@ -22,9 +22,8 @@ func TestMain(m *testing.M) { unittest.MainTest(m) } -func TestUploadAttachment(t *testing.T) { +func TestNewAttachment(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) fPath := "./attachment_test.go" @@ -47,3 +46,46 @@ func TestUploadAttachment(t *testing.T) { assert.Equal(t, user.ID, attachment.UploaderID) assert.Equal(t, int64(0), attachment.DownloadCount) } + +func TestUploadAttachment(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + fPath := "./attachment_test.go" + f, err := os.Open(fPath) + require.NoError(t, err) + defer f.Close() + fs, err := f.Stat() + require.NoError(t, err) + + t.Run("size to big", func(t *testing.T) { + attach, err := UploadAttachment(t.Context(), f, "", 10, fs.Size(), &repo_model.Attachment{ + RepoID: 1, + UploaderID: user.ID, + Name: filepath.Base(fPath), + }) + assert.ErrorIs(t, err, &ErrAttachmentSizeExceed{}) + assert.Nil(t, attach) + }) + + t.Run("size was lied about", func(t *testing.T) { + attach, err := UploadAttachment(t.Context(), f, "", 10, 10, &repo_model.Attachment{ + RepoID: 1, + UploaderID: user.ID, + Name: filepath.Base(fPath), + }) + assert.ErrorIs(t, err, &ErrAttachmentSizeExceed{}) + assert.Nil(t, attach) + }) + + t.Run("size was correct", func(t *testing.T) { + attach, err := UploadAttachment(t.Context(), f, "", fs.Size(), fs.Size(), &repo_model.Attachment{ + RepoID: 1, + UploaderID: user.ID, + Name: filepath.Base(fPath), + }) + assert.NoError(t, err) + require.NotNil(t, attach) + assert.Equal(t, user.ID, attach.UploaderID) + }) +} diff --git a/services/attachment/reader.go b/services/attachment/reader.go new file mode 100644 index 0000000000000..ed37a8e3e8069 --- /dev/null +++ b/services/attachment/reader.go @@ -0,0 +1,32 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package attachment + +import "io" + +// attachmentLimitedReader returns a Reader that reads from r +// but stops with EOF after n bytes. +// The underlying implementation is a *attachmentReader. +func attachmentLimitedReader(r io.Reader, n int64) io.Reader { return &attachmentReader{r, n} } + +// A attachmentReader reads from R but limits the amount of +// data returned to just N bytes. Each call to Read +// updates N to reflect the new amount remaining. +// Read returns EOF when N <= 0 or when the underlying R returns EOF. +type attachmentReader struct { + R io.Reader // underlying reader + N int64 // max bytes remaining +} + +func (l *attachmentReader) Read(p []byte) (n int, err error) { + if l.N <= 0 { + return 0, &ErrAttachmentSizeExceed{MaxSize: l.N} + } + if int64(len(p)) > l.N { + p = p[0:l.N] + } + n, err = l.R.Read(p) + l.N -= int64(n) + return +} From 73165938115ef75ad9cbb3e19ef1b6b1e774b0a3 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 7 Oct 2025 10:27:55 +0200 Subject: [PATCH 08/10] Update services/attachment/reader.go --- services/attachment/reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/attachment/reader.go b/services/attachment/reader.go index ed37a8e3e8069..ba8e67a1f9524 100644 --- a/services/attachment/reader.go +++ b/services/attachment/reader.go @@ -6,7 +6,7 @@ package attachment import "io" // attachmentLimitedReader returns a Reader that reads from r -// but stops with EOF after n bytes. +// but errors with ErrAttachmentSizeExceed after n bytes. // The underlying implementation is a *attachmentReader. func attachmentLimitedReader(r io.Reader, n int64) io.Reader { return &attachmentReader{r, n} } From 9e1648288a465dd73c9e60592f2109297c07daa6 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 7 Oct 2025 10:29:29 +0200 Subject: [PATCH 09/10] Update services/attachment/reader.go --- services/attachment/reader.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/attachment/reader.go b/services/attachment/reader.go index ba8e67a1f9524..d424bf2ccaec2 100644 --- a/services/attachment/reader.go +++ b/services/attachment/reader.go @@ -13,7 +13,8 @@ func attachmentLimitedReader(r io.Reader, n int64) io.Reader { return &attachmen // A attachmentReader reads from R but limits the amount of // data returned to just N bytes. Each call to Read // updates N to reflect the new amount remaining. -// Read returns EOF when N <= 0 or when the underlying R returns EOF. +// Read returns ErrAttachmentSizeExceed when N <= 0. +// Underlying errors are passed through. type attachmentReader struct { R io.Reader // underlying reader N int64 // max bytes remaining From 9c1e0b5e45af675df157fd6c56b13781453e1c6d Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 7 Oct 2025 10:31:32 +0200 Subject: [PATCH 10/10] Update services/attachment/reader.go --- services/attachment/reader.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/attachment/reader.go b/services/attachment/reader.go index d424bf2ccaec2..db0e44301eec0 100644 --- a/services/attachment/reader.go +++ b/services/attachment/reader.go @@ -5,6 +5,8 @@ package attachment import "io" +// modified version of io.LimitReader: https://cs.opensource.google/go/go/+/refs/tags/go1.25.1:src/io/io.go;l=458-482 + // attachmentLimitedReader returns a Reader that reads from r // but errors with ErrAttachmentSizeExceed after n bytes. // The underlying implementation is a *attachmentReader.