From 73e0e442988704487b285d59f7494a85229e41e2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 Apr 2026 01:17:05 +0800 Subject: [PATCH] Fix various problems (#37129) * Fix #37128 * Manually tested with various cases (issue, pr) X (close, reopen) * Fix #36792 * Fix the comment * Fix #36755 * Add a "sleep 3" * Follow up #36697 * Clarify the "attachment uploading" problem and function call --------- Signed-off-by: wxiaoguang Co-authored-by: TheFox0x7 --- custom/conf/app.example.ini | 23 ++++++------ docker/root/etc/s6/openssh/finish | 6 ++++ routers/api/v1/repo/issue_attachment.go | 2 +- .../api/v1/repo/issue_comment_attachment.go | 2 +- routers/api/v1/repo/release_attachment.go | 2 +- routers/web/repo/attachment.go | 12 +++---- routers/web/repo/issue_comment.go | 36 +++++++++---------- services/attachment/attachment.go | 13 ++++--- services/mailer/incoming/incoming_handler.go | 2 +- 9 files changed, 56 insertions(+), 42 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index e5d5ca669f..26c512b6f6 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -175,14 +175,15 @@ RUN_USER = ; git ;; The port number the builtin SSH server should listen on, defaults to SSH_PORT ;SSH_LISTEN_PORT = ;; -;; Root path of SSH directory, default is '~/.ssh', but you have to use '/home/git/.ssh'. +;; Root path of SSH user directory for the system's standalone SSH server if Gitea is not using its builtin SSH server. +;; Default is the '.ssh' directory in the run user's home directory. ;SSH_ROOT_PATH = ;; -;; Gitea will create a authorized_keys file by default when it is not using the internal ssh server +;; Gitea will create an authorized_keys file by default when it is not using the builtin SSH server ;; If you intend to use the AuthorizedKeysCommand functionality then you should turn this off. ;SSH_CREATE_AUTHORIZED_KEYS_FILE = true ;; -;; Gitea will create a authorized_principals file by default when it is not using the internal ssh server +;; Gitea will create an authorized_principals file by default when it is not using the builtin SSH server ;; If you intend to use the AuthorizedPrincipalsCommand functionality then you should turn this off. ;SSH_CREATE_AUTHORIZED_PRINCIPALS_FILE = true ;; @@ -1178,16 +1179,16 @@ LEVEL = Info ;[repository.release] ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types. +;; Comma-separated list of allowed release attachment file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types. ;ALLOWED_TYPES = ;; ;; Number of releases that are displayed on release page ;DEFAULT_PAGING_NUM = 10 ;; -;; Max size of each file in megabytes. Defaults to 2GB +;; Max size of each release attachment file in megabytes. Defaults to 2GB ;FILE_MAX_SIZE = 2048 ;; -;; Max number of files per upload. Defaults to 5 +;; Max number of release attachment files per upload. Defaults to 5 ;MAX_FILES = 5 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -1994,16 +1995,18 @@ LEVEL = Info ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; -;; Whether issue and pull request attachments are enabled. Defaults to `true` +;; Whether issue, pull-request and release attachments are enabled. Defaults to `true` +;; ALLOWED_TYPES/MAX_SIZE/MAX_FILES in this section only affect issue and pull-request attachments, not release attachments. +;; Release attachment has its own config options in [repository.release] section. ;ENABLED = true ;; -;; Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types. +;; Comma-separated list of allowed issue/pull-request attachment file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types. ;ALLOWED_TYPES = .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 ;; -;; Max size of each file. Defaults to 100MB +;; Max size of each issue/pull-request attachment file. Defaults to 100MB ;MAX_SIZE = 100 ;; -;; Max number of files per upload. Defaults to 5 +;; Max number of issue/pull-request attachment files per upload. Defaults to 5 ;MAX_FILES = 5 ;; ;; Storage type for attachments, `local` for local disk or `minio` for s3 compatible diff --git a/docker/root/etc/s6/openssh/finish b/docker/root/etc/s6/openssh/finish index 06bd986563..8e89b35c51 100755 --- a/docker/root/etc/s6/openssh/finish +++ b/docker/root/etc/s6/openssh/finish @@ -1,2 +1,8 @@ #!/bin/bash +# $1 = exit code of the run script, $2 = signal +if [ "$1" -ne 0 ]; then + # avoid immediately restarting the sshd service, which may cause CPU 100% if the error (permission, configuration) is not fixed + echo "openssh failed with exit code $1 - waiting a short delay before attempting a restart" + sleep 3 +fi exit 0 diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index bfe9c92f1c..b64f713401 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -186,7 +186,7 @@ func CreateIssueAttachment(ctx *context.APIContext) { } uploaderFile := attachment_service.NewLimitedUploaderKnownSize(file, header.Size) - attachment, err := attachment_service.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, setting.Attachment.AllowedTypes, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachmentForIssue(ctx, uploaderFile, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 3227f5ddee..30b79a1d54 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -193,7 +193,7 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { } uploaderFile := attachment_service.NewLimitedUploaderKnownSize(file, header.Size) - attachment, err := attachment_service.UploadAttachmentGeneralSizeLimit(ctx, uploaderFile, setting.Attachment.AllowedTypes, &repo_model.Attachment{ + attachment, err := attachment_service.UploadAttachmentForIssue(ctx, uploaderFile, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index 19075961f3..c0bf89a9d0 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -242,7 +242,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { } // Create a new attachment and save the file - attach, err := attachment_service.UploadAttachmentReleaseSizeLimit(ctx, uploaderFile, setting.Repository.Release.AllowedTypes, &repo_model.Attachment{ + attach, err := attachment_service.UploadAttachmentForRelease(ctx, uploaderFile, &repo_model.Attachment{ Name: filename, UploaderID: ctx.Doer.ID, RepoID: ctx.Repo.Repository.ID, diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 8b35f52ed6..bb2002521c 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -23,16 +23,16 @@ import ( // UploadIssueAttachment response for Issue/PR attachments func UploadIssueAttachment(ctx *context.Context) { - uploadAttachment(ctx, ctx.Repo.Repository.ID, setting.Attachment.AllowedTypes) + uploadAttachment(ctx, ctx.Repo.Repository.ID, attachment.UploadAttachmentForIssue) } // UploadReleaseAttachment response for uploading release attachments func UploadReleaseAttachment(ctx *context.Context) { - uploadAttachment(ctx, ctx.Repo.Repository.ID, setting.Repository.Release.AllowedTypes) + uploadAttachment(ctx, ctx.Repo.Repository.ID, attachment.UploadAttachmentForRelease) } // UploadAttachment response for uploading attachments -func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { +func uploadAttachment(ctx *context.Context, repoID int64, uploadFunc attachment.UploadAttachmentFunc) { if !setting.Attachment.Enabled { ctx.HTTPError(http.StatusNotFound, "attachment is not enabled") return @@ -46,7 +46,7 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { defer file.Close() uploaderFile := attachment.NewLimitedUploaderKnownSize(file, header.Size) - attach, err := attachment.UploadAttachmentReleaseSizeLimit(ctx, uploaderFile, allowedTypes, &repo_model.Attachment{ + attach, err := uploadFunc(ctx, uploaderFile, &repo_model.Attachment{ Name: header.Filename, UploaderID: ctx.Doer.ID, RepoID: repoID, @@ -56,7 +56,7 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { ctx.HTTPError(http.StatusBadRequest, err.Error()) return } - ctx.ServerError("UploadAttachmentReleaseSizeLimit", err) + ctx.ServerError("uploadAttachment(uploadFunc)", err) return } @@ -119,7 +119,7 @@ func DeleteAttachment(ctx *context.Context) { }) } -// GetAttachment serve attachments with the given UUID +// ServeAttachment serve attachments with the given UUID func ServeAttachment(ctx *context.Context, uuid string) { attach, err := repo_model.GetAttachmentByUUID(ctx, uuid) if err != nil { diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index ac30b678c3..860dcd7442 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -57,22 +57,26 @@ func NewComment(ctx *context.Context) { return } + redirect := fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, issueType, issue.Index) attachments := util.Iif(setting.Attachment.Enabled, form.Files, nil) - // Can allow empty comments if there are attachments or a status change (close, reopen, approve, reject) - // So, only stop if there is no content, no attachments, and no status change. - if form.Content == "" && len(attachments) == 0 && form.Status == "" { - ctx.JSONError(ctx.Tr("repo.issues.comment_no_content")) - return - } - - comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments) - if err != nil { - if errors.Is(err, user_model.ErrBlockedUser) { - ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user")) - } else { - ctx.ServerError("CreateIssueComment", err) + // allow empty content if there are attachments + if form.Content != "" || len(attachments) > 0 { + comment, err := issue_service.CreateIssueComment(ctx, ctx.Doer, ctx.Repo.Repository, issue, form.Content, attachments) + if err != nil { + if errors.Is(err, user_model.ErrBlockedUser) { + ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user")) + } else { + ctx.ServerError("CreateIssueComment", err) + } + return } + // redirect to the comment's hashtag + redirect += "#" + comment.HashTag() + } else if form.Status == "" { + // if no status change (close, reopen), it is a plain comment, and content is required + // "approve/reject" are handled differently in SubmitReview + ctx.JSONError(ctx.Tr("repo.issues.comment_no_content")) return } @@ -86,6 +90,7 @@ func NewComment(ctx *context.Context) { !(issue.IsPull && issue.PullRequest.HasMerged) { // Duplication and conflict check should apply to reopen pull request. var branchOtherUnmergedPR *issues_model.PullRequest + var err error if form.Status == "reopen" && issue.IsPull { pull := issue.PullRequest branchOtherUnmergedPR, err = issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow) @@ -179,11 +184,6 @@ func NewComment(ctx *context.Context) { } } // end if: handle close or reopen - // Redirect to the comment, add hashtag if it exists - redirect := fmt.Sprintf("%s/%s/%d", ctx.Repo.RepoLink, issueType, issue.Index) - if comment != nil { - redirect += "#" + comment.HashTag() - } ctx.JSONRedirect(redirect) } diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index d69253dd59..a824cdb246 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -54,12 +54,17 @@ func NewLimitedUploaderMaxBytesReader(r io.ReadCloser, w http.ResponseWriter) *U 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) +type UploadAttachmentFunc func(ctx context.Context, file *UploaderFile, attach *repo_model.Attachment) (*repo_model.Attachment, error) + +func UploadAttachmentForIssue(ctx context.Context, file *UploaderFile, attach *repo_model.Attachment) (*repo_model.Attachment, error) { + return uploadAttachment(ctx, file, setting.Attachment.AllowedTypes, setting.Attachment.MaxSize<<20, attach) } -func UploadAttachmentReleaseSizeLimit(ctx context.Context, file *UploaderFile, allowedTypes string, attach *repo_model.Attachment) (*repo_model.Attachment, error) { - return uploadAttachment(ctx, file, allowedTypes, setting.Repository.Release.FileMaxSize<<20, attach) +func UploadAttachmentForRelease(ctx context.Context, file *UploaderFile, attach *repo_model.Attachment) (*repo_model.Attachment, error) { + // FIXME: although the release attachment has different settings from the issue attachment, + // it still uses the same attachment table, the same storage and the same upload logic + // So if the "issue attachment [attachment]" is not enabled, it will also affect the release attachment, which is not expected. + return uploadAttachment(ctx, file, setting.Repository.Release.AllowedTypes, setting.Repository.Release.FileMaxSize<<20, attach) } func uploadAttachment(ctx context.Context, file *UploaderFile, allowedTypes string, maxFileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { diff --git a/services/mailer/incoming/incoming_handler.go b/services/mailer/incoming/incoming_handler.go index dfb7b1244c..3c350e3d5d 100644 --- a/services/mailer/incoming/incoming_handler.go +++ b/services/mailer/incoming/incoming_handler.go @@ -88,7 +88,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u for _, attachment := range content.Attachments { 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{ + a, err := attachment_service.UploadAttachmentForIssue(ctx, uploaderFile, &repo_model.Attachment{ Name: attachment.Name, UploaderID: doer.ID, RepoID: issue.Repo.ID,