Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API to get single commit via SHA and Ref #10915

Merged
merged 12 commits into from
Apr 8, 2020
40 changes: 32 additions & 8 deletions integrations/api_repo_git_commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,41 @@ func TestAPIReposGitCommits(t *testing.T) {
session := loginUser(t, user.Name)
token := getTokenForLoggedInUser(t, session)

//check invalid requests for GetCommitsBySHA
req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/master?token="+token, user.Name)
session.MakeRequest(t, req, http.StatusUnprocessableEntity)

req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/12345?token="+token, user.Name)
session.MakeRequest(t, req, http.StatusNotFound)

//check invalid requests for GetCommitsByRef
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/..?token="+token, user.Name)
session.MakeRequest(t, req, http.StatusUnprocessableEntity)

req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/branch-not-exist?token="+token, user.Name)
session.MakeRequest(t, req, http.StatusNotFound)

for _, ref := range [...]string{
"commits/master", // Branch
"commits/v1.1", // Tag
"master", // Branch
"v1.1", // Tag
"65f1", // short sha
"65f1bf27bc3bf70f64657658635e66094edbcb4d", // full sha
} {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/%s?token="+token, user.Name, ref)
session.MakeRequest(t, req, http.StatusOK)
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/%s?token="+token, user.Name, ref)
resp := session.MakeRequest(t, req, http.StatusOK)
commitByRef := new(api.Commit)
DecodeJSON(t, resp, commitByRef)
assert.Len(t, commitByRef.SHA, 40)
assert.EqualValues(t, commitByRef.SHA, commitByRef.RepoCommit.Tree.SHA)
req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/%s?token="+token, user.Name, commitByRef.SHA)
resp = session.MakeRequest(t, req, http.StatusOK)
commitBySHA := new(api.Commit)
DecodeJSON(t, resp, commitBySHA)

assert.EqualValues(t, commitByRef.SHA, commitBySHA.SHA)
assert.EqualValues(t, commitByRef.HTMLURL, commitBySHA.HTMLURL)
assert.EqualValues(t, commitByRef.RepoCommit.Message, commitBySHA.RepoCommit.Message)
}

// Test getting non-existent refs
req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/unknown?token="+token, user.Name)
session.MakeRequest(t, req, http.StatusNotFound)
}

func TestAPIReposGitCommitList(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion modules/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ func ToCommitUser(sig *git.Signature) *api.CommitUser {
func ToCommitMeta(repo *models.Repository, tag *git.Tag) *api.CommitMeta {
return &api.CommitMeta{
SHA: tag.Object.String(),
// TODO: Add the /commits API endpoint and use it here (https://developer.github.com/v3/repos/commits/#get-a-single-commit)
URL: util.URLJoin(repo.APIURL(), "git/commits", tag.ID.String()),
}
}
Expand Down
4 changes: 4 additions & 0 deletions modules/git/sha1.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package git
import (
"encoding/hex"
"fmt"
"regexp"
"strings"

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

// SHAPattern can be used to determine if a string is an valid sha
var SHAPattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`)

// SHA1 a git commit name
type SHA1 = plumbing.Hash

Expand Down
40 changes: 25 additions & 15 deletions modules/validation/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,32 @@ const (
)

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

// CheckGitRefAdditionalRulesValid check name is valid on additional rules
func CheckGitRefAdditionalRulesValid(name string) bool {

// Additional rules as described at https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
if strings.HasPrefix(name, "/") || strings.HasSuffix(name, "/") ||
strings.HasSuffix(name, ".") || strings.Contains(name, "..") ||
strings.Contains(name, "//") || strings.Contains(name, "@{") ||
name == "@" {
return false
}
parts := strings.Split(name, "/")
for _, part := range parts {
if strings.HasSuffix(part, ".lock") || strings.HasPrefix(part, ".") {
return false
}
}

return true
}

// AddBindingRules adds additional binding rules
func AddBindingRules() {
addGitRefNameBindingRule()
Expand All @@ -44,25 +64,15 @@ func addGitRefNameBindingRule() {
IsValid: func(errs binding.Errors, name string, val interface{}) (bool, binding.Errors) {
str := fmt.Sprintf("%v", val)

if GitRefNamePattern.MatchString(str) {
if GitRefNamePatternInvalid.MatchString(str) {
errs.Add([]string{name}, ErrGitRefName, "GitRefName")
return false, errs
}
// Additional rules as described at https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
if strings.HasPrefix(str, "/") || strings.HasSuffix(str, "/") ||
strings.HasSuffix(str, ".") || strings.Contains(str, "..") ||
strings.Contains(str, "//") || strings.Contains(str, "@{") ||
str == "@" {

if !CheckGitRefAdditionalRulesValid(str) {
errs.Add([]string{name}, ErrGitRefName, "GitRefName")
return false, errs
}
parts := strings.Split(str, "/")
for _, part := range parts {
if strings.HasSuffix(part, ".lock") || strings.HasPrefix(part, ".") {
errs.Add([]string{name}, ErrGitRefName, "GitRefName")
return false, errs
}
}

return true, errs
},
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,14 +798,14 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Group("/commits", func() {
m.Get("", repo.GetAllCommits)
m.Group("/:ref", func() {
// TODO: Add m.Get("") for single commit (https://developer.github.com/v3/repos/commits/#get-a-single-commit)
m.Get("", repo.GetSingleCommitByRef)
m.Get("/status", repo.GetCombinedCommitStatusByRef)
m.Get("/statuses", repo.GetCommitStatusesByRef)
})
}, reqRepoReader(models.UnitTypeCode))
m.Group("/git", func() {
m.Group("/commits", func() {
m.Get("/:sha", repo.GetSingleCommit)
m.Get("/:sha", repo.GetSingleCommitBySHA)
})
m.Get("/refs", repo.GetGitAllRefs)
m.Get("/refs/*", repo.GetGitRefs)
Expand Down
63 changes: 58 additions & 5 deletions routers/api/v1/repo/commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package repo

import (
"fmt"
"math"
"net/http"
"strconv"
Expand All @@ -16,12 +17,13 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/validation"
"code.gitea.io/gitea/routers/api/v1/utils"
)

// GetSingleCommit get a commit via
func GetSingleCommit(ctx *context.APIContext) {
// swagger:operation GET /repos/{owner}/{repo}/git/commits/{sha} repository repoGetSingleCommit
// GetSingleCommitBySHA get a commit via sha
func GetSingleCommitBySHA(ctx *context.APIContext) {
// swagger:operation GET /repos/{owner}/{repo}/git/commits/{sha} repository repoGetSingleCommitBySHA
// ---
// summary: Get a single commit from a repository
// produces:
Expand All @@ -45,16 +47,68 @@ func GetSingleCommit(ctx *context.APIContext) {
// responses:
// "200":
// "$ref": "#/responses/Commit"
// "422":
// "$ref": "#/responses/validationError"
// "404":
// "$ref": "#/responses/notFound"

sha := ctx.Params(":sha")
if !git.SHAPattern.MatchString(sha) {
ctx.Error(http.StatusUnprocessableEntity, "no valid sha", fmt.Sprintf("no valid sha: %s", sha))
return
}
getCommit(ctx, sha)
}

// GetSingleCommitByRef get a commit via ref
func GetSingleCommitByRef(ctx *context.APIContext) {
6543 marked this conversation as resolved.
Show resolved Hide resolved
// swagger:operation GET /repos/{owner}/{repo}/commits/{ref} repository repoGetSingleCommitByRef
// ---
// summary: Get a single commit from a repository
// produces:
// - application/json
// parameters:
// - name: owner
// in: path
// description: owner of the repo
// type: string
// required: true
// - name: repo
// in: path
// description: name of the repo
// type: string
// required: true
// - name: ref
// in: path
// description: a git ref
// type: string
// required: true
// responses:
// "200":
// "$ref": "#/responses/Commit"
// "422":
// "$ref": "#/responses/validationError"
// "404":
// "$ref": "#/responses/notFound"

ref := ctx.Params("ref")

if validation.GitRefNamePatternInvalid.MatchString(ref) || !validation.CheckGitRefAdditionalRulesValid(ref) {
ctx.Error(http.StatusUnprocessableEntity, "no valid sha", fmt.Sprintf("no valid ref: %s", ref))
return
}

getCommit(ctx, ref)
}

func getCommit(ctx *context.APIContext, identifier string) {
gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath())
if err != nil {
ctx.ServerError("OpenRepository", err)
return
}
defer gitRepo.Close()
commit, err := gitRepo.GetCommit(ctx.Params(":sha"))
commit, err := gitRepo.GetCommit(identifier)
if err != nil {
ctx.NotFoundOrServerError("GetCommit", git.IsErrNotExist, err)
return
Expand All @@ -65,7 +119,6 @@ func GetSingleCommit(ctx *context.APIContext) {
ctx.ServerError("toCommit", err)
return
}

ctx.JSON(http.StatusOK, json)
}

Expand Down
51 changes: 50 additions & 1 deletion templates/swagger/v1_json.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2511,6 +2511,52 @@
}
}
},
"/repos/{owner}/{repo}/commits/{ref}": {
"get": {
"produces": [
"application/json"
],
"tags": [
"repository"
],
"summary": "Get a single commit from a repository",
"operationId": "repoGetSingleCommitByRef",
"parameters": [
{
"type": "string",
"description": "owner of the repo",
"name": "owner",
"in": "path",
"required": true
},
{
"type": "string",
"description": "name of the repo",
"name": "repo",
"in": "path",
"required": true
},
{
"type": "string",
"description": "a git ref",
"name": "ref",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"$ref": "#/responses/Commit"
},
"404": {
"$ref": "#/responses/notFound"
},
"422": {
"$ref": "#/responses/validationError"
}
}
}
},
"/repos/{owner}/{repo}/commits/{ref}/statuses": {
"get": {
"produces": [
Expand Down Expand Up @@ -2976,7 +3022,7 @@
"repository"
],
"summary": "Get a single commit from a repository",
"operationId": "repoGetSingleCommit",
"operationId": "repoGetSingleCommitBySHA",
"parameters": [
{
"type": "string",
Expand Down Expand Up @@ -3006,6 +3052,9 @@
},
"404": {
"$ref": "#/responses/notFound"
},
"422": {
"$ref": "#/responses/validationError"
}
}
}
Expand Down