Fix SHA1 hash linking (#2143)

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.

2 files changed, 15 insertions, 6 deletions

diff --git a/modules/markdown/markdown.go b/modules/markdown/markdown.go
index 9773e8c2f8..bebf454ff1 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(
 			`<a href="%s">%s</a>`, URLJoin(urlPrefix, "commit", string(hash)), base.ShortSha(string(hash)))), -1)
 	}
diff --git a/modules/markdown/markdown_test.go b/modules/markdown/markdown_test.go
index c622009e82..2943ffde95 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, `<p><a href="`+commit+`" rel="nofollow">b6dd6210ea</a></p>`)
+	test(sha[:7], `<p><a href="`+commit[:len(commit)-(40-7)]+`" rel="nofollow">b6dd621</a></p>`)
+	test(sha[:39], `<p><a href="`+commit[:len(commit)-(40-39)]+`" rel="nofollow">b6dd6210ea</a></p>`)
 	test(commit, `<p><a href="`+commit+`" rel="nofollow">b6dd6210ea</a></p>`)
 	test(tree, `<p><a href="`+src+`" rel="nofollow">b6dd6210ea/src</a></p>`)
 	test("commit "+sha, `<p>commit <a href="`+commit+`" rel="nofollow">b6dd6210ea</a></p>`)