Fix issue link rendering in commit messages (#2897)

* Fix issue link rendering in commit messages

* Update page.tmpl

* No links for parens

* remove comment

diff --git a/modules/markup/html.go b/modules/markup/html.go
index 9daf0b0c6c2bf1da4924668ce4c63c81700a82dd..532533976257c1a353af826d5351b6483b8911ea 100644 (file)
--- 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 ab2ca5ef47ec7566636432942d42c7cd16683264..e3597a4c3a1b03608e0e0afe8d085675af9ab77d 100644 (file)
--- 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

diff --git a/modules/templates/helper.go b/modules/templates/helper.go
index 34881b788a28c9fd0823080ab8e9c0495073c196..454bd648a3cbbf8a931256abdce343980a5f849a 100644 (file)
--- a/modules/templates/helper.go
+++ b/modules/templates/helper.go
@@ -110,7 +110,8 @@ func NewFuncMap() []template.FuncMap {
                "EscapePound": func(str string) string {
                        return strings.NewReplacer("%", "%25", "#", "%23", " ", "%20", "?", "%3F").Replace(str)
                },
-               "RenderCommitMessage": RenderCommitMessage,
+               "RenderCommitMessage":     RenderCommitMessage,
+               "RenderCommitMessageLink": RenderCommitMessageLink,
                "ThemeColorMetaTag": func() string {
                        return setting.UI.ThemeColorMetaTag
                },
@@ -252,28 +253,31 @@ func ReplaceLeft(s, old, new string) string {
 }
 
 // RenderCommitMessage renders commit message with XSS-safe and special links.
-func RenderCommitMessage(full bool, msg, urlPrefix string, metas map[string]string) template.HTML {
+func RenderCommitMessage(msg, urlPrefix string, metas map[string]string) template.HTML {
+       return renderCommitMessage(msg, markup.RenderIssueIndexPatternOptions{
+               URLPrefix: urlPrefix,
+               Metas:     metas,
+       })
+}
+
+// RenderCommitMessageLink renders commit message as a XXS-safe link to the provided
+// default url, handling for special links.
+func RenderCommitMessageLink(msg, urlPrefix string, urlDefault string, metas map[string]string) template.HTML {
+       return renderCommitMessage(msg, markup.RenderIssueIndexPatternOptions{
+               DefaultURL: urlDefault,
+               URLPrefix:  urlPrefix,
+               Metas:      metas,
+       })
+}
+
+func renderCommitMessage(msg string, opts markup.RenderIssueIndexPatternOptions) template.HTML {
        cleanMsg := template.HTMLEscapeString(msg)
-       fullMessage := string(markup.RenderIssueIndexPattern([]byte(cleanMsg), urlPrefix, metas))
+       fullMessage := string(markup.RenderIssueIndexPattern([]byte(cleanMsg), opts))
        msgLines := strings.Split(strings.TrimSpace(fullMessage), "\n")
-       numLines := len(msgLines)
-       if numLines == 0 {
+       if len(msgLines) == 0 {
                return template.HTML("")
-       } else if !full {
-               return template.HTML(msgLines[0])
-       } else if numLines == 1 || (numLines >= 2 && len(msgLines[1]) == 0) {
-               // First line is a header, standalone or followed by empty line
-               header := fmt.Sprintf("<h3>%s</h3>", msgLines[0])
-               if numLines >= 2 {
-                       fullMessage = header + fmt.Sprintf("\n<pre>%s</pre>", strings.Join(msgLines[2:], "\n"))
-               } else {
-                       fullMessage = header
-               }
-       } else {
-               // Non-standard git message, there is no header line
-               fullMessage = fmt.Sprintf("<h4>%s</h4>", strings.Join(msgLines, "<br>"))
        }
-       return template.HTML(fullMessage)
+       return template.HTML(msgLines[0])
 }
 
 // Actioner describes an action

diff --git a/templates/repo/commits_table.tmpl b/templates/repo/commits_table.tmpl
index 2ca861c623ebf98879ec4b65694fe8ef862e6776..5550ad7493c2c104881e2fa49503762923eb4e03 100644 (file)
--- a/templates/repo/commits_table.tmpl
+++ b/templates/repo/commits_table.tmpl
@@ -60,7 +60,7 @@
                                                        </a>
                                                </td>
                                                <td class="message collapsing">
-                                                       <span class="has-emoji{{if gt .ParentCount 1}} grey text{{end}}">{{RenderCommitMessage false .Summary $.RepoLink $.Repository.ComposeMetas}}</span>
+                                                       <span class="has-emoji{{if gt .ParentCount 1}} grey text{{end}}">{{RenderCommitMessage .Summary $.RepoLink $.Repository.ComposeMetas}}</span>
                                                        {{template "repo/commit_status" .Status}}
                                                </td>
                                                <td class="grey text right aligned">{{TimeSince .Author.When $.Lang}}</td>

diff --git a/templates/repo/diff/page.tmpl b/templates/repo/diff/page.tmpl
index 2736daa7f909f5cafea90ebcea645c3f821b76ce..cb67b36f73f5baba10f8c710674808c4249b20c7 100644 (file)
--- a/templates/repo/diff/page.tmpl
+++ b/templates/repo/diff/page.tmpl
@@ -9,7 +9,7 @@
                                <a class="ui floated right blue tiny button" href="{{EscapePound .SourcePath}}">
                                        {{.i18n.Tr "repo.diff.browse_source"}}
                                </a>
-                               <h3>{{RenderCommitMessage false .Commit.Message $.RepoLink $.Repository.ComposeMetas}}{{template "repo/commit_status" .CommitStatus}}</h3>
+                               <h3>{{RenderCommitMessage .Commit.Message $.RepoLink $.Repository.ComposeMetas}}{{template "repo/commit_status" .CommitStatus}}</h3>
                        </div>
                        <div class="ui attached info segment {{if .Commit.Signature}} isSigned {{if .Verification.Verified }} isVerified {{end}}{{end}}">
                                {{if .Author}}

diff --git a/templates/repo/graph.tmpl b/templates/repo/graph.tmpl
index 02ce3df280c82572e6563fcaff134180b19e281c..e8da069a9d7ec2afe879b20fd2ccb0a582d23230 100644 (file)
--- a/templates/repo/graph.tmpl
+++ b/templates/repo/graph.tmpl
@@ -26,7 +26,7 @@
                    <a href="{{AppSubUrl}}/{{$.Username}}/{{$.Reponame}}/commit/{{.Rev}}">{{ .ShortRev}}</a>
                  </code>
                  <strong> {{.Branch}}</strong>
-                 <em>{{RenderCommitMessage false .Subject $.RepoLink $.Repository.ComposeMetas}}</em> by
+                 <em>{{RenderCommitMessage .Subject $.RepoLink $.Repository.ComposeMetas}}</em> by
                  <span class="author">
                    {{.Author}}
                  </span>

diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl
index 7be2183fa80db8f80548394050fad65cf187268b..a2bb72c2276eed9c7c9f4d0d5576c02836aff367 100644 (file)
--- a/templates/repo/view_list.tmpl
+++ b/templates/repo/view_list.tmpl
@@ -27,7 +27,7 @@
                                                        </div>
                                                {{end}}
                                </a>
-                               <span class="grey has-emoji">{{RenderCommitMessage false .LatestCommit.Summary .RepoLink $.Repository.ComposeMetas}}
+                               <span class="grey has-emoji">{{RenderCommitMessage .LatestCommit.Summary .RepoLink $.Repository.ComposeMetas}}
                                {{template "repo/commit_status" .LatestCommitStatus}}</span>
                        </th>
                        <th class="nine wide">
@@ -75,9 +75,7 @@
                                        </td>
                                {{end}}
                                <td class="message collapsing has-emoji">
-                                       <a rel="nofollow" href="{{$.RepoLink}}/commit/{{$commit.ID}}">
-                                               {{RenderCommitMessage false $commit.Summary $.RepoLink $.Repository.ComposeMetas}}
-                                       </a>
+                                       {{RenderCommitMessageLink $commit.Summary $.RepoLink (print $.RepoLink "/commit/" $commit.ID) $.Repository.ComposeMetas}}
                                </td>
                                <td class="text grey right age">{{TimeSince $commit.Committer.When $.Lang}}</td>
                        </tr>