diff --git a/modules/git/blame.go b/modules/git/blame.go index 2d992f83b44..68b43b75f87 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -72,12 +72,14 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { } var objectID string - objectFormatLength := r.objectFormat.FullLength() - - if len(lineBytes) > objectFormatLength && lineBytes[objectFormatLength] == ' ' && r.objectFormat.IsValid(string(lineBytes[0:objectFormatLength])) { - objectID = string(lineBytes[0:objectFormatLength]) + if lineField1, _, ok := bytes.Cut(lineBytes, []byte(" ")); ok { + lineFieldStr := string(lineField1) + if IsStringValidObjectID(r.objectFormat, lineFieldStr) { + objectID = lineFieldStr + } } - if len(objectID) > 0 { + + if objectID != "" { if blamePart == nil { blamePart = &BlamePart{ Sha: objectID, @@ -99,6 +101,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { } else if lineBytes[0] == '\t' { blamePart.Lines = append(blamePart.Lines, string(lineBytes[1:])) } else if bytes.HasPrefix(lineBytes, []byte(previousHeader)) { + objectFormatLength := r.objectFormat.FullLength() offset := len(previousHeader) // already includes a space blamePart.PreviousSha = string(lineBytes[offset : offset+objectFormatLength]) offset += objectFormatLength + 1 // +1 for space diff --git a/modules/git/blame_sha256_test.go b/modules/git/blame_sha256_test.go index 2b9b82040af..3a17f8d0de6 100644 --- a/modules/git/blame_sha256_test.go +++ b/modules/git/blame_sha256_test.go @@ -4,7 +4,6 @@ package git import ( - "context" "testing" "gitea.dev/modules/setting" @@ -14,8 +13,7 @@ import ( func TestReadingBlameOutputSha256(t *testing.T) { setting.AppDataPath = t.TempDir() - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() + ctx := t.Context() if DefaultFeatures().UsingGogit { t.Skip("Skipping test since gogit does not support sha256") diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index ad378033ec7..3c258d0dd76 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -4,7 +4,6 @@ package git import ( - "context" "testing" "gitea.dev/modules/setting" @@ -14,8 +13,7 @@ import ( func TestReadingBlameOutput(t *testing.T) { setting.AppDataPath = t.TempDir() - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() + ctx := t.Context() t.Run("Without .git-blame-ignore-revs", func(t *testing.T) { storage := mockRepository("repo5_pulls") diff --git a/modules/git/commit.go b/modules/git/commit.go index 600ed15ef87..e67e02777bd 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -248,25 +248,3 @@ func GetFullCommitID(ctx context.Context, repo RepositoryFacade, shortID string) } return strings.TrimSpace(commitID), nil } - -func IsStringLikelyCommitID(objFmt ObjectFormat, s string, minLength ...int) bool { - maxLen := 64 // sha256 - if objFmt != nil { - maxLen = objFmt.FullLength() - } - minLen := util.OptionalArg(minLength, maxLen) - if len(s) < minLen || len(s) > maxLen { - return false - } - return isStringLowerHex(s) -} - -func isStringLowerHex(s string) bool { - for _, c := range s { - isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') - if !isHex { - return false - } - } - return len(s) > 0 // it accepts odd length because "shorten commit id" can be 7-chars -} diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index 3e0c9f28543..6896ce15564 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -195,10 +195,3 @@ func Test_GetCommitBranchStart(t *testing.T) { assert.NotEmpty(t, startCommitID) assert.Equal(t, "95bb4d39648ee7e325106df01a621c530863a653", startCommitID) } - -func TestIsStringLikelyCommitID(t *testing.T) { - assert.True(t, IsStringLikelyCommitID(nil, "abc", 3)) - assert.False(t, IsStringLikelyCommitID(nil, "abc", 4)) - assert.True(t, IsStringLikelyCommitID(nil, strings.Repeat("a", 64), 4)) - assert.False(t, IsStringLikelyCommitID(nil, strings.Repeat("a", 65), 4)) -} diff --git a/modules/git/object_format.go b/modules/git/object_format.go index 081dc5a2127..b0aef23c947 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -6,16 +6,9 @@ package git import ( "crypto/sha1" "crypto/sha256" - "regexp" "strconv" ) -// sha1Pattern can be used to determine if a string is a valid sha -var sha1Pattern = regexp.MustCompile(`^[0-9a-f]{4,40}$`) - -// sha256Pattern can be used to determine if a string is a valid sha -var sha256Pattern = regexp.MustCompile(`^[0-9a-f]{4,64}$`) - type ObjectFormat interface { // Name returns the name of the object format Name() string @@ -25,8 +18,6 @@ type ObjectFormat interface { EmptyTree() ObjectID // FullLength is the length of the hash's hex string FullLength() int - // IsValid returns true if the input is a valid hash - IsValid(input string) bool // MustID creates a new ObjectID from a byte slice MustID(b []byte) ObjectID // ComputeHash compute the hash for a given ObjectType and content @@ -41,19 +32,10 @@ var ( emptySha1Tree = &Sha1Hash{0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60, 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04} ) -func (Sha1ObjectFormatImpl) Name() string { return "sha1" } -func (Sha1ObjectFormatImpl) EmptyObjectID() ObjectID { - return emptySha1ObjectID -} - -func (Sha1ObjectFormatImpl) EmptyTree() ObjectID { - return emptySha1Tree -} -func (Sha1ObjectFormatImpl) FullLength() int { return 40 } -func (Sha1ObjectFormatImpl) IsValid(input string) bool { - return sha1Pattern.MatchString(input) -} - +func (Sha1ObjectFormatImpl) Name() string { return "sha1" } +func (Sha1ObjectFormatImpl) EmptyObjectID() ObjectID { return emptySha1ObjectID } +func (Sha1ObjectFormatImpl) EmptyTree() ObjectID { return emptySha1Tree } +func (Sha1ObjectFormatImpl) FullLength() int { return 40 } func (Sha1ObjectFormatImpl) MustID(b []byte) ObjectID { var id Sha1Hash copy(id[0:20], b) @@ -83,19 +65,10 @@ var ( } ) -func (Sha256ObjectFormatImpl) Name() string { return "sha256" } -func (Sha256ObjectFormatImpl) EmptyObjectID() ObjectID { - return emptySha256ObjectID -} - -func (Sha256ObjectFormatImpl) EmptyTree() ObjectID { - return emptySha256Tree -} -func (Sha256ObjectFormatImpl) FullLength() int { return 64 } -func (Sha256ObjectFormatImpl) IsValid(input string) bool { - return sha256Pattern.MatchString(input) -} - +func (Sha256ObjectFormatImpl) Name() string { return "sha256" } +func (Sha256ObjectFormatImpl) EmptyObjectID() ObjectID { return emptySha256ObjectID } +func (Sha256ObjectFormatImpl) EmptyTree() ObjectID { return emptySha256Tree } +func (Sha256ObjectFormatImpl) FullLength() int { return 64 } func (Sha256ObjectFormatImpl) MustID(b []byte) ObjectID { var id Sha256Hash copy(id[0:32], b) @@ -115,34 +88,13 @@ func (h Sha256ObjectFormatImpl) ComputeHash(t ObjectType, content []byte) Object type invalidObjectFormatImpl struct{} -var emptyInvalidObjectID = &Sha1Hash{} - -func (h invalidObjectFormatImpl) Name() string { - return "invalid-object-format" -} - -func (h invalidObjectFormatImpl) EmptyObjectID() ObjectID { - return emptyInvalidObjectID -} - -func (h invalidObjectFormatImpl) EmptyTree() ObjectID { - return emptyInvalidObjectID -} - -func (h invalidObjectFormatImpl) FullLength() int { - return len(emptyInvalidObjectID) * 2 -} - -func (h invalidObjectFormatImpl) IsValid(input string) bool { - return false -} - -func (h invalidObjectFormatImpl) MustID(b []byte) ObjectID { - return emptyInvalidObjectID -} - +func (h invalidObjectFormatImpl) Name() string { return "invalid-object-format" } +func (h invalidObjectFormatImpl) EmptyObjectID() ObjectID { return &Sha1Hash{} } +func (h invalidObjectFormatImpl) EmptyTree() ObjectID { return h.EmptyObjectID() } +func (h invalidObjectFormatImpl) FullLength() int { return len(h.EmptyObjectID().RawValue()) * 2 } +func (h invalidObjectFormatImpl) MustID(b []byte) ObjectID { return h.EmptyObjectID() } func (h invalidObjectFormatImpl) ComputeHash(t ObjectType, content []byte) ObjectID { - return emptyInvalidObjectID + return h.EmptyObjectID() } var ( diff --git a/modules/git/object_id.go b/modules/git/object_id.go index 53ae5ec1a1a..78397872fa4 100644 --- a/modules/git/object_id.go +++ b/modules/git/object_id.go @@ -7,6 +7,8 @@ import ( "bytes" "encoding/hex" "fmt" + + "gitea.dev/modules/util" ) type ObjectID interface { @@ -104,3 +106,37 @@ func IsEmptyCommitID(commitID string) bool { func ComputeBlobHash(hashType ObjectFormat, content []byte) ObjectID { return hashType.ComputeHash(ObjectBlob, content) } + +func IsStringValidObjectID(objFmt ObjectFormat, s string, optMinLen ...int) bool { + if objFmt == invalidObjectFormat { + return false + } + var minLen, maxLen int + if objFmt != nil { + maxLen = objFmt.FullLength() + minLen = util.OptionalArg(optMinLen, maxLen) + } else { + if len(optMinLen) == 0 { + // if no "min length" is applied, then the length must exactly match one of the formats + if len(s) != Sha1ObjectFormat.FullLength() && len(s) != Sha256ObjectFormat.FullLength() { + return false + } + } + maxLen = Sha256ObjectFormat.FullLength() + minLen = util.OptionalArg(optMinLen, Sha1ObjectFormat.FullLength()) + } + if len(s) < minLen || len(s) > maxLen { + return false + } + return isStringLowerHex(s) +} + +func isStringLowerHex(s string) bool { + for _, c := range s { + isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') + if !isHex { + return false + } + } + return len(s) > 0 // it accepts odd length because "shorten commit id" can be 7-chars +} diff --git a/modules/git/object_id_test.go b/modules/git/object_id_test.go index 64b0c70303e..d8923267d73 100644 --- a/modules/git/object_id_test.go +++ b/modules/git/object_id_test.go @@ -4,19 +4,13 @@ package git import ( + "strings" "testing" "github.com/stretchr/testify/assert" ) func TestIsValidSHAPattern(t *testing.T) { - assert.True(t, Sha1ObjectFormat.IsValid("fee1")) - assert.True(t, Sha1ObjectFormat.IsValid("abc000")) - assert.True(t, Sha1ObjectFormat.IsValid("9023902390239023902390239023902390239023")) - assert.False(t, Sha1ObjectFormat.IsValid("90239023902390239023902390239023902390239023")) - assert.False(t, Sha1ObjectFormat.IsValid("abc")) - assert.False(t, Sha1ObjectFormat.IsValid("123g")) - assert.False(t, Sha1ObjectFormat.IsValid("some random text")) assert.Equal(t, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", ComputeBlobHash(Sha1ObjectFormat, nil).String()) assert.Equal(t, "2e65efe2a145dda7ee51d1741299f848e5bf752e", ComputeBlobHash(Sha1ObjectFormat, []byte("a")).String()) assert.Equal(t, "473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813", ComputeBlobHash(Sha256ObjectFormat, nil).String()) @@ -30,3 +24,30 @@ func TestInvalidObjectFormat(t *testing.T) { assert.NotNil(t, of) assert.False(t, IsValidObjectFormat("no-such")) } + +func TestIsStringValidObjectID(t *testing.T) { + assert.True(t, IsStringValidObjectID(nil, "abc", 3)) + assert.False(t, IsStringValidObjectID(nil, "abg", 3)) + + assert.False(t, IsStringValidObjectID(nil, "abc", 4)) + assert.True(t, IsStringValidObjectID(nil, "0123456789abcdef", 4)) + + assert.False(t, IsStringValidObjectID(nil, strings.Repeat("a", 3), 4)) + assert.True(t, IsStringValidObjectID(nil, strings.Repeat("a", 4), 4)) + assert.True(t, IsStringValidObjectID(nil, strings.Repeat("a", 5), 4)) + assert.True(t, IsStringValidObjectID(nil, strings.Repeat("a", 64), 4)) + assert.False(t, IsStringValidObjectID(nil, strings.Repeat("a", 65), 4)) + + assert.False(t, IsStringValidObjectID(nil, strings.Repeat("a", 39))) + assert.True(t, IsStringValidObjectID(nil, strings.Repeat("a", 40))) + assert.False(t, IsStringValidObjectID(nil, strings.Repeat("a", 41))) + assert.True(t, IsStringValidObjectID(nil, strings.Repeat("a", 64))) + assert.False(t, IsStringValidObjectID(nil, strings.Repeat("a", 65))) + + assert.False(t, IsStringValidObjectID(Sha1ObjectFormat, strings.Repeat("a", 39))) + assert.True(t, IsStringValidObjectID(Sha1ObjectFormat, strings.Repeat("a", 40))) + assert.False(t, IsStringValidObjectID(Sha1ObjectFormat, strings.Repeat("a", 41))) + + assert.False(t, IsStringValidObjectID(invalidObjectFormat, strings.Repeat("a", 40))) + assert.False(t, IsStringValidObjectID(invalidObjectFormat, strings.Repeat("a", 40), 4)) +} diff --git a/modules/git/ref.go b/modules/git/ref.go index eaed84e2b51..cf7640003ad 100644 --- a/modules/git/ref.go +++ b/modules/git/ref.go @@ -224,7 +224,7 @@ func (ref RefName) RefType() RefType { return RefTypeBranch case ref.IsTag(): return RefTypeTag - case IsStringLikelyCommitID(nil, string(ref), 6): + case IsStringValidObjectID(nil, string(ref), 6): return RefTypeCommit } return "" diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 2395f5be08d..5532044345c 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -203,8 +203,8 @@ func (repo *Repository) searchCommits(ctx context.Context, id ObjectID, opts Sea // if there are any keywords (ie not committer:, author:, time:) // then let's iterate over them for _, v := range opts.Keywords { - // ignore anything not matching a valid sha pattern - if id.Type().IsValid(v) { + // ignore anything not matching a valid sha pattern (TODO: the legacy min-length is pretty small, is it right or fine?) + if IsStringValidObjectID(id.Type(), v, 4) { // create new git log command with 1 commit limit hashCmd := gitcmd.NewCommand("log", "-1", prettyLogFormat) // add previous arguments except for --grep and --all diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index 53e35b6ff72..0014417f57f 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -13,7 +13,6 @@ import ( "gitea.dev/modules/git/gitcmd" "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/hash" "github.com/go-git/go-git/v5/plumbing/object" ) @@ -48,7 +47,7 @@ func (repo *Repository) ConvertToGitID(ctx context.Context, commitID string) (Ob if err != nil { return nil, err } - if len(commitID) == hash.HexSize && objectFormat.IsValid(commitID) { + if IsStringValidObjectID(objectFormat, commitID) { ID, err := NewIDFromString(commitID) if err == nil { return ID, nil diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index fb77f037d22..ac71df58a1b 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/modules/git/repo_commit_nogogit.go @@ -116,7 +116,7 @@ func (repo *Repository) ConvertToGitID(ctx context.Context, ref string) (ObjectI if err != nil { return nil, err } - if len(ref) == objectFormat.FullLength() && objectFormat.IsValid(ref) { + if IsStringValidObjectID(objectFormat, ref) { id, err := NewIDFromString(ref) if err == nil { return id, nil diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 94cc9c3b9ba..d4849d822be 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1157,8 +1157,8 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git log.Trace("Repo: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.FullName(), compareReq.BaseOriRef+compareReq.BaseOriRefSuffix, baseRef, compareReq.HeadOriRef+compareReq.HeadOriRefSuffix, headRef) - baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName()) - headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName()) + baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringValidObjectID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName()) + headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringValidObjectID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName()) // Check if base&head ref are valid. if !baseRefValid || !headRefValid { ctx.APIErrorNotFound() diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index d63ab2274ce..8f1c335ad03 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -52,7 +52,7 @@ func ResolveRefCommit(ctx reqctx.RequestContext, repo *repo_model.Repository, in } } if refCommit.RefName == "" { - if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.ObjectFormatName), inputRef, minCommitIDLen...) { + if git.IsStringValidObjectID(git.ObjectFormatFromName(repo.ObjectFormatName), inputRef, minCommitIDLen...) { refCommit.RefName = git.RefNameFromCommit(inputRef) } } diff --git a/services/context/repo.go b/services/context/repo.go index b54e1c4641e..1226459d47a 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -849,7 +849,7 @@ func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (re if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeTag); refName != "" { return refName, git.RefTypeTag, false } - if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), reqRefPathParts[0]) { + if git.IsStringValidObjectID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), reqRefPathParts[0]) { // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists repo.TreePath = strings.Join(reqRefPathParts[1:], "/") return reqRefPathParts[0], git.RefTypeCommit, false @@ -900,7 +900,7 @@ func getRefName(ctx *Base, repo *Repository, path string, refType git.RefType) s }) case git.RefTypeCommit: parts := strings.Split(path, "/") - if git.IsStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) { + if git.IsStringValidObjectID(repo.GetObjectFormat(), parts[0], 7) { // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists repo.TreePath = strings.Join(parts[1:], "/") return parts[0] @@ -1032,7 +1032,7 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) { return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if git.IsStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refShortName, 7) { + } else if git.IsStringValidObjectID(ctx.Repo.GetObjectFormat(), refShortName, 7) { ctx.Repo.RefFullName = git.RefNameFromCommit(refShortName) ctx.Repo.CommitID = refShortName diff --git a/services/migrations/common.go b/services/migrations/common.go index ba809e4045e..f1aab4ee1e3 100644 --- a/services/migrations/common.go +++ b/services/migrations/common.go @@ -50,19 +50,18 @@ func CheckAndEnsureSafePR(pr *base.PullRequest, commonCloneBaseURL string, g bas valid = false } - // SECURITY: SHAs Must be a SHA - // FIXME: hash only a SHA1 - CommitType := git.Sha1ObjectFormat - if pr.MergeCommitSHA != "" && !CommitType.IsValid(pr.MergeCommitSHA) { + // SECURITY: SHAs must be valid Git object IDs. + // The repository object format is not yet available at this stage. + if pr.MergeCommitSHA != "" && !git.IsStringValidObjectID(nil, pr.MergeCommitSHA) { WarnAndNotice("PR #%d in %s has invalid MergeCommitSHA: %s", pr.Number, g, pr.MergeCommitSHA) pr.MergeCommitSHA = "" } - if pr.Head.SHA != "" && !CommitType.IsValid(pr.Head.SHA) { + if pr.Head.SHA != "" && !git.IsStringValidObjectID(nil, pr.Head.SHA) { WarnAndNotice("PR #%d in %s has invalid HeadSHA: %s", pr.Number, g, pr.Head.SHA) pr.Head.SHA = "" valid = false } - if pr.Base.SHA != "" && !CommitType.IsValid(pr.Base.SHA) { + if pr.Base.SHA != "" && !git.IsStringValidObjectID(nil, pr.Base.SHA) { WarnAndNotice("PR #%d in %s has invalid BaseSHA: %s", pr.Number, g, pr.Base.SHA) pr.Base.SHA = "" valid = false diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 57f407a3afb..a76b0293969 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -918,7 +918,7 @@ func (g *GiteaLocalUploader) CreateReviews(ctx context.Context, reviews ...*base } objectFormat := git.ObjectFormatFromName(g.repo.ObjectFormatName) - if !objectFormat.IsValid(comment.CommitID) { + if !git.IsStringValidObjectID(objectFormat, comment.CommitID) { log.Warn("Invalid comment CommitID[%s] on comment[%d] in PR #%d of %s/%s replaced with %s", comment.CommitID, pr.Index, g.repoOwner, g.repoName, headCommitID) comment.CommitID = headCommitID }