Fix issue link rendering in commit messages (#2897)

* Fix issue link rendering in commit messages

* Update page.tmpl

* No links for parens

* remove comment

2 files changed, 98 insertions, 26 deletions

diff --git a/modules/markup/html.go b/modules/markup/html.go
index 9daf0b0c6c..5325339762 100644
--- a/modules/markup/html.go
+++ b/modules/markup/html.go
@@ -126,35 +126,82 @@ func URLJoin(base string, elems ...string) string {
 	return u.String()
 }
 
+// RenderIssueIndexPatternOptions options for RenderIssueIndexPattern function
+type RenderIssueIndexPatternOptions struct {
+	// url to which non-special formatting should be linked. If empty,
+	// no such links will be added
+	DefaultURL string
+	URLPrefix  string
+	Metas      map[string]string
+}
+
+// addText add text to the given buffer, adding a link to the default url
+// if appropriate
+func (opts RenderIssueIndexPatternOptions) addText(text []byte, buf *bytes.Buffer) {
+	if len(text) == 0 {
+		return
+	} else if len(opts.DefaultURL) == 0 {
+		buf.Write(text)
+		return
+	}
+	buf.WriteString(`<a rel="nofollow" href="`)
+	buf.WriteString(opts.DefaultURL)
+	buf.WriteString(`">`)
+	buf.Write(text)
+	buf.WriteString(`</a>`)
+}
+
 // RenderIssueIndexPattern renders issue indexes to corresponding links.
-func RenderIssueIndexPattern(rawBytes []byte, urlPrefix string, metas map[string]string) []byte {
-	urlPrefix = cutoutVerbosePrefix(urlPrefix)
+func RenderIssueIndexPattern(rawBytes []byte, opts RenderIssueIndexPatternOptions) []byte {
+	opts.URLPrefix = cutoutVerbosePrefix(opts.URLPrefix)
 
 	pattern := IssueNumericPattern
-	if metas["style"] == IssueNameStyleAlphanumeric {
+	if opts.Metas["style"] == IssueNameStyleAlphanumeric {
 		pattern = IssueAlphanumericPattern
 	}
 
-	ms := pattern.FindAll(rawBytes, -1)
-	for _, m := range ms {
-		if m[0] == ' ' || m[0] == '(' {
-			m = m[1:] // ignore leading space or opening parentheses
+	var buf bytes.Buffer
+	remainder := rawBytes
+	for {
+		indices := pattern.FindIndex(remainder)
+		if indices == nil || len(indices) < 2 {
+			opts.addText(remainder, &buf)
+			return buf.Bytes()
 		}
-		var link string
-		if metas == nil {
-			link = fmt.Sprintf(`<a href="%s">%s</a>`, URLJoin(urlPrefix, "issues", string(m[1:])), m)
+		startIndex := indices[0]
+		endIndex := indices[1]
+		opts.addText(remainder[:startIndex], &buf)
+		if remainder[startIndex] == '(' || remainder[startIndex] == ' ' {
+			buf.WriteByte(remainder[startIndex])
+			startIndex++
+		}
+		if opts.Metas == nil {
+			buf.WriteString(`<a href="`)
+			buf.WriteString(URLJoin(
+				opts.URLPrefix, "issues", string(remainder[startIndex+1:endIndex])))
+			buf.WriteString(`">`)
+			buf.Write(remainder[startIndex:endIndex])
+			buf.WriteString(`</a>`)
 		} else {
 			// Support for external issue tracker
-			if metas["style"] == IssueNameStyleAlphanumeric {
-				metas["index"] = string(m)
+			buf.WriteString(`<a href="`)
+			if opts.Metas["style"] == IssueNameStyleAlphanumeric {
+				opts.Metas["index"] = string(remainder[startIndex:endIndex])
 			} else {
-				metas["index"] = string(m[1:])
+				opts.Metas["index"] = string(remainder[startIndex+1 : endIndex])
 			}
-			link = fmt.Sprintf(`<a href="%s">%s</a>`, com.Expand(metas["format"], metas), m)
+			buf.WriteString(com.Expand(opts.Metas["format"], opts.Metas))
+			buf.WriteString(`">`)
+			buf.Write(remainder[startIndex:endIndex])
+			buf.WriteString(`</a>`)
 		}
-		rawBytes = bytes.Replace(rawBytes, m, []byte(link), 1)
+		if endIndex < len(remainder) &&
+			(remainder[endIndex] == ')' || remainder[endIndex] == ' ') {
+			buf.WriteByte(remainder[endIndex])
+			endIndex++
+		}
+		remainder = remainder[endIndex:]
 	}
-	return rawBytes
 }
 
 // IsSameDomain checks if given url string has the same hostname as current Gitea instance
@@ -432,7 +479,10 @@ func RenderSpecialLink(rawBytes []byte, urlPrefix string, metas map[string]strin
 
 	rawBytes = RenderFullIssuePattern(rawBytes)
 	rawBytes = RenderShortLinks(rawBytes, urlPrefix, false, isWikiMarkdown)
-	rawBytes = RenderIssueIndexPattern(rawBytes, urlPrefix, metas)
+	rawBytes = RenderIssueIndexPattern(rawBytes, RenderIssueIndexPatternOptions{
+		URLPrefix: urlPrefix,
+		Metas:     metas,
+	})
 	rawBytes = RenderCrossReferenceIssueIndexPattern(rawBytes, urlPrefix, metas)
 	rawBytes = renderFullSha1Pattern(rawBytes, urlPrefix)
 	rawBytes = renderSha1CurrentPattern(rawBytes, urlPrefix)
diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go
index ab2ca5ef47..e3597a4c3a 100644
--- a/modules/markup/html_test.go
+++ b/modules/markup/html_test.go
@@ -55,9 +55,12 @@ func link(href, contents string) string {
 	return fmt.Sprintf("<a href=\"%s\">%s</a>", href, contents)
 }
 
-func testRenderIssueIndexPattern(t *testing.T, input, expected string, metas map[string]string) {
-	assert.Equal(t, expected,
-		string(RenderIssueIndexPattern([]byte(input), AppSubURL, metas)))
+func testRenderIssueIndexPattern(t *testing.T, input, expected string, opts RenderIssueIndexPatternOptions) {
+	if len(opts.URLPrefix) == 0 {
+		opts.URLPrefix = AppSubURL
+	}
+	actual := string(RenderIssueIndexPattern([]byte(input), opts))
+	assert.Equal(t, expected, actual)
 }
 
 func TestURLJoin(t *testing.T) {
@@ -88,8 +91,8 @@ func TestURLJoin(t *testing.T) {
 func TestRender_IssueIndexPattern(t *testing.T) {
 	// numeric: render inputs without valid mentions
 	test := func(s string) {
-		testRenderIssueIndexPattern(t, s, s, nil)
-		testRenderIssueIndexPattern(t, s, s, numericMetas)
+		testRenderIssueIndexPattern(t, s, s, RenderIssueIndexPatternOptions{})
+		testRenderIssueIndexPattern(t, s, s, RenderIssueIndexPatternOptions{Metas: numericMetas})
 	}
 
 	// should not render anything when there are no mentions
@@ -123,13 +126,13 @@ func TestRender_IssueIndexPattern2(t *testing.T) {
 			links[i] = numericIssueLink(URLJoin(setting.AppSubURL, "issues"), index)
 		}
 		expectedNil := fmt.Sprintf(expectedFmt, links...)
-		testRenderIssueIndexPattern(t, s, expectedNil, nil)
+		testRenderIssueIndexPattern(t, s, expectedNil, RenderIssueIndexPatternOptions{})
 
 		for i, index := range indices {
 			links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", index)
 		}
 		expectedNum := fmt.Sprintf(expectedFmt, links...)
-		testRenderIssueIndexPattern(t, s, expectedNum, numericMetas)
+		testRenderIssueIndexPattern(t, s, expectedNum, RenderIssueIndexPatternOptions{Metas: numericMetas})
 	}
 
 	// should render freestanding mentions
@@ -155,7 +158,7 @@ func TestRender_IssueIndexPattern3(t *testing.T) {
 
 	// alphanumeric: render inputs without valid mentions
 	test := func(s string) {
-		testRenderIssueIndexPattern(t, s, s, alphanumericMetas)
+		testRenderIssueIndexPattern(t, s, s, RenderIssueIndexPatternOptions{Metas: alphanumericMetas})
 	}
 	test("")
 	test("this is a test")
@@ -187,13 +190,32 @@ func TestRender_IssueIndexPattern4(t *testing.T) {
 			links[i] = alphanumIssueLink("https://someurl.com/someUser/someRepo/", name)
 		}
 		expected := fmt.Sprintf(expectedFmt, links...)
-		testRenderIssueIndexPattern(t, s, expected, alphanumericMetas)
+		testRenderIssueIndexPattern(t, s, expected, RenderIssueIndexPatternOptions{Metas: alphanumericMetas})
 	}
 	test("OTT-1234 test", "%s test", "OTT-1234")
 	test("test T-12 issue", "test %s issue", "T-12")
 	test("test issue ABCDEFGHIJ-1234567890", "test issue %s", "ABCDEFGHIJ-1234567890")
 }
 
+func TestRenderIssueIndexPatternWithDefaultURL(t *testing.T) {
+	setting.AppURL = AppURL
+	setting.AppSubURL = AppSubURL
+
+	test := func(input string, expected string) {
+		testRenderIssueIndexPattern(t, input, expected, RenderIssueIndexPatternOptions{
+			DefaultURL: AppURL,
+		})
+	}
+	test("hello #123 world",
+		fmt.Sprintf(`<a rel="nofollow" href="%s">hello</a> `, AppURL)+
+			fmt.Sprintf(`<a href="%sissues/123">#123</a> `, AppSubURL)+
+			fmt.Sprintf(`<a rel="nofollow" href="%s">world</a>`, AppURL))
+	test("hello (#123) world",
+		fmt.Sprintf(`<a rel="nofollow" href="%s">hello </a>`, AppURL)+
+			fmt.Sprintf(`(<a href="%sissues/123">#123</a>)`, AppSubURL)+
+			fmt.Sprintf(`<a rel="nofollow" href="%s"> world</a>`, AppURL))
+}
+
 func TestRender_AutoLink(t *testing.T) {
 	setting.AppURL = AppURL
 	setting.AppSubURL = AppSubURL