From 55c84382f2416c2eb00265e55dfbaccb72907976 Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Tue, 11 Jul 2017 15:30:58 +0100 Subject: [PATCH 1/2] Match abbreviated SHA1 hash to link to This changes the regex to look for a hash from 7 to 40 characters, to match the use of abbreviated hash lookups in both git and github. The restriction of not being a pure number is also removed because 1234567 is now considered a valid abbreviated hash, as is deadbeef. A note has been added to the top of the code to state that the literal regex match is fine, but no extra validation is currently performed so some false positives are expected. A future change could ensure that the hash exists in the repository before rendering it as a link, although this might incur a slight performance penalty. Reverts part of commit 4a46613 and fixes #2053. --- modules/markdown/markdown.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/modules/markdown/markdown.go b/modules/markdown/markdown.go index 9773e8c2f8398..bebf454ff1911 100644 --- a/modules/markdown/markdown.go +++ b/modules/markdown/markdown.go @@ -43,6 +43,10 @@ func IsMarkdownFile(name string) bool { } var ( + // NOTE: All below regex matching do not perform any extra validation. + // Thus a link is produced even if the user does not exist, the issue does not exist, the commit does not exist, etc. + // While fast, this is also incorrect and lead to false positives. + // MentionPattern matches string that mentions someone, e.g. @Unknwon MentionPattern = regexp.MustCompile(`(\s|^|\W)@[0-9a-zA-Z-_\.]+`) @@ -55,9 +59,9 @@ var ( CrossReferenceIssueNumericPattern = regexp.MustCompile(`( |^)[0-9a-zA-Z]+/[0-9a-zA-Z]+#[0-9]+\b`) // Sha1CurrentPattern matches string that represents a commit SHA, e.g. d8a994ef243349f321568f9e36d5c3f444b99cae - // FIXME: this pattern matches pure numbers as well, right now we do a hack to check in renderSha1CurrentPattern - // by converting string to a number. - Sha1CurrentPattern = regexp.MustCompile(`(?:^|\s|\()([0-9a-f]{40})\b`) + // Although SHA1 hashes are 40 chars long, the regex matches the hash from 7 to 40 chars in length + // so that abbreviated hash links can be used as well. This matches git and github useability. + Sha1CurrentPattern = regexp.MustCompile(`(?:^|\s|\()([0-9a-f]{7,40})\b`) // ShortLinkPattern matches short but difficult to parse [[name|link|arg=test]] syntax ShortLinkPattern = regexp.MustCompile(`(\[\[.*\]\]\w*)`) @@ -525,9 +529,12 @@ func renderSha1CurrentPattern(rawBytes []byte, urlPrefix string) []byte { ms := Sha1CurrentPattern.FindAllSubmatch(rawBytes, -1) for _, m := range ms { hash := m[1] - if com.StrTo(hash).MustInt() > 0 { - continue - } + // The regex does not lie, it matches the hash pattern. + // However, a regex cannot know if a hash actually exists or not. + // We could assume that a SHA1 hash should probably contain alphas AND numerics + // 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. rawBytes = bytes.Replace(rawBytes, hash, []byte(fmt.Sprintf( `%s`, URLJoin(urlPrefix, "commit", string(hash)), base.ShortSha(string(hash)))), -1) } From 6a70c8355b705daa883692323b8d03a269094f1f Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Tue, 11 Jul 2017 21:38:47 +0100 Subject: [PATCH 2/2] Add tests for hashs of length 7 and 39 characters --- modules/markdown/markdown_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/markdown/markdown_test.go b/modules/markdown/markdown_test.go index c622009e82533..2943ffde95077 100644 --- a/modules/markdown/markdown_test.go +++ b/modules/markdown/markdown_test.go @@ -319,6 +319,8 @@ func TestRender_Commits(t *testing.T) { var src = strings.Replace(subtree, "/commit/", "/src/", -1) test(sha, `

b6dd6210ea

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

b6dd621

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

b6dd6210ea

`) test(commit, `

b6dd6210ea

`) test(tree, `

b6dd6210ea/src

`) test("commit "+sha, `

commit b6dd6210ea

`)