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'
This commit is contained in:
parent
396f16e7b2
commit
9cd0441cd3
5 changed files with 78 additions and 2 deletions
|
@ -15,6 +15,7 @@ import (
|
|||
"code.gitea.io/gitea/modules/web"
|
||||
"code.gitea.io/gitea/services/attachment"
|
||||
"code.gitea.io/gitea/services/context"
|
||||
"code.gitea.io/gitea/services/context/upload"
|
||||
"code.gitea.io/gitea/services/convert"
|
||||
issue_service "code.gitea.io/gitea/services/issue"
|
||||
)
|
||||
|
@ -159,6 +160,8 @@ func CreateIssueAttachment(ctx *context.APIContext) {
|
|||
// "$ref": "#/responses/error"
|
||||
// "404":
|
||||
// "$ref": "#/responses/error"
|
||||
// "422":
|
||||
// "$ref": "#/responses/validationError"
|
||||
// "423":
|
||||
// "$ref": "#/responses/repoArchivedError"
|
||||
|
||||
|
@ -207,7 +210,11 @@ func CreateIssueAttachment(ctx *context.APIContext) {
|
|||
CreatedUnix: issue.UpdatedUnix,
|
||||
})
|
||||
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
|
||||
}
|
||||
|
||||
|
|
|
@ -15,6 +15,7 @@ import (
|
|||
"code.gitea.io/gitea/modules/web"
|
||||
"code.gitea.io/gitea/services/attachment"
|
||||
"code.gitea.io/gitea/services/context"
|
||||
"code.gitea.io/gitea/services/context/upload"
|
||||
"code.gitea.io/gitea/services/convert"
|
||||
issue_service "code.gitea.io/gitea/services/issue"
|
||||
)
|
||||
|
@ -156,6 +157,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) {
|
|||
// "$ref": "#/responses/error"
|
||||
// "404":
|
||||
// "$ref": "#/responses/error"
|
||||
// "422":
|
||||
// "$ref": "#/responses/validationError"
|
||||
// "423":
|
||||
// "$ref": "#/responses/repoArchivedError"
|
||||
|
||||
|
@ -209,9 +212,14 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) {
|
|||
CreatedUnix: comment.Issue.UpdatedUnix,
|
||||
})
|
||||
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
|
||||
}
|
||||
|
||||
if err := comment.LoadAttachments(ctx); err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "LoadAttachments", err)
|
||||
return
|
||||
|
|
6
templates/swagger/v1_json.tmpl
generated
6
templates/swagger/v1_json.tmpl
generated
|
@ -7606,6 +7606,9 @@
|
|||
"404": {
|
||||
"$ref": "#/responses/error"
|
||||
},
|
||||
"422": {
|
||||
"$ref": "#/responses/validationError"
|
||||
},
|
||||
"423": {
|
||||
"$ref": "#/responses/repoArchivedError"
|
||||
}
|
||||
|
@ -8232,6 +8235,9 @@
|
|||
"404": {
|
||||
"$ref": "#/responses/error"
|
||||
},
|
||||
"422": {
|
||||
"$ref": "#/responses/validationError"
|
||||
},
|
||||
"423": {
|
||||
"$ref": "#/responses/repoArchivedError"
|
||||
}
|
||||
|
|
|
@ -240,3 +240,31 @@ func TestAPIDeleteCommentAttachment(t *testing.T) {
|
|||
|
||||
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)
|
||||
}
|
||||
|
|
|
@ -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) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
|
|
Loading…
Reference in a new issue