Skip to content

Commit 3d63caa

Browse files
65436543
authored
[API] Get a single commit via Ref (#10915)
* GET /repos/:owner/:repo/commits/:ref * add Validation Checks * Fix & Extend TEST * add two new tast cases
1 parent 71979d9 commit 3d63caa

File tree

7 files changed

+171
-32
lines changed

7 files changed

+171
-32
lines changed

‎integrations/api_repo_git_commits_test.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,41 @@ func TestAPIReposGitCommits(t *testing.T) {
2121
session := loginUser(t, user.Name)
2222
token := getTokenForLoggedInUser(t, session)
2323

24+
//check invalid requests for GetCommitsBySHA
25+
req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/master?token="+token, user.Name)
26+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
27+
28+
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/12345?token="+token, user.Name)
29+
session.MakeRequest(t, req, http.StatusNotFound)
30+
31+
//check invalid requests for GetCommitsByRef
32+
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/..?token="+token, user.Name)
33+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
34+
35+
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/branch-not-exist?token="+token, user.Name)
36+
session.MakeRequest(t, req, http.StatusNotFound)
37+
2438
for _, ref := range [...]string{
25-
"commits/master", // Branch
26-
"commits/v1.1", // Tag
39+
"master", // Branch
40+
"v1.1", // Tag
41+
"65f1", // short sha
42+
"65f1bf27bc3bf70f64657658635e66094edbcb4d", // full sha
2743
} {
28-
req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/%s?token="+token, user.Name, ref)
29-
session.MakeRequest(t, req, http.StatusOK)
44+
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/%s?token="+token, user.Name, ref)
45+
resp := session.MakeRequest(t, req, http.StatusOK)
46+
commitByRef := new(api.Commit)
47+
DecodeJSON(t, resp, commitByRef)
48+
assert.Len(t, commitByRef.SHA, 40)
49+
assert.EqualValues(t, commitByRef.SHA, commitByRef.RepoCommit.Tree.SHA)
50+
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/%s?token="+token, user.Name, commitByRef.SHA)
51+
resp = session.MakeRequest(t, req, http.StatusOK)
52+
commitBySHA := new(api.Commit)
53+
DecodeJSON(t, resp, commitBySHA)
54+
55+
assert.EqualValues(t, commitByRef.SHA, commitBySHA.SHA)
56+
assert.EqualValues(t, commitByRef.HTMLURL, commitBySHA.HTMLURL)
57+
assert.EqualValues(t, commitByRef.RepoCommit.Message, commitBySHA.RepoCommit.Message)
3058
}
31-
32-
// Test getting non-existent refs
33-
req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/unknown?token="+token, user.Name)
34-
session.MakeRequest(t, req, http.StatusNotFound)
3559
}
3660

3761
func TestAPIReposGitCommitList(t *testing.T) {

‎modules/convert/convert.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,6 @@ func ToCommitUser(sig *git.Signature) *api.CommitUser {
386386
func ToCommitMeta(repo *models.Repository, tag *git.Tag) *api.CommitMeta {
387387
return &api.CommitMeta{
388388
SHA: tag.Object.String(),
389-
// TODO: Add the /commits API endpoint and use it here (https://developer.github.com/v3/repos/commits/#get-a-single-commit)
390389
URL: util.URLJoin(repo.APIURL(), "git/commits", tag.ID.String()),
391390
}
392391
}

‎modules/git/sha1.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package git
88
import (
99
"encoding/hex"
1010
"fmt"
11+
"regexp"
1112
"strings"
1213

1314
"github.com/go-git/go-git/v5/plumbing"
@@ -16,6 +17,9 @@ import (
1617
// EmptySHA defines empty git SHA
1718
const EmptySHA = "0000000000000000000000000000000000000000"
1819

20+
// SHAPattern can be used to determine if a string is an valid sha
21+
var SHAPattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`)
22+
1923
// SHA1 a git commit name
2024
type SHA1 = plumbing.Hash
2125

‎modules/validation/binding.go

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,32 @@ const (
2222
)
2323

2424
var (
25-
// GitRefNamePattern is regular expression with unallowed characters in git reference name
25+
// GitRefNamePatternInvalid is regular expression with unallowed characters in git reference name
2626
// They cannot have ASCII control characters (i.e. bytes whose values are lower than \040, or \177 DEL), space, tilde ~, caret ^, or colon : anywhere.
2727
// They cannot have question-mark ?, asterisk *, or open bracket [ anywhere
28-
GitRefNamePattern = regexp.MustCompile(`[\000-\037\177 \\~^:?*[]+`)
28+
GitRefNamePatternInvalid = regexp.MustCompile(`[\000-\037\177 \\~^:?*[]+`)
2929
)
3030

31+
// CheckGitRefAdditionalRulesValid check name is valid on additional rules
32+
func CheckGitRefAdditionalRulesValid(name string) bool {
33+
34+
// Additional rules as described at https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
35+
if strings.HasPrefix(name, "/") || strings.HasSuffix(name, "/") ||
36+
strings.HasSuffix(name, ".") || strings.Contains(name, "..") ||
37+
strings.Contains(name, "//") || strings.Contains(name, "@{") ||
38+
name == "@" {
39+
return false
40+
}
41+
parts := strings.Split(name, "/")
42+
for _, part := range parts {
43+
if strings.HasSuffix(part, ".lock") || strings.HasPrefix(part, ".") {
44+
return false
45+
}
46+
}
47+
48+
return true
49+
}
50+
3151
// AddBindingRules adds additional binding rules
3252
func AddBindingRules() {
3353
addGitRefNameBindingRule()
@@ -44,25 +64,15 @@ func addGitRefNameBindingRule() {
4464
IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) {
4565
str := fmt.Sprintf("%v", val)
4666

47-
if GitRefNamePattern.MatchString(str) {
67+
if GitRefNamePatternInvalid.MatchString(str) {
4868
errs.Add([]string{name}, ErrGitRefName, "GitRefName")
4969
return false, errs
5070
}
51-
// Additional rules as described at https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
52-
if strings.HasPrefix(str, "/") || strings.HasSuffix(str, "/") ||
53-
strings.HasSuffix(str, ".") || strings.Contains(str, "..") ||
54-
strings.Contains(str, "//") || strings.Contains(str, "@{") ||
55-
str == "@" {
71+
72+
if !CheckGitRefAdditionalRulesValid(str) {
5673
errs.Add([]string{name}, ErrGitRefName, "GitRefName")
5774
return false, errs
5875
}
59-
parts := strings.Split(str, "/")
60-
for _, part := range parts {
61-
if strings.HasSuffix(part, ".lock") || strings.HasPrefix(part, ".") {
62-
errs.Add([]string{name}, ErrGitRefName, "GitRefName")
63-
return false, errs
64-
}
65-
}
6676

6777
return true, errs
6878
},

‎routers/api/v1/api.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -798,14 +798,14 @@ func RegisterRoutes(m *macaron.Macaron) {
798798
m.Group("/commits", func() {
799799
m.Get("", repo.GetAllCommits)
800800
m.Group("/:ref", func() {
801-
// TODO: Add m.Get("") for single commit (https://developer.github.com/v3/repos/commits/#get-a-single-commit)
801+
m.Get("", repo.GetSingleCommitByRef)
802802
m.Get("/status", repo.GetCombinedCommitStatusByRef)
803803
m.Get("/statuses", repo.GetCommitStatusesByRef)
804804
})
805805
}, reqRepoReader(models.UnitTypeCode))
806806
m.Group("/git", func() {
807807
m.Group("/commits", func() {
808-
m.Get("/:sha", repo.GetSingleCommit)
808+
m.Get("/:sha", repo.GetSingleCommitBySHA)
809809
})
810810
m.Get("/refs", repo.GetGitAllRefs)
811811
m.Get("/refs/*", repo.GetGitRefs)

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

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package repo
77

88
import (
9+
"fmt"
910
"math"
1011
"net/http"
1112
"strconv"
@@ -16,12 +17,13 @@ import (
1617
"code.gitea.io/gitea/modules/git"
1718
"code.gitea.io/gitea/modules/setting"
1819
api "code.gitea.io/gitea/modules/structs"
20+
"code.gitea.io/gitea/modules/validation"
1921
"code.gitea.io/gitea/routers/api/v1/utils"
2022
)
2123

22-
// GetSingleCommit get a commit via
23-
func GetSingleCommit(ctx *context.APIContext) {
24-
// swagger:operation GET /repos/{owner}/{repo}/git/commits/{sha} repository repoGetSingleCommit
24+
// GetSingleCommitBySHA get a commit via sha
25+
func GetSingleCommitBySHA(ctx *context.APIContext) {
26+
// swagger:operation GET /repos/{owner}/{repo}/git/commits/{sha} repository repoGetSingleCommitBySHA
2527
// ---
2628
// summary: Get a single commit from a repository
2729
// produces:
@@ -45,16 +47,68 @@ func GetSingleCommit(ctx *context.APIContext) {
4547
// responses:
4648
// "200":
4749
// "$ref": "#/responses/Commit"
50+
// "422":
51+
// "$ref": "#/responses/validationError"
4852
// "404":
4953
// "$ref": "#/responses/notFound"
5054

55+
sha := ctx.Params(":sha")
56+
if !git.SHAPattern.MatchString(sha) {
57+
ctx.Error(http.StatusUnprocessableEntity, "no valid sha", fmt.Sprintf("no valid sha: %s", sha))
58+
return
59+
}
60+
getCommit(ctx, sha)
61+
}
62+
63+
// GetSingleCommitByRef get a commit via ref
64+
func GetSingleCommitByRef(ctx *context.APIContext) {
65+
// swagger:operation GET /repos/{owner}/{repo}/commits/{ref} repository repoGetSingleCommitByRef
66+
// ---
67+
// summary: Get a single commit from a repository
68+
// produces:
69+
// - application/json
70+
// parameters:
71+
// - name: owner
72+
// in: path
73+
// description: owner of the repo
74+
// type: string
75+
// required: true
76+
// - name: repo
77+
// in: path
78+
// description: name of the repo
79+
// type: string
80+
// required: true
81+
// - name: ref
82+
// in: path
83+
// description: a git ref
84+
// type: string
85+
// required: true
86+
// responses:
87+
// "200":
88+
// "$ref": "#/responses/Commit"
89+
// "422":
90+
// "$ref": "#/responses/validationError"
91+
// "404":
92+
// "$ref": "#/responses/notFound"
93+
94+
ref := ctx.Params("ref")
95+
96+
if validation.GitRefNamePatternInvalid.MatchString(ref) || !validation.CheckGitRefAdditionalRulesValid(ref) {
97+
ctx.Error(http.StatusUnprocessableEntity, "no valid sha", fmt.Sprintf("no valid ref: %s", ref))
98+
return
99+
}
100+
101+
getCommit(ctx, ref)
102+
}
103+
104+
func getCommit(ctx *context.APIContext, identifier string) {
51105
gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath())
52106
if err != nil {
53107
ctx.ServerError("OpenRepository", err)
54108
return
55109
}
56110
defer gitRepo.Close()
57-
commit, err := gitRepo.GetCommit(ctx.Params(":sha"))
111+
commit, err := gitRepo.GetCommit(identifier)
58112
if err != nil {
59113
ctx.NotFoundOrServerError("GetCommit", git.IsErrNotExist, err)
60114
return
@@ -65,7 +119,6 @@ func GetSingleCommit(ctx *context.APIContext) {
65119
ctx.ServerError("toCommit", err)
66120
return
67121
}
68-
69122
ctx.JSON(http.StatusOK, json)
70123
}
71124

‎templates/swagger/v1_json.tmpl

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2511,6 +2511,52 @@
25112511
}
25122512
}
25132513
},
2514+
"/repos/{owner}/{repo}/commits/{ref}": {
2515+
"get": {
2516+
"produces": [
2517+
"application/json"
2518+
],
2519+
"tags": [
2520+
"repository"
2521+
],
2522+
"summary": "Get a single commit from a repository",
2523+
"operationId": "repoGetSingleCommitByRef",
2524+
"parameters": [
2525+
{
2526+
"type": "string",
2527+
"description": "owner of the repo",
2528+
"name": "owner",
2529+
"in": "path",
2530+
"required": true
2531+
},
2532+
{
2533+
"type": "string",
2534+
"description": "name of the repo",
2535+
"name": "repo",
2536+
"in": "path",
2537+
"required": true
2538+
},
2539+
{
2540+
"type": "string",
2541+
"description": "a git ref",
2542+
"name": "ref",
2543+
"in": "path",
2544+
"required": true
2545+
}
2546+
],
2547+
"responses": {
2548+
"200": {
2549+
"$ref": "#/responses/Commit"
2550+
},
2551+
"404": {
2552+
"$ref": "#/responses/notFound"
2553+
},
2554+
"422": {
2555+
"$ref": "#/responses/validationError"
2556+
}
2557+
}
2558+
}
2559+
},
25142560
"/repos/{owner}/{repo}/commits/{ref}/statuses": {
25152561
"get": {
25162562
"produces": [
@@ -2976,7 +3022,7 @@
29763022
"repository"
29773023
],
29783024
"summary": "Get a single commit from a repository",
2979-
"operationId": "repoGetSingleCommit",
3025+
"operationId": "repoGetSingleCommitBySHA",
29803026
"parameters": [
29813027
{
29823028
"type": "string",
@@ -3006,6 +3052,9 @@
30063052
},
30073053
"404": {
30083054
"$ref": "#/responses/notFound"
3055+
},
3056+
"422": {
3057+
"$ref": "#/responses/validationError"
30093058
}
30103059
}
30113060
}

0 commit comments

Comments
 (0)