Detect full references to issues and pulls in commit messages (#12399)
Fix #10269 Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
		
					parent
					
						
							
								e17e3f71f4
							
						
					
				
			
			
				commit
				
					
						e770c2b850
					
				
			
		
					 3 changed files with 200 additions and 8 deletions
				
			
		| 
						 | 
				
			
			@ -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,7 +25,13 @@ import (
 | 
			
		|||
	"github.com/yuin/goldmark/text"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
var (
 | 
			
		||||
	giteaHostInit sync.Once
 | 
			
		||||
	giteaHost     *url.URL
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
type stripRenderer struct {
 | 
			
		||||
	localhost *url.URL
 | 
			
		||||
	links     []string
 | 
			
		||||
	empty     bool
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -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,6 +174,7 @@ func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) {
 | 
			
		|||
		stripParser = gdMarkdown.Parser()
 | 
			
		||||
	})
 | 
			
		||||
	stripper := &stripRenderer{
 | 
			
		||||
		localhost: getGiteaHost(),
 | 
			
		||||
		links:     make([]string, 0, 10),
 | 
			
		||||
		empty:     true,
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -43,6 +43,7 @@ var (
 | 
			
		|||
 | 
			
		||||
	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.
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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{})
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue