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 <wxiaoguang@gmail.com>
Co-authored-by: TheFox0x7 <thefox0x7@gmail.com>

Author: wxiaoguang

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

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

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,

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,

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,

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 {

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

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

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,