Skip to content

Commit

Permalink
Fix SHA1 hash linking (go-gitea#2143) (go-gitea#2293)
Browse files Browse the repository at this point in the history
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 go-gitea#2053.
  • Loading branch information
lunny committed Aug 12, 2017
1 parent 5fe8fee commit 1709297
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
25 changes: 16 additions & 9 deletions modules/markdown/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func IsReadmeFile(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-_\.]+`)

Expand All @@ -65,9 +69,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*)`)
Expand Down Expand Up @@ -553,12 +557,15 @@ func RenderCrossReferenceIssueIndexPattern(rawBytes []byte, urlPrefix string, me
func renderSha1CurrentPattern(rawBytes []byte, urlPrefix string) []byte {
ms := Sha1CurrentPattern.FindAllSubmatch(rawBytes, -1)
for _, m := range ms {
all := m[0]
if com.StrTo(all).MustInt() > 0 {
continue
}
rawBytes = bytes.Replace(rawBytes, all, []byte(fmt.Sprintf(
`<a href="%s">%s</a>`, URLJoin(urlPrefix, "commit", string(all)), base.ShortSha(string(all)))), -1)
hash := m[1]
// 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)
}
return rawBytes
}
Expand Down
2 changes: 2 additions & 0 deletions modules/markdown/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,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>`)
}
Expand Down

0 comments on commit 1709297

Please sign in to comment.