diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 128fcd53695..f8757ee53d1 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -27,7 +27,6 @@ import ( "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/routers/web/feed" - shared_user "gitea.dev/routers/web/shared/user" "gitea.dev/services/context" "gitea.dev/services/context/upload" "gitea.dev/services/forms" @@ -338,7 +337,6 @@ func LatestRelease(ctx *context.Context) { func newReleaseCommon(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.release.new_release") - ctx.Data["PageIsReleaseList"] = true tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) if err != nil { @@ -346,17 +344,8 @@ func newReleaseCommon(ctx *context.Context) { return } ctx.Data["Tags"] = tags - ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled - assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository) - if err != nil { - ctx.ServerError("GetRepoAssignees", err) - return - } - ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) - upload.AddUploadContext(ctx, "release") - PrepareBranchList(ctx) // for New Release page } @@ -425,8 +414,8 @@ func GenerateReleaseNotes(ctx *context.Context) { // NewReleasePost response for creating a release func NewReleasePost(ctx *context.Context) { - newReleaseCommon(ctx) - if ctx.Written() { + if ctx.HasError() { + ctx.JSONError(ctx.GetErrMsg()) return } @@ -450,35 +439,28 @@ func NewReleasePost(ctx *context.Context) { // Or another choice is "always show the tag-only button" if error occurs. ctx.Data["ShowCreateTagOnlyButton"] = form.TagOnly || rel == nil - // do some form checks - if ctx.HasError() { - ctx.HTML(http.StatusOK, tplReleaseNew) - return - } - form.Target = util.IfZero(form.Target, ctx.Repo.Repository.DefaultBranch) if exist, _ := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, form.Target); !exist { - ctx.RenderWithErrDeprecated(ctx.Tr("form.target_branch_not_exist"), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("form.target_branch_not_exist")) return } if !form.TagOnly && form.Title == "" { // if not "tag only", then the title of the release cannot be empty - ctx.RenderWithErrDeprecated(ctx.Tr("repo.release.title_empty"), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("repo.release.title_empty")) return } handleTagReleaseError := func(err error) { - ctx.Data["Err_TagName"] = true switch { case release_service.IsErrTagAlreadyExists(err): - ctx.RenderWithErrDeprecated(ctx.Tr("repo.branch.tag_collision", form.TagName), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("repo.branch.tag_collision", form.TagName)) case repo_model.IsErrReleaseAlreadyExist(err): - ctx.RenderWithErrDeprecated(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("repo.release.tag_name_already_exist")) case release_service.IsErrInvalidTagName(err): - ctx.RenderWithErrDeprecated(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("repo.release.tag_name_invalid")) case release_service.IsErrProtectedTagName(err): - ctx.RenderWithErrDeprecated(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("repo.release.tag_name_protected")) default: ctx.ServerError("handleTagReleaseError", err) } @@ -497,7 +479,7 @@ func NewReleasePost(ctx *context.Context) { return } ctx.Flash.Success(ctx.Tr("repo.tag.create_success", form.TagName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName)) + ctx.JSONRedirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName)) return } @@ -522,7 +504,7 @@ func NewReleasePost(ctx *context.Context) { handleTagReleaseError(err) return } - ctx.Redirect(ctx.Repo.RepoLink + "/releases") + ctx.JSONRedirect(ctx.Repo.RepoLink + "/releases") return } @@ -530,8 +512,7 @@ func NewReleasePost(ctx *context.Context) { // old logic: if the release is not a tag (it is a real release), do not update it on the "new release" page // add new logic: if tag-only, do not convert the tag to a release if form.TagOnly || !rel.IsTag { - ctx.Data["Err_TagName"] = true - ctx.RenderWithErrDeprecated(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) + ctx.JSONError(ctx.Tr("repo.release.tag_name_already_exist")) return } @@ -547,7 +528,7 @@ func NewReleasePost(ctx *context.Context) { handleTagReleaseError(err) return } - ctx.Redirect(ctx.Repo.RepoLink + "/releases") + ctx.JSONRedirect(ctx.Repo.RepoLink + "/releases") } // EditRelease render release edit page @@ -589,55 +570,39 @@ func EditRelease(ctx *context.Context) { return } ctx.Data["attachments"] = rel.Attachments - - // Get assignees. - assigneeUsers, err := repo_model.GetRepoAssignees(ctx, rel.Repo) - if err != nil { - ctx.ServerError("GetRepoAssignees", err) - return - } - ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) - ctx.HTML(http.StatusOK, tplReleaseNew) } // EditReleasePost response for edit release func EditReleasePost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.EditReleaseForm) - - newReleaseCommon(ctx) - if ctx.Written() { + if ctx.HasError() { + ctx.JSONError(ctx.GetErrMsg()) return } - ctx.Data["Title"] = ctx.Tr("repo.release.edit_release") - ctx.Data["PageIsEditRelease"] = true + form := web.GetForm(ctx).(*forms.EditReleaseForm) tagName := ctx.PathParam("*") rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, tagName) if err != nil { if repo_model.IsErrReleaseNotExist(err) { - ctx.NotFound(err) + ctx.JSONErrorNotFound(err.Error()) } else { ctx.ServerError("GetRelease", err) } return } if rel.IsTag { - ctx.NotFound(err) // for a pure tag release, don't allow to edit it as a release + ctx.JSONErrorNotFound() // for a pure tag release, don't allow to edit it as a release return } + ctx.Data["tag_name"] = rel.TagName ctx.Data["tag_target"] = util.IfZero(rel.Target, ctx.Repo.Repository.DefaultBranch) ctx.Data["title"] = rel.Title ctx.Data["content"] = rel.Note ctx.Data["prerelease"] = rel.IsPrerelease - if ctx.HasError() { - ctx.HTML(http.StatusOK, tplReleaseNew) - return - } - const delPrefix = "attachment-del-" const editPrefix = "attachment-edit-" var addAttachmentUUIDs, delAttachmentUUIDs []string @@ -659,10 +624,14 @@ func EditReleasePost(ctx *context.Context) { rel.IsPrerelease = form.Prerelease if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments); err != nil { - ctx.ServerError("UpdateRelease", err) + if upload.IsErrFileTypeForbidden(err) { + ctx.JSONError(err.Error()) + } else { + ctx.ServerError("UpdateRelease", err) + } return } - ctx.Redirect(ctx.Repo.RepoLink + "/releases") + ctx.JSONRedirect(ctx.Repo.RepoLink + "/releases") } // DeleteRelease deletes a release diff --git a/routers/web/repo/release_test.go b/routers/web/repo/release_test.go index 82f3115ee5d..57f831bffda 100644 --- a/routers/web/repo/release_test.go +++ b/routers/web/repo/release_test.go @@ -4,12 +4,14 @@ package repo import ( + "net/http/httptest" "testing" "gitea.dev/models/db" repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" "gitea.dev/models/unittest" + "gitea.dev/modules/test" "gitea.dev/modules/web" "gitea.dev/services/context" "gitea.dev/services/contexttest" @@ -39,15 +41,15 @@ func TestNewReleasePost(t *testing.T) { assert.NotEmpty(t, ctx.Data["ShowCreateTagOnlyButton"]) }) - post := func(t *testing.T, form forms.NewReleaseForm) *context.Context { - ctx, _ := contexttest.MockContext(t, "user2/repo1/releases/new") + post := func(t *testing.T, form forms.NewReleaseForm) (*context.Context, *httptest.ResponseRecorder) { + ctx, resp := contexttest.MockContext(t, "user2/repo1/releases/new") contexttest.LoadUser(t, ctx, 2) contexttest.LoadRepo(t, ctx, 1) contexttest.LoadGitRepo(t, ctx) defer ctx.Repo.GitRepo.Close() web.SetForm(ctx, &form) NewReleasePost(ctx) - return ctx + return ctx, resp } loadRelease := func(t *testing.T, tagName string) *repo_model.Release { @@ -70,7 +72,7 @@ func TestNewReleasePost(t *testing.T) { }) t.Run("ReleaseExistsDoUpdate(non-tag)", func(t *testing.T) { - ctx := post(t, forms.NewReleaseForm{ + _, resp := post(t, forms.NewReleaseForm{ TagName: "v1.1", Target: "master", Title: "updated-title", @@ -80,11 +82,11 @@ func TestNewReleasePost(t *testing.T) { require.NotNil(t, rel) assert.False(t, rel.IsTag) assert.Equal(t, "testing-release", rel.Title) - assert.NotEmpty(t, ctx.Flash.ErrorMsg) + assert.NotEmpty(t, test.ParseJSONError(resp.Body.Bytes()).ErrorMessage) }) t.Run("ReleaseExistsDoUpdate(tag-only)", func(t *testing.T) { - ctx := post(t, forms.NewReleaseForm{ + ctx, resp := post(t, forms.NewReleaseForm{ TagName: "delete-tag", // a strange name, but it is the only "is_tag=true" fixture Target: "master", Title: "updated-title", @@ -95,12 +97,12 @@ func TestNewReleasePost(t *testing.T) { require.NotNil(t, rel) assert.True(t, rel.IsTag) // the record should not be updated because the request is "tag-only". TODO: need to improve the logic? assert.Equal(t, "delete-tag", rel.Title) - assert.NotEmpty(t, ctx.Flash.ErrorMsg) + assert.NotEmpty(t, test.ParseJSONError(resp.Body.Bytes()).ErrorMessage) assert.NotEmpty(t, ctx.Data["ShowCreateTagOnlyButton"]) // still show the "tag-only" button }) t.Run("ReleaseExistsDoUpdate(tag-release)", func(t *testing.T) { - ctx := post(t, forms.NewReleaseForm{ + ctx, _ := post(t, forms.NewReleaseForm{ TagName: "delete-tag", // a strange name, but it is the only "is_tag=true" fixture Target: "master", Title: "updated-title", @@ -114,7 +116,7 @@ func TestNewReleasePost(t *testing.T) { }) t.Run("TagOnly", func(t *testing.T) { - ctx := post(t, forms.NewReleaseForm{ + ctx, _ := post(t, forms.NewReleaseForm{ TagName: "new-tag-only", Target: "master", Title: "title", @@ -128,7 +130,7 @@ func TestNewReleasePost(t *testing.T) { }) t.Run("TagOnlyConflict", func(t *testing.T) { - ctx := post(t, forms.NewReleaseForm{ + _, resp := post(t, forms.NewReleaseForm{ TagName: "v1.1", Target: "master", Title: "title", @@ -138,7 +140,7 @@ func TestNewReleasePost(t *testing.T) { rel := loadRelease(t, "v1.1") require.NotNil(t, rel) assert.False(t, rel.IsTag) - assert.NotEmpty(t, ctx.Flash.ErrorMsg) + assert.NotEmpty(t, test.ParseJSONError(resp.Body.Bytes()).ErrorMessage) }) } diff --git a/services/release/release.go b/services/release/release.go index 1be60731a93..5d4063c0839 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -20,9 +20,11 @@ import ( "gitea.dev/modules/graceful" "gitea.dev/modules/log" "gitea.dev/modules/repository" + "gitea.dev/modules/setting" "gitea.dev/modules/storage" "gitea.dev/modules/timeutil" "gitea.dev/modules/util" + "gitea.dev/services/context/upload" notify_service "gitea.dev/services/notify" ) @@ -319,13 +321,17 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo } for uuid, newName := range editAttachments { - if !deletedUUIDs.Contains(uuid) { - if err = repo_model.UpdateAttachmentByUUID(ctx, &repo_model.Attachment{ - UUID: uuid, - Name: newName, - }, "name"); err != nil { - return err - } + if deletedUUIDs.Contains(uuid) { + continue + } + if err = upload.Verify(nil, newName, setting.Repository.Release.AllowedTypes); err != nil { + return err + } + if err = repo_model.UpdateAttachmentByUUID(ctx, &repo_model.Attachment{ + UUID: uuid, + Name: newName, + }, "name"); err != nil { + return err } } } diff --git a/services/release/release_test.go b/services/release/release_test.go index 579d92f2f4b..1c0f910bbfb 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -12,8 +12,11 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/gitrepo" + "gitea.dev/modules/setting" + "gitea.dev/modules/test" "gitea.dev/modules/timeutil" "gitea.dev/services/attachment" + "gitea.dev/services/context/upload" _ "gitea.dev/models/actions" @@ -270,6 +273,17 @@ func TestRelease_Update(t *testing.T) { assert.Equal(t, release.ID, release.Attachments[0].ReleaseID) assert.Equal(t, "test2.txt", release.Attachments[0].Name) + defer test.MockVariableValue(&setting.Repository.Release.AllowedTypes, ".zip")() + err = UpdateRelease(t.Context(), user, gitRepo, release, nil, nil, map[string]string{ + attach.UUID: "test.exe", + }) + assert.Error(t, err) + assert.True(t, upload.IsErrFileTypeForbidden(err)) + release.Attachments = nil + assert.NoError(t, repo_model.GetReleaseAttachments(t.Context(), release)) + assert.Len(t, release.Attachments, 1) + assert.Equal(t, "test2.txt", release.Attachments[0].Name) + // delete the attachment assert.NoError(t, UpdateRelease(t.Context(), user, gitRepo, release, nil, []string{attach.UUID}, nil)) release.Attachments = nil diff --git a/templates/repo/release/new.tmpl b/templates/repo/release/new.tmpl index cc73bdf13c5..a930664554e 100644 --- a/templates/repo/release/new.tmpl +++ b/templates/repo/release/new.tmpl @@ -13,7 +13,7 @@ {{template "base/alert" .}} -