Properly handle the name query param in create issue attachment API

models/repo/attachment.go

@@ -26,6 +26,7 @@ type Attachment struct {
 	UploaderID        int64  `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added
 	CommentID         int64  `xorm:"INDEX"`
 	Name              string
+	OriginalName      string             `xorm:"-"` // only used while its being created
 	DownloadCount     int64              `xorm:"DEFAULT 0"`
 	Size              int64              `xorm:"DEFAULT 0"`
 	CreatedUnix       timeutil.TimeStamp `xorm:"created"`

routers/api/v1/repo/issue_attachment.go

@@ -173,16 +173,17 @@ func CreateIssueAttachment(ctx *context.APIContext) {
 	}
 	defer file.Close()
 
-	filename := header.Filename
+	attachmentName := header.Filename
 	if query := ctx.FormString("name"); query != "" {
-		filename = query
+		attachmentName = query
 	}
 
 	attachment, err := attachment.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{
-		Name:       filename,
-		UploaderID: ctx.Doer.ID,
-		RepoID:     ctx.Repo.Repository.ID,
-		IssueID:    issue.ID,
+		Name:         attachmentName,
+		OriginalName: header.Filename,
+		UploaderID:   ctx.Doer.ID,
+		RepoID:       ctx.Repo.Repository.ID,
+		IssueID:      issue.ID,
 	})
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "UploadAttachment", err)

routers/api/v1/repo/issue_comment_attachment.go

@@ -181,17 +181,18 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) {
 	}
 	defer file.Close()
 
-	filename := header.Filename
+	attachmentName := header.Filename
 	if query := ctx.FormString("name"); query != "" {
-		filename = query
+		attachmentName = query
 	}
 
 	attachment, err := attachment.UploadAttachment(ctx, file, setting.Attachment.AllowedTypes, header.Size, &repo_model.Attachment{
-		Name:       filename,
-		UploaderID: ctx.Doer.ID,
-		RepoID:     ctx.Repo.Repository.ID,
-		IssueID:    comment.IssueID,
-		CommentID:  comment.ID,
+		Name:         attachmentName,
+		OriginalName: header.Filename,
+		UploaderID:   ctx.Doer.ID,
+		RepoID:       ctx.Repo.Repository.ID,
+		IssueID:      comment.IssueID,
+		CommentID:    comment.ID,
 	})
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "UploadAttachment", err)

routers/api/v1/repo/release_attachment.go

@@ -206,7 +206,8 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 
 	// Get uploaded file from request
 	var content io.ReadCloser
-	var filename string
+	var originalFileName string
+	var attachmentName string
 	var size int64 = -1
 
 	if strings.HasPrefix(strings.ToLower(ctx.Req.Header.Get("Content-Type")), "multipart/form-data") {
@@ -219,26 +220,29 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 
 		content = file
 		size = header.Size
-		filename = header.Filename
+		originalFileName = header.Filename
+		attachmentName = originalFileName
 		if name := ctx.FormString("name"); name != "" {
-			filename = name
+			attachmentName = name
 		}
 	} else {
 		content = ctx.Req.Body
-		filename = ctx.FormString("name")
+		attachmentName = ctx.FormString("name")
+		originalFileName = attachmentName
 	}
 
-	if filename == "" {
+	if attachmentName == "" {
 		ctx.Error(http.StatusBadRequest, "CreateReleaseAttachment", "Could not determine name of attachment.")
 		return
 	}
 
 	// Create a new attachment and save the file
 	attach, err := attachment.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{
-		Name:       filename,
-		UploaderID: ctx.Doer.ID,
-		RepoID:     ctx.Repo.Repository.ID,
-		ReleaseID:  releaseID,
+		Name:         attachmentName,
+		OriginalName: originalFileName,
+		UploaderID:   ctx.Doer.ID,
+		RepoID:       ctx.Repo.Repository.ID,
+		ReleaseID:    releaseID,
 	})
 	if err != nil {
 		if upload.IsErrFileTypeForbidden(err) {

routers/web/repo/attachment.go

@@ -46,9 +46,10 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) {
 	defer file.Close()
 
 	attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, &repo_model.Attachment{
-		Name:       header.Filename,
-		UploaderID: ctx.Doer.ID,
-		RepoID:     repoID,
+		Name:         header.Filename,
+		OriginalName: header.Filename,
+		UploaderID:   ctx.Doer.ID,
+		RepoID:       repoID,
 	})
 	if err != nil {
 		if upload.IsErrFileTypeForbidden(err) {

services/attachment/attachment.go

@@ -44,7 +44,7 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string,
 	n, _ := util.ReadAtMost(file, buf)
 	buf = buf[:n]
 
-	if err := upload.Verify(buf, attach.Name, allowedTypes); err != nil {
+	if err := upload.Verify(buf, attach.OriginalName, allowedTypes); err != nil {
 		return nil, err
 	}
 

services/mailer/incoming/incoming_handler.go

@@ -88,9 +88,10 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u
 		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{
-					Name:       attachment.Name,
-					UploaderID: doer.ID,
-					RepoID:     issue.Repo.ID,
+					Name:         attachment.Name,
+					OriginalName: attachment.Name,
+					UploaderID:   doer.ID,
+					RepoID:       issue.Repo.ID,
 				})
 				if err != nil {
 					if upload.IsErrFileTypeForbidden(err) {