Skip to content

Commit 6cf09cc

Browse files
wolfogredelvhlafrikslunny
authored
Use complete SHA to create and query commit status (#22244)
Fix #13485. Co-authored-by: delvh <[email protected]> Co-authored-by: Lauris BH <[email protected]> Co-authored-by: Lunny Xiao <[email protected]>
1 parent 90237d8 commit 6cf09cc

File tree

19 files changed

+68
-23
lines changed

19 files changed

+68
-23
lines changed

‎models/activities/action.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ func (a *Action) GetRefLink() string {
272272
return a.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.BranchPrefix))
273273
case strings.HasPrefix(a.RefName, git.TagPrefix):
274274
return a.GetRepoLink() + "/src/tag/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.TagPrefix))
275-
case len(a.RefName) == 40 && git.IsValidSHAPattern(a.RefName):
275+
case len(a.RefName) == git.SHAFullLength && git.IsValidSHAPattern(a.RefName):
276276
return a.GetRepoLink() + "/src/commit/" + a.RefName
277277
default:
278278
// 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.

‎models/git/commit_status.go

+4
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,10 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
279279
return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
280280
}
281281

282+
if _, err := git.NewIDFromString(opts.SHA); err != nil {
283+
return fmt.Errorf("NewCommitStatus[%s, %s]: invalid sha: %w", repoPath, opts.SHA, err)
284+
}
285+
282286
ctx, committer, err := db.TxContext(db.DefaultContext)
283287
if err != nil {
284288
return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err)

‎modules/context/api.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ func RepoRefForAPI(next http.Handler) http.Handler {
387387
return
388388
}
389389
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
390-
} else if len(refName) == 40 {
390+
} else if len(refName) == git.SHAFullLength {
391391
ctx.Repo.CommitID = refName
392392
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
393393
if err != nil {

‎modules/context/repo.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
817817
}
818818
// For legacy and API support only full commit sha
819819
parts := strings.Split(path, "/")
820-
if len(parts) > 0 && len(parts[0]) == 40 {
820+
if len(parts) > 0 && len(parts[0]) == git.SHAFullLength {
821821
ctx.Repo.TreePath = strings.Join(parts[1:], "/")
822822
return parts[0]
823823
}
@@ -853,7 +853,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
853853
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist)
854854
case RepoRefCommit:
855855
parts := strings.Split(path, "/")
856-
if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= 40 {
856+
if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= git.SHAFullLength {
857857
ctx.Repo.TreePath = strings.Join(parts[1:], "/")
858858
return parts[0]
859859
}
@@ -962,7 +962,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
962962
return
963963
}
964964
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
965-
} else if len(refName) >= 7 && len(refName) <= 40 {
965+
} else if len(refName) >= 7 && len(refName) <= git.SHAFullLength {
966966
ctx.Repo.IsViewCommit = true
967967
ctx.Repo.CommitID = refName
968968

@@ -972,7 +972,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
972972
return
973973
}
974974
// If short commit ID add canonical link header
975-
if len(refName) < 40 {
975+
if len(refName) < git.SHAFullLength {
976976
ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"",
977977
util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1))))
978978
}

‎modules/git/repo_commit_gogit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func (repo *Repository) RemoveReference(name string) error {
4141

4242
// ConvertToSHA1 returns a Hash object from a potential ID string
4343
func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
44-
if len(commitID) == 40 {
44+
if len(commitID) == SHAFullLength {
4545
sha1, err := NewIDFromString(commitID)
4646
if err == nil {
4747
return sha1, nil

‎modules/git/repo_commit_nogogit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co
137137

138138
// ConvertToSHA1 returns a Hash object from a potential ID string
139139
func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
140-
if len(commitID) == 40 && IsValidSHAPattern(commitID) {
140+
if len(commitID) == SHAFullLength && IsValidSHAPattern(commitID) {
141141
sha1, err := NewIDFromString(commitID)
142142
if err == nil {
143143
return sha1, nil

‎modules/git/repo_index.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616

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

‎modules/git/repo_tree_gogit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
1919

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

‎modules/git/repo_tree_nogogit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
6666

6767
// GetTree find the tree object in the repository.
6868
func (repo *Repository) GetTree(idStr string) (*Tree, error) {
69-
if len(idStr) != 40 {
69+
if len(idStr) != SHAFullLength {
7070
res, err := repo.GetRefCommitID(idStr)
7171
if err != nil {
7272
return nil, err

‎modules/git/sha1.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ const EmptySHA = "0000000000000000000000000000000000000000"
1717
// EmptyTreeSHA is the SHA of an empty tree
1818
const EmptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
1919

20+
// SHAFullLength is the full length of a git SHA
21+
const SHAFullLength = 40
22+
2023
// SHAPattern can be used to determine if a string is an valid sha
2124
var shaPattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`)
2225

@@ -50,7 +53,7 @@ func MustIDFromString(s string) SHA1 {
5053
func NewIDFromString(s string) (SHA1, error) {
5154
var id SHA1
5255
s = strings.TrimSpace(s)
53-
if len(s) != 40 {
56+
if len(s) != SHAFullLength {
5457
return id, fmt.Errorf("Length must be 40: %s", s)
5558
}
5659
b, err := hex.DecodeString(s)

‎routers/api/v1/repo/status.go

+1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ func getCommitStatuses(ctx *context.APIContext, sha string) {
183183
ctx.Error(http.StatusBadRequest, "ref/sha not given", nil)
184184
return
185185
}
186+
sha = utils.MustConvertToSHA1(ctx.Context, sha)
186187
repo := ctx.Repo.Repository
187188

188189
listOptions := utils.GetListOptions(ctx)

‎routers/api/v1/utils/git.go

+29
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ func ResolveRefOrSha(ctx *context.APIContext, ref string) string {
3333
}
3434
}
3535

36+
sha = MustConvertToSHA1(ctx.Context, sha)
37+
3638
if ctx.Repo.GitRepo != nil {
3739
err := ctx.Repo.GitRepo.AddLastCommitCache(ctx.Repo.Repository.GetCommitsCountCacheKey(ref, ref != sha), ctx.Repo.Repository.FullName(), sha)
3840
if err != nil {
@@ -65,3 +67,30 @@ func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (str
6567
}
6668
return "", "", nil
6769
}
70+
71+
// ConvertToSHA1 returns a full-length SHA1 from a potential ID string
72+
func ConvertToSHA1(ctx *context.Context, commitID string) (git.SHA1, error) {
73+
if len(commitID) == git.SHAFullLength && git.IsValidSHAPattern(commitID) {
74+
sha1, err := git.NewIDFromString(commitID)
75+
if err == nil {
76+
return sha1, nil
77+
}
78+
}
79+
80+
gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.Repo.Repository.RepoPath())
81+
if err != nil {
82+
return git.SHA1{}, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
83+
}
84+
defer closer.Close()
85+
86+
return gitRepo.ConvertToSHA1(commitID)
87+
}
88+
89+
// MustConvertToSHA1 returns a full-length SHA1 string from a potential ID string, or returns origin input if it can't convert to SHA1
90+
func MustConvertToSHA1(ctx *context.Context, commitID string) string {
91+
sha, err := ConvertToSHA1(ctx, commitID)
92+
if err != nil {
93+
return commitID
94+
}
95+
return sha.String()
96+
}

‎routers/web/repo/commit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ func Diff(ctx *context.Context) {
283283
}
284284
return
285285
}
286-
if len(commitID) != 40 {
286+
if len(commitID) != git.SHAFullLength {
287287
commitID = commit.ID.String()
288288
}
289289

‎services/pull/check.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -199,19 +199,19 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
199199
return nil, fmt.Errorf("ReadFile(%s): %w", headFile, err)
200200
}
201201
commitID := string(commitIDBytes)
202-
if len(commitID) < 40 {
202+
if len(commitID) < git.SHAFullLength {
203203
return nil, fmt.Errorf(`ReadFile(%s): invalid commit-ID "%s"`, headFile, commitID)
204204
}
205-
cmd := commitID[:40] + ".." + pr.BaseBranch
205+
cmd := commitID[:git.SHAFullLength] + ".." + pr.BaseBranch
206206

207207
// Get the commit from BaseBranch where the pull request got merged
208208
mergeCommit, _, err := git.NewCommand(ctx, "rev-list", "--ancestry-path", "--merges", "--reverse").AddDynamicArguments(cmd).
209209
RunStdString(&git.RunOpts{Dir: "", Env: []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()}})
210210
if err != nil {
211211
return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %w", err)
212-
} else if len(mergeCommit) < 40 {
212+
} else if len(mergeCommit) < git.SHAFullLength {
213213
// PR was maybe fast-forwarded, so just use last commit of PR
214-
mergeCommit = commitID[:40]
214+
mergeCommit = commitID[:git.SHAFullLength]
215215
}
216216

217217
gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo.RepoPath())
@@ -220,9 +220,9 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
220220
}
221221
defer gitRepo.Close()
222222

223-
commit, err := gitRepo.GetCommit(mergeCommit[:40])
223+
commit, err := gitRepo.GetCommit(mergeCommit[:git.SHAFullLength])
224224
if err != nil {
225-
return nil, fmt.Errorf("GetMergeCommit[%v]: %w", mergeCommit[:40], err)
225+
return nil, fmt.Errorf("GetMergeCommit[%v]: %w", mergeCommit[:git.SHAFullLength], err)
226226
}
227227

228228
return commit, nil

‎services/pull/merge.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ func MergedManually(pr *issues_model.PullRequest, doer *user_model.User, baseGit
839839
return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: repo_model.MergeStyleManuallyMerged}
840840
}
841841

842-
if len(commitID) < 40 {
842+
if len(commitID) < git.SHAFullLength {
843843
return fmt.Errorf("Wrong commit ID")
844844
}
845845

‎services/pull/temp_repo.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str
166166
var headBranch string
167167
if pr.Flow == issues_model.PullRequestFlowGithub {
168168
headBranch = git.BranchPrefix + pr.HeadBranch
169-
} else if len(pr.HeadCommitID) == 40 { // for not created pull request
169+
} else if len(pr.HeadCommitID) == git.SHAFullLength { // for not created pull request
170170
headBranch = pr.HeadCommitID
171171
} else {
172172
headBranch = pr.GetGitRefName()

‎services/repository/files/commit.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato
2929
}
3030
defer closer.Close()
3131

32-
if _, err := gitRepo.GetCommit(sha); err != nil {
32+
if commit, err := gitRepo.GetCommit(sha); err != nil {
3333
gitRepo.Close()
3434
return fmt.Errorf("GetCommit[%s]: %w", sha, err)
35+
} else if len(sha) != git.SHAFullLength {
36+
// use complete commit sha
37+
sha = commit.ID.String()
3538
}
3639
gitRepo.Close()
3740

‎services/repository/files/tree.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func GetTreeBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git
4949
copy(treeURL[apiURLLen:], "/git/trees/")
5050

5151
// 40 is the size of the sha1 hash in hexadecimal format.
52-
copyPos := len(treeURL) - 40
52+
copyPos := len(treeURL) - git.SHAFullLength
5353

5454
if perPage <= 0 || perPage > setting.API.DefaultGitTreesPerPage {
5555
perPage = setting.API.DefaultGitTreesPerPage

‎tests/integration/repo_commits_test.go

+5
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
6868
reqOne := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/status")
6969
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
7070

71+
// By short SHA
72+
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/statuses")
73+
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/status")
74+
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
75+
7176
// By Ref
7277
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/statuses")
7378
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/status")

0 commit comments

Comments
 (0)