Backport #30791 by kemzeb 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. Co-authored-by: Kemal Zebari <60799661+kemzeb@users.noreply.github.com>tags/v1.22.0
@@ -14,6 +14,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" | |||
) | |||
@@ -153,6 +154,8 @@ func CreateIssueAttachment(ctx *context.APIContext) { | |||
// "$ref": "#/responses/error" | |||
// "404": | |||
// "$ref": "#/responses/error" | |||
// "422": | |||
// "$ref": "#/responses/validationError" | |||
// "423": | |||
// "$ref": "#/responses/repoArchivedError" | |||
@@ -185,7 +188,11 @@ func CreateIssueAttachment(ctx *context.APIContext) { | |||
IssueID: issue.ID, | |||
}) | |||
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 | |||
} | |||
@@ -16,6 +16,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" | |||
) | |||
@@ -160,6 +161,8 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { | |||
// "$ref": "#/responses/forbidden" | |||
// "404": | |||
// "$ref": "#/responses/error" | |||
// "422": | |||
// "$ref": "#/responses/validationError" | |||
// "423": | |||
// "$ref": "#/responses/repoArchivedError" | |||
@@ -194,9 +197,14 @@ func CreateIssueCommentAttachment(ctx *context.APIContext) { | |||
CommentID: comment.ID, | |||
}) | |||
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 |
@@ -7418,6 +7418,9 @@ | |||
"404": { | |||
"$ref": "#/responses/error" | |||
}, | |||
"422": { | |||
"$ref": "#/responses/validationError" | |||
}, | |||
"423": { | |||
"$ref": "#/responses/repoArchivedError" | |||
} | |||
@@ -8037,6 +8040,9 @@ | |||
"404": { | |||
"$ref": "#/responses/error" | |||
}, | |||
"422": { | |||
"$ref": "#/responses/validationError" | |||
}, | |||
"423": { | |||
"$ref": "#/responses/repoArchivedError" | |||
} |
@@ -120,6 +120,34 @@ func TestAPICreateCommentAttachment(t *testing.T) { | |||
unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: apiAttachment.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) | |||
} | |||
func TestAPIEditCommentAttachment(t *testing.T) { | |||
defer tests.PrepareTestEnv(t)() | |||
@@ -96,6 +96,33 @@ func TestAPICreateIssueAttachment(t *testing.T) { | |||
unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: apiAttachment.ID, IssueID: issue.ID}) | |||
} | |||
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)() | |||