Prevent duplicate form submissions when creating forks (#34714)

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
pull/34537/merge
Kerwin Bryant 2025-06-16 12:19:16 +08:00 committed by GitHub
parent 637070e07b
commit bbee652e29
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 17 additions and 15 deletions

View File

@ -151,7 +151,7 @@ func ForkPost(ctx *context.Context) {
ctx.Data["ContextUser"] = ctxUser ctx.Data["ContextUser"] = ctxUser
if ctx.HasError() { if ctx.HasError() {
ctx.HTML(http.StatusOK, tplFork) ctx.JSONError(ctx.GetErrMsg())
return return
} }
@ -159,12 +159,12 @@ func ForkPost(ctx *context.Context) {
traverseParentRepo := forkRepo traverseParentRepo := forkRepo
for { for {
if !repository.CanUserForkBetweenOwners(ctxUser.ID, traverseParentRepo.OwnerID) { if !repository.CanUserForkBetweenOwners(ctxUser.ID, traverseParentRepo.OwnerID) {
ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) ctx.JSONError(ctx.Tr("repo.settings.new_owner_has_same_repo"))
return return
} }
repo := repo_model.GetForkedRepo(ctx, ctxUser.ID, traverseParentRepo.ID) repo := repo_model.GetForkedRepo(ctx, ctxUser.ID, traverseParentRepo.ID)
if repo != nil { if repo != nil {
ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) ctx.JSONRedirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name))
return return
} }
if !traverseParentRepo.IsFork { if !traverseParentRepo.IsFork {
@ -201,26 +201,26 @@ func ForkPost(ctx *context.Context) {
case repo_model.IsErrReachLimitOfRepo(err): case repo_model.IsErrReachLimitOfRepo(err):
maxCreationLimit := ctxUser.MaxCreationLimit() maxCreationLimit := ctxUser.MaxCreationLimit()
msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit)
ctx.RenderWithErr(msg, tplFork, &form) ctx.JSONError(msg)
case repo_model.IsErrRepoAlreadyExist(err): case repo_model.IsErrRepoAlreadyExist(err):
ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) ctx.JSONError(ctx.Tr("repo.settings.new_owner_has_same_repo"))
case repo_model.IsErrRepoFilesAlreadyExist(err): case repo_model.IsErrRepoFilesAlreadyExist(err):
switch { switch {
case ctx.IsUserSiteAdmin() || (setting.Repository.AllowAdoptionOfUnadoptedRepositories && setting.Repository.AllowDeleteOfUnadoptedRepositories): case ctx.IsUserSiteAdmin() || (setting.Repository.AllowAdoptionOfUnadoptedRepositories && setting.Repository.AllowDeleteOfUnadoptedRepositories):
ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.adopt_or_delete"), tplFork, form) ctx.JSONError(ctx.Tr("form.repository_files_already_exist.adopt_or_delete"))
case setting.Repository.AllowAdoptionOfUnadoptedRepositories: case setting.Repository.AllowAdoptionOfUnadoptedRepositories:
ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.adopt"), tplFork, form) ctx.JSONError(ctx.Tr("form.repository_files_already_exist.adopt"))
case setting.Repository.AllowDeleteOfUnadoptedRepositories: case setting.Repository.AllowDeleteOfUnadoptedRepositories:
ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.delete"), tplFork, form) ctx.JSONError(ctx.Tr("form.repository_files_already_exist.delete"))
default: default:
ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist"), tplFork, form) ctx.JSONError(ctx.Tr("form.repository_files_already_exist"))
} }
case db.IsErrNameReserved(err): case db.IsErrNameReserved(err):
ctx.RenderWithErr(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name), tplFork, &form) ctx.JSONError(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name))
case db.IsErrNamePatternNotAllowed(err): case db.IsErrNamePatternNotAllowed(err):
ctx.RenderWithErr(ctx.Tr("repo.form.name_pattern_not_allowed", err.(db.ErrNamePatternNotAllowed).Pattern), tplFork, &form) ctx.JSONError(ctx.Tr("repo.form.name_pattern_not_allowed", err.(db.ErrNamePatternNotAllowed).Pattern))
case errors.Is(err, user_model.ErrBlockedUser): case errors.Is(err, user_model.ErrBlockedUser):
ctx.RenderWithErr(ctx.Tr("repo.fork.blocked_user"), tplFork, form) ctx.JSONError(ctx.Tr("repo.fork.blocked_user"))
default: default:
ctx.ServerError("ForkPost", err) ctx.ServerError("ForkPost", err)
} }
@ -228,5 +228,5 @@ func ForkPost(ctx *context.Context) {
} }
log.Trace("Repository forked[%d]: %s/%s", forkRepo.ID, ctxUser.Name, repo.Name) log.Trace("Repository forked[%d]: %s/%s", forkRepo.ID, ctxUser.Name, repo.Name)
ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) ctx.JSONRedirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name))
} }

View File

@ -6,7 +6,7 @@
</h3> </h3>
<div class="ui attached segment"> <div class="ui attached segment">
{{template "base/alert" .}} {{template "base/alert" .}}
<form class="ui form left-right-form" action="{{.Link}}" method="post"> <form class="ui form form-fetch-action left-right-form" action="{{.Link}}" method="post">
{{.CsrfTokenHtml}} {{.CsrfTokenHtml}}
<div class="inline required field {{if .Err_Owner}}error{{end}}"> <div class="inline required field {{if .Err_Owner}}error{{end}}">
<label>{{ctx.Locale.Tr "repo.owner"}}</label> <label>{{ctx.Locale.Tr "repo.owner"}}</label>

View File

@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
org_service "code.gitea.io/gitea/services/org" org_service "code.gitea.io/gitea/services/org"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
@ -51,7 +52,8 @@ func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkO
"repo_name": forkRepoName, "repo_name": forkRepoName,
"fork_single_branch": forkBranch, "fork_single_branch": forkBranch,
}) })
session.MakeRequest(t, req, http.StatusSeeOther) resp = session.MakeRequest(t, req, http.StatusOK)
assert.Equal(t, fmt.Sprintf("/%s/%s", forkOwnerName, forkRepoName), test.RedirectURL(resp))
// Step4: check the existence of the forked repo // Step4: check the existence of the forked repo
req = NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName) req = NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)