From e887f922ca3f62cba5001f2fda580515d3422f27 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Fri, 2 Aug 2019 15:31:41 +0800 Subject: [PATCH 1/9] Check commit message hashes before making links Previously, when formatting commit messages, anything that looked like SHA1 hashes was turned into a link using regex. This meant that certain phrases or numbers such as `777777` or `deadbeef` could be recognized as a commit even if the repository has no commit with those hashes. This change will make it so that anything that looks like a SHA1 hash using regex will then also be checked to ensure that there is a commit in the repository with that hash before making a link. Signed-off-by: Gary Kim --- models/repo.go | 5 +++-- modules/markup/html.go | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/models/repo.go b/models/repo.go index fe296c1a11a6f..dbd1f2b688533 100644 --- a/models/repo.go +++ b/models/repo.go @@ -508,8 +508,9 @@ func (repo *Repository) mustOwnerName(e Engine) string { func (repo *Repository) ComposeMetas() map[string]string { if repo.ExternalMetas == nil { repo.ExternalMetas = map[string]string{ - "user": repo.MustOwner().Name, - "repo": repo.Name, + "user": repo.MustOwner().Name, + "repo": repo.Name, + "repoPath": repo.RepoPath(), } unit, err := repo.GetUnit(UnitTypeExternalTracker) if err != nil { diff --git a/modules/markup/html.go b/modules/markup/html.go index 825a41dd1f6f3..a3db925f76275 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -13,6 +13,7 @@ import ( "strings" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -657,6 +658,14 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // but that is not always the case. // Although unlikely, deadbeef and 1234567 are valid short forms of SHA1 hash // as used by git and github for linking and thus we have to do similar. + // Because of this, we check to make sure that a matched hash is actually + // a commit in the repository before making it a link. + if ctx.metas["repoPath"] != "" { + if _, err := git.NewCommand("log", "-1", hash).RunInDirBytes(ctx.metas["repoPath"]); err != nil { + return + } + } + replaceContent(node, m[2], m[3], createCodeLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "commit", hash), base.ShortSha(hash))) } From d7788bff7d20e43879f0c411a1170d0d318840dd Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Fri, 2 Aug 2019 16:01:17 +0800 Subject: [PATCH 2/9] Use gogit to check if commit exists This commit modifies the commit hash check in the render for commit messages to use gogit for better performance. Signed-off-by: Gary Kim --- modules/markup/html.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index a3db925f76275..eabdf69a0e501 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -661,7 +661,11 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // Because of this, we check to make sure that a matched hash is actually // a commit in the repository before making it a link. if ctx.metas["repoPath"] != "" { - if _, err := git.NewCommand("log", "-1", hash).RunInDirBytes(ctx.metas["repoPath"]); err != nil { + repo, err := git.OpenRepository(ctx.metas["repoPath"]) + if err != nil { + return + } + if !repo.IsCommitExist(hash) { return } } From 582a18062125045af5cfe6e654239f8232840469 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Fri, 2 Aug 2019 16:06:08 +0800 Subject: [PATCH 3/9] Make code cleaner Signed-off-by: Gary Kim --- modules/markup/html.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index eabdf69a0e501..1dcd3389bce00 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -662,10 +662,7 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // a commit in the repository before making it a link. if ctx.metas["repoPath"] != "" { repo, err := git.OpenRepository(ctx.metas["repoPath"]) - if err != nil { - return - } - if !repo.IsCommitExist(hash) { + if err != nil || !repo.IsCommitExist(hash) { return } } From 58529741373f592f6023de97b89a154f27c1f09e Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Mon, 5 Aug 2019 13:01:26 +0800 Subject: [PATCH 4/9] Use rev-parse to check if commit exists Signed-off-by: Gary Kim --- modules/markup/html.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 1dcd3389bce00..e6c4be64cd249 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -647,6 +647,9 @@ func fullSha1PatternProcessor(ctx *postProcessCtx, node *html.Node) { // sha1CurrentPatternProcessor renders SHA1 strings to corresponding links that // are assumed to be in the same repository. func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { + if ctx.metas == nil || ctx.metas["user"] == "" || ctx.metas["repo"] == "" || ctx.metas["repoPath"] == "" { + return + } m := sha1CurrentPattern.FindStringSubmatchIndex(node.Data) if m == nil { return @@ -660,11 +663,8 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // as used by git and github for linking and thus we have to do similar. // Because of this, we check to make sure that a matched hash is actually // a commit in the repository before making it a link. - if ctx.metas["repoPath"] != "" { - repo, err := git.OpenRepository(ctx.metas["repoPath"]) - if err != nil || !repo.IsCommitExist(hash) { - return - } + if _, err := git.NewCommand("rev-parse", "--verify", hash).RunInDirBytes(ctx.metas["repoPath"]); err != nil { + return } replaceContent(node, m[2], m[3], From 026c84a3e30599079fa213dfabf63b37ff491fdc Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Mon, 5 Aug 2019 14:42:20 +0800 Subject: [PATCH 5/9] Add and modify tests for checking hashes in html link rendering Signed-off-by: Gary Kim --- modules/markup/html_test.go | 22 ++++++++++++---------- modules/markup/markdown/markdown_test.go | 9 +++++---- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index be7fb0efc57ad..66e56f71a74d3 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -17,8 +17,9 @@ import ( ) var localMetas = map[string]string{ - "user": "gogits", - "repo": "gogs", + "user": "gogits", + "repo": "gogs", + "repoPath": "../../integrations/gitea-repositories-meta/user13/repo11.git/", } func TestRender_Commits(t *testing.T) { @@ -30,19 +31,20 @@ func TestRender_Commits(t *testing.T) { assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } - var sha = "b6dd6210eaebc915fd5be5579c58cce4da2e2579" + var sha = "65f1bf27bc3bf70f64657658635e66094edbcb4d" var commit = util.URLJoin(AppSubURL, "commit", sha) var subtree = util.URLJoin(commit, "src") var tree = strings.Replace(subtree, "/commit/", "/tree/", -1) - test(sha, `

b6dd6210ea

`) - test(sha[:7], `

b6dd621

`) - test(sha[:39], `

b6dd6210ea

`) - test(commit, `

b6dd6210ea

`) - test(tree, `

b6dd6210ea/src

`) - test("commit "+sha, `

commit b6dd6210ea

`) + test(sha, `

65f1bf27bc

`) + test(sha[:7], `

65f1bf2

`) + test(sha[:39], `

65f1bf27bc

`) + test(commit, `

65f1bf27bc

`) + test(tree, `

65f1bf27bc/src

`) + test("commit "+sha, `

commit 65f1bf27bc

`) test("/home/gitea/"+sha, "

/home/gitea/"+sha+"

") - + test("deadbeef", `

deadbeef

`) + test("d27ace93", `

d27ace93

`) } func TestRender_CrossReferences(t *testing.T) { diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 92b4f03e34f87..669b49367ee0a 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -21,8 +21,9 @@ const AppSubURL = AppURL + Repo + "/" // these values should match the Repo const above var localMetas = map[string]string{ - "user": "gogits", - "repo": "gogs", + "user": "gogits", + "repo": "gogs", + "repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/", } func TestRender_StandardLinks(t *testing.T) { @@ -103,7 +104,7 @@ func testAnswers(baseURLContent, baseURLImages string) []string {
  • Tips
    • -

    See commit fc7f44dadf

    +

    See commit 65f1bf27bc

    Ideas and codes

    @@ -194,7 +195,7 @@ var sameCases = []string{ - [[Links, Language bindings, Engine bindings|Links]] - [[Tips]] -See commit fc7f44dadf +See commit 65f1bf27bc Ideas and codes From 28f561cac46ef7e51aa26aefcbe9aca4671366a6 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Tue, 6 Aug 2019 01:30:02 +0000 Subject: [PATCH 6/9] Return error in sha1CurrentPatternProcessor Co-Authored-By: mrsdizzie --- modules/markup/html.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/markup/html.go b/modules/markup/html.go index e6c4be64cd249..2fe0e4eb438cf 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -664,6 +664,7 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // Because of this, we check to make sure that a matched hash is actually // a commit in the repository before making it a link. if _, err := git.NewCommand("rev-parse", "--verify", hash).RunInDirBytes(ctx.metas["repoPath"]); err != nil { + log.Error("sha1CurrentPatternProcessor git rev-parse: %v", err) return } From 024057138d6c5d5c01746a83d02122fbd48d8908 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Tue, 6 Aug 2019 09:47:54 +0800 Subject: [PATCH 7/9] Import Gitea log module Signed-off-by: Gary Kim --- modules/markup/html.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/markup/html.go b/modules/markup/html.go index 2fe0e4eb438cf..b12c7c62d73bf 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" From d6b0392109b23fb4c1020d1479223b853732a9e4 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Tue, 6 Aug 2019 10:29:39 +0800 Subject: [PATCH 8/9] Revert "Return error in sha1CurrentPatternProcessor" This reverts commit 28f561cac46ef7e51aa26aefcbe9aca4671366a6. Signed-off-by: Gary Kim --- modules/markup/html.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index b12c7c62d73bf..e6c4be64cd249 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -665,7 +664,6 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // Because of this, we check to make sure that a matched hash is actually // a commit in the repository before making it a link. if _, err := git.NewCommand("rev-parse", "--verify", hash).RunInDirBytes(ctx.metas["repoPath"]); err != nil { - log.Error("sha1CurrentPatternProcessor git rev-parse: %v", err) return } From 950152ecfceb47baa56136693f3310ae95ab7494 Mon Sep 17 00:00:00 2001 From: Gary Kim Date: Wed, 7 Aug 2019 14:58:12 +0800 Subject: [PATCH 9/9] Add debug logging to sha1CurrentPatternProcessor This will log errors by the git command run in sha1CurrentPatternProcessor if the error is one that was unexpected. Signed-off-by: Gary Kim --- modules/markup/html.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/markup/html.go b/modules/markup/html.go index e6c4be64cd249..28533cdd3f54b 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -664,6 +665,9 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) { // Because of this, we check to make sure that a matched hash is actually // a commit in the repository before making it a link. if _, err := git.NewCommand("rev-parse", "--verify", hash).RunInDirBytes(ctx.metas["repoPath"]); err != nil { + if !strings.Contains(err.Error(), "fatal: Needed a single revision") { + log.Debug("sha1CurrentPatternProcessor git rev-parse: %v", err) + } return }