Use complete SHA to create and query commit status (#22244) (#22257)

Backport #22244.

Fix #13485.

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
Jason Song 2022-12-28 18:03:21 +08:00 committed by GitHub
parent 9b4da56963
commit e9bc2c77c3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 68 additions and 23 deletions

View file

@ -272,7 +272,7 @@ func (a *Action) GetRefLink() string {
return a.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.BranchPrefix)) return a.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.BranchPrefix))
case strings.HasPrefix(a.RefName, git.TagPrefix): case strings.HasPrefix(a.RefName, git.TagPrefix):
return a.GetRepoLink() + "/src/tag/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.TagPrefix)) return a.GetRepoLink() + "/src/tag/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.TagPrefix))
case len(a.RefName) == 40 && git.IsValidSHAPattern(a.RefName): case len(a.RefName) == git.SHAFullLength && git.IsValidSHAPattern(a.RefName):
return a.GetRepoLink() + "/src/commit/" + a.RefName return a.GetRepoLink() + "/src/commit/" + a.RefName
default: default:
// FIXME: we will just assume it's a branch - this was the old way - at some point we may want to enforce that there is always a ref here. // FIXME: we will just assume it's a branch - this was the old way - at some point we may want to enforce that there is always a ref here.

View file

@ -281,6 +281,10 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA) return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
} }
if _, err := git.NewIDFromString(opts.SHA); err != nil {
return fmt.Errorf("NewCommitStatus[%s, %s]: invalid sha: %w", repoPath, opts.SHA, err)
}
ctx, committer, err := db.TxContext() ctx, committer, err := db.TxContext()
if err != nil { if err != nil {
return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err) return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err)

View file

@ -388,7 +388,7 @@ func RepoRefForAPI(next http.Handler) http.Handler {
return return
} }
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
} else if len(refName) == 40 { } else if len(refName) == git.SHAFullLength {
ctx.Repo.CommitID = refName ctx.Repo.CommitID = refName
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
if err != nil { if err != nil {

View file

@ -816,7 +816,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
} }
// For legacy and API support only full commit sha // For legacy and API support only full commit sha
parts := strings.Split(path, "/") parts := strings.Split(path, "/")
if len(parts) > 0 && len(parts[0]) == 40 { if len(parts) > 0 && len(parts[0]) == git.SHAFullLength {
ctx.Repo.TreePath = strings.Join(parts[1:], "/") ctx.Repo.TreePath = strings.Join(parts[1:], "/")
return parts[0] return parts[0]
} }
@ -852,7 +852,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist) return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist)
case RepoRefCommit: case RepoRefCommit:
parts := strings.Split(path, "/") parts := strings.Split(path, "/")
if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= 40 { if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= git.SHAFullLength {
ctx.Repo.TreePath = strings.Join(parts[1:], "/") ctx.Repo.TreePath = strings.Join(parts[1:], "/")
return parts[0] return parts[0]
} }
@ -961,7 +961,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
return return
} }
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
} else if len(refName) >= 7 && len(refName) <= 40 { } else if len(refName) >= 7 && len(refName) <= git.SHAFullLength {
ctx.Repo.IsViewCommit = true ctx.Repo.IsViewCommit = true
ctx.Repo.CommitID = refName ctx.Repo.CommitID = refName
@ -971,7 +971,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
return return
} }
// If short commit ID add canonical link header // If short commit ID add canonical link header
if len(refName) < 40 { if len(refName) < git.SHAFullLength {
ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"", ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"",
util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1)))) util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1))))
} }

View file

@ -42,7 +42,7 @@ func (repo *Repository) RemoveReference(name string) error {
// ConvertToSHA1 returns a Hash object from a potential ID string // ConvertToSHA1 returns a Hash object from a potential ID string
func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) { func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
if len(commitID) == 40 { if len(commitID) == SHAFullLength {
sha1, err := NewIDFromString(commitID) sha1, err := NewIDFromString(commitID)
if err == nil { if err == nil {
return sha1, nil return sha1, nil

View file

@ -138,7 +138,7 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co
// ConvertToSHA1 returns a Hash object from a potential ID string // ConvertToSHA1 returns a Hash object from a potential ID string
func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) { func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
if len(commitID) == 40 && IsValidSHAPattern(commitID) { if len(commitID) == SHAFullLength && IsValidSHAPattern(commitID) {
sha1, err := NewIDFromString(commitID) sha1, err := NewIDFromString(commitID)
if err == nil { if err == nil {
return sha1, nil return sha1, nil

View file

@ -17,7 +17,7 @@ import (
// ReadTreeToIndex reads a treeish to the index // ReadTreeToIndex reads a treeish to the index
func (repo *Repository) ReadTreeToIndex(treeish string, indexFilename ...string) error { func (repo *Repository) ReadTreeToIndex(treeish string, indexFilename ...string) error {
if len(treeish) != 40 { if len(treeish) != SHAFullLength {
res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(treeish).RunStdString(&RunOpts{Dir: repo.Path}) res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(treeish).RunStdString(&RunOpts{Dir: repo.Path})
if err != nil { if err != nil {
return err return err

View file

@ -20,7 +20,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
// GetTree find the tree object in the repository. // GetTree find the tree object in the repository.
func (repo *Repository) GetTree(idStr string) (*Tree, error) { func (repo *Repository) GetTree(idStr string) (*Tree, error) {
if len(idStr) != 40 { if len(idStr) != SHAFullLength {
res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(idStr).RunStdString(&RunOpts{Dir: repo.Path}) res, _, err := NewCommand(repo.Ctx, "rev-parse", "--verify").AddDynamicArguments(idStr).RunStdString(&RunOpts{Dir: repo.Path})
if err != nil { if err != nil {
return nil, err return nil, err

View file

@ -67,7 +67,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
// GetTree find the tree object in the repository. // GetTree find the tree object in the repository.
func (repo *Repository) GetTree(idStr string) (*Tree, error) { func (repo *Repository) GetTree(idStr string) (*Tree, error) {
if len(idStr) != 40 { if len(idStr) != SHAFullLength {
res, err := repo.GetRefCommitID(idStr) res, err := repo.GetRefCommitID(idStr)
if err != nil { if err != nil {
return nil, err return nil, err

View file

@ -18,6 +18,9 @@ const EmptySHA = "0000000000000000000000000000000000000000"
// EmptyTreeSHA is the SHA of an empty tree // EmptyTreeSHA is the SHA of an empty tree
const EmptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" const EmptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
// SHAFullLength is the full length of a git SHA
const SHAFullLength = 40
// SHAPattern can be used to determine if a string is an valid sha // SHAPattern can be used to determine if a string is an valid sha
var shaPattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) var shaPattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`)
@ -51,7 +54,7 @@ func MustIDFromString(s string) SHA1 {
func NewIDFromString(s string) (SHA1, error) { func NewIDFromString(s string) (SHA1, error) {
var id SHA1 var id SHA1
s = strings.TrimSpace(s) s = strings.TrimSpace(s)
if len(s) != 40 { if len(s) != SHAFullLength {
return id, fmt.Errorf("Length must be 40: %s", s) return id, fmt.Errorf("Length must be 40: %s", s)
} }
b, err := hex.DecodeString(s) b, err := hex.DecodeString(s)

View file

@ -184,6 +184,7 @@ func getCommitStatuses(ctx *context.APIContext, sha string) {
ctx.Error(http.StatusBadRequest, "ref/sha not given", nil) ctx.Error(http.StatusBadRequest, "ref/sha not given", nil)
return return
} }
sha = utils.MustConvertToSHA1(ctx.Context, sha)
repo := ctx.Repo.Repository repo := ctx.Repo.Repository
listOptions := utils.GetListOptions(ctx) listOptions := utils.GetListOptions(ctx)

View file

@ -34,6 +34,8 @@ func ResolveRefOrSha(ctx *context.APIContext, ref string) string {
} }
} }
sha = MustConvertToSHA1(ctx.Context, sha)
if ctx.Repo.GitRepo != nil { if ctx.Repo.GitRepo != nil {
err := ctx.Repo.GitRepo.AddLastCommitCache(ctx.Repo.Repository.GetCommitsCountCacheKey(ref, ref != sha), ctx.Repo.Repository.FullName(), sha) err := ctx.Repo.GitRepo.AddLastCommitCache(ctx.Repo.Repository.GetCommitsCountCacheKey(ref, ref != sha), ctx.Repo.Repository.FullName(), sha)
if err != nil { if err != nil {
@ -66,3 +68,30 @@ func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (str
} }
return "", "", nil return "", "", nil
} }
// ConvertToSHA1 returns a full-length SHA1 from a potential ID string
func ConvertToSHA1(ctx *context.Context, commitID string) (git.SHA1, error) {
if len(commitID) == git.SHAFullLength && git.IsValidSHAPattern(commitID) {
sha1, err := git.NewIDFromString(commitID)
if err == nil {
return sha1, nil
}
}
gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.Repo.Repository.RepoPath())
if err != nil {
return git.SHA1{}, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
}
defer closer.Close()
return gitRepo.ConvertToSHA1(commitID)
}
// MustConvertToSHA1 returns a full-length SHA1 string from a potential ID string, or returns origin input if it can't convert to SHA1
func MustConvertToSHA1(ctx *context.Context, commitID string) string {
sha, err := ConvertToSHA1(ctx, commitID)
if err != nil {
return commitID
}
return sha.String()
}

View file

@ -284,7 +284,7 @@ func Diff(ctx *context.Context) {
} }
return return
} }
if len(commitID) != 40 { if len(commitID) != git.SHAFullLength {
commitID = commit.ID.String() commitID = commit.ID.String()
} }

View file

@ -200,19 +200,19 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
return nil, fmt.Errorf("ReadFile(%s): %w", headFile, err) return nil, fmt.Errorf("ReadFile(%s): %w", headFile, err)
} }
commitID := string(commitIDBytes) commitID := string(commitIDBytes)
if len(commitID) < 40 { if len(commitID) < git.SHAFullLength {
return nil, fmt.Errorf(`ReadFile(%s): invalid commit-ID "%s"`, headFile, commitID) return nil, fmt.Errorf(`ReadFile(%s): invalid commit-ID "%s"`, headFile, commitID)
} }
cmd := commitID[:40] + ".." + pr.BaseBranch cmd := commitID[:git.SHAFullLength] + ".." + pr.BaseBranch
// Get the commit from BaseBranch where the pull request got merged // Get the commit from BaseBranch where the pull request got merged
mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse").AddDynamicArguments(cmd). mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse").AddDynamicArguments(cmd).
RunStdString(&git.RunOpts{Dir: "", Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}}) RunStdString(&git.RunOpts{Dir: "", Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}})
if err != nil { if err != nil {
return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %w", err) return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %w", err)
} else if len(mergeCommit) < 40 { } else if len(mergeCommit) < git.SHAFullLength {
// PR was maybe fast-forwarded, so just use last commit of PR // PR was maybe fast-forwarded, so just use last commit of PR
mergeCommit = commitID[:40] mergeCommit = commitID[:git.SHAFullLength]
} }
gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo.RepoPath()) gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo.RepoPath())
@ -221,9 +221,9 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
} }
defer gitRepo.Close() defer gitRepo.Close()
commit, err := gitRepo.GetCommit(mergeCommit[:40]) commit, err := gitRepo.GetCommit(mergeCommit[:git.SHAFullLength])
if err != nil { if err != nil {
return nil, fmt.Errorf("GetMergeCommit[%v]: %w", mergeCommit[:40], err) return nil, fmt.Errorf("GetMergeCommit[%v]: %w", mergeCommit[:git.SHAFullLength], err)
} }
return commit, nil return commit, nil

View file

@ -836,7 +836,7 @@ func MergedManually(pr *issues_model.PullRequest, doer *user_model.User, baseGit
return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: repo_model.MergeStyleManuallyMerged} return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: repo_model.MergeStyleManuallyMerged}
} }
if len(commitID) < 40 { if len(commitID) < git.SHAFullLength {
return fmt.Errorf("Wrong commit ID") return fmt.Errorf("Wrong commit ID")
} }

View file

@ -167,7 +167,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str
var headBranch string var headBranch string
if pr.Flow == issues_model.PullRequestFlowGithub { if pr.Flow == issues_model.PullRequestFlowGithub {
headBranch = git.BranchPrefix + pr.HeadBranch headBranch = git.BranchPrefix + pr.HeadBranch
} else if len(pr.HeadCommitID) == 40 { // for not created pull request } else if len(pr.HeadCommitID) == git.SHAFullLength { // for not created pull request
headBranch = pr.HeadCommitID headBranch = pr.HeadCommitID
} else { } else {
headBranch = pr.GetGitRefName() headBranch = pr.GetGitRefName()

View file

@ -30,9 +30,12 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato
} }
defer closer.Close() defer closer.Close()
if _, err := gitRepo.GetCommit(sha); err != nil { if commit, err := gitRepo.GetCommit(sha); err != nil {
gitRepo.Close() gitRepo.Close()
return fmt.Errorf("GetCommit[%s]: %w", sha, err) return fmt.Errorf("GetCommit[%s]: %w", sha, err)
} else if len(sha) != git.SHAFullLength {
// use complete commit sha
sha = commit.ID.String()
} }
gitRepo.Close() gitRepo.Close()

View file

@ -50,7 +50,7 @@ func GetTreeBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git
copy(treeURL[apiURLLen:], "/git/trees/") copy(treeURL[apiURLLen:], "/git/trees/")
// 40 is the size of the sha1 hash in hexadecimal format. // 40 is the size of the sha1 hash in hexadecimal format.
copyPos := len(treeURL) - 40 copyPos := len(treeURL) - git.SHAFullLength
if perPage <= 0 || perPage > setting.API.DefaultGitTreesPerPage { if perPage <= 0 || perPage > setting.API.DefaultGitTreesPerPage {
perPage = setting.API.DefaultGitTreesPerPage perPage = setting.API.DefaultGitTreesPerPage

View file

@ -69,6 +69,11 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
reqOne := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/status") reqOne := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/status")
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state) testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
// By short SHA
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/statuses")
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/status")
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
// By Ref // By Ref
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/statuses") req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/statuses")
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/status") reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/status")