Catch and handle unallowed file type errors in issue attachment API (#30791)

Before, we would just throw 500 if a user passes an attachment that is
not an allowed type. This commit catches this error and throws a 422
instead since this should be considered a validation error.

(cherry picked from commit 872caa17c0a30d95f85ab75c068d606e07bd10b3)

Conflicts:
	tests/integration/api_comment_attachment_test.go
	tests/integration/api_issue_attachment_test.go
	trivial context conflict because of 'allow setting the update date on issues and comments'
(cherry picked from commit 9cd0441cd384f7fbff8e973182c6094270e7f97e)
This commit is contained in:
Kemal Zebari 2024-05-02 09:33:31 -07:00 committed by Earl Warren
parent a7124df0c5
commit 5678e9ab20
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
5 changed files with 78 additions and 2 deletions

View file

@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/attachment"
"code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/context/upload"
"code.gitea.io/gitea/services/convert" "code.gitea.io/gitea/services/convert"
issue_service "code.gitea.io/gitea/services/issue" issue_service "code.gitea.io/gitea/services/issue"
) )
@ -159,6 +160,8 @@ func CreateIssueAttachment(ctx *context.APIContext) {
// "$ref": "#/responses/error" // "$ref": "#/responses/error"
// "404": // "404":
// "$ref": "#/responses/error" // "$ref": "#/responses/error"
// "422":
// "$ref": "#/responses/validationError"
// "423": // "423":
// "$ref": "#/responses/repoArchivedError" // "$ref": "#/responses/repoArchivedError"
@ -207,7 +210,11 @@ func CreateIssueAttachment(ctx *context.APIContext) {
CreatedUnix: issue.UpdatedUnix, CreatedUnix: issue.UpdatedUnix,
}) })
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "UploadAttachment", err) if upload.IsErrFileTypeForbidden(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
} else {
ctx.Error(http.StatusInternalServerError, "UploadAttachment", err)
}
return return
} }

View file

@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/attachment"
"code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/context/upload"
"code.gitea.io/gitea/services/convert" "code.gitea.io/gitea/services/convert"
issue_service "code.gitea.io/gitea/services/issue" issue_service "code.gitea.io/gitea/services/issue"
) )
@ -156,6 +157,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) {
// "$ref": "#/responses/error" // "$ref": "#/responses/error"
// "404": // "404":
// "$ref": "#/responses/error" // "$ref": "#/responses/error"
// "422":
// "$ref": "#/responses/validationError"
// "423": // "423":
// "$ref": "#/responses/repoArchivedError" // "$ref": "#/responses/repoArchivedError"
@ -209,9 +212,14 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) {
CreatedUnix: comment.Issue.UpdatedUnix, CreatedUnix: comment.Issue.UpdatedUnix,
}) })
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "UploadAttachment", err) if upload.IsErrFileTypeForbidden(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
} else {
ctx.Error(http.StatusInternalServerError, "UploadAttachment", err)
}
return return
} }
if err := comment.LoadAttachments(ctx); err != nil { if err := comment.LoadAttachments(ctx); err != nil {
ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
return return

View file

@ -6974,6 +6974,9 @@
"404": { "404": {
"$ref": "#/responses/error" "$ref": "#/responses/error"
}, },
"422": {
"$ref": "#/responses/validationError"
},
"423": { "423": {
"$ref": "#/responses/repoArchivedError" "$ref": "#/responses/repoArchivedError"
} }
@ -7600,6 +7603,9 @@
"404": { "404": {
"$ref": "#/responses/error" "$ref": "#/responses/error"
}, },
"422": {
"$ref": "#/responses/validationError"
},
"423": { "423": {
"$ref": "#/responses/repoArchivedError" "$ref": "#/responses/repoArchivedError"
} }

View file

@ -240,3 +240,31 @@ func TestAPIDeleteCommentAttachment(t *testing.T) {
unittest.AssertNotExistsBean(t, &repo_model.Attachment{ID: attachment.ID, CommentID: comment.ID}) unittest.AssertNotExistsBean(t, &repo_model.Attachment{ID: attachment.ID, CommentID: comment.ID})
} }
func TestAPICreateCommentAttachmentWithUnallowedFile(t *testing.T) {
defer tests.PrepareTestEnv(t)()
comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 2})
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
session := loginUser(t, repoOwner.Name)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
filename := "file.bad"
body := &bytes.Buffer{}
// Setup multi-part.
writer := multipart.NewWriter(body)
_, err := writer.CreateFormFile("attachment", filename)
assert.NoError(t, err)
err = writer.Close()
assert.NoError(t, err)
req := NewRequestWithBody(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/assets", repoOwner.Name, repo.Name, comment.ID), body).
AddTokenAuth(token).
SetHeader("Content-Type", writer.FormDataContentType())
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
}

View file

@ -173,6 +173,33 @@ func TestAPICreateIssueAttachmentAutoDate(t *testing.T) {
}) })
} }
func TestAPICreateIssueAttachmentWithUnallowedFile(t *testing.T) {
defer tests.PrepareTestEnv(t)()
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID})
repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
session := loginUser(t, repoOwner.Name)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
filename := "file.bad"
body := &bytes.Buffer{}
// Setup multi-part.
writer := multipart.NewWriter(body)
_, err := writer.CreateFormFile("attachment", filename)
assert.NoError(t, err)
err = writer.Close()
assert.NoError(t, err)
req := NewRequestWithBody(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets", repoOwner.Name, repo.Name, issue.Index), body).
AddTokenAuth(token)
req.Header.Add("Content-Type", writer.FormDataContentType())
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
}
func TestAPIEditIssueAttachment(t *testing.T) { func TestAPIEditIssueAttachment(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()