Skip to content

Detect full references to issues and pulls in commit messages #12399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 6, 2020
Merged
76 changes: 71 additions & 5 deletions modules/markup/mdstripper/mdstripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
54 changes: 51 additions & 3 deletions modules/references/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
78 changes: 78 additions & 0 deletions modules/repofiles/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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: "[email protected]",
CommitterName: "User Two",
AuthorEmail: "[email protected]",
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) {
Expand Down Expand Up @@ -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: "[email protected]",
CommitterName: "User Two",
AuthorEmail: "[email protected]",
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)
Expand All @@ -319,6 +381,14 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
AuthorName: "User Ten",
Message: "close user3/repo3#1",
},
{
Sha1: "abcdef4",
CommitterEmail: "[email protected]",
CommitterName: "User Ten",
AuthorEmail: "[email protected]",
AuthorName: "User Ten",
Message: "close " + setting.AppURL + "user3/repo3/issues/1",
},
}

repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 6}).(*models.Repository)
Expand All @@ -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{})
}