Skip to content

Commit

Permalink
Do not read or write git reference files directly (#18079)
Browse files Browse the repository at this point in the history
Git will and can pack references into packfiles and therefore if you write/read the
files directly you will get false results. Instead you should use update-ref and
show-ref. To that end I have created three new functions in git/repo_commit.go that
will do this correctly.

Related #17191

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath authored Dec 23, 2021
1 parent e0cf3d8 commit ffc08c1
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 45 deletions.
10 changes: 10 additions & 0 deletions modules/git/repo_commit_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
return ref.Hash().String(), nil
}

// SetReference sets the commit ID string of given reference (e.g. branch or tag).
func (repo *Repository) SetReference(name, commitID string) error {
return repo.gogitRepo.Storer.SetReference(plumbing.NewReferenceFromStrings(name, commitID))
}

// RemoveReference removes the given reference (e.g. branch or tag).
func (repo *Repository) RemoveReference(name string) error {
return repo.gogitRepo.Storer.RemoveReference(plumbing.ReferenceName(name))
}

// ConvertToSHA1 returns a Hash object from a potential ID string
func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
if len(commitID) == 40 {
Expand Down
12 changes: 12 additions & 0 deletions modules/git/repo_commit_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) {
return string(shaBs), nil
}

// SetReference sets the commit ID string of given reference (e.g. branch or tag).
func (repo *Repository) SetReference(name, commitID string) error {
_, err := NewCommandContext(repo.Ctx, "update-ref", name, commitID).RunInDir(repo.Path)
return err
}

// RemoveReference removes the given reference (e.g. branch or tag).
func (repo *Repository) RemoveReference(name string) error {
_, err := NewCommandContext(repo.Ctx, "update-ref", "--no-deref", "-d", name).RunInDir(repo.Path)
return err
}

// IsCommitExist returns true if given commit exists in current repository.
func (repo *Repository) IsCommitExist(name string) bool {
_, err := NewCommandContext(repo.Ctx, "cat-file", "-e", name).RunInDir(repo.Path)
Expand Down
33 changes: 0 additions & 33 deletions modules/git/repo_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -275,25 +274,6 @@ func (repo *Repository) GetDiffFromMergeBase(base, head string, w io.Writer) err
return err
}

// ReadPullHead will fetch a pull ref if possible or return an error
func (repo *Repository) ReadPullHead(prID int64) (commitSHA string, err error) {
headPath := fmt.Sprintf("refs/pull/%d/head", prID)
fullHeadPath := filepath.Join(repo.Path, headPath)
loadHead, err := os.Open(fullHeadPath)
if err != nil {
return "", err
}
defer loadHead.Close()
// Read only the first line of the patch - usually it contains the first commit made in patch
scanner := bufio.NewScanner(loadHead)
scanner.Scan()
commitHead := scanner.Text()
if len(commitHead) != 40 {
return "", errors.New("head file doesn't contain valid commit ID")
}
return commitHead, nil
}

// ReadPatchCommit will check if a diff patch exists and return stats
func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error) {
// Migrated repositories download patches to "pulls" location
Expand All @@ -315,16 +295,3 @@ func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error
}
return commitSHA, nil
}

// WritePullHead will populate a PR head retrieved from patch file
func (repo *Repository) WritePullHead(prID int64, commitSHA string) error {
headPath := fmt.Sprintf("refs/pull/%d", prID)
fullHeadPath := filepath.Join(repo.Path, headPath)
// Create missing directory just in case
if err := os.MkdirAll(fullHeadPath, os.ModePerm); err != nil {
return err
}
commitBytes := []byte(commitSHA)
pullPath := filepath.Join(fullHeadPath, "head")
return ioutil.WriteFile(pullPath, commitBytes, os.ModePerm)
}
15 changes: 7 additions & 8 deletions modules/git/repo_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"bytes"
"io"
"path/filepath"
"strings"
"testing"

"code.gitea.io/gitea/modules/util"
Expand Down Expand Up @@ -63,18 +62,18 @@ func TestReadWritePullHead(t *testing.T) {
assert.NoError(t, err)
defer repo.Close()
// Try to open non-existing Pull
_, err = repo.ReadPullHead(0)
_, err = repo.GetRefCommitID(PullPrefix + "0/head")
assert.Error(t, err)
// Write a fake sha1 with only 40 zeros
newCommit := strings.Repeat("0", 40)
err = repo.WritePullHead(1, newCommit)
newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2"
err = repo.SetReference(PullPrefix+"1/head", newCommit)
assert.NoError(t, err)
headFile := filepath.Join(repo.Path, "refs/pull/1/head")
// Remove file after the test
defer util.Remove(headFile)
assert.FileExists(t, headFile)
defer func() {
_ = repo.RemoveReference(PullPrefix + "1/head")
}()
// Read the file created
headContents, err := repo.ReadPullHead(1)
headContents, err := repo.GetRefCommitID(PullPrefix + "1/head")
assert.NoError(t, err)
assert.Len(t, string(headContents), 40)
assert.True(t, string(headContents) == newCommit)
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,13 +325,13 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C
if pull.MergeBase == "" {
var commitSHA, parentCommit string
// If there is a head or a patch file, and it is readable, grab info
commitSHA, err := ctx.Repo.GitRepo.ReadPullHead(pull.Index)
commitSHA, err := ctx.Repo.GitRepo.GetRefCommitID(pull.GetGitRefName())
if err != nil {
// Head File does not exist, try the patch
commitSHA, err = ctx.Repo.GitRepo.ReadPatchCommit(pull.Index)
if err == nil {
// Recreate pull head in files for next time
if err := ctx.Repo.GitRepo.WritePullHead(pull.Index, commitSHA); err != nil {
if err := ctx.Repo.GitRepo.SetReference(pull.GetGitRefName(), commitSHA); err != nil {
log.Error("Could not write head file", err)
}
} else {
Expand Down
3 changes: 1 addition & 2 deletions services/migrations/gitea_uploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,7 @@ func (g *GiteaLocalUploader) newPullRequest(pr *base.PullRequest) (*models.PullR
if pr.Head.SHA != "" {
// Git update-ref remove bad references with a relative path
log.Warn("Deprecated local head, removing : %v", pr.Head.SHA)
relPath := pr.GetGitRefName()
_, err = git.NewCommand("update-ref", "--no-deref", "-d", relPath).RunInDir(g.repo.RepoPath())
err = g.gitRepo.RemoveReference(pr.GetGitRefName())
} else {
// The SHA is empty, remove the head file
log.Warn("Empty reference, removing : %v", pullHead)
Expand Down

0 comments on commit ffc08c1

Please sign in to comment.