Skip to content

Commit 92796dc

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

File tree

19 files changed

+67
-24
lines changed

19 files changed

+67
-24
lines changed

‎integrations/repo_commits_test.go

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

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

‎models/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ func (a *Action) GetRefLink() string {
287287
return a.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.BranchPrefix))
288288
case strings.HasPrefix(a.RefName, git.TagPrefix):
289289
return a.GetRepoLink() + "/src/tag/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.TagPrefix))
290-
case len(a.RefName) == 40 && git.IsValidSHAPattern(a.RefName):
290+
case len(a.RefName) == git.SHAFullLength && git.IsValidSHAPattern(a.RefName):
291291
return a.GetRepoLink() + "/src/commit/" + a.RefName
292292
default:
293293
// 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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,10 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
291291
return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
292292
}
293293

294+
if _, err := git.NewIDFromString(opts.SHA); err != nil {
295+
return fmt.Errorf("NewCommitStatus[%s, %s]: invalid sha: %w", repoPath, opts.SHA, err)
296+
}
297+
294298
// Get the next Status Index
295299
idx, err := GetNextCommitStatusIndex(opts.Repo.ID, opts.SHA)
296300
if err != nil {

‎modules/context/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ func RepoRefForAPI(next http.Handler) http.Handler {
388388
return
389389
}
390390
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
391-
} else if len(refName) == 40 {
391+
} else if len(refName) == git.SHAFullLength {
392392
ctx.Repo.CommitID = refName
393393
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
394394
if err != nil {

‎modules/context/repo.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
807807
}
808808
// For legacy and API support only full commit sha
809809
parts := strings.Split(path, "/")
810-
if len(parts) > 0 && len(parts[0]) == 40 {
810+
if len(parts) > 0 && len(parts[0]) == git.SHAFullLength {
811811
ctx.Repo.TreePath = strings.Join(parts[1:], "/")
812812
return parts[0]
813813
}
@@ -843,7 +843,7 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
843843
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist)
844844
case RepoRefCommit:
845845
parts := strings.Split(path, "/")
846-
if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= 40 {
846+
if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= git.SHAFullLength {
847847
ctx.Repo.TreePath = strings.Join(parts[1:], "/")
848848
return parts[0]
849849
}
@@ -952,7 +952,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
952952
return
953953
}
954954
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
955-
} else if len(refName) >= 7 && len(refName) <= 40 {
955+
} else if len(refName) >= 7 && len(refName) <= git.SHAFullLength {
956956
ctx.Repo.IsViewCommit = true
957957
ctx.Repo.CommitID = refName
958958

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

‎modules/git/repo_commit_gogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (repo *Repository) RemoveReference(name string) error {
4242

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

‎modules/git/repo_commit_nogogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co
138138

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

‎modules/git/repo_index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717

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

‎modules/git/repo_tree_gogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
2020

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

‎modules/git/repo_tree_nogogit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
6767

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

‎modules/git/sha1.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ const EmptySHA = "0000000000000000000000000000000000000000"
1818
// EmptyTreeSHA is the SHA of an empty tree
1919
const EmptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
2020

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

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ func getCommitStatuses(ctx *context.APIContext, sha string) {
184184
ctx.Error(http.StatusBadRequest, "ref/sha not given", nil)
185185
return
186186
}
187+
sha = utils.MustConvertToSHA1(ctx.Context, sha)
187188
repo := ctx.Repo.Repository
188189

189190
listOptions := utils.GetListOptions(ctx)

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func ResolveRefOrSha(ctx *context.APIContext, ref string) string {
3030
return refSHA
3131
}
3232
}
33-
return ref
33+
return MustConvertToSHA1(ctx.Context, ref)
3434
}
3535

3636
// GetGitRefs return git references based on filter
@@ -55,3 +55,30 @@ func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (str
5555
}
5656
return "", "", nil
5757
}
58+
59+
// ConvertToSHA1 returns a full-length SHA1 from a potential ID string
60+
func ConvertToSHA1(ctx *context.Context, commitID string) (git.SHA1, error) {
61+
if len(commitID) == git.SHAFullLength && git.IsValidSHAPattern(commitID) {
62+
sha1, err := git.NewIDFromString(commitID)
63+
if err == nil {
64+
return sha1, nil
65+
}
66+
}
67+
68+
gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.Repo.Repository.RepoPath())
69+
if err != nil {
70+
return git.SHA1{}, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
71+
}
72+
defer closer.Close()
73+
74+
return gitRepo.ConvertToSHA1(commitID)
75+
}
76+
77+
// MustConvertToSHA1 returns a full-length SHA1 string from a potential ID string, or returns origin input if it can't convert to SHA1
78+
func MustConvertToSHA1(ctx *context.Context, commitID string) string {
79+
sha, err := ConvertToSHA1(ctx, commitID)
80+
if err != nil {
81+
return commitID
82+
}
83+
return sha.String()
84+
}

‎routers/web/repo/commit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ func Diff(ctx *context.Context) {
284284
}
285285
return
286286
}
287-
if len(commitID) != 40 {
287+
if len(commitID) != git.SHAFullLength {
288288
commitID = commit.ID.String()
289289
}
290290

‎services/pull/check.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,19 +200,19 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
200200
return nil, fmt.Errorf("ReadFile(%s): %v", headFile, err)
201201
}
202202
commitID := string(commitIDBytes)
203-
if len(commitID) < 40 {
203+
if len(commitID) < git.SHAFullLength {
204204
return nil, fmt.Errorf(`ReadFile(%s): invalid commit-ID "%s"`, headFile, commitID)
205205
}
206-
cmd := commitID[:40] + ".." + pr.BaseBranch
206+
cmd := commitID[:git.SHAFullLength] + ".." + pr.BaseBranch
207207

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

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

224-
commit, err := gitRepo.GetCommit(mergeCommit[:40])
224+
commit, err := gitRepo.GetCommit(mergeCommit[:git.SHAFullLength])
225225
if err != nil {
226-
return nil, fmt.Errorf("GetMergeCommit[%v]: %v", mergeCommit[:40], err)
226+
return nil, fmt.Errorf("GetMergeCommit[%v]: %v", mergeCommit[:git.SHAFullLength], err)
227227
}
228228

229229
return commit, nil

‎services/pull/merge.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ func MergedManually(pr *issues_model.PullRequest, doer *user_model.User, baseGit
833833
return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: repo_model.MergeStyleManuallyMerged}
834834
}
835835

836-
if len(commitID) < 40 {
836+
if len(commitID) < git.SHAFullLength {
837837
return fmt.Errorf("Wrong commit ID")
838838
}
839839

‎services/pull/temp_repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str
167167
var headBranch string
168168
if pr.Flow == issues_model.PullRequestFlowGithub {
169169
headBranch = git.BranchPrefix + pr.HeadBranch
170-
} else if len(pr.HeadCommitID) == 40 { // for not created pull request
170+
} else if len(pr.HeadCommitID) == git.SHAFullLength { // for not created pull request
171171
headBranch = pr.HeadCommitID
172172
} else {
173173
headBranch = pr.GetGitRefName()

‎services/repository/files/commit.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,12 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato
3030
}
3131
defer closer.Close()
3232

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

‎services/repository/files/tree.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func GetTreeBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git
5050
copy(treeURL[apiURLLen:], "/git/trees/")
5151

5252
// 40 is the size of the sha1 hash in hexadecimal format.
53-
copyPos := len(treeURL) - 40
53+
copyPos := len(treeURL) - git.SHAFullLength
5454

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

0 commit comments

Comments
 (0)