From 5481be0ac5e137a1127ca37975a6d319bf0ad940 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Sun, 12 Nov 2017 17:35:55 -0800 Subject: [PATCH] Fix issue link rendering in commit messages (#2897) * Fix issue link rendering in commit messages * Update page.tmpl * No links for parens * remove comment --- modules/markup/html.go | 84 ++++++++++++++++++++++++------- modules/markup/html_test.go | 40 +++++++++++---- modules/templates/helper.go | 42 +++++++++------- templates/repo/commits_table.tmpl | 2 +- templates/repo/diff/page.tmpl | 2 +- templates/repo/graph.tmpl | 2 +- templates/repo/view_list.tmpl | 6 +-- 7 files changed, 126 insertions(+), 52 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(``) + buf.Write(text) + buf.WriteString(``) +} + // 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(`%s`, 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(``) + buf.Write(remainder[startIndex:endIndex]) + buf.WriteString(``) } else { // Support for external issue tracker - if metas["style"] == IssueNameStyleAlphanumeric { - metas["index"] = string(m) + buf.WriteString(`%s`, com.Expand(metas["format"], metas), m) + buf.WriteString(com.Expand(opts.Metas["format"], opts.Metas)) + buf.WriteString(`">`) + buf.Write(remainder[startIndex:endIndex]) + buf.WriteString(``) } - 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("%s", 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(`hello `, AppURL)+ + fmt.Sprintf(`#123 `, AppSubURL)+ + fmt.Sprintf(`world`, AppURL)) + test("hello (#123) world", + fmt.Sprintf(`hello `, AppURL)+ + fmt.Sprintf(`(#123)`, AppSubURL)+ + fmt.Sprintf(` world`, 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 34881b788a..454bd648a3 100644 --- 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("

%s

", msgLines[0]) - if numLines >= 2 { - fullMessage = header + fmt.Sprintf("\n
%s
", strings.Join(msgLines[2:], "\n")) - } else { - fullMessage = header - } - } else { - // Non-standard git message, there is no header line - fullMessage = fmt.Sprintf("

%s

", strings.Join(msgLines, "
")) } - 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 2ca861c623..5550ad7493 100644 --- a/templates/repo/commits_table.tmpl +++ b/templates/repo/commits_table.tmpl @@ -60,7 +60,7 @@ - {{RenderCommitMessage false .Summary $.RepoLink $.Repository.ComposeMetas}} + {{RenderCommitMessage .Summary $.RepoLink $.Repository.ComposeMetas}} {{template "repo/commit_status" .Status}} {{TimeSince .Author.When $.Lang}} diff --git a/templates/repo/diff/page.tmpl b/templates/repo/diff/page.tmpl index 2736daa7f9..cb67b36f73 100644 --- a/templates/repo/diff/page.tmpl +++ b/templates/repo/diff/page.tmpl @@ -9,7 +9,7 @@ {{.i18n.Tr "repo.diff.browse_source"}} -

{{RenderCommitMessage false .Commit.Message $.RepoLink $.Repository.ComposeMetas}}{{template "repo/commit_status" .CommitStatus}}

+

{{RenderCommitMessage .Commit.Message $.RepoLink $.Repository.ComposeMetas}}{{template "repo/commit_status" .CommitStatus}}

{{if .Author}} diff --git a/templates/repo/graph.tmpl b/templates/repo/graph.tmpl index 02ce3df280..e8da069a9d 100644 --- a/templates/repo/graph.tmpl +++ b/templates/repo/graph.tmpl @@ -26,7 +26,7 @@ {{ .ShortRev}} {{.Branch}} - {{RenderCommitMessage false .Subject $.RepoLink $.Repository.ComposeMetas}} by + {{RenderCommitMessage .Subject $.RepoLink $.Repository.ComposeMetas}} by {{.Author}} diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl index 7be2183fa8..a2bb72c227 100644 --- a/templates/repo/view_list.tmpl +++ b/templates/repo/view_list.tmpl @@ -27,7 +27,7 @@
{{end}} - {{RenderCommitMessage false .LatestCommit.Summary .RepoLink $.Repository.ComposeMetas}} + {{RenderCommitMessage .LatestCommit.Summary .RepoLink $.Repository.ComposeMetas}} {{template "repo/commit_status" .LatestCommitStatus}} @@ -75,9 +75,7 @@ {{end}} - - {{RenderCommitMessage false $commit.Summary $.RepoLink $.Repository.ComposeMetas}} - + {{RenderCommitMessageLink $commit.Summary $.RepoLink (print $.RepoLink "/commit/" $commit.ID) $.Repository.ComposeMetas}} {{TimeSince $commit.Committer.When $.Lang}}