Handle more pathological branch and tag names (#11843) (#11863)

Backport #11843

It's possible to push quite pathological appearing branch names to gitea
using git push gitea reasonable-branch:refs/heads/-- at which point
large parts of the UI will break. Similarly you can git push origin
reasonable-tag:refs/tags/-- which wil return an error.

This PR fixes the problems these cause. It also changes the code from
creating branches to pushing to ensure that branch restoration has to
pass hooks.

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
This commit is contained in:
zeripath 2020-06-12 19:01:44 +01:00 committed by GitHub
parent ef2f18964e
commit 320031fce6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 41 additions and 87 deletions

View file

@ -32,14 +32,14 @@ func TestDeleteBranch(t *testing.T) {
} }
func TestUndoDeleteBranch(t *testing.T) { func TestUndoDeleteBranch(t *testing.T) {
defer prepareTestEnv(t)() onGiteaRun(t, func(t *testing.T, u *url.URL) {
deleteBranch(t) deleteBranch(t)
htmlDoc, name := branchAction(t, ".undo-button") htmlDoc, name := branchAction(t, ".undo-button")
assert.Contains(t, assert.Contains(t,
htmlDoc.doc.Find(".ui.positive.message").Text(), htmlDoc.doc.Find(".ui.positive.message").Text(),
i18n.Tr("en", "repo.branch.restore_success", name), i18n.Tr("en", "repo.branch.restore_success", name),
) )
})
} }
func deleteBranch(t *testing.T) { func deleteBranch(t *testing.T) {

View file

@ -46,7 +46,7 @@ func (repo *Repository) GetBranchCommitID(name string) (string, error) {
// GetTagCommitID returns last commit ID string of given tag. // GetTagCommitID returns last commit ID string of given tag.
func (repo *Repository) GetTagCommitID(name string) (string, error) { func (repo *Repository) GetTagCommitID(name string) (string, error) {
stdout, err := NewCommand("rev-list", "-n", "1", name).RunInDir(repo.Path) stdout, err := NewCommand("rev-list", "-n", "1", TagPrefix+name).RunInDir(repo.Path)
if err != nil { if err != nil {
if strings.Contains(err.Error(), "unknown revision or path") { if strings.Contains(err.Error(), "unknown revision or path") {
return "", ErrNotExist{name, ""} return "", ErrNotExist{name, ""}

View file

@ -9,7 +9,6 @@ import (
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
) )
// GetBranch returns a branch by its name // GetBranch returns a branch by its name
@ -74,39 +73,9 @@ func CreateNewBranch(doer *models.User, repo *models.Repository, oldBranchName,
return fmt.Errorf("OldBranch: %s does not exist. Cannot create new branch from this", oldBranchName) return fmt.Errorf("OldBranch: %s does not exist. Cannot create new branch from this", oldBranchName)
} }
basePath, err := models.CreateTemporaryPath("branch-maker") if err := git.Push(repo.RepoPath(), git.PushOptions{
if err != nil { Remote: repo.RepoPath(),
return err Branch: fmt.Sprintf("%s:%s%s", oldBranchName, git.BranchPrefix, branchName),
}
defer func() {
if err := models.RemoveTemporaryPath(basePath); err != nil {
log.Error("CreateNewBranch: RemoveTemporaryPath: %s", err)
}
}()
if err := git.Clone(repo.RepoPath(), basePath, git.CloneRepoOptions{
Bare: true,
Shared: true,
}); err != nil {
log.Error("Failed to clone repository: %s (%v)", repo.FullName(), err)
return fmt.Errorf("Failed to clone repository: %s (%v)", repo.FullName(), err)
}
gitRepo, err := git.OpenRepository(basePath)
if err != nil {
log.Error("Unable to open temporary repository: %s (%v)", basePath, err)
return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err)
}
defer gitRepo.Close()
if err = gitRepo.CreateBranch(branchName, oldBranchName); err != nil {
log.Error("Unable to create branch: %s from %s. (%v)", branchName, oldBranchName, err)
return fmt.Errorf("Unable to create branch: %s from %s. (%v)", branchName, oldBranchName, err)
}
if err = git.Push(basePath, git.PushOptions{
Remote: "origin",
Branch: branchName,
Env: models.PushingEnvironment(doer, repo), Env: models.PushingEnvironment(doer, repo),
}); err != nil { }); err != nil {
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) { if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
@ -124,39 +93,10 @@ func CreateNewBranchFromCommit(doer *models.User, repo *models.Repository, commi
if err := checkBranchName(repo, branchName); err != nil { if err := checkBranchName(repo, branchName); err != nil {
return err return err
} }
basePath, err := models.CreateTemporaryPath("branch-maker")
if err != nil {
return err
}
defer func() {
if err := models.RemoveTemporaryPath(basePath); err != nil {
log.Error("CreateNewBranchFromCommit: RemoveTemporaryPath: %s", err)
}
}()
if err := git.Clone(repo.RepoPath(), basePath, git.CloneRepoOptions{ if err := git.Push(repo.RepoPath(), git.PushOptions{
Bare: true, Remote: repo.RepoPath(),
Shared: true, Branch: fmt.Sprintf("%s:%s%s", commit, git.BranchPrefix, branchName),
}); err != nil {
log.Error("Failed to clone repository: %s (%v)", repo.FullName(), err)
return fmt.Errorf("Failed to clone repository: %s (%v)", repo.FullName(), err)
}
gitRepo, err := git.OpenRepository(basePath)
if err != nil {
log.Error("Unable to open temporary repository: %s (%v)", basePath, err)
return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err)
}
defer gitRepo.Close()
if err = gitRepo.CreateBranch(branchName, commit); err != nil {
log.Error("Unable to create branch: %s from %s. (%v)", branchName, commit, err)
return fmt.Errorf("Unable to create branch: %s from %s. (%v)", branchName, commit, err)
}
if err = git.Push(basePath, git.PushOptions{
Remote: "origin",
Branch: branchName,
Env: models.PushingEnvironment(doer, repo), Env: models.PushingEnvironment(doer, repo),
}); err != nil { }); err != nil {
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) { if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {

View file

@ -6,6 +6,7 @@
package repo package repo
import ( import (
"fmt"
"strings" "strings"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
@ -102,7 +103,11 @@ func RestoreBranchPost(ctx *context.Context) {
return return
} }
if err := ctx.Repo.GitRepo.CreateBranch(deletedBranch.Name, deletedBranch.Commit); err != nil { if err := git.Push(ctx.Repo.Repository.RepoPath(), git.PushOptions{
Remote: ctx.Repo.Repository.RepoPath(),
Branch: fmt.Sprintf("%s:%s%s", deletedBranch.Commit, git.BranchPrefix, deletedBranch.Name),
Env: models.PushingEnvironment(ctx.User, ctx.Repo.Repository),
}); err != nil {
if strings.Contains(err.Error(), "already exists") { if strings.Contains(err.Error(), "already exists") {
ctx.Flash.Error(ctx.Tr("repo.branch.already_exists", deletedBranch.Name)) ctx.Flash.Error(ctx.Tr("repo.branch.already_exists", deletedBranch.Name))
return return
@ -112,12 +117,6 @@ func RestoreBranchPost(ctx *context.Context) {
return return
} }
if err := ctx.Repo.Repository.RemoveDeletedBranch(deletedBranch.ID); err != nil {
log.Error("RemoveDeletedBranch: %v", err)
ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name))
return
}
// Don't return error below this // Don't return error below this
if err := repofiles.PushUpdate( if err := repofiles.PushUpdate(
ctx.Repo.Repository, ctx.Repo.Repository,
@ -216,7 +215,7 @@ func loadBranches(ctx *context.Context) []*Branch {
} }
} }
divergence, divergenceError := repofiles.CountDivergingCommits(ctx.Repo.Repository, branchName) divergence, divergenceError := repofiles.CountDivergingCommits(ctx.Repo.Repository, git.BranchPrefix+branchName)
if divergenceError != nil { if divergenceError != nil {
ctx.ServerError("CountDivergingCommits", divergenceError) ctx.ServerError("CountDivergingCommits", divergenceError)
return nil return nil
@ -331,6 +330,8 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
var err error var err error
if ctx.Repo.IsViewBranch { if ctx.Repo.IsViewBranch {
err = repo_module.CreateNewBranch(ctx.User, ctx.Repo.Repository, ctx.Repo.BranchName, form.NewBranchName) err = repo_module.CreateNewBranch(ctx.User, ctx.Repo.Repository, ctx.Repo.BranchName, form.NewBranchName)
} else if ctx.Repo.IsViewTag {
err = repo_module.CreateNewBranchFromCommit(ctx.User, ctx.Repo.Repository, ctx.Repo.CommitID, form.NewBranchName)
} else { } else {
err = repo_module.CreateNewBranchFromCommit(ctx.User, ctx.Repo.Repository, ctx.Repo.BranchName, form.NewBranchName) err = repo_module.CreateNewBranchFromCommit(ctx.User, ctx.Repo.Repository, ctx.Repo.BranchName, form.NewBranchName)
} }

View file

@ -381,7 +381,20 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
return nil, nil, nil, nil, "", "" return nil, nil, nil, nil, "", ""
} }
compareInfo, err := headGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranch, headBranch) baseBranchRef := baseBranch
if baseIsBranch {
baseBranchRef = git.BranchPrefix + baseBranch
} else if baseIsTag {
baseBranchRef = git.TagPrefix + baseBranch
}
headBranchRef := headBranch
if headIsBranch {
headBranchRef = git.BranchPrefix + headBranch
} else if headIsTag {
headBranchRef = git.TagPrefix + headBranch
}
compareInfo, err := headGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef)
if err != nil { if err != nil {
ctx.ServerError("GetCompareInfo", err) ctx.ServerError("GetCompareInfo", err)
return nil, nil, nil, nil, "", "" return nil, nil, nil, nil, "", ""

View file

@ -484,7 +484,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
} }
compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(), compareInfo, err := baseGitRepo.GetCompareInfo(pull.BaseRepo.RepoPath(),
pull.BaseBranch, pull.GetGitRefName()) git.BranchPrefix+pull.BaseBranch, pull.GetGitRefName())
if err != nil { if err != nil {
if strings.Contains(err.Error(), "fatal: Not a valid object name") { if strings.Contains(err.Error(), "fatal: Not a valid object name") {
ctx.Data["IsPullRequestBroken"] = true ctx.Data["IsPullRequestBroken"] = true

View file

@ -100,7 +100,7 @@ func createTemporaryRepo(pr *models.PullRequest) (string, error) {
outbuf.Reset() outbuf.Reset()
errbuf.Reset() errbuf.Reset()
if err := git.NewCommand("fetch", "origin", "--no-tags", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { if err := git.NewCommand("fetch", "origin", "--no-tags", "--", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) log.Error("Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
if err := models.RemoveTemporaryPath(tmpBasePath); err != nil { if err := models.RemoveTemporaryPath(tmpBasePath); err != nil {
log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err)
@ -140,7 +140,7 @@ func createTemporaryRepo(pr *models.PullRequest) (string, error) {
trackingBranch := "tracking" trackingBranch := "tracking"
// Fetch head branch // Fetch head branch
if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil { if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, git.BranchPrefix+pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
if err := models.RemoveTemporaryPath(tmpBasePath); err != nil { if err := models.RemoveTemporaryPath(tmpBasePath); err != nil {
log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err)