From 15809d81f7d36759f289b941352a9754611c5dba Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Sun, 13 Oct 2019 19:29:10 -0300 Subject: [PATCH] Rewrite reference processing code in preparation for opening/closing from comment references (#8261) * Add a markdown stripper for mentions and xrefs * Improve comments * Small code simplification * Move reference code to modules/references * Fix typo * Make MarkdownStripper return [][]byte * Implement preliminary keywords parsing * Add FIXME comment * Fix comment * make fmt * Fix permissions check * Fix text assumptions * Fix imports * Fix lint, fmt * Fix unused import * Add missing export comment * Bypass revive on implemented interface * Move mdstripper into its own package * Support alphanumeric patterns * Refactor FindAllMentions * Move mentions test to references * Parse mentions from reference package * Refactor code to implement renderizable references * Fix typo * Move patterns and tests to the references package * Fix nil reference * Preliminary rendering attempt of closing keywords * Normalize names, comments, general tidy-up * Add CSS style for action keywords * Fix permission for admin and owner * Fix golangci-lint * Fix golangci-lint --- integrations/issue_test.go | 13 +- models/action.go | 181 +++-------- models/action_test.go | 53 +-- models/issue_comment.go | 11 +- models/issue_xref.go | 109 +++---- modules/markup/html.go | 136 ++++---- modules/markup/html_internal_test.go | 92 ------ modules/markup/mdstripper/mdstripper.go | 260 +++++++++++++++ modules/markup/mdstripper/mdstripper_test.go | 71 ++++ modules/markup/sanitizer.go | 3 + modules/references/references.go | 322 +++++++++++++++++++ modules/references/references_test.go | 296 +++++++++++++++++ public/css/index.css | 1 + public/less/_repository.less | 5 + services/mailer/mail_comment.go | 4 +- services/mailer/mail_issue.go | 4 +- 16 files changed, 1123 insertions(+), 438 deletions(-) create mode 100644 modules/markup/mdstripper/mdstripper.go create mode 100644 modules/markup/mdstripper/mdstripper_test.go create mode 100644 modules/references/references.go create mode 100644 modules/references/references_test.go diff --git a/integrations/issue_test.go b/integrations/issue_test.go index 0b153607ee..aa17c44254 100644 --- a/integrations/issue_test.go +++ b/integrations/issue_test.go @@ -13,6 +13,7 @@ import ( "testing" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" @@ -207,7 +208,7 @@ func TestIssueCrossReference(t *testing.T) { RefIssueID: issueRef.ID, RefCommentID: 0, RefIsPull: false, - RefAction: models.XRefActionNone}) + RefAction: references.XRefActionNone}) // Edit title, neuter ref testIssueChangeInfo(t, "user2", issueRefURL, "title", "Title no ref") @@ -217,7 +218,7 @@ func TestIssueCrossReference(t *testing.T) { RefIssueID: issueRef.ID, RefCommentID: 0, RefIsPull: false, - RefAction: models.XRefActionNeutered}) + RefAction: references.XRefActionNeutered}) // Ref from issue content issueRefURL, issueRef = testIssueWithBean(t, "user2", 1, "TitleXRef", fmt.Sprintf("Description ref #%d", issueBase.Index)) @@ -227,7 +228,7 @@ func TestIssueCrossReference(t *testing.T) { RefIssueID: issueRef.ID, RefCommentID: 0, RefIsPull: false, - RefAction: models.XRefActionNone}) + RefAction: references.XRefActionNone}) // Edit content, neuter ref testIssueChangeInfo(t, "user2", issueRefURL, "content", "Description no ref") @@ -237,7 +238,7 @@ func TestIssueCrossReference(t *testing.T) { RefIssueID: issueRef.ID, RefCommentID: 0, RefIsPull: false, - RefAction: models.XRefActionNeutered}) + RefAction: references.XRefActionNeutered}) // Ref from a comment session := loginUser(t, "user2") @@ -248,7 +249,7 @@ func TestIssueCrossReference(t *testing.T) { RefIssueID: issueRef.ID, RefCommentID: commentID, RefIsPull: false, - RefAction: models.XRefActionNone} + RefAction: references.XRefActionNone} models.AssertExistsAndLoadBean(t, comment) // Ref from a different repository @@ -259,7 +260,7 @@ func TestIssueCrossReference(t *testing.T) { RefIssueID: issueRef.ID, RefCommentID: 0, RefIsPull: false, - RefAction: models.XRefActionNone}) + RefAction: references.XRefActionNone}) } func testIssueWithBean(t *testing.T, user string, repoID int64, title, content string) (string, *models.Issue) { diff --git a/models/action.go b/models/action.go index 87088101f9..2d2999f880 100644 --- a/models/action.go +++ b/models/action.go @@ -10,15 +10,14 @@ import ( "fmt" "html" "path" - "regexp" "strconv" "strings" "time" - "unicode" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -54,29 +53,6 @@ const ( ActionMirrorSyncDelete // 20 ) -var ( - // Same as GitHub. See - // https://help.github.com/articles/closing-issues-via-commit-messages - issueCloseKeywords = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"} - issueReopenKeywords = []string{"reopen", "reopens", "reopened"} - - issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp - issueReferenceKeywordsPat *regexp.Regexp -) - -const issueRefRegexpStr = `(?:([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+))?(#[0-9]+)+` -const issueRefRegexpStrNoKeyword = `(?:\s|^|\(|\[)(?:([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+))?(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))` - -func assembleKeywordsPattern(words []string) string { - return fmt.Sprintf(`(?i)(?:%s)(?::?) %s`, strings.Join(words, "|"), issueRefRegexpStr) -} - -func init() { - issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)) - issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)) - issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStrNoKeyword) -} - // Action represents user operation type and other information to // repository. It implemented interface base.Actioner so that can be // used in template render. @@ -351,10 +327,6 @@ func RenameRepoAction(actUser *User, oldRepoName string, repo *Repository) error return renameRepoAction(x, actUser, oldRepoName, repo) } -func issueIndexTrimRight(c rune) bool { - return !unicode.IsDigit(c) -} - // PushCommit represents a commit in a push operation. type PushCommit struct { Sha1 string @@ -480,39 +452,9 @@ func (pc *PushCommits) AvatarLink(email string) string { } // getIssueFromRef returns the issue referenced by a ref. Returns a nil *Issue -// if the provided ref is misformatted or references a non-existent issue. -func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { - ref = ref[strings.IndexByte(ref, ' ')+1:] - ref = strings.TrimRightFunc(ref, issueIndexTrimRight) - - var refRepo *Repository - poundIndex := strings.IndexByte(ref, '#') - if poundIndex < 0 { - return nil, nil - } else if poundIndex == 0 { - refRepo = repo - } else { - slashIndex := strings.IndexByte(ref, '/') - if slashIndex < 0 || slashIndex >= poundIndex { - return nil, nil - } - ownerName := ref[:slashIndex] - repoName := ref[slashIndex+1 : poundIndex] - var err error - refRepo, err = GetRepositoryByOwnerAndName(ownerName, repoName) - if err != nil { - if IsErrRepoNotExist(err) { - return nil, nil - } - return nil, err - } - } - issueIndex, err := strconv.ParseInt(ref[poundIndex+1:], 10, 64) - if err != nil { - return nil, nil - } - - issue, err := GetIssueByIndex(refRepo.ID, issueIndex) +// if the provided ref references a non-existent issue. +func getIssueFromRef(repo *Repository, index int64) (*Issue, error) { + issue, err := GetIssueByIndex(repo.ID, index) if err != nil { if IsErrIssueNotExist(err) { return nil, nil @@ -522,20 +464,7 @@ func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { return issue, nil } -func changeIssueStatus(repo *Repository, doer *User, ref string, refMarked map[int64]bool, status bool) error { - issue, err := getIssueFromRef(repo, ref) - if err != nil { - return err - } - - if issue == nil || refMarked[issue.ID] { - return nil - } - refMarked[issue.ID] = true - - if issue.RepoID != repo.ID || issue.IsClosed == status { - return nil - } +func changeIssueStatus(repo *Repository, issue *Issue, doer *User, status bool) error { stopTimerIfAvailable := func(doer *User, issue *Issue) error { @@ -549,7 +478,7 @@ func changeIssueStatus(repo *Repository, doer *User, ref string, refMarked map[i } issue.Repo = repo - if err = issue.ChangeStatus(doer, status); err != nil { + if err := issue.ChangeStatus(doer, status); err != nil { // Don't return an error when dependencies are open as this would let the push fail if IsErrDependenciesLeft(err) { return stopTimerIfAvailable(doer, issue) @@ -566,99 +495,67 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra for i := len(commits) - 1; i >= 0; i-- { c := commits[i] - refMarked := make(map[int64]bool) + type markKey struct { + ID int64 + Action references.XRefAction + } + + refMarked := make(map[markKey]bool) var refRepo *Repository + var refIssue *Issue var err error - for _, m := range issueReferenceKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { - continue - } - ref := m[3] + for _, ref := range references.FindAllIssueReferences(c.Message) { // issue is from another repo - if len(m[1]) > 0 && len(m[2]) > 0 { - refRepo, err = GetRepositoryFromMatch(m[1], m[2]) + if len(ref.Owner) > 0 && len(ref.Name) > 0 { + refRepo, err = GetRepositoryFromMatch(ref.Owner, ref.Name) if err != nil { continue } } else { refRepo = repo } - issue, err := getIssueFromRef(refRepo, ref) - if err != nil { + if refIssue, err = getIssueFromRef(refRepo, ref.Index); err != nil { return err } - - if issue == nil || refMarked[issue.ID] { + if refIssue == nil { continue } - refMarked[issue.ID] = true - - message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, html.EscapeString(c.Message)) - if err = CreateRefComment(doer, refRepo, issue, message, c.Sha1); err != nil { - return err - } - } - - // Change issue status only if the commit has been pushed to the default branch. - // and if the repo is configured to allow only that - if repo.DefaultBranch != branchName && !repo.CloseIssuesViaCommitInAnyBranch { - continue - } - refMarked = make(map[int64]bool) - for _, m := range issueCloseKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { - continue - } - ref := m[3] - - // issue is from another repo - if len(m[1]) > 0 && len(m[2]) > 0 { - refRepo, err = GetRepositoryFromMatch(m[1], m[2]) - if err != nil { - continue - } - } else { - refRepo = repo - } perm, err := GetUserRepoPermission(refRepo, doer) if err != nil { return err } - // only close issues in another repo if user has push access - if perm.CanWrite(UnitTypeCode) { - if err := changeIssueStatus(refRepo, doer, ref, refMarked, true); err != nil { + + key := markKey{ID: refIssue.ID, Action: ref.Action} + if refMarked[key] { + continue + } + refMarked[key] = true + + // only create comments for issues if user has permission for it + if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeIssues) { + message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, html.EscapeString(c.Message)) + if err = CreateRefComment(doer, refRepo, refIssue, message, c.Sha1); err != nil { return err } } - } - // It is conflict to have close and reopen at same time, so refsMarked doesn't need to reinit here. - for _, m := range issueReopenKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { + // Process closing/reopening keywords + if ref.Action != references.XRefActionCloses && ref.Action != references.XRefActionReopens { continue } - ref := m[3] - // issue is from another repo - if len(m[1]) > 0 && len(m[2]) > 0 { - refRepo, err = GetRepositoryFromMatch(m[1], m[2]) - if err != nil { - continue - } - } else { - refRepo = repo + // Change issue status only if the commit has been pushed to the default branch. + // and if the repo is configured to allow only that + // FIXME: we should be using Issue.ref if set instead of repo.DefaultBranch + if repo.DefaultBranch != branchName && !repo.CloseIssuesViaCommitInAnyBranch { + continue } - perm, err := GetUserRepoPermission(refRepo, doer) - if err != nil { - return err - } - - // only reopen issues in another repo if user has push access - if perm.CanWrite(UnitTypeCode) { - if err := changeIssueStatus(refRepo, doer, ref, refMarked, false); err != nil { + // only close issues in another repo if user has push access + if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeCode) { + if err := changeIssueStatus(refRepo, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil { return err } } diff --git a/models/action_test.go b/models/action_test.go index c90538ebe6..df41556850 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -1,7 +1,6 @@ package models import ( - "fmt" "path" "strings" "testing" @@ -181,56 +180,6 @@ func TestPushCommits_AvatarLink(t *testing.T) { pushCommits.AvatarLink("nonexistent@example.com")) } -func TestRegExp_issueReferenceKeywordsPat(t *testing.T) { - trueTestCases := []string{ - "#2", - "[#2]", - "please see go-gitea/gitea#5", - "#2:", - } - falseTestCases := []string{ - "kb#2", - "#2xy", - } - - for _, testCase := range trueTestCases { - assert.True(t, issueReferenceKeywordsPat.MatchString(testCase)) - } - for _, testCase := range falseTestCases { - assert.False(t, issueReferenceKeywordsPat.MatchString(testCase)) - } -} - -func Test_getIssueFromRef(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) - for _, test := range []struct { - Ref string - ExpectedIssueID int64 - }{ - {"#2", 2}, - {"reopen #2", 2}, - {"user2/repo2#1", 4}, - {"fixes user2/repo2#1", 4}, - {"fixes: user2/repo2#1", 4}, - } { - issue, err := getIssueFromRef(repo, test.Ref) - assert.NoError(t, err) - if assert.NotNil(t, issue) { - assert.EqualValues(t, test.ExpectedIssueID, issue.ID) - } - } - - for _, badRef := range []string{ - "doesnotexist/doesnotexist#1", - fmt.Sprintf("#%d", NonexistentID), - } { - issue, err := getIssueFromRef(repo, badRef) - assert.NoError(t, err) - assert.Nil(t, issue) - } -} - func TestUpdateIssuesCommit(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) pushCommits := []*PushCommit{ @@ -431,7 +380,7 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) { AssertNotExistsBean(t, commentBean) AssertNotExistsBean(t, issueBean, "is_closed=1") assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) - AssertExistsAndLoadBean(t, commentBean) + AssertNotExistsBean(t, commentBean) AssertNotExistsBean(t, issueBean, "is_closed=1") CheckConsistencyFor(t, &Action{}) } diff --git a/models/issue_comment.go b/models/issue_comment.go index e8043c1ec7..7d38302b98 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/references" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -144,10 +145,10 @@ type Comment struct { // Reference an issue or pull from another comment, issue or PR // All information is about the origin of the reference - RefRepoID int64 `xorm:"index"` // Repo where the referencing - RefIssueID int64 `xorm:"index"` - RefCommentID int64 `xorm:"index"` // 0 if origin is Issue title or content (or PR's) - RefAction XRefAction `xorm:"SMALLINT"` // What hapens if RefIssueID resolves + RefRepoID int64 `xorm:"index"` // Repo where the referencing + RefIssueID int64 `xorm:"index"` + RefCommentID int64 `xorm:"index"` // 0 if origin is Issue title or content (or PR's) + RefAction references.XRefAction `xorm:"SMALLINT"` // What hapens if RefIssueID resolves RefIsPull bool RefRepo *Repository `xorm:"-"` @@ -773,7 +774,7 @@ type CreateCommentOptions struct { RefRepoID int64 RefIssueID int64 RefCommentID int64 - RefAction XRefAction + RefAction references.XRefAction RefIsPull bool } diff --git a/models/issue_xref.go b/models/issue_xref.go index 1cc0bcfe6a..141a7e0e8c 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -5,42 +5,16 @@ package models import ( - "regexp" - "strconv" - "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/references" "github.com/go-xorm/xorm" "github.com/unknwon/com" ) -var ( - // TODO: Unify all regexp treatment of cross references in one place - - // issueNumericPattern matches string that references to a numeric issue, e.g. #1287 - issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(?:#)([0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) - // crossReferenceIssueNumericPattern matches string that references a numeric issue in a different repository - // e.g. gogits/gogs#12345 - crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+)#([0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) -) - -// XRefAction represents the kind of effect a cross reference has once is resolved -type XRefAction int64 - -const ( - // XRefActionNone means the cross-reference is a mention (commit, etc.) - XRefActionNone XRefAction = iota // 0 - // XRefActionCloses means the cross-reference should close an issue if it is resolved - XRefActionCloses // 1 - not implemented yet - // XRefActionReopens means the cross-reference should reopen an issue if it is resolved - XRefActionReopens // 2 - Not implemented yet - // XRefActionNeutered means the cross-reference will no longer affect the source - XRefActionNeutered // 3 -) - type crossReference struct { Issue *Issue - Action XRefAction + Action references.XRefAction } // crossReferencesContext is context to pass along findCrossReference functions @@ -72,7 +46,7 @@ func newCrossReference(e *xorm.Session, ctx *crossReferencesContext, xref *cross func neuterCrossReferences(e Engine, issueID int64, commentID int64) error { active := make([]*Comment, 0, 10) - sess := e.Where("`ref_action` IN (?, ?, ?)", XRefActionNone, XRefActionCloses, XRefActionReopens) + sess := e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens) if issueID != 0 { sess = sess.And("`ref_issue_id` = ?", issueID) } @@ -86,7 +60,7 @@ func neuterCrossReferences(e Engine, issueID int64, commentID int64) error { for i, c := range active { ids[i] = c.ID } - _, err := e.In("id", ids).Cols("`ref_action`").Update(&Comment{RefAction: XRefActionNeutered}) + _, err := e.In("id", ids).Cols("`ref_action`").Update(&Comment{RefAction: references.XRefActionNeutered}) return err } @@ -110,11 +84,11 @@ func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User) error { Doer: doer, OrigIssue: issue, } - return issue.createCrossReferences(e, ctx, issue.Title+"\n"+issue.Content) + return issue.createCrossReferences(e, ctx, issue.Title, issue.Content) } -func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesContext, content string) error { - xreflist, err := ctx.OrigIssue.getCrossReferences(e, ctx, content) +func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesContext, plaincontent, mdcontent string) error { + xreflist, err := ctx.OrigIssue.getCrossReferences(e, ctx, plaincontent, mdcontent) if err != nil { return err } @@ -126,47 +100,43 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC return nil } -func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, content string) ([]*crossReference, error) { +func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, plaincontent, mdcontent string) ([]*crossReference, error) { xreflist := make([]*crossReference, 0, 5) - var xref *crossReference + var ( + refRepo *Repository + refIssue *Issue + err error + ) - // Issues in the same repository - // FIXME: Should we support IssueNameStyleAlphanumeric? - matches := issueNumericPattern.FindAllStringSubmatch(content, -1) - for _, match := range matches { - if index, err := strconv.ParseInt(match[1], 10, 64); err == nil { - if err = ctx.OrigIssue.loadRepo(e); err != nil { + allrefs := append(references.FindAllIssueReferences(plaincontent), references.FindAllIssueReferencesMarkdown(mdcontent)...) + + for _, ref := range allrefs { + if ref.Owner == "" && ref.Name == "" { + // Issues in the same repository + if err := ctx.OrigIssue.loadRepo(e); err != nil { return nil, err } - if xref, err = ctx.OrigIssue.isValidCommentReference(e, ctx, issue.Repo, index); err != nil { - return nil, err - } - if xref != nil { - xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, xref) - } - } - } - - // Issues in other repositories - matches = crossReferenceIssueNumericPattern.FindAllStringSubmatch(content, -1) - for _, match := range matches { - if index, err := strconv.ParseInt(match[3], 10, 64); err == nil { - repo, err := getRepositoryByOwnerAndName(e, match[1], match[2]) + refRepo = ctx.OrigIssue.Repo + } else { + // Issues in other repositories + refRepo, err = getRepositoryByOwnerAndName(e, ref.Owner, ref.Name) if err != nil { if IsErrRepoNotExist(err) { continue } return nil, err } - if err = ctx.OrigIssue.loadRepo(e); err != nil { - return nil, err - } - if xref, err = issue.isValidCommentReference(e, ctx, repo, index); err != nil { - return nil, err - } - if xref != nil { - xreflist = issue.updateCrossReferenceList(xreflist, xref) - } + } + if refIssue, err = ctx.OrigIssue.findReferencedIssue(e, ctx, refRepo, ref.Index); err != nil { + return nil, err + } + if refIssue != nil { + xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, &crossReference{ + Issue: refIssue, + // FIXME: currently ignore keywords + // Action: ref.Action, + Action: references.XRefActionNone, + }) } } @@ -179,7 +149,7 @@ func (issue *Issue) updateCrossReferenceList(list []*crossReference, xref *cross } for i, r := range list { if r.Issue.ID == xref.Issue.ID { - if xref.Action != XRefActionNone { + if xref.Action != references.XRefActionNone { list[i].Action = xref.Action } return list @@ -188,7 +158,7 @@ func (issue *Issue) updateCrossReferenceList(list []*crossReference, xref *cross return append(list, xref) } -func (issue *Issue) isValidCommentReference(e Engine, ctx *crossReferencesContext, repo *Repository, index int64) (*crossReference, error) { +func (issue *Issue) findReferencedIssue(e Engine, ctx *crossReferencesContext, repo *Repository, index int64) (*Issue, error) { refIssue := &Issue{RepoID: repo.ID, Index: index} if has, _ := e.Get(refIssue); !has { return nil, nil @@ -206,10 +176,7 @@ func (issue *Issue) isValidCommentReference(e Engine, ctx *crossReferencesContex return nil, nil } } - return &crossReference{ - Issue: refIssue, - Action: XRefActionNone, - }, nil + return refIssue, nil } func (issue *Issue) neuterCrossReferences(e Engine) error { @@ -237,7 +204,7 @@ func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error { OrigIssue: comment.Issue, OrigComment: comment, } - return comment.Issue.createCrossReferences(e, ctx, comment.Content) + return comment.Issue.createCrossReferences(e, ctx, "", comment.Content) } func (comment *Comment) neuterCrossReferences(e Engine) error { diff --git a/modules/markup/html.go b/modules/markup/html.go index f07993bc4c..fc823b1f30 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -36,17 +37,6 @@ var ( // While fast, this is also incorrect and lead to false positives. // TODO: fix invalid linking issue - // mentionPattern matches all mentions in the form of "@user" - mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_\.]+)(?:\s|$|\)|\])`) - - // issueNumericPattern matches string that references to a numeric issue, e.g. #1287 - issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) - // issueAlphanumericPattern matches string that references to an alphanumeric issue, e.g. ABC-1234 - issueAlphanumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([A-Z]{1,10}-[1-9][0-9]*)(?:\s|$|\)|\]|:|\.(\s|$))`) - // crossReferenceIssueNumericPattern matches string that references a numeric issue in a different repository - // e.g. gogits/gogs#12345 - crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+#[0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) - // sha1CurrentPattern matches string that represents a commit SHA, e.g. d8a994ef243349f321568f9e36d5c3f444b99cae // Although SHA1 hashes are 40 chars long, the regex matches the hash from 7 to 40 chars in length // so that abbreviated hash links can be used as well. This matches git and github useability. @@ -70,6 +60,9 @@ var ( linkRegex, _ = xurls.StrictMatchingScheme("https?://") ) +// CSS class for action keywords (e.g. "closes: #1") +const keywordClass = "issue-keyword" + // regexp for full links to issues/pulls var issueFullPattern *regexp.Regexp @@ -99,17 +92,6 @@ func getIssueFullPattern() *regexp.Regexp { return issueFullPattern } -// FindAllMentions matches mention patterns in given content -// and returns a list of found user names without @ prefix. -func FindAllMentions(content string) []string { - mentions := mentionPattern.FindAllStringSubmatch(content, -1) - ret := make([]string, len(mentions)) - for i, val := range mentions { - ret[i] = val[1][1:] - } - return ret -} - // IsSameDomain checks if given url string has the same hostname as current Gitea instance func IsSameDomain(s string) bool { if strings.HasPrefix(s, "/") { @@ -142,7 +124,6 @@ var defaultProcessors = []processor{ linkProcessor, mentionProcessor, issueIndexPatternProcessor, - crossReferenceIssueIndexPatternProcessor, sha1CurrentPatternProcessor, emailAddressProcessor, } @@ -183,7 +164,6 @@ var commitMessageProcessors = []processor{ linkProcessor, mentionProcessor, issueIndexPatternProcessor, - crossReferenceIssueIndexPatternProcessor, sha1CurrentPatternProcessor, emailAddressProcessor, } @@ -217,7 +197,6 @@ var commitMessageSubjectProcessors = []processor{ linkProcessor, mentionProcessor, issueIndexPatternProcessor, - crossReferenceIssueIndexPatternProcessor, sha1CurrentPatternProcessor, } @@ -330,6 +309,24 @@ func (ctx *postProcessCtx) textNode(node *html.Node) { } } +// createKeyword() renders a highlighted version of an action keyword +func createKeyword(content string) *html.Node { + span := &html.Node{ + Type: html.ElementNode, + Data: atom.Span.String(), + Attr: []html.Attribute{}, + } + span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: keywordClass}) + + text := &html.Node{ + Type: html.TextNode, + Data: content, + } + span.AppendChild(text) + + return span +} + func createLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, @@ -377,10 +374,16 @@ func createCodeLink(href, content, class string) *html.Node { return a } -// replaceContent takes a text node, and in its content it replaces a section of -// it with the specified newNode. An example to visualize how this can work can -// be found here: https://play.golang.org/p/5zP8NnHZ03s +// replaceContent takes text node, and in its content it replaces a section of +// it with the specified newNode. func replaceContent(node *html.Node, i, j int, newNode *html.Node) { + replaceContentList(node, i, j, []*html.Node{newNode}) +} + +// replaceContentList takes text node, and in its content it replaces a section of +// it with the specified newNodes. An example to visualize how this can work can +// be found here: https://play.golang.org/p/5zP8NnHZ03s +func replaceContentList(node *html.Node, i, j int, newNodes []*html.Node) { // get the data before and after the match before := node.Data[:i] after := node.Data[j:] @@ -392,7 +395,9 @@ func replaceContent(node *html.Node, i, j int, newNode *html.Node) { // Get the current next sibling, before which we place the replaced data, // and after that we place the new text node. nextSibling := node.NextSibling - node.Parent.InsertBefore(newNode, nextSibling) + for _, n := range newNodes { + node.Parent.InsertBefore(n, nextSibling) + } if after != "" { node.Parent.InsertBefore(&html.Node{ Type: html.TextNode, @@ -402,13 +407,13 @@ func replaceContent(node *html.Node, i, j int, newNode *html.Node) { } func mentionProcessor(_ *postProcessCtx, node *html.Node) { - m := mentionPattern.FindStringSubmatchIndex(node.Data) - if m == nil { + // We replace only the first mention; other mentions will be addressed later + found, loc := references.FindFirstMentionBytes([]byte(node.Data)) + if !found { return } - // Replace the mention with a link to the specified user. - mention := node.Data[m[2]:m[3]] - replaceContent(node, m[2], m[3], createLink(util.URLJoin(setting.AppURL, mention[1:]), mention, "mention")) + mention := node.Data[loc.Start:loc.End] + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, mention[1:]), mention, "mention")) } func shortLinkProcessor(ctx *postProcessCtx, node *html.Node) { @@ -597,45 +602,44 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { if ctx.metas == nil { return } - // default to numeric pattern, unless alphanumeric is requested. - pattern := issueNumericPattern + + var ( + found bool + ref *references.RenderizableReference + ) + if ctx.metas["style"] == IssueNameStyleAlphanumeric { - pattern = issueAlphanumericPattern - } - - match := pattern.FindStringSubmatchIndex(node.Data) - if match == nil { - return - } - - id := node.Data[match[2]:match[3]] - var link *html.Node - if _, ok := ctx.metas["format"]; ok { - // Support for external issue tracker - if ctx.metas["style"] == IssueNameStyleAlphanumeric { - ctx.metas["index"] = id - } else { - ctx.metas["index"] = id[1:] - } - link = createLink(com.Expand(ctx.metas["format"], ctx.metas), id, "issue") + found, ref = references.FindRenderizableReferenceAlphanumeric(node.Data) } else { - link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "issues", id[1:]), id, "issue") + found, ref = references.FindRenderizableReferenceNumeric(node.Data) } - replaceContent(node, match[2], match[3], link) -} - -func crossReferenceIssueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { - m := crossReferenceIssueNumericPattern.FindStringSubmatchIndex(node.Data) - if m == nil { + if !found { return } - ref := node.Data[m[2]:m[3]] - parts := strings.SplitN(ref, "#", 2) - repo, issue := parts[0], parts[1] + var link *html.Node + reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] + if _, ok := ctx.metas["format"]; ok { + ctx.metas["index"] = ref.Issue + link = createLink(com.Expand(ctx.metas["format"], ctx.metas), reftext, "issue") + } else if ref.Owner == "" { + link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "issues", ref.Issue), reftext, "issue") + } else { + link = createLink(util.URLJoin(setting.AppURL, ref.Owner, ref.Name, "issues", ref.Issue), reftext, "issue") + } - replaceContent(node, m[2], m[3], - createLink(util.URLJoin(setting.AppURL, repo, "issues", issue), ref, issue)) + if ref.Action == references.XRefActionNone { + replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) + return + } + + // Decorate action keywords + keyword := createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) + spaces := &html.Node{ + Type: html.TextNode, + Data: node.Data[ref.ActionLocation.End:ref.RefLocation.Start], + } + replaceContentList(node, ref.ActionLocation.Start, ref.RefLocation.End, []*html.Node{keyword, spaces, link}) } // fullSha1PatternProcessor renders SHA containing URLs diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 2824ce3e68..9722063e17 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -239,34 +239,6 @@ func TestRender_FullIssueURLs(t *testing.T) { `#4`) } -func TestRegExp_issueNumericPattern(t *testing.T) { - trueTestCases := []string{ - "#1234", - "#0", - "#1234567890987654321", - " #12", - "#12:", - "ref: #12: msg", - } - falseTestCases := []string{ - "# 1234", - "# 0", - "# ", - "#", - "#ABC", - "#1A2B", - "", - "ABC", - } - - for _, testCase := range trueTestCases { - assert.True(t, issueNumericPattern.MatchString(testCase)) - } - for _, testCase := range falseTestCases { - assert.False(t, issueNumericPattern.MatchString(testCase)) - } -} - func TestRegExp_sha1CurrentPattern(t *testing.T) { trueTestCases := []string{ "d8a994ef243349f321568f9e36d5c3f444b99cae", @@ -325,70 +297,6 @@ func TestRegExp_anySHA1Pattern(t *testing.T) { } } -func TestRegExp_mentionPattern(t *testing.T) { - trueTestCases := []string{ - "@Unknwon", - "@ANT_123", - "@xxx-DiN0-z-A..uru..s-xxx", - " @lol ", - " @Te-st", - "(@gitea)", - "[@gitea]", - } - falseTestCases := []string{ - "@ 0", - "@ ", - "@", - "", - "ABC", - "/home/gitea/@gitea", - "\"@gitea\"", - } - - for _, testCase := range trueTestCases { - res := mentionPattern.MatchString(testCase) - assert.True(t, res) - } - for _, testCase := range falseTestCases { - res := mentionPattern.MatchString(testCase) - assert.False(t, res) - } -} - -func TestRegExp_issueAlphanumericPattern(t *testing.T) { - trueTestCases := []string{ - "ABC-1234", - "A-1", - "RC-80", - "ABCDEFGHIJ-1234567890987654321234567890", - "ABC-123.", - "(ABC-123)", - "[ABC-123]", - "ABC-123:", - } - falseTestCases := []string{ - "RC-08", - "PR-0", - "ABCDEFGHIJK-1", - "PR_1", - "", - "#ABC", - "", - "ABC", - "GG-", - "rm-1", - "/home/gitea/ABC-1234", - "MY-STRING-ABC-123", - } - - for _, testCase := range trueTestCases { - assert.True(t, issueAlphanumericPattern.MatchString(testCase)) - } - for _, testCase := range falseTestCases { - assert.False(t, issueAlphanumericPattern.MatchString(testCase)) - } -} - func TestRegExp_shortLinkPattern(t *testing.T) { trueTestCases := []string{ "[[stuff]]", diff --git a/modules/markup/mdstripper/mdstripper.go b/modules/markup/mdstripper/mdstripper.go new file mode 100644 index 0000000000..7a901b17a9 --- /dev/null +++ b/modules/markup/mdstripper/mdstripper.go @@ -0,0 +1,260 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package mdstripper + +import ( + "bytes" + + "github.com/russross/blackfriday" +) + +// MarkdownStripper extends blackfriday.Renderer +type MarkdownStripper struct { + blackfriday.Renderer + links []string + coallesce bool +} + +const ( + blackfridayExtensions = 0 | + blackfriday.EXTENSION_NO_INTRA_EMPHASIS | + blackfriday.EXTENSION_TABLES | + blackfriday.EXTENSION_FENCED_CODE | + blackfriday.EXTENSION_STRIKETHROUGH | + blackfriday.EXTENSION_NO_EMPTY_LINE_BEFORE_BLOCK | + blackfriday.EXTENSION_DEFINITION_LISTS | + blackfriday.EXTENSION_FOOTNOTES | + blackfriday.EXTENSION_HEADER_IDS | + blackfriday.EXTENSION_AUTO_HEADER_IDS | + // Not included in modules/markup/markdown/markdown.go; + // required here to process inline links + blackfriday.EXTENSION_AUTOLINK +) + +//revive:disable:var-naming Implementing the Rendering interface requires breaking some linting rules + +// StripMarkdown parses markdown content by removing all markup and code blocks +// in order to extract links and other references +func StripMarkdown(rawBytes []byte) (string, []string) { + stripper := &MarkdownStripper{ + links: make([]string, 0, 10), + } + body := blackfriday.Markdown(rawBytes, stripper, blackfridayExtensions) + return string(body), stripper.GetLinks() +} + +// StripMarkdownBytes parses markdown content by removing all markup and code blocks +// in order to extract links and other references +func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) { + stripper := &MarkdownStripper{ + links: make([]string, 0, 10), + } + body := blackfriday.Markdown(rawBytes, stripper, blackfridayExtensions) + return body, stripper.GetLinks() +} + +// block-level callbacks + +// BlockCode dummy function to proceed with rendering +func (r *MarkdownStripper) BlockCode(out *bytes.Buffer, text []byte, infoString string) { + // Not rendered + r.coallesce = false +} + +// BlockQuote dummy function to proceed with rendering +func (r *MarkdownStripper) BlockQuote(out *bytes.Buffer, text []byte) { + // FIXME: perhaps it's better to leave out block quote for this? + r.processString(out, text, false) +} + +// BlockHtml dummy function to proceed with rendering +func (r *MarkdownStripper) BlockHtml(out *bytes.Buffer, text []byte) { //nolint + // Not rendered + r.coallesce = false +} + +// Header dummy function to proceed with rendering +func (r *MarkdownStripper) Header(out *bytes.Buffer, text func() bool, level int, id string) { + text() + r.coallesce = false +} + +// HRule dummy function to proceed with rendering +func (r *MarkdownStripper) HRule(out *bytes.Buffer) { + // Not rendered + r.coallesce = false +} + +// List dummy function to proceed with rendering +func (r *MarkdownStripper) List(out *bytes.Buffer, text func() bool, flags int) { + text() + r.coallesce = false +} + +// ListItem dummy function to proceed with rendering +func (r *MarkdownStripper) ListItem(out *bytes.Buffer, text []byte, flags int) { + r.processString(out, text, false) +} + +// Paragraph dummy function to proceed with rendering +func (r *MarkdownStripper) Paragraph(out *bytes.Buffer, text func() bool) { + text() + r.coallesce = false +} + +// Table dummy function to proceed with rendering +func (r *MarkdownStripper) Table(out *bytes.Buffer, header []byte, body []byte, columnData []int) { + r.processString(out, header, false) + r.processString(out, body, false) +} + +// TableRow dummy function to proceed with rendering +func (r *MarkdownStripper) TableRow(out *bytes.Buffer, text []byte) { + r.processString(out, text, false) +} + +// TableHeaderCell dummy function to proceed with rendering +func (r *MarkdownStripper) TableHeaderCell(out *bytes.Buffer, text []byte, flags int) { + r.processString(out, text, false) +} + +// TableCell dummy function to proceed with rendering +func (r *MarkdownStripper) TableCell(out *bytes.Buffer, text []byte, flags int) { + r.processString(out, text, false) +} + +// Footnotes dummy function to proceed with rendering +func (r *MarkdownStripper) Footnotes(out *bytes.Buffer, text func() bool) { + text() +} + +// FootnoteItem dummy function to proceed with rendering +func (r *MarkdownStripper) FootnoteItem(out *bytes.Buffer, name, text []byte, flags int) { + r.processString(out, text, false) +} + +// TitleBlock dummy function to proceed with rendering +func (r *MarkdownStripper) TitleBlock(out *bytes.Buffer, text []byte) { + r.processString(out, text, false) +} + +// Span-level callbacks + +// AutoLink dummy function to proceed with rendering +func (r *MarkdownStripper) AutoLink(out *bytes.Buffer, link []byte, kind int) { + r.processLink(out, link, []byte{}) +} + +// CodeSpan dummy function to proceed with rendering +func (r *MarkdownStripper) CodeSpan(out *bytes.Buffer, text []byte) { + // Not rendered + r.coallesce = false +} + +// DoubleEmphasis dummy function to proceed with rendering +func (r *MarkdownStripper) DoubleEmphasis(out *bytes.Buffer, text []byte) { + r.processString(out, text, false) +} + +// Emphasis dummy function to proceed with rendering +func (r *MarkdownStripper) Emphasis(out *bytes.Buffer, text []byte) { + r.processString(out, text, false) +} + +// Image dummy function to proceed with rendering +func (r *MarkdownStripper) Image(out *bytes.Buffer, link []byte, title []byte, alt []byte) { + // Not rendered + r.coallesce = false +} + +// LineBreak dummy function to proceed with rendering +func (r *MarkdownStripper) LineBreak(out *bytes.Buffer) { + // Not rendered + r.coallesce = false +} + +// Link dummy function to proceed with rendering +func (r *MarkdownStripper) Link(out *bytes.Buffer, link []byte, title []byte, content []byte) { + r.processLink(out, link, content) +} + +// RawHtmlTag dummy function to proceed with rendering +func (r *MarkdownStripper) RawHtmlTag(out *bytes.Buffer, tag []byte) { //nolint + // Not rendered + r.coallesce = false +} + +// TripleEmphasis dummy function to proceed with rendering +func (r *MarkdownStripper) TripleEmphasis(out *bytes.Buffer, text []byte) { + r.processString(out, text, false) +} + +// StrikeThrough dummy function to proceed with rendering +func (r *MarkdownStripper) StrikeThrough(out *bytes.Buffer, text []byte) { + r.processString(out, text, false) +} + +// FootnoteRef dummy function to proceed with rendering +func (r *MarkdownStripper) FootnoteRef(out *bytes.Buffer, ref []byte, id int) { + // Not rendered + r.coallesce = false +} + +// Low-level callbacks + +// Entity dummy function to proceed with rendering +func (r *MarkdownStripper) Entity(out *bytes.Buffer, entity []byte) { + // FIXME: literal entities are not parsed; perhaps they should + r.coallesce = false +} + +// NormalText dummy function to proceed with rendering +func (r *MarkdownStripper) NormalText(out *bytes.Buffer, text []byte) { + r.processString(out, text, true) +} + +// Header and footer + +// DocumentHeader dummy function to proceed with rendering +func (r *MarkdownStripper) DocumentHeader(out *bytes.Buffer) { + r.coallesce = false +} + +// DocumentFooter dummy function to proceed with rendering +func (r *MarkdownStripper) DocumentFooter(out *bytes.Buffer) { + r.coallesce = false +} + +// GetFlags returns rendering flags +func (r *MarkdownStripper) GetFlags() int { + return 0 +} + +//revive:enable:var-naming + +func doubleSpace(out *bytes.Buffer) { + if out.Len() > 0 { + out.WriteByte('\n') + } +} + +func (r *MarkdownStripper) processString(out *bytes.Buffer, text []byte, coallesce bool) { + // Always break-up words + if !coallesce || !r.coallesce { + doubleSpace(out) + } + out.Write(text) + r.coallesce = coallesce +} +func (r *MarkdownStripper) processLink(out *bytes.Buffer, link []byte, content []byte) { + // Links are processed out of band + r.links = append(r.links, string(link)) + r.coallesce = false +} + +// GetLinks returns the list of link data collected while parsing +func (r *MarkdownStripper) GetLinks() []string { + return r.links +} diff --git a/modules/markup/mdstripper/mdstripper_test.go b/modules/markup/mdstripper/mdstripper_test.go new file mode 100644 index 0000000000..157fe1975b --- /dev/null +++ b/modules/markup/mdstripper/mdstripper_test.go @@ -0,0 +1,71 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package mdstripper + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMarkdownStripper(t *testing.T) { + type testItem struct { + markdown string + expectedText []string + expectedLinks []string + } + + list := []testItem{ + { + ` +## This is a title + +This is [one](link) to paradise. +This **is emphasized**. +This: should coallesce. + +` + "```" + ` +This is a code block. +This should not appear in the output at all. +` + "```" + ` + +* Bullet 1 +* Bullet 2 + +A HIDDEN ` + "`" + `GHOST` + "`" + ` IN THIS LINE. + `, + []string{ + "This is a title", + "This is", + "to paradise.", + "This", + "is emphasized", + ".", + "This: should coallesce.", + "Bullet 1", + "Bullet 2", + "A HIDDEN", + "IN THIS LINE.", + }, + []string{ + "link", + }}, + } + + for _, test := range list { + text, links := StripMarkdown([]byte(test.markdown)) + rawlines := strings.Split(text, "\n") + lines := make([]string, 0, len(rawlines)) + for _, line := range rawlines { + line := strings.TrimSpace(line) + if line != "" { + lines = append(lines, line) + } + } + assert.EqualValues(t, test.expectedText, lines) + assert.EqualValues(t, test.expectedLinks, links) + } +} diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go index 2ec43cf4fd..fd6f90b2ab 100644 --- a/modules/markup/sanitizer.go +++ b/modules/markup/sanitizer.go @@ -38,6 +38,9 @@ func NewSanitizer() { // Custom URL-Schemes sanitizer.policy.AllowURLSchemes(setting.Markdown.CustomURLSchemes...) + + // Allow keyword markup + sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^` + keywordClass + `$`)).OnElements("span") }) } diff --git a/modules/references/references.go b/modules/references/references.go new file mode 100644 index 0000000000..9c74d0d081 --- /dev/null +++ b/modules/references/references.go @@ -0,0 +1,322 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package references + +import ( + "net/url" + "regexp" + "strconv" + "strings" + "sync" + + "code.gitea.io/gitea/modules/markup/mdstripper" + "code.gitea.io/gitea/modules/setting" +) + +var ( + // validNamePattern performs only the most basic validation for user or repository names + // Repository name should contain only alphanumeric, dash ('-'), underscore ('_') and dot ('.') characters. + validNamePattern = regexp.MustCompile(`^[a-z0-9_.-]+$`) + + // NOTE: All below regex matching do not perform any extra validation. + // Thus a link is produced even if the linked entity does not exist. + // While fast, this is also incorrect and lead to false positives. + // TODO: fix invalid linking issue + + // mentionPattern matches all mentions in the form of "@user" + mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_\.]+)(?:\s|$|\)|\])`) + // issueNumericPattern matches string that references to a numeric issue, e.g. #1287 + issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) + // issueAlphanumericPattern matches string that references to an alphanumeric issue, e.g. ABC-1234 + issueAlphanumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([A-Z]{1,10}-[1-9][0-9]*)(?:\s|$|\)|\]|:|\.(\s|$))`) + // crossReferenceIssueNumericPattern matches string that references a numeric issue in a different repository + // e.g. gogits/gogs#12345 + crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+#[0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) + + // Same as GitHub. See + // https://help.github.com/articles/closing-issues-via-commit-messages + issueCloseKeywords = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"} + issueReopenKeywords = []string{"reopen", "reopens", "reopened"} + + issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp + + giteaHostInit sync.Once + giteaHost string +) + +// XRefAction represents the kind of effect a cross reference has once is resolved +type XRefAction int64 + +const ( + // XRefActionNone means the cross-reference is simply a comment + XRefActionNone XRefAction = iota // 0 + // XRefActionCloses means the cross-reference should close an issue if it is resolved + XRefActionCloses // 1 + // XRefActionReopens means the cross-reference should reopen an issue if it is resolved + XRefActionReopens // 2 + // XRefActionNeutered means the cross-reference will no longer affect the source + XRefActionNeutered // 3 +) + +// IssueReference contains an unverified cross-reference to a local issue or pull request +type IssueReference struct { + Index int64 + Owner string + Name string + Action XRefAction +} + +// RenderizableReference contains an unverified cross-reference to with rendering information +type RenderizableReference struct { + Issue string + Owner string + Name string + RefLocation *RefSpan + Action XRefAction + ActionLocation *RefSpan +} + +type rawReference struct { + index int64 + owner string + name string + action XRefAction + issue string + refLocation *RefSpan + actionLocation *RefSpan +} + +func rawToIssueReferenceList(reflist []*rawReference) []IssueReference { + refarr := make([]IssueReference, len(reflist)) + for i, r := range reflist { + refarr[i] = IssueReference{ + Index: r.index, + Owner: r.owner, + Name: r.name, + Action: r.action, + } + } + return refarr +} + +// RefSpan is the position where the reference was found within the parsed text +type RefSpan struct { + Start int + End int +} + +func makeKeywordsPat(keywords []string) *regexp.Regexp { + return regexp.MustCompile(`(?i)(?:\s|^|\(|\[)(` + strings.Join(keywords, `|`) + `):? $`) +} + +func init() { + issueCloseKeywordsPat = makeKeywordsPat(issueCloseKeywords) + issueReopenKeywordsPat = makeKeywordsPat(issueReopenKeywords) +} + +// getGiteaHostName returns a normalized string with the local host name, with no scheme or port information +func getGiteaHostName() string { + giteaHostInit.Do(func() { + if uapp, err := url.Parse(setting.AppURL); err == nil { + giteaHost = strings.ToLower(uapp.Host) + } else { + giteaHost = "" + } + }) + return giteaHost +} + +// 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 { + bcontent, _ := mdstripper.StripMarkdownBytes([]byte(content)) + locations := FindAllMentionsBytes(bcontent) + mentions := make([]string, len(locations)) + for i, val := range locations { + mentions[i] = string(bcontent[val.Start+1 : val.End]) + } + return mentions +} + +// FindAllMentionsBytes matches mention patterns in given content +// and returns a list of locations for the unvalidated user names, including the @ prefix. +func FindAllMentionsBytes(content []byte) []RefSpan { + mentions := mentionPattern.FindAllSubmatchIndex(content, -1) + ret := make([]RefSpan, len(mentions)) + for i, val := range mentions { + ret[i] = RefSpan{Start: val[2], End: val[3]} + } + return ret +} + +// FindFirstMentionBytes matches the first mention in then given content +// and returns the location of the unvalidated user name, including the @ prefix. +func FindFirstMentionBytes(content []byte) (bool, RefSpan) { + mention := mentionPattern.FindSubmatchIndex(content) + if mention == nil { + return false, RefSpan{} + } + return true, RefSpan{Start: mention[2], End: mention[3]} +} + +// FindAllIssueReferencesMarkdown strips content from markdown markup +// and returns a list of unvalidated references found in it. +func FindAllIssueReferencesMarkdown(content string) []IssueReference { + return rawToIssueReferenceList(findAllIssueReferencesMarkdown(content)) +} + +func findAllIssueReferencesMarkdown(content string) []*rawReference { + bcontent, links := mdstripper.StripMarkdownBytes([]byte(content)) + return findAllIssueReferencesBytes(bcontent, links) +} + +// FindAllIssueReferences returns a list of unvalidated references found in a string. +func FindAllIssueReferences(content string) []IssueReference { + return rawToIssueReferenceList(findAllIssueReferencesBytes([]byte(content), []string{})) +} + +// FindRenderizableReferenceNumeric returns the first unvalidated reference found in a string. +func FindRenderizableReferenceNumeric(content string) (bool, *RenderizableReference) { + match := issueNumericPattern.FindStringSubmatchIndex(content) + if match == nil { + if match = crossReferenceIssueNumericPattern.FindStringSubmatchIndex(content); match == nil { + return false, nil + } + } + r := getCrossReference([]byte(content), match[2], match[3], false) + if r == nil { + return false, nil + } + + return true, &RenderizableReference{ + Issue: r.issue, + Owner: r.owner, + Name: r.name, + RefLocation: r.refLocation, + Action: r.action, + ActionLocation: r.actionLocation, + } +} + +// FindRenderizableReferenceAlphanumeric returns the first alphanumeric unvalidated references found in a string. +func FindRenderizableReferenceAlphanumeric(content string) (bool, *RenderizableReference) { + match := issueAlphanumericPattern.FindStringSubmatchIndex(content) + if match == nil { + return false, nil + } + + action, location := findActionKeywords([]byte(content), match[2]) + + return true, &RenderizableReference{ + Issue: string(content[match[2]:match[3]]), + RefLocation: &RefSpan{Start: match[2], End: match[3]}, + Action: action, + ActionLocation: location, + } +} + +// FindAllIssueReferencesBytes returns a list of unvalidated references found in a byte slice. +func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference { + + ret := make([]*rawReference, 0, 10) + + matches := issueNumericPattern.FindAllSubmatchIndex(content, -1) + for _, match := range matches { + if ref := getCrossReference(content, match[2], match[3], false); ref != nil { + ret = append(ret, ref) + } + } + + matches = crossReferenceIssueNumericPattern.FindAllSubmatchIndex(content, -1) + for _, match := range matches { + if ref := getCrossReference(content, match[2], match[3], false); ref != nil { + ret = append(ret, ref) + } + } + + localhost := getGiteaHostName() + for _, link := range links { + if u, err := url.Parse(link); err == nil { + // Note: we're not attempting to match the URL scheme (http/https) + host := strings.ToLower(u.Host) + if host != "" && host != localhost { + continue + } + parts := strings.Split(u.EscapedPath(), "/") + // /user/repo/issues/3 + if len(parts) != 5 || parts[0] != "" { + continue + } + if parts[3] != "issues" && parts[3] != "pulls" { + continue + } + // Note: closing/reopening keywords not supported with URLs + bytes := []byte(parts[1] + "/" + parts[2] + "#" + parts[4]) + if ref := getCrossReference(bytes, 0, len(bytes), true); ref != nil { + ref.refLocation = nil + ret = append(ret, ref) + } + } + } + + return ret +} + +func getCrossReference(content []byte, start, end int, fromLink bool) *rawReference { + refid := string(content[start:end]) + parts := strings.Split(refid, "#") + if len(parts) != 2 { + return nil + } + repo, issue := parts[0], parts[1] + index, err := strconv.ParseInt(issue, 10, 64) + if err != nil { + return nil + } + if repo == "" { + if fromLink { + // Markdown links must specify owner/repo + return nil + } + action, location := findActionKeywords(content, start) + return &rawReference{ + index: index, + action: action, + issue: issue, + refLocation: &RefSpan{Start: start, End: end}, + actionLocation: location, + } + } + parts = strings.Split(strings.ToLower(repo), "/") + if len(parts) != 2 { + return nil + } + owner, name := parts[0], parts[1] + if !validNamePattern.MatchString(owner) || !validNamePattern.MatchString(name) { + return nil + } + action, location := findActionKeywords(content, start) + return &rawReference{ + index: index, + owner: owner, + name: name, + action: action, + issue: issue, + refLocation: &RefSpan{Start: start, End: end}, + actionLocation: location, + } +} + +func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { + m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) + if m != nil { + return XRefActionCloses, &RefSpan{Start: m[2], End: m[3]} + } + m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]) + if m != nil { + return XRefActionReopens, &RefSpan{Start: m[2], End: m[3]} + } + return XRefActionNone, nil +} diff --git a/modules/references/references_test.go b/modules/references/references_test.go new file mode 100644 index 0000000000..f8153ffe36 --- /dev/null +++ b/modules/references/references_test.go @@ -0,0 +1,296 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package references + +import ( + "testing" + + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func TestFindAllIssueReferences(t *testing.T) { + + type result struct { + Index int64 + Owner string + Name string + Issue string + Action XRefAction + RefLocation *RefSpan + ActionLocation *RefSpan + } + + type testFixture struct { + input string + expected []result + } + + fixtures := []testFixture{ + { + "Simply closes: #29 yes", + []result{ + {29, "", "", "29", XRefActionCloses, &RefSpan{Start: 15, End: 18}, &RefSpan{Start: 7, End: 13}}, + }, + }, + { + "#123 no, this is a title.", + []result{}, + }, + { + " #124 yes, this is a reference.", + []result{ + {124, "", "", "124", XRefActionNone, &RefSpan{Start: 0, End: 4}, nil}, + }, + }, + { + "```\nThis is a code block.\n#723 no, it's a code block.```", + []result{}, + }, + { + "This `#724` no, it's inline code.", + []result{}, + }, + { + "This user3/repo4#200 yes.", + []result{ + {200, "user3", "repo4", "200", XRefActionNone, &RefSpan{Start: 5, End: 20}, nil}, + }, + }, + { + "This [one](#919) no, this is a URL fragment.", + []result{}, + }, + { + "This [two](/user2/repo1/issues/921) yes.", + []result{ + {921, "user2", "repo1", "921", XRefActionNone, nil, nil}, + }, + }, + { + "This [three](/user2/repo1/pulls/922) yes.", + []result{ + {922, "user2", "repo1", "922", XRefActionNone, nil, nil}, + }, + }, + { + "This [four](http://gitea.com:3000/user3/repo4/issues/203) yes.", + []result{ + {203, "user3", "repo4", "203", XRefActionNone, nil, nil}, + }, + }, + { + "This [five](http://github.com/user3/repo4/issues/204) no.", + []result{}, + }, + { + "This http://gitea.com:3000/user4/repo5/201 no, bad URL.", + []result{}, + }, + { + "This http://gitea.com:3000/user4/repo5/pulls/202 yes.", + []result{ + {202, "user4", "repo5", "202", XRefActionNone, nil, nil}, + }, + }, + { + "This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.", + []result{ + {205, "user4", "repo6", "205", XRefActionNone, nil, nil}, + }, + }, + { + "Reopens #15 yes", + []result{ + {15, "", "", "15", XRefActionReopens, &RefSpan{Start: 8, End: 11}, &RefSpan{Start: 0, End: 7}}, + }, + }, + { + "This closes #20 for you yes", + []result{ + {20, "", "", "20", XRefActionCloses, &RefSpan{Start: 12, End: 15}, &RefSpan{Start: 5, End: 11}}, + }, + }, + { + "Do you fix user6/repo6#300 ? yes", + []result{ + {300, "user6", "repo6", "300", XRefActionCloses, &RefSpan{Start: 11, End: 26}, &RefSpan{Start: 7, End: 10}}, + }, + }, + { + "For 999 #1235 no keyword, but yes", + []result{ + {1235, "", "", "1235", XRefActionNone, &RefSpan{Start: 8, End: 13}, nil}, + }, + }, + { + "Which abc. #9434 same as above", + []result{ + {9434, "", "", "9434", XRefActionNone, &RefSpan{Start: 11, End: 16}, nil}, + }, + }, + { + "This closes #600 and reopens #599", + []result{ + {600, "", "", "600", XRefActionCloses, &RefSpan{Start: 12, End: 16}, &RefSpan{Start: 5, End: 11}}, + {599, "", "", "599", XRefActionReopens, &RefSpan{Start: 29, End: 33}, &RefSpan{Start: 21, End: 28}}, + }, + }, + } + + // Save original value for other tests that may rely on it + prevURL := setting.AppURL + setting.AppURL = "https://gitea.com:3000/" + + for _, fixture := range fixtures { + expraw := make([]*rawReference, len(fixture.expected)) + for i, e := range fixture.expected { + expraw[i] = &rawReference{ + index: e.Index, + owner: e.Owner, + name: e.Name, + action: e.Action, + issue: e.Issue, + refLocation: e.RefLocation, + actionLocation: e.ActionLocation, + } + } + expref := rawToIssueReferenceList(expraw) + refs := FindAllIssueReferencesMarkdown(fixture.input) + assert.EqualValues(t, expref, refs, "Failed to parse: {%s}", fixture.input) + rawrefs := findAllIssueReferencesMarkdown(fixture.input) + assert.EqualValues(t, expraw, rawrefs, "Failed to parse: {%s}", fixture.input) + } + + // Restore for other tests that may rely on the original value + setting.AppURL = prevURL + + type alnumFixture struct { + input string + issue string + refLocation *RefSpan + action XRefAction + actionLocation *RefSpan + } + + alnumFixtures := []alnumFixture{ + { + "This ref ABC-123 is alphanumeric", + "ABC-123", &RefSpan{Start: 9, End: 16}, + XRefActionNone, nil, + }, + { + "This closes ABCD-1234 alphanumeric", + "ABCD-1234", &RefSpan{Start: 12, End: 21}, + XRefActionCloses, &RefSpan{Start: 5, End: 11}, + }, + } + + for _, fixture := range alnumFixtures { + found, ref := FindRenderizableReferenceAlphanumeric(fixture.input) + if fixture.issue == "" { + assert.False(t, found, "Failed to parse: {%s}", fixture.input) + } else { + assert.True(t, found, "Failed to parse: {%s}", fixture.input) + assert.Equal(t, fixture.issue, ref.Issue, "Failed to parse: {%s}", fixture.input) + assert.Equal(t, fixture.refLocation, ref.RefLocation, "Failed to parse: {%s}", fixture.input) + assert.Equal(t, fixture.action, ref.Action, "Failed to parse: {%s}", fixture.input) + assert.Equal(t, fixture.actionLocation, ref.ActionLocation, "Failed to parse: {%s}", fixture.input) + } + } +} + +func TestRegExp_mentionPattern(t *testing.T) { + trueTestCases := []string{ + "@Unknwon", + "@ANT_123", + "@xxx-DiN0-z-A..uru..s-xxx", + " @lol ", + " @Te-st", + "(@gitea)", + "[@gitea]", + } + falseTestCases := []string{ + "@ 0", + "@ ", + "@", + "", + "ABC", + "/home/gitea/@gitea", + "\"@gitea\"", + } + + for _, testCase := range trueTestCases { + res := mentionPattern.MatchString(testCase) + assert.True(t, res) + } + for _, testCase := range falseTestCases { + res := mentionPattern.MatchString(testCase) + assert.False(t, res) + } +} + +func TestRegExp_issueNumericPattern(t *testing.T) { + trueTestCases := []string{ + "#1234", + "#0", + "#1234567890987654321", + " #12", + "#12:", + "ref: #12: msg", + } + falseTestCases := []string{ + "# 1234", + "# 0", + "# ", + "#", + "#ABC", + "#1A2B", + "", + "ABC", + } + + for _, testCase := range trueTestCases { + assert.True(t, issueNumericPattern.MatchString(testCase)) + } + for _, testCase := range falseTestCases { + assert.False(t, issueNumericPattern.MatchString(testCase)) + } +} + +func TestRegExp_issueAlphanumericPattern(t *testing.T) { + trueTestCases := []string{ + "ABC-1234", + "A-1", + "RC-80", + "ABCDEFGHIJ-1234567890987654321234567890", + "ABC-123.", + "(ABC-123)", + "[ABC-123]", + "ABC-123:", + } + falseTestCases := []string{ + "RC-08", + "PR-0", + "ABCDEFGHIJK-1", + "PR_1", + "", + "#ABC", + "", + "ABC", + "GG-", + "rm-1", + "/home/gitea/ABC-1234", + "MY-STRING-ABC-123", + } + + for _, testCase := range trueTestCases { + assert.True(t, issueAlphanumericPattern.MatchString(testCase)) + } + for _, testCase := range falseTestCases { + assert.False(t, issueAlphanumericPattern.MatchString(testCase)) + } +} diff --git a/public/css/index.css b/public/css/index.css index 0efd787122..fda26f4e08 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -878,6 +878,7 @@ tbody.commit-list{vertical-align:baseline} .repo-buttons .disabled-repo-button a.button:hover{background:0 0!important;color:rgba(0,0,0,.6)!important;box-shadow:0 0 0 1px rgba(34,36,38,.15) inset!important} .repo-buttons .ui.labeled.button>.label{border-left:0!important;margin:0!important} .tag-code,.tag-code td{background-color:#f0f0f0!important;border-color:#d3cfcf!important;padding-top:8px;padding-bottom:8px} +.issue-keyword{border-bottom:1px dotted #959da5;display:inline-block} .file-header{display:flex;justify-content:space-between;align-items:center;padding:8px 12px!important} .file-info{display:flex;align-items:center} .file-info-entry+.file-info-entry{border-left:1px solid currentColor;margin-left:8px;padding-left:8px} diff --git a/public/less/_repository.less b/public/less/_repository.less index 0527759ed4..5f6a7fbd97 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -2384,6 +2384,11 @@ tbody.commit-list { padding-bottom: 8px; } +.issue-keyword { + border-bottom: 1px dotted #959da5; + display: inline-block; +} + .file-header { display: flex; justify-content: space-between; diff --git a/services/mailer/mail_comment.go b/services/mailer/mail_comment.go index f64db04fff..d306c14f42 100644 --- a/services/mailer/mail_comment.go +++ b/services/mailer/mail_comment.go @@ -9,7 +9,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/references" ) // MailParticipantsComment sends new comment emails to repository watchers @@ -19,7 +19,7 @@ func MailParticipantsComment(c *models.Comment, opType models.ActionType, issue } func mailParticipantsComment(ctx models.DBContext, c *models.Comment, opType models.ActionType, issue *models.Issue) (err error) { - rawMentions := markup.FindAllMentions(c.Content) + rawMentions := references.FindAllMentionsMarkdown(c.Content) userMentions, err := issue.ResolveMentionsByVisibility(ctx, c.Poster, rawMentions) if err != nil { return fmt.Errorf("ResolveMentionsByVisibility [%d]: %v", c.IssueID, err) diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index da0249d595..b16323909c 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -9,7 +9,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" "github.com/unknwon/com" @@ -123,7 +123,7 @@ func MailParticipants(issue *models.Issue, doer *models.User, opType models.Acti } func mailParticipants(ctx models.DBContext, issue *models.Issue, doer *models.User, opType models.ActionType) (err error) { - rawMentions := markup.FindAllMentions(issue.Content) + rawMentions := references.FindAllMentionsMarkdown(issue.Content) userMentions, err := issue.ResolveMentionsByVisibility(ctx, doer, rawMentions) if err != nil { return fmt.Errorf("ResolveMentionsByVisibility [%d]: %v", issue.ID, err)