From e770c2b850b6e1ce6a6b554cf376c0f7575f80eb Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 6 Aug 2020 20:20:05 +0100 Subject: [PATCH] Detect full references to issues and pulls in commit messages (#12399) Fix #10269 Signed-off-by: Andrew Thornton --- modules/markup/mdstripper/mdstripper.go | 76 ++++++++++++++++++++++-- modules/references/references.go | 54 ++++++++++++++++- modules/repofiles/action_test.go | 78 +++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 8 deletions(-) diff --git a/modules/markup/mdstripper/mdstripper.go b/modules/markup/mdstripper/mdstripper.go index 9d05ee396..52464f3e1 100644 --- a/modules/markup/mdstripper/mdstripper.go +++ b/modules/markup/mdstripper/mdstripper.go @@ -6,12 +6,15 @@ package mdstripper import ( "bytes" + "net/url" + "strings" "sync" "io" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/common" + "code.gitea.io/gitea/modules/setting" "github.com/yuin/goldmark" "github.com/yuin/goldmark/ast" @@ -22,9 +25,15 @@ import ( "github.com/yuin/goldmark/text" ) +var ( + giteaHostInit sync.Once + giteaHost *url.URL +) + type stripRenderer struct { - links []string - empty bool + localhost *url.URL + links []string + empty bool } 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 { r.processLink(w, v.Destination) return ast.WalkSkipChildren, nil case *ast.AutoLink: - r.processLink(w, v.URL(source)) + // This could be a reference to an issue or pull - if so convert it + r.processAutoLink(w, v.URL(source)) return ast.WalkSkipChildren, nil } return ast.WalkContinue, nil @@ -72,6 +82,50 @@ func (r *stripRenderer) processString(w io.Writer, text []byte, coalesce bool) { r.empty = false } +// ProcessAutoLinks to detect and handle links to issues and pulls +func (r *stripRenderer) processAutoLink(w io.Writer, link []byte) { + linkStr := string(link) + u, err := url.Parse(linkStr) + if err != nil { + // Process out of band + r.links = append(r.links, linkStr) + return + } + + // Note: we're not attempting to match the URL scheme (http/https) + host := strings.ToLower(u.Host) + if host != "" && host != strings.ToLower(r.localhost.Host) { + // Process out of band + r.links = append(r.links, linkStr) + return + } + + // We want: /user/repo/issues/3 + parts := strings.Split(strings.TrimPrefix(u.EscapedPath(), r.localhost.EscapedPath()), "/") + if len(parts) != 5 || parts[0] != "" { + // Process out of band + r.links = append(r.links, linkStr) + return + } + + var sep string + if parts[3] == "issues" { + sep = "#" + } else if parts[3] == "pulls" { + sep = "!" + } else { + // Process out of band + r.links = append(r.links, linkStr) + return + } + + _, _ = w.Write([]byte(parts[1])) + _, _ = w.Write([]byte("/")) + _, _ = w.Write([]byte(parts[2])) + _, _ = w.Write([]byte(sep)) + _, _ = w.Write([]byte(parts[4])) +} + func (r *stripRenderer) processLink(w io.Writer, link []byte) { // Links are processed out of band r.links = append(r.links, string(link)) @@ -120,8 +174,9 @@ func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) { stripParser = gdMarkdown.Parser() }) stripper := &stripRenderer{ - links: make([]string, 0, 10), - empty: true, + localhost: getGiteaHost(), + links: make([]string, 0, 10), + empty: true, } reader := text.NewReader(rawBytes) doc := stripParser.Parse(reader) @@ -131,3 +186,14 @@ func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) { } return buf.Bytes(), stripper.GetLinks() } + +// getGiteaHostName returns a normalized string with the local host name, with no scheme or port information +func getGiteaHost() *url.URL { + giteaHostInit.Do(func() { + var err error + if giteaHost, err = url.Parse(setting.AppURL); err != nil { + giteaHost = &url.URL{} + } + }) + return giteaHost +} diff --git a/modules/references/references.go b/modules/references/references.go index c9dabb06b..ce08dcc7a 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -41,8 +41,9 @@ var ( issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp issueKeywordsOnce sync.Once - giteaHostInit sync.Once - giteaHost string + giteaHostInit sync.Once + giteaHost string + giteaIssuePullPattern *regexp.Regexp ) // XRefAction represents the kind of effect a cross reference has once is resolved @@ -152,13 +153,25 @@ func getGiteaHostName() string { giteaHostInit.Do(func() { if uapp, err := url.Parse(setting.AppURL); err == nil { giteaHost = strings.ToLower(uapp.Host) + giteaIssuePullPattern = regexp.MustCompile( + `(\s|^|\(|\[)` + + regexp.QuoteMeta(strings.TrimSpace(setting.AppURL)) + + `([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+)/` + + `((?:issues)|(?:pulls))/([0-9]+)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)`) } else { giteaHost = "" + giteaIssuePullPattern = nil } }) return giteaHost } +// getGiteaIssuePullPattern +func getGiteaIssuePullPattern() *regexp.Regexp { + getGiteaHostName() + return giteaIssuePullPattern +} + // FindAllMentionsMarkdown matches mention patterns in given content and // returns a list of found unvalidated user names **not including** the @ prefix. func FindAllMentionsMarkdown(content string) []string { @@ -219,7 +232,42 @@ func findAllIssueReferencesMarkdown(content string) []*rawReference { // FindAllIssueReferences returns a list of unvalidated references found in a string. func FindAllIssueReferences(content string) []IssueReference { - return rawToIssueReferenceList(findAllIssueReferencesBytes([]byte(content), []string{})) + // Need to convert fully qualified html references to local system to #/! short codes + contentBytes := []byte(content) + if re := getGiteaIssuePullPattern(); re != nil { + pos := 0 + for { + match := re.FindSubmatchIndex(contentBytes[pos:]) + if match == nil { + break + } + // match[0]-match[1] is whole string + // match[2]-match[3] is preamble + pos += match[3] + // match[4]-match[5] is owner/repo + endPos := pos + match[5] - match[4] + copy(contentBytes[pos:endPos], contentBytes[match[4]:match[5]]) + pos = endPos + // match[6]-match[7] == 'issues' + contentBytes[pos] = '#' + if string(contentBytes[match[6]:match[7]]) == "pulls" { + contentBytes[pos] = '!' + } + pos++ + // match[8]-match[9] is the number + endPos = pos + match[9] - match[8] + copy(contentBytes[pos:endPos], contentBytes[match[8]:match[9]]) + copy(contentBytes[endPos:], contentBytes[match[9]:]) + // now we reset the length + // our new section has length endPos - match[3] + // our old section has length match[9] - match[3] + contentBytes = contentBytes[:len(contentBytes)-match[9]+endPos] + pos = endPos + } + } else { + log.Debug("No GiteaIssuePullPattern pattern") + } + return rawToIssueReferenceList(findAllIssueReferencesBytes(contentBytes, []string{})) } // FindRenderizableReferenceNumeric returns the first unvalidated reference found in a string. diff --git a/modules/repofiles/action_test.go b/modules/repofiles/action_test.go index 2d659cde2..8ed3ba7b3 100644 --- a/modules/repofiles/action_test.go +++ b/modules/repofiles/action_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" ) @@ -208,6 +209,32 @@ func TestUpdateIssuesCommit(t *testing.T) { models.AssertExistsAndLoadBean(t, commentBean) models.AssertNotExistsBean(t, issueBean, "is_closed=1") models.CheckConsistencyFor(t, &models.Action{}) + + pushCommits = []*repository.PushCommit{ + { + Sha1: "abcdef3", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "close " + setting.AppURL + repo.FullName() + "/pulls/1", + }, + } + repo = models.AssertExistsAndLoadBean(t, &models.Repository{ID: 3}).(*models.Repository) + commentBean = &models.Comment{ + Type: models.CommentTypeCommitRef, + CommitSHA: "abcdef3", + PosterID: user.ID, + IssueID: 6, + } + issueBean = &models.Issue{RepoID: repo.ID, Index: 1} + + models.AssertNotExistsBean(t, commentBean) + models.AssertNotExistsBean(t, &models.Issue{RepoID: repo.ID, Index: 1}, "is_closed=1") + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) + models.AssertExistsAndLoadBean(t, commentBean) + models.AssertExistsAndLoadBean(t, issueBean, "is_closed=1") + models.CheckConsistencyFor(t, &models.Action{}) } func TestUpdateIssuesCommit_Colon(t *testing.T) { @@ -304,6 +331,41 @@ func TestUpdateIssuesCommit_AnotherRepo(t *testing.T) { models.CheckConsistencyFor(t, &models.Action{}) } +func TestUpdateIssuesCommit_AnotherRepo_FullAddress(t *testing.T) { + assert.NoError(t, models.PrepareTestDatabase()) + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + + // Test that a push to default branch closes issue in another repo + // If the user also has push permissions to that repo + pushCommits := []*repository.PushCommit{ + { + Sha1: "abcdef1", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "close " + setting.AppURL + "user2/repo1/issues/1", + }, + } + + repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 2}).(*models.Repository) + commentBean := &models.Comment{ + Type: models.CommentTypeCommitRef, + CommitSHA: "abcdef1", + PosterID: user.ID, + IssueID: 1, + } + + issueBean := &models.Issue{RepoID: 1, Index: 1, ID: 1} + + models.AssertNotExistsBean(t, commentBean) + models.AssertNotExistsBean(t, issueBean, "is_closed=1") + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) + models.AssertExistsAndLoadBean(t, commentBean) + models.AssertExistsAndLoadBean(t, issueBean, "is_closed=1") + models.CheckConsistencyFor(t, &models.Action{}) +} + func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) { assert.NoError(t, models.PrepareTestDatabase()) user := models.AssertExistsAndLoadBean(t, &models.User{ID: 10}).(*models.User) @@ -319,6 +381,14 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) { AuthorName: "User Ten", Message: "close user3/repo3#1", }, + { + Sha1: "abcdef4", + CommitterEmail: "user10@example.com", + CommitterName: "User Ten", + AuthorEmail: "user10@example.com", + AuthorName: "User Ten", + Message: "close " + setting.AppURL + "user3/repo3/issues/1", + }, } repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 6}).(*models.Repository) @@ -328,13 +398,21 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) { PosterID: user.ID, IssueID: 6, } + commentBean2 := &models.Comment{ + Type: models.CommentTypeCommitRef, + CommitSHA: "abcdef4", + PosterID: user.ID, + IssueID: 6, + } issueBean := &models.Issue{RepoID: 3, Index: 1, ID: 6} models.AssertNotExistsBean(t, commentBean) + models.AssertNotExistsBean(t, commentBean2) models.AssertNotExistsBean(t, issueBean, "is_closed=1") assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) models.AssertNotExistsBean(t, commentBean) + models.AssertNotExistsBean(t, commentBean2) models.AssertNotExistsBean(t, issueBean, "is_closed=1") models.CheckConsistencyFor(t, &models.Action{}) }