Check commit message hashes before making links (#7713)
* 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 <gary@garykim.dev> * 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 <gary@garykim.dev> * Make code cleaner Signed-off-by: Gary Kim <gary@garykim.dev> * Use rev-parse to check if commit exists Signed-off-by: Gary Kim <gary@garykim.dev> * Add and modify tests for checking hashes in html link rendering Signed-off-by: Gary Kim <gary@garykim.dev> * Return error in sha1CurrentPatternProcessor Co-Authored-By: mrsdizzie <info@mrsdizzie.com> * Import Gitea log module Signed-off-by: Gary Kim <gary@garykim.dev> * Revert "Return error in sha1CurrentPatternProcessor" This reverts commit 28f561cac46ef7e51aa26aefcbe9aca4671366a6. Signed-off-by: Gary Kim <gary@garykim.dev> * 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 <gary@garykim.dev>
This commit is contained in:
parent
46d6b92ad0
commit
7eed11e5e9
4 changed files with 34 additions and 16 deletions
|
@ -510,6 +510,7 @@ func (repo *Repository) ComposeMetas() map[string]string {
|
||||||
repo.ExternalMetas = map[string]string{
|
repo.ExternalMetas = map[string]string{
|
||||||
"user": repo.MustOwner().Name,
|
"user": repo.MustOwner().Name,
|
||||||
"repo": repo.Name,
|
"repo": repo.Name,
|
||||||
|
"repoPath": repo.RepoPath(),
|
||||||
}
|
}
|
||||||
unit, err := repo.GetUnit(UnitTypeExternalTracker)
|
unit, err := repo.GetUnit(UnitTypeExternalTracker)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
@ -13,6 +13,8 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"code.gitea.io/gitea/modules/base"
|
"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/setting"
|
||||||
"code.gitea.io/gitea/modules/util"
|
"code.gitea.io/gitea/modules/util"
|
||||||
|
|
||||||
|
@ -646,6 +648,9 @@ func fullSha1PatternProcessor(ctx *postProcessCtx, node *html.Node) {
|
||||||
// sha1CurrentPatternProcessor renders SHA1 strings to corresponding links that
|
// sha1CurrentPatternProcessor renders SHA1 strings to corresponding links that
|
||||||
// are assumed to be in the same repository.
|
// are assumed to be in the same repository.
|
||||||
func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) {
|
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)
|
m := sha1CurrentPattern.FindStringSubmatchIndex(node.Data)
|
||||||
if m == nil {
|
if m == nil {
|
||||||
return
|
return
|
||||||
|
@ -657,6 +662,15 @@ func sha1CurrentPatternProcessor(ctx *postProcessCtx, node *html.Node) {
|
||||||
// but that is not always the case.
|
// but that is not always the case.
|
||||||
// Although unlikely, deadbeef and 1234567 are valid short forms of SHA1 hash
|
// 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.
|
// 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 _, 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
|
||||||
|
}
|
||||||
|
|
||||||
replaceContent(node, m[2], m[3],
|
replaceContent(node, m[2], m[3],
|
||||||
createCodeLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "commit", hash), base.ShortSha(hash)))
|
createCodeLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "commit", hash), base.ShortSha(hash)))
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,6 +19,7 @@ import (
|
||||||
var localMetas = map[string]string{
|
var localMetas = map[string]string{
|
||||||
"user": "gogits",
|
"user": "gogits",
|
||||||
"repo": "gogs",
|
"repo": "gogs",
|
||||||
|
"repoPath": "../../integrations/gitea-repositories-meta/user13/repo11.git/",
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRender_Commits(t *testing.T) {
|
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))
|
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
|
||||||
}
|
}
|
||||||
|
|
||||||
var sha = "b6dd6210eaebc915fd5be5579c58cce4da2e2579"
|
var sha = "65f1bf27bc3bf70f64657658635e66094edbcb4d"
|
||||||
var commit = util.URLJoin(AppSubURL, "commit", sha)
|
var commit = util.URLJoin(AppSubURL, "commit", sha)
|
||||||
var subtree = util.URLJoin(commit, "src")
|
var subtree = util.URLJoin(commit, "src")
|
||||||
var tree = strings.Replace(subtree, "/commit/", "/tree/", -1)
|
var tree = strings.Replace(subtree, "/commit/", "/tree/", -1)
|
||||||
|
|
||||||
test(sha, `<p><a href="`+commit+`" rel="nofollow"><code>b6dd6210ea</code></a></p>`)
|
test(sha, `<p><a href="`+commit+`" rel="nofollow"><code>65f1bf27bc</code></a></p>`)
|
||||||
test(sha[:7], `<p><a href="`+commit[:len(commit)-(40-7)]+`" rel="nofollow"><code>b6dd621</code></a></p>`)
|
test(sha[:7], `<p><a href="`+commit[:len(commit)-(40-7)]+`" rel="nofollow"><code>65f1bf2</code></a></p>`)
|
||||||
test(sha[:39], `<p><a href="`+commit[:len(commit)-(40-39)]+`" rel="nofollow"><code>b6dd6210ea</code></a></p>`)
|
test(sha[:39], `<p><a href="`+commit[:len(commit)-(40-39)]+`" rel="nofollow"><code>65f1bf27bc</code></a></p>`)
|
||||||
test(commit, `<p><a href="`+commit+`" rel="nofollow"><code>b6dd6210ea</code></a></p>`)
|
test(commit, `<p><a href="`+commit+`" rel="nofollow"><code>65f1bf27bc</code></a></p>`)
|
||||||
test(tree, `<p><a href="`+tree+`" rel="nofollow"><code>b6dd6210ea/src</code></a></p>`)
|
test(tree, `<p><a href="`+tree+`" rel="nofollow"><code>65f1bf27bc/src</code></a></p>`)
|
||||||
test("commit "+sha, `<p>commit <a href="`+commit+`" rel="nofollow"><code>b6dd6210ea</code></a></p>`)
|
test("commit "+sha, `<p>commit <a href="`+commit+`" rel="nofollow"><code>65f1bf27bc</code></a></p>`)
|
||||||
test("/home/gitea/"+sha, "<p>/home/gitea/"+sha+"</p>")
|
test("/home/gitea/"+sha, "<p>/home/gitea/"+sha+"</p>")
|
||||||
|
test("deadbeef", `<p>deadbeef</p>`)
|
||||||
|
test("d27ace93", `<p>d27ace93</p>`)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRender_CrossReferences(t *testing.T) {
|
func TestRender_CrossReferences(t *testing.T) {
|
||||||
|
|
|
@ -23,6 +23,7 @@ const AppSubURL = AppURL + Repo + "/"
|
||||||
var localMetas = map[string]string{
|
var localMetas = map[string]string{
|
||||||
"user": "gogits",
|
"user": "gogits",
|
||||||
"repo": "gogs",
|
"repo": "gogs",
|
||||||
|
"repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/",
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRender_StandardLinks(t *testing.T) {
|
func TestRender_StandardLinks(t *testing.T) {
|
||||||
|
@ -103,7 +104,7 @@ func testAnswers(baseURLContent, baseURLImages string) []string {
|
||||||
<li><a href="` + baseURLContent + `/Tips" rel="nofollow">Tips</a></li>
|
<li><a href="` + baseURLContent + `/Tips" rel="nofollow">Tips</a></li>
|
||||||
</ul>
|
</ul>
|
||||||
|
|
||||||
<p>See commit <a href="http://localhost:3000/gogits/gogs/commit/fc7f44dadf" rel="nofollow"><code>fc7f44dadf</code></a></p>
|
<p>See commit <a href="http://localhost:3000/gogits/gogs/commit/65f1bf27bc" rel="nofollow"><code>65f1bf27bc</code></a></p>
|
||||||
|
|
||||||
<p>Ideas and codes</p>
|
<p>Ideas and codes</p>
|
||||||
|
|
||||||
|
@ -194,7 +195,7 @@ var sameCases = []string{
|
||||||
- [[Links, Language bindings, Engine bindings|Links]]
|
- [[Links, Language bindings, Engine bindings|Links]]
|
||||||
- [[Tips]]
|
- [[Tips]]
|
||||||
|
|
||||||
See commit fc7f44dadf
|
See commit 65f1bf27bc
|
||||||
|
|
||||||
Ideas and codes
|
Ideas and codes
|
||||||
|
|
||||||
|
|
Reference in a new issue