Skip to content

Commit e770c2b

Browse files
zeripathzeripath
authored
Detect full references to issues and pulls in commit messages (#12399)
 Fix #10269 Signed-off-by: Andrew Thornton <[email protected]>
1 parent e17e3f7 commit e770c2b

File tree

3 files changed

+200
-8
lines changed

3 files changed

+200
-8
lines changed

‎modules/markup/mdstripper/mdstripper.go

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ package mdstripper
66

77
import (
88
"bytes"
9+
"net/url"
10+
"strings"
911
"sync"
1012

1113
"io"
1214

1315
"code.gitea.io/gitea/modules/log"
1416
"code.gitea.io/gitea/modules/markup/common"
17+
"code.gitea.io/gitea/modules/setting"
1518

1619
"github.com/yuin/goldmark"
1720
"github.com/yuin/goldmark/ast"
@@ -22,9 +25,15 @@ import (
2225
"github.com/yuin/goldmark/text"
2326
)
2427

28+
var (
29+
giteaHostInit sync.Once
30+
giteaHost *url.URL
31+
)
32+
2533
type stripRenderer struct {
26-
links []string
27-
empty bool
34+
localhost *url.URL
35+
links []string
36+
empty bool
2837
}
2938

3039
func (r *stripRenderer) Render(w io.Writer, source []byte, doc ast.Node) error {
@@ -50,7 +59,8 @@ func (r *stripRenderer) Render(w io.Writer, source []byte, doc ast.Node) error {
5059
r.processLink(w, v.Destination)
5160
return ast.WalkSkipChildren, nil
5261
case *ast.AutoLink:
53-
r.processLink(w, v.URL(source))
62+
// This could be a reference to an issue or pull - if so convert it
63+
r.processAutoLink(w, v.URL(source))
5464
return ast.WalkSkipChildren, nil
5565
}
5666
return ast.WalkContinue, nil
@@ -72,6 +82,50 @@ func (r *stripRenderer) processString(w io.Writer, text []byte, coalesce bool) {
7282
r.empty = false
7383
}
7484

85+
// ProcessAutoLinks to detect and handle links to issues and pulls
86+
func (r *stripRenderer) processAutoLink(w io.Writer, link []byte) {
87+
linkStr := string(link)
88+
u, err := url.Parse(linkStr)
89+
if err != nil {
90+
// Process out of band
91+
r.links = append(r.links, linkStr)
92+
return
93+
}
94+
95+
// Note: we're not attempting to match the URL scheme (http/https)
96+
host := strings.ToLower(u.Host)
97+
if host != "" && host != strings.ToLower(r.localhost.Host) {
98+
// Process out of band
99+
r.links = append(r.links, linkStr)
100+
return
101+
}
102+
103+
// We want: /user/repo/issues/3
104+
parts := strings.Split(strings.TrimPrefix(u.EscapedPath(), r.localhost.EscapedPath()), "/")
105+
if len(parts) != 5 || parts[0] != "" {
106+
// Process out of band
107+
r.links = append(r.links, linkStr)
108+
return
109+
}
110+
111+
var sep string
112+
if parts[3] == "issues" {
113+
sep = "#"
114+
} else if parts[3] == "pulls" {
115+
sep = "!"
116+
} else {
117+
// Process out of band
118+
r.links = append(r.links, linkStr)
119+
return
120+
}
121+
122+
_, _ = w.Write([]byte(parts[1]))
123+
_, _ = w.Write([]byte("/"))
124+
_, _ = w.Write([]byte(parts[2]))
125+
_, _ = w.Write([]byte(sep))
126+
_, _ = w.Write([]byte(parts[4]))
127+
}
128+
75129
func (r *stripRenderer) processLink(w io.Writer, link []byte) {
76130
// Links are processed out of band
77131
r.links = append(r.links, string(link))
@@ -120,8 +174,9 @@ func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) {
120174
stripParser = gdMarkdown.Parser()
121175
})
122176
stripper := &stripRenderer{
123-
links: make([]string, 0, 10),
124-
empty: true,
177+
localhost: getGiteaHost(),
178+
links: make([]string, 0, 10),
179+
empty: true,
125180
}
126181
reader := text.NewReader(rawBytes)
127182
doc := stripParser.Parse(reader)
@@ -131,3 +186,14 @@ func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) {
131186
}
132187
return buf.Bytes(), stripper.GetLinks()
133188
}
189+
190+
// getGiteaHostName returns a normalized string with the local host name, with no scheme or port information
191+
func getGiteaHost() *url.URL {
192+
giteaHostInit.Do(func() {
193+
var err error
194+
if giteaHost, err = url.Parse(setting.AppURL); err != nil {
195+
giteaHost = &url.URL{}
196+
}
197+
})
198+
return giteaHost
199+
}

‎modules/references/references.go

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ var (
4141
issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp
4242
issueKeywordsOnce sync.Once
4343

44-
giteaHostInit sync.Once
45-
giteaHost string
44+
giteaHostInit sync.Once
45+
giteaHost string
46+
giteaIssuePullPattern *regexp.Regexp
4647
)
4748

4849
// XRefAction represents the kind of effect a cross reference has once is resolved
@@ -152,13 +153,25 @@ func getGiteaHostName() string {
152153
giteaHostInit.Do(func() {
153154
if uapp, err := url.Parse(setting.AppURL); err == nil {
154155
giteaHost = strings.ToLower(uapp.Host)
156+
giteaIssuePullPattern = regexp.MustCompile(
157+
`(\s|^|\(|\[)` +
158+
regexp.QuoteMeta(strings.TrimSpace(setting.AppURL)) +
159+
`([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+)/` +
160+
`((?:issues)|(?:pulls))/([0-9]+)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)`)
155161
} else {
156162
giteaHost = ""
163+
giteaIssuePullPattern = nil
157164
}
158165
})
159166
return giteaHost
160167
}
161168

169+
// getGiteaIssuePullPattern
170+
func getGiteaIssuePullPattern() *regexp.Regexp {
171+
getGiteaHostName()
172+
return giteaIssuePullPattern
173+
}
174+
162175
// FindAllMentionsMarkdown matches mention patterns in given content and
163176
// returns a list of found unvalidated user names **not including** the @ prefix.
164177
func FindAllMentionsMarkdown(content string) []string {
@@ -219,7 +232,42 @@ func findAllIssueReferencesMarkdown(content string) []*rawReference {
219232

220233
// FindAllIssueReferences returns a list of unvalidated references found in a string.
221234
func FindAllIssueReferences(content string) []IssueReference {
222-
return rawToIssueReferenceList(findAllIssueReferencesBytes([]byte(content), []string{}))
235+
// Need to convert fully qualified html references to local system to #/! short codes
236+
contentBytes := []byte(content)
237+
if re := getGiteaIssuePullPattern(); re != nil {
238+
pos := 0
239+
for {
240+
match := re.FindSubmatchIndex(contentBytes[pos:])
241+
if match == nil {
242+
break
243+
}
244+
// match[0]-match[1] is whole string
245+
// match[2]-match[3] is preamble
246+
pos += match[3]
247+
// match[4]-match[5] is owner/repo
248+
endPos := pos + match[5] - match[4]
249+
copy(contentBytes[pos:endPos], contentBytes[match[4]:match[5]])
250+
pos = endPos
251+
// match[6]-match[7] == 'issues'
252+
contentBytes[pos] = '#'
253+
if string(contentBytes[match[6]:match[7]]) == "pulls" {
254+
contentBytes[pos] = '!'
255+
}
256+
pos++
257+
// match[8]-match[9] is the number
258+
endPos = pos + match[9] - match[8]
259+
copy(contentBytes[pos:endPos], contentBytes[match[8]:match[9]])
260+
copy(contentBytes[endPos:], contentBytes[match[9]:])
261+
// now we reset the length
262+
// our new section has length endPos - match[3]
263+
// our old section has length match[9] - match[3]
264+
contentBytes = contentBytes[:len(contentBytes)-match[9]+endPos]
265+
pos = endPos
266+
}
267+
} else {
268+
log.Debug("No GiteaIssuePullPattern pattern")
269+
}
270+
return rawToIssueReferenceList(findAllIssueReferencesBytes(contentBytes, []string{}))
223271
}
224272

225273
// FindRenderizableReferenceNumeric returns the first unvalidated reference found in a string.

‎modules/repofiles/action_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"code.gitea.io/gitea/models"
1111
"code.gitea.io/gitea/modules/git"
1212
"code.gitea.io/gitea/modules/repository"
13+
"code.gitea.io/gitea/modules/setting"
1314

1415
"github.com/stretchr/testify/assert"
1516
)
@@ -208,6 +209,32 @@ func TestUpdateIssuesCommit(t *testing.T) {
208209
models.AssertExistsAndLoadBean(t, commentBean)
209210
models.AssertNotExistsBean(t, issueBean, "is_closed=1")
210211
models.CheckConsistencyFor(t, &models.Action{})
212+
213+
pushCommits = []*repository.PushCommit{
214+
{
215+
Sha1: "abcdef3",
216+
CommitterEmail: "[email protected]",
217+
CommitterName: "User Two",
218+
AuthorEmail: "[email protected]",
219+
AuthorName: "User Two",
220+
Message: "close " + setting.AppURL + repo.FullName() + "/pulls/1",
221+
},
222+
}
223+
repo = models.AssertExistsAndLoadBean(t, &models.Repository{ID: 3}).(*models.Repository)
224+
commentBean = &models.Comment{
225+
Type: models.CommentTypeCommitRef,
226+
CommitSHA: "abcdef3",
227+
PosterID: user.ID,
228+
IssueID: 6,
229+
}
230+
issueBean = &models.Issue{RepoID: repo.ID, Index: 1}
231+
232+
models.AssertNotExistsBean(t, commentBean)
233+
models.AssertNotExistsBean(t, &models.Issue{RepoID: repo.ID, Index: 1}, "is_closed=1")
234+
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
235+
models.AssertExistsAndLoadBean(t, commentBean)
236+
models.AssertExistsAndLoadBean(t, issueBean, "is_closed=1")
237+
models.CheckConsistencyFor(t, &models.Action{})
211238
}
212239

213240
func TestUpdateIssuesCommit_Colon(t *testing.T) {
@@ -304,6 +331,41 @@ func TestUpdateIssuesCommit_AnotherRepo(t *testing.T) {
304331
models.CheckConsistencyFor(t, &models.Action{})
305332
}
306333

334+
func TestUpdateIssuesCommit_AnotherRepo_FullAddress(t *testing.T) {
335+
assert.NoError(t, models.PrepareTestDatabase())
336+
user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
337+
338+
// Test that a push to default branch closes issue in another repo
339+
// If the user also has push permissions to that repo
340+
pushCommits := []*repository.PushCommit{
341+
{
342+
Sha1: "abcdef1",
343+
CommitterEmail: "[email protected]",
344+
CommitterName: "User Two",
345+
AuthorEmail: "[email protected]",
346+
AuthorName: "User Two",
347+
Message: "close " + setting.AppURL + "user2/repo1/issues/1",
348+
},
349+
}
350+
351+
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 2}).(*models.Repository)
352+
commentBean := &models.Comment{
353+
Type: models.CommentTypeCommitRef,
354+
CommitSHA: "abcdef1",
355+
PosterID: user.ID,
356+
IssueID: 1,
357+
}
358+
359+
issueBean := &models.Issue{RepoID: 1, Index: 1, ID: 1}
360+
361+
models.AssertNotExistsBean(t, commentBean)
362+
models.AssertNotExistsBean(t, issueBean, "is_closed=1")
363+
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
364+
models.AssertExistsAndLoadBean(t, commentBean)
365+
models.AssertExistsAndLoadBean(t, issueBean, "is_closed=1")
366+
models.CheckConsistencyFor(t, &models.Action{})
367+
}
368+
307369
func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
308370
assert.NoError(t, models.PrepareTestDatabase())
309371
user := models.AssertExistsAndLoadBean(t, &models.User{ID: 10}).(*models.User)
@@ -319,6 +381,14 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
319381
AuthorName: "User Ten",
320382
Message: "close user3/repo3#1",
321383
},
384+
{
385+
Sha1: "abcdef4",
386+
CommitterEmail: "[email protected]",
387+
CommitterName: "User Ten",
388+
AuthorEmail: "[email protected]",
389+
AuthorName: "User Ten",
390+
Message: "close " + setting.AppURL + "user3/repo3/issues/1",
391+
},
322392
}
323393

324394
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 6}).(*models.Repository)
@@ -328,13 +398,21 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
328398
PosterID: user.ID,
329399
IssueID: 6,
330400
}
401+
commentBean2 := &models.Comment{
402+
Type: models.CommentTypeCommitRef,
403+
CommitSHA: "abcdef4",
404+
PosterID: user.ID,
405+
IssueID: 6,
406+
}
331407

332408
issueBean := &models.Issue{RepoID: 3, Index: 1, ID: 6}
333409

334410
models.AssertNotExistsBean(t, commentBean)
411+
models.AssertNotExistsBean(t, commentBean2)
335412
models.AssertNotExistsBean(t, issueBean, "is_closed=1")
336413
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
337414
models.AssertNotExistsBean(t, commentBean)
415+
models.AssertNotExistsBean(t, commentBean2)
338416
models.AssertNotExistsBean(t, issueBean, "is_closed=1")
339417
models.CheckConsistencyFor(t, &models.Action{})
340418
}

0 commit comments

Comments
 (0)