Fix issue link rendering in commit messages (#2897)
* Fix issue link rendering in commit messages * Update page.tmpl * No links for parens * remove comment
This commit is contained in:
		
					parent
					
						
							
								47f40ccd5e
							
						
					
				
			
			
				commit
				
					
						5481be0ac5
					
				
			
		
					 7 changed files with 126 additions and 52 deletions
				
			
		|  | @ -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) | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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> | ||||
|  |  | |||
|  | @ -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}} | ||||
|  |  | |||
|  | @ -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> | ||||
|  |  | |||
|  | @ -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> | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Ethan Koenig
				Ethan Koenig