From 7d7bd6c8da1ef291d8e500ca43ec87f463eb6da3 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 20 Sep 2019 12:19:00 -0300 Subject: [PATCH 01/32] Add a markdown stripper for mentions and xrefs --- models/issue_xref.go | 65 ++--- modules/markup/html.go | 11 - modules/markup/markdown_stripper.go | 336 +++++++++++++++++++++++ modules/markup/markdown_stripper_test.go | 114 ++++++++ 4 files changed, 471 insertions(+), 55 deletions(-) create mode 100644 modules/markup/markdown_stripper.go create mode 100644 modules/markup/markdown_stripper_test.go diff --git a/models/issue_xref.go b/models/issue_xref.go index 3631bf3246ad7..2b19dc66b0804 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -5,25 +5,13 @@ package models import ( - "regexp" - "strconv" - "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/markup" "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 @@ -128,45 +116,34 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, content string) ([]*crossReference, error) { xreflist := make([]*crossReference, 0, 5) - var xref *crossReference - - // 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 { + var ( + xref *crossReference + refRepo *Repository + err error + ) + + for _, ref := range markup.FindAllIssueReferences(content) { + 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 xref, err = ctx.OrigIssue.isValidCommentReference(e, ctx, refRepo, ref.Index); err != nil { + return nil, err + } + if xref != nil { + xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, xref) } } diff --git a/modules/markup/html.go b/modules/markup/html.go index f07993bc4cc8f..78a06039d1676 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -99,17 +99,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, "/") { diff --git a/modules/markup/markdown_stripper.go b/modules/markup/markdown_stripper.go new file mode 100644 index 0000000000000..ea8960d33bd19 --- /dev/null +++ b/modules/markup/markdown_stripper.go @@ -0,0 +1,336 @@ +// 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 markup + +import ( + "bytes" + "net/url" + "regexp" + "strconv" + "strings" + + "code.gitea.io/gitea/modules/setting" + + "github.com/russross/blackfriday" +) + +// MarkdownStripper extends blackfriday.Renderer +type MarkdownStripper struct { + blackfriday.Renderer + links []string +} + +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 +) + +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_.-]+$`) +) + +// 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() +} + +// block-level callbacks + +// BlockCode dummy function to proceed with rendering +func (r *MarkdownStripper) BlockCode(out *bytes.Buffer, text []byte, infoString string) { + // Not rendered +} + +// 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) +} + +// BlockHtml dummy function to proceed with rendering +func (r *MarkdownStripper) BlockHtml(out *bytes.Buffer, text []byte) { //nolint + // Not rendered +} + +// Header dummy function to proceed with rendering +func (r *MarkdownStripper) Header(out *bytes.Buffer, text func() bool, level int, id string) { + text() +} + +// HRule dummy function to proceed with rendering +func (r *MarkdownStripper) HRule(out *bytes.Buffer) { + // Not rendered +} + +// List dummy function to proceed with rendering +func (r *MarkdownStripper) List(out *bytes.Buffer, text func() bool, flags int) { + text() +} + +// ListItem dummy function to proceed with rendering +func (r *MarkdownStripper) ListItem(out *bytes.Buffer, text []byte, flags int) { + r.processString(out, text) +} + +// Paragraph dummy function to proceed with rendering +func (r *MarkdownStripper) Paragraph(out *bytes.Buffer, text func() bool) { + text() +} + +// Table dummy function to proceed with rendering +func (r *MarkdownStripper) Table(out *bytes.Buffer, header []byte, body []byte, columnData []int) { + r.processString(out, header) + r.processString(out, body) +} + +// TableRow dummy function to proceed with rendering +func (r *MarkdownStripper) TableRow(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// TableHeaderCell dummy function to proceed with rendering +func (r *MarkdownStripper) TableHeaderCell(out *bytes.Buffer, text []byte, flags int) { + r.processString(out, text) +} + +// TableCell dummy function to proceed with rendering +func (r *MarkdownStripper) TableCell(out *bytes.Buffer, text []byte, flags int) { + r.processString(out, text) +} + +// 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) +} + +// TitleBlock dummy function to proceed with rendering +func (r *MarkdownStripper) TitleBlock(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// 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 +} + +// DoubleEmphasis dummy function to proceed with rendering +func (r *MarkdownStripper) DoubleEmphasis(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// Emphasis dummy function to proceed with rendering +func (r *MarkdownStripper) Emphasis(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// Image dummy function to proceed with rendering +func (r *MarkdownStripper) Image(out *bytes.Buffer, link []byte, title []byte, alt []byte) { + // Not rendered +} + +// LineBreak dummy function to proceed with rendering +func (r *MarkdownStripper) LineBreak(out *bytes.Buffer) { + // Not rendered +} + +// 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 +} + +// TripleEmphasis dummy function to proceed with rendering +func (r *MarkdownStripper) TripleEmphasis(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// StrikeThrough dummy function to proceed with rendering +func (r *MarkdownStripper) StrikeThrough(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// FootnoteRef dummy function to proceed with rendering +func (r *MarkdownStripper) FootnoteRef(out *bytes.Buffer, ref []byte, id int) { + // Not rendered +} + +// 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 +} + +// NormalText dummy function to proceed with rendering +func (r *MarkdownStripper) NormalText(out *bytes.Buffer, text []byte) { + r.processString(out, text) +} + +// Header and footer + +// DocumentHeader dummy function to proceed with rendering +func (r *MarkdownStripper) DocumentHeader(out *bytes.Buffer) { +} + +// DocumentFooter dummy function to proceed with rendering +func (r *MarkdownStripper) DocumentFooter(out *bytes.Buffer) { +} + +// GetFlags returns rendering flags +func (r *MarkdownStripper) GetFlags() int { + return 0 +} + +func doubleSpace(out *bytes.Buffer) { + if out.Len() > 0 { + out.WriteByte('\n') + } +} + +func (r *MarkdownStripper) processString(out *bytes.Buffer, text []byte) { + // Always break-up words + doubleSpace(out) + out.Write(text) +} +func (r *MarkdownStripper) processLink(out *bytes.Buffer, link []byte, content []byte) { + // Links are processed out of band + r.links = append(r.links, string(link)) +} + +// GetLinks returns the list of link data collected while parsing +func (r *MarkdownStripper) GetLinks() []string { + return r.links +} + +// FindAllMentions matches mention patterns in given content +// and returns a list of found user names without @ prefix. +func FindAllMentions(content string) []string { + content, _ = StripMarkdown([]byte(content)) + mentions := mentionPattern.FindAllStringSubmatch(content, -1) + ret := make([]string, len(mentions)) + for i, val := range mentions { + ret[i] = val[1][1:] + } + return ret +} + +type RawIssueReference struct { + Index int64 + Owner string + Name string +} + +// FindAllIssueReferences matches issue reference patterns in given content +// and returns a list of found references *with* #. +func FindAllIssueReferences(content string) []*RawIssueReference { + + content, links := StripMarkdown([]byte(content)) + ret := make([]*RawIssueReference, 0, 10) + + matches := issueNumericPattern.FindAllStringSubmatch(content, -1) + for _, match := range matches { + if ref := getCrossReference(match[1], false); ref != nil { + ret = append(ret, ref) + } + } + + matches = crossReferenceIssueNumericPattern.FindAllStringSubmatch(content, -1) + for _, match := range matches { + if ref := getCrossReference(match[1], false); ref != nil { + ret = append(ret, ref) + } + } + + var giteahost string + if uapp, err := url.Parse(setting.AppURL); err == nil { + giteahost = strings.ToLower(uapp.Host) + } + + for _, link := range links { + if u, err := url.Parse(link); err == nil { + host := strings.ToLower(u.Host) + if host != "" && host != giteahost { + continue + } + if u.EscapedPath() == "" || u.EscapedPath()[0] != '/' { + continue + } + parts := strings.Split(u.EscapedPath()[1:], "/") + // user/repo/issues/3 + if len(parts) != 4 { + continue + } + if parts[2] != "issues" && parts[2] != "pulls" { + continue + } + if ref := getCrossReference(parts[0]+"/"+parts[1]+"#"+parts[3], true); ref != nil { + ret = append(ret, ref) + } + } + } + return ret +} + +func getCrossReference(s string, fromLink bool) *RawIssueReference { + parts := strings.Split(s, "#") + 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 + } + return &RawIssueReference{Index: index} + } + 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 + } + return &RawIssueReference{Index: index, Owner: owner, Name: name} +} diff --git a/modules/markup/markdown_stripper_test.go b/modules/markup/markdown_stripper_test.go new file mode 100644 index 0000000000000..dd3113aae75b0 --- /dev/null +++ b/modules/markup/markdown_stripper_test.go @@ -0,0 +1,114 @@ +// 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 markup + +import ( + "strings" + "testing" + + "code.gitea.io/gitea/modules/setting" + + "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 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", + ".", + "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) + } +} + +func TestFindAllIssueReferences(t *testing.T) { + text := ` +#123 no + #124 yes +This [one](#919) no. +This [two](/user2/repo1/issues/921) yes. +This [three](/user2/repo1/pulls/922) yes. +This [four](http://gitea.com:3000/user3/repo4/issues/203) yes. +This [five](http://github.com/user3/repo4/issues/204) no. + +` + "```" + ` +This is a code block. +#723 no +` + "```" + ` + +This ` + "`" + `#724` + "`" + ` no. +This user3/repo4#200 yes. +This http://gitea.com:3000/user4/repo5/201 no. +This http://gitea.com:3000/user4/repo5/pulls/202 yes. +This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes. + + ` + expected := []*RawIssueReference{ + {124, "", ""}, + {200, "user3", "repo4"}, + {921, "user2", "repo1"}, + {922, "user2", "repo1"}, + {203, "user3", "repo4"}, + {202, "user4", "repo5"}, + {205, "user4", "repo6"}, + } + + // Save original value for other tests that may rely on it + prevURL := setting.AppURL + setting.AppURL = "https://gitea.com:3000/" + + refs := FindAllIssueReferences(text) + assert.EqualValues(t, expected, refs) + + // Restore for other tests that may rely on the original value + setting.AppURL = prevURL +} From c20afe104471d6f89ba02f3f11469f259eabd887 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 20 Sep 2019 12:36:04 -0300 Subject: [PATCH 02/32] Improve comments --- modules/markup/markdown_stripper.go | 5 +++-- modules/markup/markdown_stripper_test.go | 16 ++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/modules/markup/markdown_stripper.go b/modules/markup/markdown_stripper.go index ea8960d33bd19..6b8b10c8675cb 100644 --- a/modules/markup/markdown_stripper.go +++ b/modules/markup/markdown_stripper.go @@ -239,7 +239,7 @@ func (r *MarkdownStripper) GetLinks() []string { } // FindAllMentions matches mention patterns in given content -// and returns a list of found user names without @ prefix. +// and returns a list of found unvalidated user names without @ prefix. func FindAllMentions(content string) []string { content, _ = StripMarkdown([]byte(content)) mentions := mentionPattern.FindAllStringSubmatch(content, -1) @@ -257,7 +257,7 @@ type RawIssueReference struct { } // FindAllIssueReferences matches issue reference patterns in given content -// and returns a list of found references *with* #. +// and returns a list of unvalidated references. func FindAllIssueReferences(content string) []*RawIssueReference { content, links := StripMarkdown([]byte(content)) @@ -284,6 +284,7 @@ func FindAllIssueReferences(content string) []*RawIssueReference { 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 != giteahost { continue diff --git a/modules/markup/markdown_stripper_test.go b/modules/markup/markdown_stripper_test.go index dd3113aae75b0..d0f613ce3ebc6 100644 --- a/modules/markup/markdown_stripper_test.go +++ b/modules/markup/markdown_stripper_test.go @@ -72,9 +72,9 @@ A HIDDEN ` + "`" + `GHOST` + "`" + ` IN THIS LINE. func TestFindAllIssueReferences(t *testing.T) { text := ` -#123 no - #124 yes -This [one](#919) no. +#123 no, this is a title. + #124 yes, this is a reference. +This [one](#919) no, this is a URL fragment. This [two](/user2/repo1/issues/921) yes. This [three](/user2/repo1/pulls/922) yes. This [four](http://gitea.com:3000/user3/repo4/issues/203) yes. @@ -82,19 +82,23 @@ This [five](http://github.com/user3/repo4/issues/204) no. ` + "```" + ` This is a code block. -#723 no +#723 no, it's a code block. ` + "```" + ` -This ` + "`" + `#724` + "`" + ` no. +This ` + "`" + `#724` + "`" + ` no, it's inline code. This user3/repo4#200 yes. -This http://gitea.com:3000/user4/repo5/201 no. +This http://gitea.com:3000/user4/repo5/201 no, bad URL. This http://gitea.com:3000/user4/repo5/pulls/202 yes. This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes. ` + // Note, FindAllIssueReferences() processes inline + // references first, then link references. expected := []*RawIssueReference{ + // Inline references {124, "", ""}, {200, "user3", "repo4"}, + // Link references {921, "user2", "repo1"}, {922, "user2", "repo1"}, {203, "user3", "repo4"}, From 69ef1c3179d35cb082a0810095591ff8df758272 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sat, 21 Sep 2019 00:03:59 -0300 Subject: [PATCH 03/32] Small code simplification --- modules/markup/markdown_stripper.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/modules/markup/markdown_stripper.go b/modules/markup/markdown_stripper.go index 6b8b10c8675cb..a8928eeba56d2 100644 --- a/modules/markup/markdown_stripper.go +++ b/modules/markup/markdown_stripper.go @@ -289,18 +289,15 @@ func FindAllIssueReferences(content string) []*RawIssueReference { if host != "" && host != giteahost { continue } - if u.EscapedPath() == "" || u.EscapedPath()[0] != '/' { + parts := strings.Split(u.EscapedPath(), "/") + // /user/repo/issues/3 + if len(parts) != 5 || parts[0] != "" { continue } - parts := strings.Split(u.EscapedPath()[1:], "/") - // user/repo/issues/3 - if len(parts) != 4 { + if parts[3] != "issues" && parts[3] != "pulls" { continue } - if parts[2] != "issues" && parts[2] != "pulls" { - continue - } - if ref := getCrossReference(parts[0]+"/"+parts[1]+"#"+parts[3], true); ref != nil { + if ref := getCrossReference(parts[1]+"/"+parts[2]+"#"+parts[4], true); ref != nil { ret = append(ret, ref) } } From ae66b145e59e2a8c08bb00cbc559ea13ddeeebc2 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sat, 21 Sep 2019 00:58:17 -0300 Subject: [PATCH 04/32] Move reference code to modules/references --- models/issue_comment.go | 4 +- models/issue_mail.go | 4 +- models/issue_xref.go | 4 +- modules/markup/markdown_stripper_test.go | 118 ---------------- .../{markdown_stripper.go => mdstripper.go} | 107 --------------- modules/markup/mdstripper_test.go | 69 ++++++++++ modules/references/references.go | 127 ++++++++++++++++++ modules/references/references_test.go | 72 ++++++++++ 8 files changed, 274 insertions(+), 231 deletions(-) delete mode 100644 modules/markup/markdown_stripper_test.go rename modules/markup/{markdown_stripper.go => mdstripper.go} (70%) create mode 100644 modules/markup/mdstripper_test.go create mode 100644 modules/references/references.go create mode 100644 modules/references/references_test.go diff --git a/models/issue_comment.go b/models/issue_comment.go index 0bb313c30bbf9..7b5759b7ddb95 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -12,8 +12,8 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/markup" "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" @@ -402,7 +402,7 @@ func (c *Comment) MailParticipants(opType ActionType, issue *Issue) (err error) } func (c *Comment) mailParticipants(e Engine, opType ActionType, issue *Issue) (err error) { - mentions := markup.FindAllMentions(c.Content) + mentions := references.FindAllMentions(c.Content) if err = UpdateIssueMentions(e, c.IssueID, mentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err) } diff --git a/models/issue_mail.go b/models/issue_mail.go index 87d991e500918..1b16f1fe544d5 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -9,7 +9,7 @@ import ( "fmt" "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 (issue *Issue) MailParticipants(doer *User, opType ActionType) (err error) } func (issue *Issue) mailParticipants(e Engine, doer *User, opType ActionType) (err error) { - mentions := markup.FindAllMentions(issue.Content) + mentions := references.FindAllMentions(issue.Content) if err = UpdateIssueMentions(e, issue.ID, mentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) diff --git a/models/issue_xref.go b/models/issue_xref.go index 2b19dc66b0804..3bf7bd30cda04 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -6,7 +6,7 @@ package models import ( "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/references" "github.com/go-xorm/xorm" "github.com/unknwon/com" @@ -122,7 +122,7 @@ func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesCont err error ) - for _, ref := range markup.FindAllIssueReferences(content) { + for _, ref := range references.FindAllIssueReferences(content) { if ref.Owner == "" && ref.Name == "" { // Issues in the same repository if err := ctx.OrigIssue.loadRepo(e); err != nil { diff --git a/modules/markup/markdown_stripper_test.go b/modules/markup/markdown_stripper_test.go deleted file mode 100644 index d0f613ce3ebc6..0000000000000 --- a/modules/markup/markdown_stripper_test.go +++ /dev/null @@ -1,118 +0,0 @@ -// 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 markup - -import ( - "strings" - "testing" - - "code.gitea.io/gitea/modules/setting" - - "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 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", - ".", - "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) - } -} - -func TestFindAllIssueReferences(t *testing.T) { - text := ` -#123 no, this is a title. - #124 yes, this is a reference. -This [one](#919) no, this is a URL fragment. -This [two](/user2/repo1/issues/921) yes. -This [three](/user2/repo1/pulls/922) yes. -This [four](http://gitea.com:3000/user3/repo4/issues/203) yes. -This [five](http://github.com/user3/repo4/issues/204) no. - -` + "```" + ` -This is a code block. -#723 no, it's a code block. -` + "```" + ` - -This ` + "`" + `#724` + "`" + ` no, it's inline code. -This user3/repo4#200 yes. -This http://gitea.com:3000/user4/repo5/201 no, bad URL. -This http://gitea.com:3000/user4/repo5/pulls/202 yes. -This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes. - - ` - // Note, FindAllIssueReferences() processes inline - // references first, then link references. - expected := []*RawIssueReference{ - // Inline references - {124, "", ""}, - {200, "user3", "repo4"}, - // Link references - {921, "user2", "repo1"}, - {922, "user2", "repo1"}, - {203, "user3", "repo4"}, - {202, "user4", "repo5"}, - {205, "user4", "repo6"}, - } - - // Save original value for other tests that may rely on it - prevURL := setting.AppURL - setting.AppURL = "https://gitea.com:3000/" - - refs := FindAllIssueReferences(text) - assert.EqualValues(t, expected, refs) - - // Restore for other tests that may rely on the original value - setting.AppURL = prevURL -} diff --git a/modules/markup/markdown_stripper.go b/modules/markup/mdstripper.go similarity index 70% rename from modules/markup/markdown_stripper.go rename to modules/markup/mdstripper.go index a8928eeba56d2..6b5723020cc90 100644 --- a/modules/markup/markdown_stripper.go +++ b/modules/markup/mdstripper.go @@ -6,12 +6,6 @@ package markup import ( "bytes" - "net/url" - "regexp" - "strconv" - "strings" - - "code.gitea.io/gitea/modules/setting" "github.com/russross/blackfriday" ) @@ -38,12 +32,6 @@ const ( blackfriday.EXTENSION_AUTOLINK ) -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_.-]+$`) -) - // 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) { @@ -237,98 +225,3 @@ func (r *MarkdownStripper) processLink(out *bytes.Buffer, link []byte, content [ func (r *MarkdownStripper) GetLinks() []string { return r.links } - -// FindAllMentions matches mention patterns in given content -// and returns a list of found unvalidated user names without @ prefix. -func FindAllMentions(content string) []string { - content, _ = StripMarkdown([]byte(content)) - mentions := mentionPattern.FindAllStringSubmatch(content, -1) - ret := make([]string, len(mentions)) - for i, val := range mentions { - ret[i] = val[1][1:] - } - return ret -} - -type RawIssueReference struct { - Index int64 - Owner string - Name string -} - -// FindAllIssueReferences matches issue reference patterns in given content -// and returns a list of unvalidated references. -func FindAllIssueReferences(content string) []*RawIssueReference { - - content, links := StripMarkdown([]byte(content)) - ret := make([]*RawIssueReference, 0, 10) - - matches := issueNumericPattern.FindAllStringSubmatch(content, -1) - for _, match := range matches { - if ref := getCrossReference(match[1], false); ref != nil { - ret = append(ret, ref) - } - } - - matches = crossReferenceIssueNumericPattern.FindAllStringSubmatch(content, -1) - for _, match := range matches { - if ref := getCrossReference(match[1], false); ref != nil { - ret = append(ret, ref) - } - } - - var giteahost string - if uapp, err := url.Parse(setting.AppURL); err == nil { - giteahost = strings.ToLower(uapp.Host) - } - - 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 != giteahost { - 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 - } - if ref := getCrossReference(parts[1]+"/"+parts[2]+"#"+parts[4], true); ref != nil { - ret = append(ret, ref) - } - } - } - return ret -} - -func getCrossReference(s string, fromLink bool) *RawIssueReference { - parts := strings.Split(s, "#") - 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 - } - return &RawIssueReference{Index: index} - } - 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 - } - return &RawIssueReference{Index: index, Owner: owner, Name: name} -} diff --git a/modules/markup/mdstripper_test.go b/modules/markup/mdstripper_test.go new file mode 100644 index 0000000000000..32fce4f201919 --- /dev/null +++ b/modules/markup/mdstripper_test.go @@ -0,0 +1,69 @@ +// 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 markup + +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 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", + ".", + "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/references/references.go b/modules/references/references.go new file mode 100644 index 0000000000000..0738bbbeccec0 --- /dev/null +++ b/modules/references/references.go @@ -0,0 +1,127 @@ +// 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" + + "code.gitea.io/gitea/modules/markup" + "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_.-]+$`) + + // 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|^|\(|\[)([\pL]+ )?(#[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|^|\(|\[)([\pL]+ )?([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+#[0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) +) + +// RawIssueReference contains information about a cross-reference in the text +type RawIssueReference struct { + Index int64 + Owner string + Name string + Keyword string +} + +// FindAllMentions matches mention patterns in given content +// and returns a list of found unvalidated user names without @ prefix. +func FindAllMentions(content string) []string { + content, _ = markup.StripMarkdown([]byte(content)) + mentions := mentionPattern.FindAllStringSubmatch(content, -1) + ret := make([]string, len(mentions)) + for i, val := range mentions { + ret[i] = val[1][1:] + } + return ret +} + +// FindAllIssueReferences matches issue reference patterns in given content +// and returns a list of unvalidated references. +func FindAllIssueReferences(content string) []*RawIssueReference { + + content, links := markup.StripMarkdown([]byte(content)) + ret := make([]*RawIssueReference, 0, 10) + + matches := issueNumericPattern.FindAllStringSubmatch(content, -1) + for _, match := range matches { + if ref := getCrossReference(match[2], strings.TrimSuffix(match[1], " "), false); ref != nil { + ret = append(ret, ref) + } + } + + matches = crossReferenceIssueNumericPattern.FindAllStringSubmatch(content, -1) + for _, match := range matches { + if ref := getCrossReference(match[2], strings.TrimSuffix(match[1], " "), false); ref != nil { + ret = append(ret, ref) + } + } + + var giteahost string + if uapp, err := url.Parse(setting.AppURL); err == nil { + giteahost = strings.ToLower(uapp.Host) + } + + 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 != giteahost { + 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 + if ref := getCrossReference(parts[1]+"/"+parts[2]+"#"+parts[4], "", true); ref != nil { + ret = append(ret, ref) + } + } + } + return ret +} + +func getCrossReference(s string, keyword string, fromLink bool) *RawIssueReference { + parts := strings.Split(s, "#") + 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 + } + return &RawIssueReference{Index: index, Keyword: keyword} + } + 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 + } + return &RawIssueReference{Index: index, Owner: owner, Name: name, Keyword: keyword} +} diff --git a/modules/references/references_test.go b/modules/references/references_test.go new file mode 100644 index 0000000000000..a6e5ad586d8d1 --- /dev/null +++ b/modules/references/references_test.go @@ -0,0 +1,72 @@ +// 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) { + text := ` +#123 no, this is a title. + #124 yes, this is a reference. +This [one](#919) no, this is a URL fragment. +This [two](/user2/repo1/issues/921) yes. +This [three](/user2/repo1/pulls/922) yes. +This [four](http://gitea.com:3000/user3/repo4/issues/203) yes. +This [five](http://github.com/user3/repo4/issues/204) no. + +` + "```" + ` +This is a code block. +#723 no, it's a code block. +` + "```" + ` + +This ` + "`" + `#724` + "`" + ` no, it's inline code. +This user3/repo4#200 yes. +This http://gitea.com:3000/user4/repo5/201 no, bad URL. +This http://gitea.com:3000/user4/repo5/pulls/202 yes. +This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes. +Closing #15 +I am opening #20 for you +Do you process user6/repo6#300 ? +For 999 #1235 no keyword. +Which abc. #9434 nk neither. + ` + // Note, FindAllIssueReferences() processes references in this order: + // * Issue number: #123 + // * Repository/issue number: user/repo#123 + // * URL: http:// .... + expected := []*RawIssueReference{ + // Numeric references + {124, "", "", ""}, + {15, "", "", "Closing"}, + {20, "", "", "opening"}, + {1235, "", "", ""}, + {9434, "", "", ""}, + // Repository/issue references + {200, "user3", "repo4", "This"}, + {300, "user6", "repo6", "process"}, + // Link references + {921, "user2", "repo1", ""}, + {922, "user2", "repo1", ""}, + {203, "user3", "repo4", ""}, + {202, "user4", "repo5", ""}, + {205, "user4", "repo6", ""}, + } + + // Save original value for other tests that may rely on it + prevURL := setting.AppURL + setting.AppURL = "https://gitea.com:3000/" + + refs := FindAllIssueReferences(text) + assert.EqualValues(t, expected, refs) + + // Restore for other tests that may rely on the original value + setting.AppURL = prevURL +} From 999bc877e1fa683b48067d33d5b1f12a9221b2da Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sat, 21 Sep 2019 01:04:40 -0300 Subject: [PATCH 05/32] Fix typo --- modules/references/references_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/references/references_test.go b/modules/references/references_test.go index a6e5ad586d8d1..453c70cd003b4 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -36,7 +36,7 @@ Closing #15 I am opening #20 for you Do you process user6/repo6#300 ? For 999 #1235 no keyword. -Which abc. #9434 nk neither. +Which abc. #9434 nk either. ` // Note, FindAllIssueReferences() processes references in this order: // * Issue number: #123 From 06f092eb6be79335325763946fb5f26ccbc47649 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sat, 21 Sep 2019 19:29:38 -0300 Subject: [PATCH 06/32] Make MarkdownStripper return [][]byte --- modules/markup/mdstripper.go | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/modules/markup/mdstripper.go b/modules/markup/mdstripper.go index 6b5723020cc90..421b15e99f898 100644 --- a/modules/markup/mdstripper.go +++ b/modules/markup/mdstripper.go @@ -13,7 +13,7 @@ import ( // MarkdownStripper extends blackfriday.Renderer type MarkdownStripper struct { blackfriday.Renderer - links []string + links [][]byte } const ( @@ -36,12 +36,22 @@ const ( // in order to extract links and other references func StripMarkdown(rawBytes []byte) (string, []string) { stripper := &MarkdownStripper{ - links: make([]string, 0, 10), + links: make([][]byte, 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, [][]byte) { + stripper := &MarkdownStripper{ + links: make([][]byte, 0, 10), + } + body := blackfriday.Markdown(rawBytes, stripper, blackfridayExtensions) + return body, stripper.GetLinksBytes() +} + // block-level callbacks // BlockCode dummy function to proceed with rendering @@ -218,10 +228,19 @@ func (r *MarkdownStripper) processString(out *bytes.Buffer, text []byte) { } 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.links = append(r.links, link) +} + +// GetLinksBytes returns the list of link data collected while parsing +func (r *MarkdownStripper) GetLinksBytes() [][]byte { + return r.links } // GetLinks returns the list of link data collected while parsing func (r *MarkdownStripper) GetLinks() []string { - return r.links + links := make([]string,len(r.links)) + for i, link := range r.links { + links[i] = string(link) + } + return links } From 16c6ab48fd0e78fce8cf80b3d216d4cdb93a0f3d Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 01:51:24 -0300 Subject: [PATCH 07/32] Implement preliminary keywords parsing --- models/action.go | 137 ++++------------ models/action_test.go | 31 ---- models/issue_comment.go | 10 +- models/issue_xref.go | 60 +++---- modules/markup/mdstripper.go | 74 +++++---- modules/markup/mdstripper_test.go | 2 + modules/references/references.go | 125 ++++++++++++--- modules/references/references_test.go | 215 ++++++++++++++++++++------ 8 files changed, 376 insertions(+), 278 deletions(-) diff --git a/models/action.go b/models/action.go index 87088101f97fc..c9c4a8b4d380c 100644 --- a/models/action.go +++ b/models/action.go @@ -19,6 +19,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" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -481,38 +482,8 @@ 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) +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 +493,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 +507,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 +524,62 @@ 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 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) + refIssue, err := getIssueFromRef(refRepo, ref.Index) if 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 { + key := markKey{ID: refIssue.ID, Action: ref.Action} + if refMarked[key] { continue } - ref := m[3] + refMarked[key] = true - // 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 { + 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 } - // 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 { - 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 + 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 + // only close issues in another repo if user has push access if perm.CanWrite(UnitTypeCode) { - if err := changeIssueStatus(refRepo, doer, ref, refMarked, false); err != nil { + 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 16fdc7adcc904..23ffbbb5f307f 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -1,7 +1,6 @@ package models import ( - "fmt" "path" "strings" "testing" @@ -201,36 +200,6 @@ func TestRegExp_issueReferenceKeywordsPat(t *testing.T) { } } -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{ diff --git a/models/issue_comment.go b/models/issue_comment.go index 7b5759b7ddb95..f2c505e5b34fe 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -145,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:"-"` @@ -822,7 +822,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 3bf7bd30cda04..da991f0df2bd7 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -12,23 +12,9 @@ import ( "github.com/unknwon/com" ) -// 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 @@ -60,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) } @@ -74,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 } @@ -98,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 } @@ -114,15 +100,17 @@ 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 - refRepo *Repository - err error + refRepo *Repository + refIssue *Issue + err error ) - for _, ref := range references.FindAllIssueReferences(content) { + 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 { @@ -139,11 +127,16 @@ func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesCont return nil, err } } - if xref, err = ctx.OrigIssue.isValidCommentReference(e, ctx, refRepo, ref.Index); err != nil { + if refIssue, err = ctx.OrigIssue.findReferencedIssue(e, ctx, refRepo, ref.Index); err != nil { return nil, err } - if xref != nil { - xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, xref) + if refIssue != nil { + xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, &crossReference{ + Issue: refIssue, + // FIXME: currently ignore keywords + // Action: ref.Action, + Action: references.XRefActionNone, + }) } } @@ -156,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 @@ -165,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 @@ -183,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 { @@ -214,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/mdstripper.go b/modules/markup/mdstripper.go index 421b15e99f898..9518d9d9ab927 100644 --- a/modules/markup/mdstripper.go +++ b/modules/markup/mdstripper.go @@ -13,7 +13,8 @@ import ( // MarkdownStripper extends blackfriday.Renderer type MarkdownStripper struct { blackfriday.Renderer - links [][]byte + links []string + coallesce bool } const ( @@ -36,7 +37,7 @@ const ( // in order to extract links and other references func StripMarkdown(rawBytes []byte) (string, []string) { stripper := &MarkdownStripper{ - links: make([][]byte, 0, 10), + links: make([]string, 0, 10), } body := blackfriday.Markdown(rawBytes, stripper, blackfridayExtensions) return string(body), stripper.GetLinks() @@ -44,12 +45,12 @@ func StripMarkdown(rawBytes []byte) (string, []string) { // StripMarkdownBytes parses markdown content by removing all markup and code blocks // in order to extract links and other references -func StripMarkdownBytes(rawBytes []byte) ([]byte, [][]byte) { +func StripMarkdownBytes(rawBytes []byte) ([]byte, []string) { stripper := &MarkdownStripper{ - links: make([][]byte, 0, 10), + links: make([]string, 0, 10), } body := blackfriday.Markdown(rawBytes, stripper, blackfridayExtensions) - return body, stripper.GetLinksBytes() + return body, stripper.GetLinks() } // block-level callbacks @@ -57,63 +58,69 @@ func StripMarkdownBytes(rawBytes []byte) ([]byte, [][]byte) { // 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) + 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) + 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) - r.processString(out, body) + 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) + 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) + 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) + r.processString(out, text, false) } // Footnotes dummy function to proceed with rendering @@ -123,12 +130,12 @@ func (r *MarkdownStripper) Footnotes(out *bytes.Buffer, text func() bool) { // FootnoteItem dummy function to proceed with rendering func (r *MarkdownStripper) FootnoteItem(out *bytes.Buffer, name, text []byte, flags int) { - r.processString(out, text) + 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) + r.processString(out, text, false) } // Span-level callbacks @@ -141,26 +148,29 @@ func (r *MarkdownStripper) AutoLink(out *bytes.Buffer, link []byte, kind int) { // 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) + 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) + 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 @@ -171,21 +181,23 @@ func (r *MarkdownStripper) Link(out *bytes.Buffer, link []byte, title []byte, co // 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) + 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) + 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 @@ -193,21 +205,24 @@ func (r *MarkdownStripper) FootnoteRef(out *bytes.Buffer, ref []byte, id int) { // 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) + 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 @@ -221,26 +236,21 @@ func doubleSpace(out *bytes.Buffer) { } } -func (r *MarkdownStripper) processString(out *bytes.Buffer, text []byte) { +func (r *MarkdownStripper) processString(out *bytes.Buffer, text []byte, coallesce bool) { // Always break-up words - doubleSpace(out) + 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, link) -} - -// GetLinksBytes returns the list of link data collected while parsing -func (r *MarkdownStripper) GetLinksBytes() [][]byte { - return r.links + 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 { - links := make([]string,len(r.links)) - for i, link := range r.links { - links[i] = string(link) - } - return links + return r.links } diff --git a/modules/markup/mdstripper_test.go b/modules/markup/mdstripper_test.go index 32fce4f201919..168ea8ff4bf87 100644 --- a/modules/markup/mdstripper_test.go +++ b/modules/markup/mdstripper_test.go @@ -25,6 +25,7 @@ func TestMarkdownStripper(t *testing.T) { This is [one](link) to paradise. This **is emphasized**. +This: should coallesce. ` + "```" + ` This is a code block. @@ -43,6 +44,7 @@ A HIDDEN ` + "`" + `GHOST` + "`" + ` IN THIS LINE. "This", "is emphasized", ".", + "This: should coallesce.", "Bullet 1", "Bullet 2", "A HIDDEN", diff --git a/modules/references/references.go b/modules/references/references.go index 0738bbbeccec0..f31a2dd313770 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -22,49 +22,111 @@ var ( // 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|^|\(|\[)([\pL]+ )?(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) + issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) + // issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([\pL]+ )?(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) + // issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([\pL]+ )?(#[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|^|\(|\[)([\pL]+ )?([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+#[0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) + crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+#[0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) + // crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([\pL]+ )?([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 +) + +// 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 ) // RawIssueReference contains information about a cross-reference in the text type RawIssueReference struct { - Index int64 - Owner string - Name string - Keyword string + Index int64 + Owner string + Name string + Action XRefAction + RefLocation ReferenceLocation + ActionLocation ReferenceLocation +} + +type ReferenceLocation 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) } // FindAllMentions matches mention patterns in given content // and returns a list of found unvalidated user names without @ prefix. func FindAllMentions(content string) []string { - content, _ = markup.StripMarkdown([]byte(content)) - mentions := mentionPattern.FindAllStringSubmatch(content, -1) - ret := make([]string, len(mentions)) + bcontent := []byte(content) + locations := FindAllMentionLocations(bcontent) + mentions := make([]string, len(locations)) + for i, val := range locations { + mentions[i] = string(bcontent[val.Start:val.End]) + } + return mentions +} + +// FindAllMentionLocations matches mention patterns in given content +// and returns a list of found unvalidated user names without @ prefix. +func FindAllMentionLocations(content []byte) []ReferenceLocation { + content, _ = markup.StripMarkdownBytes([]byte(content)) + mentions := mentionPattern.FindAllSubmatchIndex(content, -1) + ret := make([]ReferenceLocation, len(mentions)) for i, val := range mentions { - ret[i] = val[1][1:] + ret[i] = ReferenceLocation{Start: val[2]+1, End: val[3]} } return ret } -// FindAllIssueReferences matches issue reference patterns in given content -// and returns a list of unvalidated references. +// FindAllIssueReferencesMarkdown strips content from markdown markup +// and returns a list of unvalidated references found in it. +func FindAllIssueReferencesMarkdown(content string) []*RawIssueReference { + bcontent, links := markup.StripMarkdownBytes([]byte(content)) + return FindAllIssueReferencesBytes(bcontent, links) +} + +// FindAllIssueReferences returns a list of unvalidated references found in a string. func FindAllIssueReferences(content string) []*RawIssueReference { + return FindAllIssueReferencesBytes([]byte(content), []string{}) +} + +// FindAllIssueReferencesBytes returns a list of unvalidated references found in a byte slice. +func FindAllIssueReferencesBytes(content []byte, links []string) []*RawIssueReference { - content, links := markup.StripMarkdown([]byte(content)) ret := make([]*RawIssueReference, 0, 10) - matches := issueNumericPattern.FindAllStringSubmatch(content, -1) + matches := issueNumericPattern.FindAllSubmatchIndex(content, -1) for _, match := range matches { - if ref := getCrossReference(match[2], strings.TrimSuffix(match[1], " "), false); ref != nil { + if ref := getCrossReference(content, match[2], match[3], false); ref != nil { ret = append(ret, ref) } } - matches = crossReferenceIssueNumericPattern.FindAllStringSubmatch(content, -1) + matches = crossReferenceIssueNumericPattern.FindAllSubmatchIndex(content, -1) for _, match := range matches { - if ref := getCrossReference(match[2], strings.TrimSuffix(match[1], " "), false); ref != nil { + if ref := getCrossReference(content, match[2], match[3], false); ref != nil { ret = append(ret, ref) } } @@ -90,15 +152,19 @@ func FindAllIssueReferences(content string) []*RawIssueReference { continue } // Note: closing/reopening keywords not supported with URLs - if ref := getCrossReference(parts[1]+"/"+parts[2]+"#"+parts[4], "", true); ref != nil { + bytes := []byte(parts[1]+"/"+parts[2]+"#"+parts[4]) + if ref := getCrossReference(bytes, 0, len(bytes), true); ref != nil { + ref.RefLocation = ReferenceLocation{} ret = append(ret, ref) } } } + return ret } -func getCrossReference(s string, keyword string, fromLink bool) *RawIssueReference { +func getCrossReference(content []byte, start, end int, fromLink bool) *RawIssueReference { + s := string(content[start:end]) parts := strings.Split(s, "#") if len(parts) != 2 { return nil @@ -113,7 +179,9 @@ func getCrossReference(s string, keyword string, fromLink bool) *RawIssueReferen // Markdown links must specify owner/repo return nil } - return &RawIssueReference{Index: index, Keyword: keyword} + action, location := findActionKeywords(content, start) + return &RawIssueReference{Index: index, RefLocation: ReferenceLocation{Start: start, End: end}, + Action: action, ActionLocation: location} } parts = strings.Split(strings.ToLower(repo), "/") if len(parts) != 2 { @@ -123,5 +191,20 @@ func getCrossReference(s string, keyword string, fromLink bool) *RawIssueReferen if !validNamePattern.MatchString(owner) || !validNamePattern.MatchString(name) { return nil } - return &RawIssueReference{Index: index, Owner: owner, Name: name, Keyword: keyword} + action, location := findActionKeywords(content, start) + return &RawIssueReference{Index: index, Owner: owner, Name: name, + RefLocation: ReferenceLocation{Start: start, End: end}, + Action: action, ActionLocation: location} +} + +func findActionKeywords(content []byte, start int) (XRefAction, ReferenceLocation) { + m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]); + if m != nil { + return XRefActionCloses, ReferenceLocation{Start: m[2], End: m[3]} + } + m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]); + if m != nil { + return XRefActionReopens, ReferenceLocation{Start: m[2], End: m[3]} + } + return XRefActionNone, ReferenceLocation{} } diff --git a/modules/references/references_test.go b/modules/references/references_test.go index 453c70cd003b4..d4204203af626 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -13,59 +13,182 @@ import ( ) func TestFindAllIssueReferences(t *testing.T) { - text := ` -#123 no, this is a title. - #124 yes, this is a reference. -This [one](#919) no, this is a URL fragment. -This [two](/user2/repo1/issues/921) yes. -This [three](/user2/repo1/pulls/922) yes. -This [four](http://gitea.com:3000/user3/repo4/issues/203) yes. -This [five](http://github.com/user3/repo4/issues/204) no. - -` + "```" + ` -This is a code block. -#723 no, it's a code block. -` + "```" + ` + type testFixture struct { + input string + expected []*RawIssueReference + } -This ` + "`" + `#724` + "`" + ` no, it's inline code. -This user3/repo4#200 yes. -This http://gitea.com:3000/user4/repo5/201 no, bad URL. -This http://gitea.com:3000/user4/repo5/pulls/202 yes. -This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes. -Closing #15 -I am opening #20 for you -Do you process user6/repo6#300 ? -For 999 #1235 no keyword. -Which abc. #9434 nk either. - ` - // Note, FindAllIssueReferences() processes references in this order: - // * Issue number: #123 - // * Repository/issue number: user/repo#123 - // * URL: http:// .... - expected := []*RawIssueReference{ - // Numeric references - {124, "", "", ""}, - {15, "", "", "Closing"}, - {20, "", "", "opening"}, - {1235, "", "", ""}, - {9434, "", "", ""}, - // Repository/issue references - {200, "user3", "repo4", "This"}, - {300, "user6", "repo6", "process"}, - // Link references - {921, "user2", "repo1", ""}, - {922, "user2", "repo1", ""}, - {203, "user3", "repo4", ""}, - {202, "user4", "repo5", ""}, - {205, "user4", "repo6", ""}, + fixtures := []testFixture { + { + "Simply closes: #29 yes", + []*RawIssueReference { + &RawIssueReference{ 29, "", "", XRefActionCloses, + ReferenceLocation{Start:15, End:18}, + ReferenceLocation{Start:7, End:13}, + }, + }, + }, + { + "#123 no, this is a title.", + []*RawIssueReference{ + }, + }, + { + " #124 yes, this is a reference.", + []*RawIssueReference { + &RawIssueReference{ 124, "", "", XRefActionNone, + ReferenceLocation{Start:0, End:4}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "```\nThis is a code block.\n#723 no, it's a code block.```", + []*RawIssueReference{ + }, + }, + { + "This `#724` no, it's inline code.", + []*RawIssueReference{ + }, + }, + { + "This user3/repo4#200 yes.", + []*RawIssueReference { + &RawIssueReference{ 200, "user3", "repo4", /* This */ XRefActionNone, + ReferenceLocation{Start:5, End:20}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "This [one](#919) no, this is a URL fragment.", + []*RawIssueReference{ + }, + }, + { + "This [two](/user2/repo1/issues/921) yes.", + []*RawIssueReference { + &RawIssueReference{ 921, "user2", "repo1", XRefActionNone, + ReferenceLocation{Start:0, End:0}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "This [three](/user2/repo1/pulls/922) yes.", + []*RawIssueReference { + &RawIssueReference{ 922, "user2", "repo1", XRefActionNone, + ReferenceLocation{Start:0, End:0}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "This [four](http://gitea.com:3000/user3/repo4/issues/203) yes.", + []*RawIssueReference { + &RawIssueReference{ 203, "user3", "repo4", XRefActionNone, + ReferenceLocation{Start:0, End:0}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "This [five](http://github.com/user3/repo4/issues/204) no.", + []*RawIssueReference{ + }, + }, + { + "This http://gitea.com:3000/user4/repo5/201 no, bad URL.", + []*RawIssueReference{ + }, + }, + { + "This http://gitea.com:3000/user4/repo5/pulls/202 yes.", + []*RawIssueReference { + &RawIssueReference{ 202, "user4", "repo5", XRefActionNone, + ReferenceLocation{Start:0, End:0}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.", + []*RawIssueReference { + &RawIssueReference{ 205, "user4", "repo6", XRefActionNone, + ReferenceLocation{Start:0, End:0}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "Reopens #15 yes", + []*RawIssueReference { + &RawIssueReference{ 15, "", "", XRefActionReopens, + ReferenceLocation{Start:8, End:11}, + ReferenceLocation{Start:0, End:7}, + }, + }, + }, + { + "This closes #20 for you yes", + []*RawIssueReference { + &RawIssueReference{ 20, "", "", XRefActionCloses, + ReferenceLocation{Start:12, End:15}, + ReferenceLocation{Start:5, End:11}, + }, + }, + }, + { + "Do you fix user6/repo6#300 ? yes", + []*RawIssueReference { + &RawIssueReference{ 300, "user6", "repo6", XRefActionCloses, + ReferenceLocation{Start:11, End:26}, + ReferenceLocation{Start:7, End:10}, + }, + }, + }, + { + "For 999 #1235 no keyword, but yes", + []*RawIssueReference { + &RawIssueReference{ 1235, "", "", XRefActionNone, + ReferenceLocation{Start:8, End:13}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "Which abc. #9434 same as above", + []*RawIssueReference { + &RawIssueReference{ 9434, "", "", XRefActionNone, + ReferenceLocation{Start:11, End:16}, + ReferenceLocation{Start:0, End:0}, + }, + }, + }, + { + "This closes #600 and reopens #599", + []*RawIssueReference { + &RawIssueReference{ 600, "", "", XRefActionCloses, + ReferenceLocation{Start:12, End:16}, + ReferenceLocation{Start:5, End:11}, + }, + &RawIssueReference{ 599, "", "", XRefActionReopens, + ReferenceLocation{Start:29, End:33}, + ReferenceLocation{Start:21, End:28}, + }, + }, + }, } // Save original value for other tests that may rely on it prevURL := setting.AppURL setting.AppURL = "https://gitea.com:3000/" - refs := FindAllIssueReferences(text) - assert.EqualValues(t, expected, refs) + for _, fixture := range fixtures { + refs := FindAllIssueReferencesMarkdown(fixture.input) + assert.EqualValues(t, fixture.expected, refs, "Failed to parse: {%s}", fixture.input) + } // Restore for other tests that may rely on the original value setting.AppURL = prevURL From dadadfa8a3e867e2b46a22d5002fbb20109346f4 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 16:46:51 -0300 Subject: [PATCH 08/32] Add FIXME comment --- models/action.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/models/action.go b/models/action.go index c9c4a8b4d380c..035b59bc3ab5e 100644 --- a/models/action.go +++ b/models/action.go @@ -531,6 +531,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra refMarked := make(map[markKey]bool) var refRepo *Repository + var refIssue *Issue var err error for _, ref := range references.FindAllIssueReferences(c.Message) { @@ -543,8 +544,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra } else { refRepo = repo } - refIssue, err := getIssueFromRef(refRepo, ref.Index) - if err != nil { + if refIssue, err = getIssueFromRef(refRepo, ref.Index); err != nil { return err } if refIssue == nil { @@ -569,6 +569,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra // 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 } From a118be2a50cd25492897e8b3774222a9ca932d3f Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 17:27:24 -0300 Subject: [PATCH 09/32] Fix comment --- models/action.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/models/action.go b/models/action.go index 035b59bc3ab5e..09af9029088dd 100644 --- a/models/action.go +++ b/models/action.go @@ -481,7 +481,7 @@ 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. +// 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 { @@ -551,15 +551,23 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra continue } + perm, err := GetUserRepoPermission(refRepo, doer) + if err != nil { + return err + } + key := markKey{ID: refIssue.ID, Action: ref.Action} if refMarked[key] { continue } refMarked[key] = true - 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 + // only create comments for issues if user has permission for it + if 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 + } } // Process closing/reopening keywords @@ -574,10 +582,6 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra continue } - 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, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil { From debb01aee683361476606ce8cd7743d14ca472bc Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 17:31:27 -0300 Subject: [PATCH 10/32] make fmt --- models/issue_comment.go | 8 +- models/issue_xref.go | 8 +- modules/markup/mdstripper.go | 2 +- modules/references/references.go | 38 +++---- modules/references/references_test.go | 144 ++++++++++++-------------- 5 files changed, 97 insertions(+), 103 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index f2c505e5b34fe..644d95fc095bb 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -145,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 references.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:"-"` diff --git a/models/issue_xref.go b/models/issue_xref.go index da991f0df2bd7..27c20119d506f 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -103,9 +103,9 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, plaincontent, mdcontent string) ([]*crossReference, error) { xreflist := make([]*crossReference, 0, 5) var ( - refRepo *Repository - refIssue *Issue - err error + refRepo *Repository + refIssue *Issue + err error ) allrefs := append(references.FindAllIssueReferences(plaincontent), references.FindAllIssueReferencesMarkdown(mdcontent)...) @@ -132,7 +132,7 @@ func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesCont } if refIssue != nil { xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, &crossReference{ - Issue: refIssue, + Issue: refIssue, // FIXME: currently ignore keywords // Action: ref.Action, Action: references.XRefActionNone, diff --git a/modules/markup/mdstripper.go b/modules/markup/mdstripper.go index 9518d9d9ab927..4ca6564c4c2c8 100644 --- a/modules/markup/mdstripper.go +++ b/modules/markup/mdstripper.go @@ -13,7 +13,7 @@ import ( // MarkdownStripper extends blackfriday.Renderer type MarkdownStripper struct { blackfriday.Renderer - links []string + links []string coallesce bool } diff --git a/modules/references/references.go b/modules/references/references.go index f31a2dd313770..3173aac156483 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -43,28 +43,28 @@ type XRefAction int64 const ( // XRefActionNone means the cross-reference is simply a comment - XRefActionNone XRefAction = iota // 0 + XRefActionNone XRefAction = iota // 0 // XRefActionCloses means the cross-reference should close an issue if it is resolved - XRefActionCloses // 1 + XRefActionCloses // 1 // XRefActionReopens means the cross-reference should reopen an issue if it is resolved - XRefActionReopens // 2 + XRefActionReopens // 2 // XRefActionNeutered means the cross-reference will no longer affect the source - XRefActionNeutered // 3 + XRefActionNeutered // 3 ) // RawIssueReference contains information about a cross-reference in the text type RawIssueReference struct { - Index int64 - Owner string - Name string - Action XRefAction - RefLocation ReferenceLocation - ActionLocation ReferenceLocation + Index int64 + Owner string + Name string + Action XRefAction + RefLocation ReferenceLocation + ActionLocation ReferenceLocation } type ReferenceLocation struct { - Start int - End int + Start int + End int } func makeKeywordsPat(keywords []string) *regexp.Regexp { @@ -95,7 +95,7 @@ func FindAllMentionLocations(content []byte) []ReferenceLocation { mentions := mentionPattern.FindAllSubmatchIndex(content, -1) ret := make([]ReferenceLocation, len(mentions)) for i, val := range mentions { - ret[i] = ReferenceLocation{Start: val[2]+1, End: val[3]} + ret[i] = ReferenceLocation{Start: val[2] + 1, End: val[3]} } return ret } @@ -152,7 +152,7 @@ func FindAllIssueReferencesBytes(content []byte, links []string) []*RawIssueRefe continue } // Note: closing/reopening keywords not supported with URLs - bytes := []byte(parts[1]+"/"+parts[2]+"#"+parts[4]) + bytes := []byte(parts[1] + "/" + parts[2] + "#" + parts[4]) if ref := getCrossReference(bytes, 0, len(bytes), true); ref != nil { ref.RefLocation = ReferenceLocation{} ret = append(ret, ref) @@ -181,7 +181,7 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *RawIssueR } action, location := findActionKeywords(content, start) return &RawIssueReference{Index: index, RefLocation: ReferenceLocation{Start: start, End: end}, - Action: action, ActionLocation: location} + Action: action, ActionLocation: location} } parts = strings.Split(strings.ToLower(repo), "/") if len(parts) != 2 { @@ -193,16 +193,16 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *RawIssueR } action, location := findActionKeywords(content, start) return &RawIssueReference{Index: index, Owner: owner, Name: name, - RefLocation: ReferenceLocation{Start: start, End: end}, - Action: action, ActionLocation: location} + RefLocation: ReferenceLocation{Start: start, End: end}, + Action: action, ActionLocation: location} } func findActionKeywords(content []byte, start int) (XRefAction, ReferenceLocation) { - m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]); + m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) if m != nil { return XRefActionCloses, ReferenceLocation{Start: m[2], End: m[3]} } - m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]); + m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]) if m != nil { return XRefActionReopens, ReferenceLocation{Start: m[2], End: m[3]} } diff --git a/modules/references/references_test.go b/modules/references/references_test.go index d4204203af626..9c9758c564efa 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -14,168 +14,162 @@ import ( func TestFindAllIssueReferences(t *testing.T) { type testFixture struct { - input string - expected []*RawIssueReference + input string + expected []*RawIssueReference } - fixtures := []testFixture { + fixtures := []testFixture{ { "Simply closes: #29 yes", - []*RawIssueReference { - &RawIssueReference{ 29, "", "", XRefActionCloses, - ReferenceLocation{Start:15, End:18}, - ReferenceLocation{Start:7, End:13}, - }, + []*RawIssueReference{ + {29, "", "", XRefActionCloses, + ReferenceLocation{Start: 15, End: 18}, + ReferenceLocation{Start: 7, End: 13}, + }, }, }, { "#123 no, this is a title.", - []*RawIssueReference{ - }, + []*RawIssueReference{}, }, { " #124 yes, this is a reference.", - []*RawIssueReference { - &RawIssueReference{ 124, "", "", XRefActionNone, - ReferenceLocation{Start:0, End:4}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {124, "", "", XRefActionNone, + ReferenceLocation{Start: 0, End: 4}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "```\nThis is a code block.\n#723 no, it's a code block.```", - []*RawIssueReference{ - }, + []*RawIssueReference{}, }, { "This `#724` no, it's inline code.", - []*RawIssueReference{ - }, + []*RawIssueReference{}, }, { "This user3/repo4#200 yes.", - []*RawIssueReference { - &RawIssueReference{ 200, "user3", "repo4", /* This */ XRefActionNone, - ReferenceLocation{Start:5, End:20}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {200, "user3", "repo4" /* This */, XRefActionNone, + ReferenceLocation{Start: 5, End: 20}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "This [one](#919) no, this is a URL fragment.", - []*RawIssueReference{ - }, + []*RawIssueReference{}, }, { "This [two](/user2/repo1/issues/921) yes.", - []*RawIssueReference { - &RawIssueReference{ 921, "user2", "repo1", XRefActionNone, - ReferenceLocation{Start:0, End:0}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {921, "user2", "repo1", XRefActionNone, + ReferenceLocation{Start: 0, End: 0}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "This [three](/user2/repo1/pulls/922) yes.", - []*RawIssueReference { - &RawIssueReference{ 922, "user2", "repo1", XRefActionNone, - ReferenceLocation{Start:0, End:0}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {922, "user2", "repo1", XRefActionNone, + ReferenceLocation{Start: 0, End: 0}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "This [four](http://gitea.com:3000/user3/repo4/issues/203) yes.", - []*RawIssueReference { - &RawIssueReference{ 203, "user3", "repo4", XRefActionNone, - ReferenceLocation{Start:0, End:0}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {203, "user3", "repo4", XRefActionNone, + ReferenceLocation{Start: 0, End: 0}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "This [five](http://github.com/user3/repo4/issues/204) no.", - []*RawIssueReference{ - }, + []*RawIssueReference{}, }, { "This http://gitea.com:3000/user4/repo5/201 no, bad URL.", - []*RawIssueReference{ - }, + []*RawIssueReference{}, }, { "This http://gitea.com:3000/user4/repo5/pulls/202 yes.", - []*RawIssueReference { - &RawIssueReference{ 202, "user4", "repo5", XRefActionNone, - ReferenceLocation{Start:0, End:0}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {202, "user4", "repo5", XRefActionNone, + ReferenceLocation{Start: 0, End: 0}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.", - []*RawIssueReference { - &RawIssueReference{ 205, "user4", "repo6", XRefActionNone, - ReferenceLocation{Start:0, End:0}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {205, "user4", "repo6", XRefActionNone, + ReferenceLocation{Start: 0, End: 0}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "Reopens #15 yes", - []*RawIssueReference { - &RawIssueReference{ 15, "", "", XRefActionReopens, - ReferenceLocation{Start:8, End:11}, - ReferenceLocation{Start:0, End:7}, + []*RawIssueReference{ + {15, "", "", XRefActionReopens, + ReferenceLocation{Start: 8, End: 11}, + ReferenceLocation{Start: 0, End: 7}, }, }, }, { "This closes #20 for you yes", - []*RawIssueReference { - &RawIssueReference{ 20, "", "", XRefActionCloses, - ReferenceLocation{Start:12, End:15}, - ReferenceLocation{Start:5, End:11}, + []*RawIssueReference{ + {20, "", "", XRefActionCloses, + ReferenceLocation{Start: 12, End: 15}, + ReferenceLocation{Start: 5, End: 11}, }, }, }, { "Do you fix user6/repo6#300 ? yes", - []*RawIssueReference { - &RawIssueReference{ 300, "user6", "repo6", XRefActionCloses, - ReferenceLocation{Start:11, End:26}, - ReferenceLocation{Start:7, End:10}, + []*RawIssueReference{ + {300, "user6", "repo6", XRefActionCloses, + ReferenceLocation{Start: 11, End: 26}, + ReferenceLocation{Start: 7, End: 10}, }, }, }, { "For 999 #1235 no keyword, but yes", - []*RawIssueReference { - &RawIssueReference{ 1235, "", "", XRefActionNone, - ReferenceLocation{Start:8, End:13}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {1235, "", "", XRefActionNone, + ReferenceLocation{Start: 8, End: 13}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "Which abc. #9434 same as above", - []*RawIssueReference { - &RawIssueReference{ 9434, "", "", XRefActionNone, - ReferenceLocation{Start:11, End:16}, - ReferenceLocation{Start:0, End:0}, + []*RawIssueReference{ + {9434, "", "", XRefActionNone, + ReferenceLocation{Start: 11, End: 16}, + ReferenceLocation{Start: 0, End: 0}, }, }, }, { "This closes #600 and reopens #599", - []*RawIssueReference { - &RawIssueReference{ 600, "", "", XRefActionCloses, - ReferenceLocation{Start:12, End:16}, - ReferenceLocation{Start:5, End:11}, + []*RawIssueReference{ + {600, "", "", XRefActionCloses, + ReferenceLocation{Start: 12, End: 16}, + ReferenceLocation{Start: 5, End: 11}, }, - &RawIssueReference{ 599, "", "", XRefActionReopens, - ReferenceLocation{Start:29, End:33}, - ReferenceLocation{Start:21, End:28}, + {599, "", "", XRefActionReopens, + ReferenceLocation{Start: 29, End: 33}, + ReferenceLocation{Start: 21, End: 28}, }, }, }, From 163ec65c3f6466ae93bbb220076ba5efa9266891 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 18:46:13 -0300 Subject: [PATCH 11/32] Fix permissions check --- models/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/action.go b/models/action.go index 09af9029088dd..45c2dafb0e607 100644 --- a/models/action.go +++ b/models/action.go @@ -563,7 +563,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra refMarked[key] = true // only create comments for issues if user has permission for it - if perm.CanWrite(UnitTypeIssues) { + 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 From 4796f433a0d5f1a7bbf96b19edfcd709e4858b0c Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 18:46:25 -0300 Subject: [PATCH 12/32] Fix text assumptions --- models/action_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/action_test.go b/models/action_test.go index 23ffbbb5f307f..ff312d5217bee 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -400,7 +400,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{}) } From 4c757232daff3d819c2368855dce88110d366f7c Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 18:51:53 -0300 Subject: [PATCH 13/32] Fix imports --- integrations/issue_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/integrations/issue_test.go b/integrations/issue_test.go index 0b153607ee645..48607c998a136 100644 --- a/integrations/issue_test.go +++ b/integrations/issue_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/test" "github.com/PuerkitoBio/goquery" @@ -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) { From 79a727598ba19b5af97d28313e29266438e0b06e Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 20:33:05 -0300 Subject: [PATCH 14/32] Fix lint, fmt --- integrations/issue_test.go | 2 +- models/action.go | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/integrations/issue_test.go b/integrations/issue_test.go index 48607c998a136..aa17c44254fc2 100644 --- a/integrations/issue_test.go +++ b/integrations/issue_test.go @@ -13,8 +13,8 @@ import ( "testing" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/references" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" "github.com/PuerkitoBio/goquery" diff --git a/models/action.go b/models/action.go index 45c2dafb0e607..769b78e8c02c1 100644 --- a/models/action.go +++ b/models/action.go @@ -352,10 +352,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 From 98ea87e2553447963b5c8757499449996047bbfd Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 20:34:52 -0300 Subject: [PATCH 15/32] Fix unused import --- models/action.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/action.go b/models/action.go index 769b78e8c02c1..f5d9a13ecea20 100644 --- a/models/action.go +++ b/models/action.go @@ -14,7 +14,6 @@ import ( "strconv" "strings" "time" - "unicode" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" From 361ba2eac4a9530355a86c1440cb6b1f2c5ecf24 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 20:44:24 -0300 Subject: [PATCH 16/32] Add missing export comment --- modules/references/references.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/references/references.go b/modules/references/references.go index 3173aac156483..74fee37c96b5e 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -62,6 +62,7 @@ type RawIssueReference struct { ActionLocation ReferenceLocation } +// ReferenceLocation is the position where the references was found within the parsed text type ReferenceLocation struct { Start int End int From ac57ac501377a9a38cdf7f4a03ec5bcac7459060 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 22 Sep 2019 21:08:24 -0300 Subject: [PATCH 17/32] Bypass revive on implemented interface --- modules/markup/mdstripper.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/markup/mdstripper.go b/modules/markup/mdstripper.go index 4ca6564c4c2c8..d6c2297fc5b5b 100644 --- a/modules/markup/mdstripper.go +++ b/modules/markup/mdstripper.go @@ -33,6 +33,8 @@ const ( 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) { @@ -230,6 +232,8 @@ func (r *MarkdownStripper) GetFlags() int { return 0 } +//revive:enable:var-naming + func doubleSpace(out *bytes.Buffer) { if out.Len() > 0 { out.WriteByte('\n') From caa53b732b6818d9973e517e5bdc3ff0e245e058 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 23 Sep 2019 22:30:12 -0300 Subject: [PATCH 18/32] Move mdstripper into its own package --- modules/markup/{ => mdstripper}/mdstripper.go | 2 +- modules/markup/{ => mdstripper}/mdstripper_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename modules/markup/{ => mdstripper}/mdstripper.go (99%) rename modules/markup/{ => mdstripper}/mdstripper_test.go (98%) diff --git a/modules/markup/mdstripper.go b/modules/markup/mdstripper/mdstripper.go similarity index 99% rename from modules/markup/mdstripper.go rename to modules/markup/mdstripper/mdstripper.go index d6c2297fc5b5b..7a901b17a925f 100644 --- a/modules/markup/mdstripper.go +++ b/modules/markup/mdstripper/mdstripper.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package markup +package mdstripper import ( "bytes" diff --git a/modules/markup/mdstripper_test.go b/modules/markup/mdstripper/mdstripper_test.go similarity index 98% rename from modules/markup/mdstripper_test.go rename to modules/markup/mdstripper/mdstripper_test.go index 168ea8ff4bf87..157fe1975b16b 100644 --- a/modules/markup/mdstripper_test.go +++ b/modules/markup/mdstripper/mdstripper_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package markup +package mdstripper import ( "strings" From b9360155c210cf639fdc5ae456f681be0d039fd5 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 23 Sep 2019 22:31:05 -0300 Subject: [PATCH 19/32] Support alphanumeric patterns --- modules/references/references.go | 73 +++++++++++++++++++++------ modules/references/references_test.go | 64 +++++++++++++++++++++++ 2 files changed, 121 insertions(+), 16 deletions(-) diff --git a/modules/references/references.go b/modules/references/references.go index 74fee37c96b5e..5c514d2a47be7 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -10,7 +10,7 @@ import ( "strconv" "strings" - "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/mdstripper" "code.gitea.io/gitea/modules/setting" ) @@ -23,12 +23,11 @@ var ( 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|$))`) - // issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([\pL]+ )?(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) - // issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([\pL]+ )?(#[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|$))`) - // crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([\pL]+ )?([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 @@ -62,6 +61,14 @@ type RawIssueReference struct { ActionLocation ReferenceLocation } +// RawAlphanumIssueReference contains information about an external reference in the text +type RawAlphanumIssueReference struct { + Index string + Action XRefAction + RefLocation ReferenceLocation + ActionLocation ReferenceLocation +} + // ReferenceLocation is the position where the references was found within the parsed text type ReferenceLocation struct { Start int @@ -77,34 +84,43 @@ func init() { issueReopenKeywordsPat = makeKeywordsPat(issueReopenKeywords) } -// FindAllMentions matches mention patterns in given content -// and returns a list of found unvalidated user names without @ prefix. -func FindAllMentions(content string) []string { - bcontent := []byte(content) - locations := FindAllMentionLocations(bcontent) +// 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:val.End]) + mentions[i] = string(bcontent[val.Start+1 : val.End]) } return mentions } -// FindAllMentionLocations matches mention patterns in given content -// and returns a list of found unvalidated user names without @ prefix. -func FindAllMentionLocations(content []byte) []ReferenceLocation { - content, _ = markup.StripMarkdownBytes([]byte(content)) +// FindAllMentionsBytes matches mention patterns in given content +// and returns a list of found unvalidated user names including the @ prefix. +func FindAllMentionsBytes(content []byte) []ReferenceLocation { mentions := mentionPattern.FindAllSubmatchIndex(content, -1) ret := make([]ReferenceLocation, len(mentions)) for i, val := range mentions { - ret[i] = ReferenceLocation{Start: val[2] + 1, End: val[3]} + ret[i] = ReferenceLocation{Start: val[2], End: val[3]} } return ret } +// FindFirstMentionBytes matches mention patterns in given content +// and returns a list of found unvalidated user names including the @ prefix. +func FindFirstMentionBytes(content []byte) (bool, ReferenceLocation) { + mention := mentionPattern.FindSubmatchIndex(content) + if mention == nil { + return false, ReferenceLocation{} + } + return true, ReferenceLocation{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) []*RawIssueReference { - bcontent, links := markup.StripMarkdownBytes([]byte(content)) + bcontent, links := mdstripper.StripMarkdownBytes([]byte(content)) return FindAllIssueReferencesBytes(bcontent, links) } @@ -113,6 +129,31 @@ func FindAllIssueReferences(content string) []*RawIssueReference { return FindAllIssueReferencesBytes([]byte(content), []string{}) } +// FindFirstIssueReferenceBytes returns the first unvalidated references found in a byte slice +func FindFirstIssueReferenceBytes(content []byte) (bool, *RawIssueReference) { + match := issueNumericPattern.FindSubmatchIndex(content) + if match == nil { + if match = crossReferenceIssueNumericPattern.FindSubmatchIndex(content); match == nil { + return false, nil + } + } + + return true, getCrossReference(content, match[2], match[3], false) +} + +// FindFirstAlphanumericIssueReferenceBytes returns the first alphanumeric unvalidated references found in a byte slice +func FindFirstAlphanumericIssueReferenceBytes(content []byte) (bool, *RawAlphanumIssueReference) { + match := issueAlphanumericPattern.FindSubmatchIndex(content) + if match == nil { + return false, nil + } + + action, location := findActionKeywords(content, match[2]) + return true, &RawAlphanumIssueReference{Index: string(content[match[2]:match[3]]), + RefLocation: ReferenceLocation{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) []*RawIssueReference { diff --git a/modules/references/references_test.go b/modules/references/references_test.go index 9c9758c564efa..1a9d6c03fb123 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -186,4 +186,68 @@ func TestFindAllIssueReferences(t *testing.T) { // Restore for other tests that may rely on the original value setting.AppURL = prevURL + + type alnumFixture struct { + input string + expected *RawAlphanumIssueReference + } + + alnumFixtures := []alnumFixture{ + { + "This ref ABC-123 is alphanumeric", + &RawAlphanumIssueReference{ + "ABC-123", XRefActionNone, + ReferenceLocation{Start: 9, End: 16}, + ReferenceLocation{Start: 0, End: 0}, + }, + }, + { + "This closes ABCD-1234 alphanumeric", + &RawAlphanumIssueReference{ + "ABCD-1234", XRefActionCloses, + ReferenceLocation{Start: 12, End: 21}, + ReferenceLocation{Start: 5, End: 11}, + }, + }, + } + + for _, fixture := range alnumFixtures { + found, ref := FindFirstAlphanumericIssueReferenceBytes([]byte(fixture.input)) + if fixture.expected == nil { + assert.False(t, found, "Failed to parse: {%s}", fixture.input) + } else { + assert.True(t, found, "Failed to parse: {%s}", fixture.input) + assert.EqualValues(t, fixture.expected, ref, "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) + } } From c4c467c5ede53900e705124762efecf1e3328c9f Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 23 Sep 2019 22:31:34 -0300 Subject: [PATCH 20/32] Refactor FindAllMentions --- models/issue_comment.go | 2 +- models/issue_mail.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 644d95fc095bb..c2662660f334e 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -402,7 +402,7 @@ func (c *Comment) MailParticipants(opType ActionType, issue *Issue) (err error) } func (c *Comment) mailParticipants(e Engine, opType ActionType, issue *Issue) (err error) { - mentions := references.FindAllMentions(c.Content) + mentions := references.FindAllMentionsMarkdown(c.Content) if err = UpdateIssueMentions(e, c.IssueID, mentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err) } diff --git a/models/issue_mail.go b/models/issue_mail.go index 1b16f1fe544d5..577decf29d76f 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -123,7 +123,7 @@ func (issue *Issue) MailParticipants(doer *User, opType ActionType) (err error) } func (issue *Issue) mailParticipants(e Engine, doer *User, opType ActionType) (err error) { - mentions := references.FindAllMentions(issue.Content) + mentions := references.FindAllMentionsMarkdown(issue.Content) if err = UpdateIssueMentions(e, issue.ID, mentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) From fdb45e6005a26017872228e78204fdaaa798c892 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 23 Sep 2019 22:31:56 -0300 Subject: [PATCH 21/32] Move mentions test to references --- modules/markup/html_internal_test.go | 30 ---------------------------- 1 file changed, 30 deletions(-) diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 2824ce3e6817e..6d40effc0fa0e 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -325,36 +325,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", From 70684f56693a0cef8581254c9e458ed9b45e4c97 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 23 Sep 2019 22:32:17 -0300 Subject: [PATCH 22/32] Parse mentions from reference package --- modules/markup/html.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 78a06039d1676..9c182005b909e 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" @@ -37,7 +38,7 @@ var ( // TODO: fix invalid linking issue // mentionPattern matches all mentions in the form of "@user" - mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_\.]+)(?:\s|$|\)|\])`) + // 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|$))`) @@ -391,13 +392,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) { From b9d709b47688a4821b2a29cd4505e9111864ec4a Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 24 Sep 2019 21:41:20 -0300 Subject: [PATCH 23/32] Refactor code to implement renderizable references --- modules/markup/html.go | 49 +++----- modules/references/references.go | 168 ++++++++++++++++++------- modules/references/references_test.go | 175 ++++++++++++-------------- 3 files changed, 217 insertions(+), 175 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 9c182005b909e..c225d81bfe253 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -132,7 +132,6 @@ var defaultProcessors = []processor{ linkProcessor, mentionProcessor, issueIndexPatternProcessor, - crossReferenceIssueIndexPatternProcessor, sha1CurrentPatternProcessor, emailAddressProcessor, } @@ -173,7 +172,6 @@ var commitMessageProcessors = []processor{ linkProcessor, mentionProcessor, issueIndexPatternProcessor, - crossReferenceIssueIndexPatternProcessor, sha1CurrentPatternProcessor, emailAddressProcessor, } @@ -207,7 +205,6 @@ var commitMessageSubjectProcessors = []processor{ linkProcessor, mentionProcessor, issueIndexPatternProcessor, - crossReferenceIssueIndexPatternProcessor, sha1CurrentPatternProcessor, } @@ -587,45 +584,33 @@ 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 + found, ref = references.FindRenderizableReferenceAlphanumeric(node.Data) + } else { + found, ref = references.FindRenderizableReferenceNumeric(node.Data) } - match := pattern.FindStringSubmatchIndex(node.Data) - if match == nil { + if !found { return } - id := node.Data[match[2]:match[3]] var link *html.Node + reftext := node.Data[ref.RefLocation().Start:ref.RefLocation().End] 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") + 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, ctx.metas["user"], ctx.metas["repo"], "issues", id[1:]), id, "issue") - } - replaceContent(node, match[2], match[3], link) -} - -func crossReferenceIssueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { - m := crossReferenceIssueNumericPattern.FindStringSubmatchIndex(node.Data) - if m == nil { - return + link = createLink(util.URLJoin(setting.AppURL, ref.Owner(), ref.Name(), "issues", ref.Issue()), reftext, "issue") } - ref := node.Data[m[2]:m[3]] - - parts := strings.SplitN(ref, "#", 2) - repo, issue := parts[0], parts[1] - - replaceContent(node, m[2], m[3], - createLink(util.URLJoin(setting.AppURL, repo, "issues", issue), ref, issue)) + replaceContent(node, ref.RefLocation().Start, ref.RefLocation().End, link) } // fullSha1PatternProcessor renders SHA containing URLs diff --git a/modules/references/references.go b/modules/references/references.go index 5c514d2a47be7..1018e769ea932 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -51,25 +51,83 @@ const ( XRefActionNeutered // 3 ) -// RawIssueReference contains information about a cross-reference in the text -type RawIssueReference struct { - Index int64 - Owner string - Name string - Action XRefAction - RefLocation ReferenceLocation - ActionLocation ReferenceLocation +// IssueReference contains an unverified cross-reference to a local issue/pull request +type IssueReference struct { + Index int64 + Owner string + Name string + Action XRefAction } -// RawAlphanumIssueReference contains information about an external reference in the text -type RawAlphanumIssueReference struct { - Index string - Action XRefAction - RefLocation ReferenceLocation - ActionLocation ReferenceLocation +// RenderizableReference contains an unverified cross-reference to with rendering information +type RenderizableReference interface { + Issue() string + Owner() string + Name() string + RefLocation() *ReferenceLocation + Action() XRefAction + ActionLocation() *ReferenceLocation } -// ReferenceLocation is the position where the references was found within the parsed text +type rawReference struct { + index int64 + owner string + name string + action XRefAction + issue string + refLocation *ReferenceLocation + actionLocation *ReferenceLocation +} + +// Index returns the number of the issue/pull request +func (r *rawReference) Index() int64 { + return r.index +} + +// Owner returns the owner of the repository for the issue/pull request +func (r *rawReference) Owner() string { + return r.owner +} + +// Owner returns the name of the repository for the issue/pull request +func (r *rawReference) Name() string { + return r.name +} + +// Issue returns the ID of the issue/pull request +func (r *rawReference) Issue() string { + return r.issue +} + +// RefLocation returns the location of the reference in the originating string +func (r *rawReference) RefLocation() *ReferenceLocation { + return r.refLocation +} + +// Action returns the action represented by the action keyword found preceeding the reference +func (r *rawReference) Action() XRefAction { + return r.action +} + +// RefLocation returns the location of the action keyword in the originating string +func (r *rawReference) ActionLocation() *ReferenceLocation { + return r.actionLocation +} + +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 +} + +// ReferenceLocation is the position where the reference was found within the parsed text type ReferenceLocation struct { Start int End int @@ -119,45 +177,52 @@ func FindFirstMentionBytes(content []byte) (bool, ReferenceLocation) { // FindAllIssueReferencesMarkdown strips content from markdown markup // and returns a list of unvalidated references found in it. -func FindAllIssueReferencesMarkdown(content string) []*RawIssueReference { +func FindAllIssueReferencesMarkdown(content string) []IssueReference { + return rawToIssueReferenceList(findAllIssueReferencesMarkdown(content)) +} + +func findAllIssueReferencesMarkdown(content string) []*rawReference { bcontent, links := mdstripper.StripMarkdownBytes([]byte(content)) - return FindAllIssueReferencesBytes(bcontent, links) + return findAllIssueReferencesBytes(bcontent, links) } // FindAllIssueReferences returns a list of unvalidated references found in a string. -func FindAllIssueReferences(content string) []*RawIssueReference { - return FindAllIssueReferencesBytes([]byte(content), []string{}) +func FindAllIssueReferences(content string) []IssueReference { + return rawToIssueReferenceList(findAllIssueReferencesBytes([]byte(content), []string{})) } -// FindFirstIssueReferenceBytes returns the first unvalidated references found in a byte slice -func FindFirstIssueReferenceBytes(content []byte) (bool, *RawIssueReference) { - match := issueNumericPattern.FindSubmatchIndex(content) +// FindRenderizableReferenceNumeric returns the first unvalidated references found in a byte slice +func FindRenderizableReferenceNumeric(content string) (bool, RenderizableReference) { + match := issueNumericPattern.FindStringSubmatchIndex(content) if match == nil { - if match = crossReferenceIssueNumericPattern.FindSubmatchIndex(content); match == nil { + if match = crossReferenceIssueNumericPattern.FindStringSubmatchIndex(content); match == nil { return false, nil } } - return true, getCrossReference(content, match[2], match[3], false) + return true, getCrossReference([]byte(content), match[2], match[3], false) } -// FindFirstAlphanumericIssueReferenceBytes returns the first alphanumeric unvalidated references found in a byte slice -func FindFirstAlphanumericIssueReferenceBytes(content []byte) (bool, *RawAlphanumIssueReference) { - match := issueAlphanumericPattern.FindSubmatchIndex(content) +// FindRenderizableReferenceAlphanumeric returns the first alphanumeric unvalidated references found in a byte slice +func FindRenderizableReferenceAlphanumeric(content string) (bool, RenderizableReference) { + match := issueAlphanumericPattern.FindStringSubmatchIndex(content) if match == nil { return false, nil } - action, location := findActionKeywords(content, match[2]) - return true, &RawAlphanumIssueReference{Index: string(content[match[2]:match[3]]), - RefLocation: ReferenceLocation{Start: match[2], End: match[3]}, - Action: action, ActionLocation: location} + action, location := findActionKeywords([]byte(content), match[2]) + return true, &rawReference{ + issue: string(content[match[2]:match[3]]), + refLocation: &ReferenceLocation{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) []*RawIssueReference { +func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference { - ret := make([]*RawIssueReference, 0, 10) + ret := make([]*rawReference, 0, 10) matches := issueNumericPattern.FindAllSubmatchIndex(content, -1) for _, match := range matches { @@ -196,7 +261,7 @@ func FindAllIssueReferencesBytes(content []byte, links []string) []*RawIssueRefe // 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 = ReferenceLocation{} + ref.refLocation = nil ret = append(ret, ref) } } @@ -205,9 +270,9 @@ func FindAllIssueReferencesBytes(content []byte, links []string) []*RawIssueRefe return ret } -func getCrossReference(content []byte, start, end int, fromLink bool) *RawIssueReference { - s := string(content[start:end]) - parts := strings.Split(s, "#") +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 } @@ -222,8 +287,13 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *RawIssueR return nil } action, location := findActionKeywords(content, start) - return &RawIssueReference{Index: index, RefLocation: ReferenceLocation{Start: start, End: end}, - Action: action, ActionLocation: location} + return &rawReference{ + index: index, + action: action, + issue: issue, + refLocation: &ReferenceLocation{Start: start, End: end}, + actionLocation: location, + } } parts = strings.Split(strings.ToLower(repo), "/") if len(parts) != 2 { @@ -234,19 +304,25 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *RawIssueR return nil } action, location := findActionKeywords(content, start) - return &RawIssueReference{Index: index, Owner: owner, Name: name, - RefLocation: ReferenceLocation{Start: start, End: end}, - Action: action, ActionLocation: location} + return &rawReference{ + index: index, + owner: owner, + name: name, + action: action, + issue: issue, + refLocation: &ReferenceLocation{Start: start, End: end}, + actionLocation: location, + } } -func findActionKeywords(content []byte, start int) (XRefAction, ReferenceLocation) { +func findActionKeywords(content []byte, start int) (XRefAction, *ReferenceLocation) { m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) if m != nil { - return XRefActionCloses, ReferenceLocation{Start: m[2], End: m[3]} + return XRefActionCloses, &ReferenceLocation{Start: m[2], End: m[3]} } m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]) if m != nil { - return XRefActionReopens, ReferenceLocation{Start: m[2], End: m[3]} + return XRefActionReopens, &ReferenceLocation{Start: m[2], End: m[3]} } - return XRefActionNone, ReferenceLocation{} + return XRefActionNone, nil } diff --git a/modules/references/references_test.go b/modules/references/references_test.go index 1a9d6c03fb123..537de212dddbf 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -13,164 +13,130 @@ import ( ) func TestFindAllIssueReferences(t *testing.T) { + + type result struct { + Index int64 + Owner string + Name string + Issue string + Action XRefAction + RefLocation *ReferenceLocation + ActionLocation *ReferenceLocation + } + type testFixture struct { input string - expected []*RawIssueReference + expected []result } fixtures := []testFixture{ { "Simply closes: #29 yes", - []*RawIssueReference{ - {29, "", "", XRefActionCloses, - ReferenceLocation{Start: 15, End: 18}, - ReferenceLocation{Start: 7, End: 13}, - }, + []result{ + {29, "", "", "29", XRefActionCloses, &ReferenceLocation{Start: 15, End: 18}, &ReferenceLocation{Start: 7, End: 13}}, }, }, { "#123 no, this is a title.", - []*RawIssueReference{}, + []result{}, }, { " #124 yes, this is a reference.", - []*RawIssueReference{ - {124, "", "", XRefActionNone, - ReferenceLocation{Start: 0, End: 4}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {124, "", "", "124", XRefActionNone, &ReferenceLocation{Start: 0, End: 4}, nil}, }, }, { "```\nThis is a code block.\n#723 no, it's a code block.```", - []*RawIssueReference{}, + []result{}, }, { "This `#724` no, it's inline code.", - []*RawIssueReference{}, + []result{}, }, { "This user3/repo4#200 yes.", - []*RawIssueReference{ - {200, "user3", "repo4" /* This */, XRefActionNone, - ReferenceLocation{Start: 5, End: 20}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {200, "user3", "repo4", "200", XRefActionNone, &ReferenceLocation{Start: 5, End: 20}, nil}, }, }, { "This [one](#919) no, this is a URL fragment.", - []*RawIssueReference{}, + []result{}, }, { "This [two](/user2/repo1/issues/921) yes.", - []*RawIssueReference{ - {921, "user2", "repo1", XRefActionNone, - ReferenceLocation{Start: 0, End: 0}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {921, "user2", "repo1", "921", XRefActionNone, nil, nil}, }, }, { "This [three](/user2/repo1/pulls/922) yes.", - []*RawIssueReference{ - {922, "user2", "repo1", XRefActionNone, - ReferenceLocation{Start: 0, End: 0}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {922, "user2", "repo1", "922", XRefActionNone, nil, nil}, }, }, { "This [four](http://gitea.com:3000/user3/repo4/issues/203) yes.", - []*RawIssueReference{ - {203, "user3", "repo4", XRefActionNone, - ReferenceLocation{Start: 0, End: 0}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {203, "user3", "repo4", "203", XRefActionNone, nil, nil}, }, }, { "This [five](http://github.com/user3/repo4/issues/204) no.", - []*RawIssueReference{}, + []result{}, }, { "This http://gitea.com:3000/user4/repo5/201 no, bad URL.", - []*RawIssueReference{}, + []result{}, }, { "This http://gitea.com:3000/user4/repo5/pulls/202 yes.", - []*RawIssueReference{ - {202, "user4", "repo5", XRefActionNone, - ReferenceLocation{Start: 0, End: 0}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {202, "user4", "repo5", "202", XRefActionNone, nil, nil}, }, }, { "This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.", - []*RawIssueReference{ - {205, "user4", "repo6", XRefActionNone, - ReferenceLocation{Start: 0, End: 0}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {205, "user4", "repo6", "205", XRefActionNone, nil, nil}, }, }, { "Reopens #15 yes", - []*RawIssueReference{ - {15, "", "", XRefActionReopens, - ReferenceLocation{Start: 8, End: 11}, - ReferenceLocation{Start: 0, End: 7}, - }, + []result{ + {15, "", "", "15", XRefActionReopens, &ReferenceLocation{Start: 8, End: 11}, &ReferenceLocation{Start: 0, End: 7}}, }, }, { "This closes #20 for you yes", - []*RawIssueReference{ - {20, "", "", XRefActionCloses, - ReferenceLocation{Start: 12, End: 15}, - ReferenceLocation{Start: 5, End: 11}, - }, + []result{ + {20, "", "", "20", XRefActionCloses, &ReferenceLocation{Start: 12, End: 15}, &ReferenceLocation{Start: 5, End: 11}}, }, }, { "Do you fix user6/repo6#300 ? yes", - []*RawIssueReference{ - {300, "user6", "repo6", XRefActionCloses, - ReferenceLocation{Start: 11, End: 26}, - ReferenceLocation{Start: 7, End: 10}, - }, + []result{ + {300, "user6", "repo6", "300", XRefActionCloses, &ReferenceLocation{Start: 11, End: 26}, &ReferenceLocation{Start: 7, End: 10}}, }, }, { "For 999 #1235 no keyword, but yes", - []*RawIssueReference{ - {1235, "", "", XRefActionNone, - ReferenceLocation{Start: 8, End: 13}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {1235, "", "", "1235", XRefActionNone, &ReferenceLocation{Start: 8, End: 13}, nil}, }, }, { "Which abc. #9434 same as above", - []*RawIssueReference{ - {9434, "", "", XRefActionNone, - ReferenceLocation{Start: 11, End: 16}, - ReferenceLocation{Start: 0, End: 0}, - }, + []result{ + {9434, "", "", "9434", XRefActionNone, &ReferenceLocation{Start: 11, End: 16}, nil}, }, }, { "This closes #600 and reopens #599", - []*RawIssueReference{ - {600, "", "", XRefActionCloses, - ReferenceLocation{Start: 12, End: 16}, - ReferenceLocation{Start: 5, End: 11}, - }, - {599, "", "", XRefActionReopens, - ReferenceLocation{Start: 29, End: 33}, - ReferenceLocation{Start: 21, End: 28}, - }, + []result{ + {600, "", "", "600", XRefActionCloses, &ReferenceLocation{Start: 12, End: 16}, &ReferenceLocation{Start: 5, End: 11}}, + {599, "", "", "599", XRefActionReopens, &ReferenceLocation{Start: 29, End: 33}, &ReferenceLocation{Start: 21, End: 28}}, }, }, } @@ -180,44 +146,59 @@ func TestFindAllIssueReferences(t *testing.T) { 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, fixture.expected, refs, "Failed to parse: {%s}", 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 - expected *RawAlphanumIssueReference + input string + issue string + refLocation *ReferenceLocation + action XRefAction + actionLocation *ReferenceLocation } alnumFixtures := []alnumFixture{ { "This ref ABC-123 is alphanumeric", - &RawAlphanumIssueReference{ - "ABC-123", XRefActionNone, - ReferenceLocation{Start: 9, End: 16}, - ReferenceLocation{Start: 0, End: 0}, - }, + "ABC-123", &ReferenceLocation{Start: 9, End: 16}, + XRefActionNone, nil, }, { "This closes ABCD-1234 alphanumeric", - &RawAlphanumIssueReference{ - "ABCD-1234", XRefActionCloses, - ReferenceLocation{Start: 12, End: 21}, - ReferenceLocation{Start: 5, End: 11}, - }, + "ABCD-1234", &ReferenceLocation{Start: 12, End: 21}, + XRefActionCloses, &ReferenceLocation{Start: 5, End: 11}, }, } for _, fixture := range alnumFixtures { - found, ref := FindFirstAlphanumericIssueReferenceBytes([]byte(fixture.input)) - if fixture.expected == nil { + 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.EqualValues(t, fixture.expected, ref, "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) } } } From b763968d0d016102cf593e054f30ca4a87a33ea3 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 24 Sep 2019 21:46:51 -0300 Subject: [PATCH 24/32] Fix typo --- modules/references/references.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/references/references.go b/modules/references/references.go index 1018e769ea932..68a1467ce29dc 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -104,7 +104,7 @@ func (r *rawReference) RefLocation() *ReferenceLocation { return r.refLocation } -// Action returns the action represented by the action keyword found preceeding the reference +// Action returns the action represented by the action keyword found preceding the reference func (r *rawReference) Action() XRefAction { return r.action } From 06b0f5191223c7a71cc3fc3fe303abe31fa2fbc9 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 24 Sep 2019 21:48:41 -0300 Subject: [PATCH 25/32] Move patterns and tests to the references package --- modules/markup/html.go | 11 ----- modules/markup/html_internal_test.go | 62 --------------------------- modules/references/references.go | 5 +++ modules/references/references_test.go | 62 +++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 73 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index c225d81bfe253..db59108cddb8e 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -37,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. diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 6d40effc0fa0e..9722063e177a2 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,40 +297,6 @@ func TestRegExp_anySHA1Pattern(t *testing.T) { } } -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/references/references.go b/modules/references/references.go index 68a1467ce29dc..194412420f2e6 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -19,6 +19,11 @@ var ( // 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 diff --git a/modules/references/references_test.go b/modules/references/references_test.go index 537de212dddbf..58db0d01e6b55 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -232,3 +232,65 @@ func TestRegExp_mentionPattern(t *testing.T) { 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)) + } +} From a00c798bb190654a5c3347c41706b30b527e130b Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 25 Sep 2019 00:04:30 -0300 Subject: [PATCH 26/32] Fix nil reference --- models/issue_xref.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_xref.go b/models/issue_xref.go index 27c20119d506f..141a7e0e8cc71 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -167,7 +167,7 @@ func (issue *Issue) findReferencedIssue(e Engine, ctx *crossReferencesContext, r return nil, err } // Check user permissions - if refIssue.Repo.ID != ctx.OrigIssue.Repo.ID { + if refIssue.RepoID != ctx.OrigIssue.RepoID { perm, err := getUserRepoPermission(e, refIssue.Repo, ctx.Doer) if err != nil { return nil, err From 86687724e770120045b3cdd664216da7039e0ece Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 25 Sep 2019 22:15:12 -0300 Subject: [PATCH 27/32] Preliminary rendering attempt of closing keywords --- modules/markup/html.go | 57 +++++++++++++++++++++++++++++++++---- modules/markup/sanitizer.go | 5 ++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index db59108cddb8e..086f12d9c9961 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -60,6 +60,9 @@ var ( linkRegex, _ = xurls.StrictMatchingScheme("https?://") ) +// CSS class for opening/closing keywords +const keywordClass = "issue-keyword" + // regexp for full links to issues/pulls var issueFullPattern *regexp.Regexp @@ -306,6 +309,30 @@ func (ctx *postProcessCtx) textNode(node *html.Node) { } } +// createKeyword() renders a highlited version of a closing/reopening keyowrd +func createKeyword(content string, class string) *html.Node { + span := &html.Node{ + Type: html.ElementNode, + Data: atom.Span.String(), + Attr: []html.Attribute{}, + } + if class != "" { + span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: class}) + } else { + span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: keywordClass}) + } + // GAP: FIXME: move this to CSS + span.Attr = append(span.Attr, html.Attribute{Key: "style", Val: "border-bottom: 1px dotted #959da5; display: inline-block;"}) + + 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, @@ -353,10 +380,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:] @@ -368,7 +401,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, @@ -599,7 +634,19 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { } else { link = createLink(util.URLJoin(setting.AppURL, ref.Owner(), ref.Name(), "issues", ref.Issue()), reftext, "issue") } - replaceContent(node, ref.RefLocation().Start, ref.RefLocation().End, link) + + 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/sanitizer.go b/modules/markup/sanitizer.go index 2ec43cf4fd3c8..bb677b9b207d1 100644 --- a/modules/markup/sanitizer.go +++ b/modules/markup/sanitizer.go @@ -38,6 +38,11 @@ func NewSanitizer() { // Custom URL-Schemes sanitizer.policy.AllowURLSchemes(setting.Markdown.CustomURLSchemes...) + + // Allow keyword markup + sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^` + keywordClass + `$`)).OnElements("span") + // GAP: FIXME: remove style + sanitizer.policy.AllowAttrs("style").Matching(regexp.MustCompile(`^border-bottom: 1px dotted #959da5; display: inline-block;$`)).OnElements("span") }) } From fc7e278434b274844b7f4557bf2b7c0bece29caa Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 26 Sep 2019 20:48:19 -0300 Subject: [PATCH 28/32] Normalize names, comments, general tidy-up --- modules/markup/html.go | 37 +++---- modules/markup/sanitizer.go | 2 - modules/references/references.go | 143 ++++++++++++-------------- modules/references/references_test.go | 42 ++++---- 4 files changed, 102 insertions(+), 122 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 086f12d9c9961..fc823b1f308ab 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -60,7 +60,7 @@ var ( linkRegex, _ = xurls.StrictMatchingScheme("https?://") ) -// CSS class for opening/closing keywords +// CSS class for action keywords (e.g. "closes: #1") const keywordClass = "issue-keyword" // regexp for full links to issues/pulls @@ -309,20 +309,14 @@ func (ctx *postProcessCtx) textNode(node *html.Node) { } } -// createKeyword() renders a highlited version of a closing/reopening keyowrd -func createKeyword(content string, class string) *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{}, } - if class != "" { - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: class}) - } else { - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: keywordClass}) - } - // GAP: FIXME: move this to CSS - span.Attr = append(span.Attr, html.Attribute{Key: "style", Val: "border-bottom: 1px dotted #959da5; display: inline-block;"}) + span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: keywordClass}) text := &html.Node{ Type: html.TextNode, @@ -611,7 +605,7 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { var ( found bool - ref references.RenderizableReference + ref *references.RenderizableReference ) if ctx.metas["style"] == IssueNameStyleAlphanumeric { @@ -619,34 +613,33 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { } else { found, ref = references.FindRenderizableReferenceNumeric(node.Data) } - if !found { return } var link *html.Node - reftext := node.Data[ref.RefLocation().Start:ref.RefLocation().End] + reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] if _, ok := ctx.metas["format"]; ok { - ctx.metas["index"] = ref.Issue() + 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 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") + link = createLink(util.URLJoin(setting.AppURL, ref.Owner, ref.Name, "issues", ref.Issue), reftext, "issue") } - if ref.Action() == references.XRefActionNone { - replaceContent(node, ref.RefLocation().Start, ref.RefLocation().End, link) + 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], "") + 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], + Data: node.Data[ref.ActionLocation.End:ref.RefLocation.Start], } - replaceContentList(node, ref.ActionLocation().Start, ref.RefLocation().End, []*html.Node{keyword, spaces, link}) + replaceContentList(node, ref.ActionLocation.Start, ref.RefLocation.End, []*html.Node{keyword, spaces, link}) } // fullSha1PatternProcessor renders SHA containing URLs diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go index bb677b9b207d1..fd6f90b2ab1bf 100644 --- a/modules/markup/sanitizer.go +++ b/modules/markup/sanitizer.go @@ -41,8 +41,6 @@ func NewSanitizer() { // Allow keyword markup sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^` + keywordClass + `$`)).OnElements("span") - // GAP: FIXME: remove style - sanitizer.policy.AllowAttrs("style").Matching(regexp.MustCompile(`^border-bottom: 1px dotted #959da5; display: inline-block;$`)).OnElements("span") }) } diff --git a/modules/references/references.go b/modules/references/references.go index 194412420f2e6..9c74d0d081ae6 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -9,6 +9,7 @@ import ( "regexp" "strconv" "strings" + "sync" "code.gitea.io/gitea/modules/markup/mdstripper" "code.gitea.io/gitea/modules/setting" @@ -40,6 +41,9 @@ var ( 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 @@ -56,7 +60,7 @@ const ( XRefActionNeutered // 3 ) -// IssueReference contains an unverified cross-reference to a local issue/pull request +// IssueReference contains an unverified cross-reference to a local issue or pull request type IssueReference struct { Index int64 Owner string @@ -65,13 +69,13 @@ type IssueReference struct { } // RenderizableReference contains an unverified cross-reference to with rendering information -type RenderizableReference interface { - Issue() string - Owner() string - Name() string - RefLocation() *ReferenceLocation - Action() XRefAction - ActionLocation() *ReferenceLocation +type RenderizableReference struct { + Issue string + Owner string + Name string + RefLocation *RefSpan + Action XRefAction + ActionLocation *RefSpan } type rawReference struct { @@ -80,43 +84,8 @@ type rawReference struct { name string action XRefAction issue string - refLocation *ReferenceLocation - actionLocation *ReferenceLocation -} - -// Index returns the number of the issue/pull request -func (r *rawReference) Index() int64 { - return r.index -} - -// Owner returns the owner of the repository for the issue/pull request -func (r *rawReference) Owner() string { - return r.owner -} - -// Owner returns the name of the repository for the issue/pull request -func (r *rawReference) Name() string { - return r.name -} - -// Issue returns the ID of the issue/pull request -func (r *rawReference) Issue() string { - return r.issue -} - -// RefLocation returns the location of the reference in the originating string -func (r *rawReference) RefLocation() *ReferenceLocation { - return r.refLocation -} - -// Action returns the action represented by the action keyword found preceding the reference -func (r *rawReference) Action() XRefAction { - return r.action -} - -// RefLocation returns the location of the action keyword in the originating string -func (r *rawReference) ActionLocation() *ReferenceLocation { - return r.actionLocation + refLocation *RefSpan + actionLocation *RefSpan } func rawToIssueReferenceList(reflist []*rawReference) []IssueReference { @@ -132,8 +101,8 @@ func rawToIssueReferenceList(reflist []*rawReference) []IssueReference { return refarr } -// ReferenceLocation is the position where the reference was found within the parsed text -type ReferenceLocation struct { +// RefSpan is the position where the reference was found within the parsed text +type RefSpan struct { Start int End int } @@ -147,6 +116,18 @@ func init() { 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 { @@ -160,24 +141,24 @@ func FindAllMentionsMarkdown(content string) []string { } // FindAllMentionsBytes matches mention patterns in given content -// and returns a list of found unvalidated user names including the @ prefix. -func FindAllMentionsBytes(content []byte) []ReferenceLocation { +// 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([]ReferenceLocation, len(mentions)) + ret := make([]RefSpan, len(mentions)) for i, val := range mentions { - ret[i] = ReferenceLocation{Start: val[2], End: val[3]} + ret[i] = RefSpan{Start: val[2], End: val[3]} } return ret } -// FindFirstMentionBytes matches mention patterns in given content -// and returns a list of found unvalidated user names including the @ prefix. -func FindFirstMentionBytes(content []byte) (bool, ReferenceLocation) { +// 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, ReferenceLocation{} + return false, RefSpan{} } - return true, ReferenceLocation{Start: mention[2], End: mention[3]} + return true, RefSpan{Start: mention[2], End: mention[3]} } // FindAllIssueReferencesMarkdown strips content from markdown markup @@ -196,31 +177,43 @@ func FindAllIssueReferences(content string) []IssueReference { return rawToIssueReferenceList(findAllIssueReferencesBytes([]byte(content), []string{})) } -// FindRenderizableReferenceNumeric returns the first unvalidated references found in a byte slice -func FindRenderizableReferenceNumeric(content string) (bool, RenderizableReference) { +// 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, getCrossReference([]byte(content), match[2], match[3], false) + 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 byte slice -func FindRenderizableReferenceAlphanumeric(content string) (bool, RenderizableReference) { +// 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, &rawReference{ - issue: string(content[match[2]:match[3]]), - refLocation: &ReferenceLocation{Start: match[2], End: match[3]}, - action: action, - actionLocation: location, + + return true, &RenderizableReference{ + Issue: string(content[match[2]:match[3]]), + RefLocation: &RefSpan{Start: match[2], End: match[3]}, + Action: action, + ActionLocation: location, } } @@ -243,16 +236,12 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference } } - var giteahost string - if uapp, err := url.Parse(setting.AppURL); err == nil { - giteahost = strings.ToLower(uapp.Host) - } - + 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 != giteahost { + if host != "" && host != localhost { continue } parts := strings.Split(u.EscapedPath(), "/") @@ -296,7 +285,7 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *rawRefere index: index, action: action, issue: issue, - refLocation: &ReferenceLocation{Start: start, End: end}, + refLocation: &RefSpan{Start: start, End: end}, actionLocation: location, } } @@ -315,19 +304,19 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *rawRefere name: name, action: action, issue: issue, - refLocation: &ReferenceLocation{Start: start, End: end}, + refLocation: &RefSpan{Start: start, End: end}, actionLocation: location, } } -func findActionKeywords(content []byte, start int) (XRefAction, *ReferenceLocation) { +func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) if m != nil { - return XRefActionCloses, &ReferenceLocation{Start: m[2], End: m[3]} + return XRefActionCloses, &RefSpan{Start: m[2], End: m[3]} } m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]) if m != nil { - return XRefActionReopens, &ReferenceLocation{Start: m[2], End: m[3]} + 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 index 58db0d01e6b55..f8153ffe36daf 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -20,8 +20,8 @@ func TestFindAllIssueReferences(t *testing.T) { Name string Issue string Action XRefAction - RefLocation *ReferenceLocation - ActionLocation *ReferenceLocation + RefLocation *RefSpan + ActionLocation *RefSpan } type testFixture struct { @@ -33,7 +33,7 @@ func TestFindAllIssueReferences(t *testing.T) { { "Simply closes: #29 yes", []result{ - {29, "", "", "29", XRefActionCloses, &ReferenceLocation{Start: 15, End: 18}, &ReferenceLocation{Start: 7, End: 13}}, + {29, "", "", "29", XRefActionCloses, &RefSpan{Start: 15, End: 18}, &RefSpan{Start: 7, End: 13}}, }, }, { @@ -43,7 +43,7 @@ func TestFindAllIssueReferences(t *testing.T) { { " #124 yes, this is a reference.", []result{ - {124, "", "", "124", XRefActionNone, &ReferenceLocation{Start: 0, End: 4}, nil}, + {124, "", "", "124", XRefActionNone, &RefSpan{Start: 0, End: 4}, nil}, }, }, { @@ -57,7 +57,7 @@ func TestFindAllIssueReferences(t *testing.T) { { "This user3/repo4#200 yes.", []result{ - {200, "user3", "repo4", "200", XRefActionNone, &ReferenceLocation{Start: 5, End: 20}, nil}, + {200, "user3", "repo4", "200", XRefActionNone, &RefSpan{Start: 5, End: 20}, nil}, }, }, { @@ -105,38 +105,38 @@ func TestFindAllIssueReferences(t *testing.T) { { "Reopens #15 yes", []result{ - {15, "", "", "15", XRefActionReopens, &ReferenceLocation{Start: 8, End: 11}, &ReferenceLocation{Start: 0, End: 7}}, + {15, "", "", "15", XRefActionReopens, &RefSpan{Start: 8, End: 11}, &RefSpan{Start: 0, End: 7}}, }, }, { "This closes #20 for you yes", []result{ - {20, "", "", "20", XRefActionCloses, &ReferenceLocation{Start: 12, End: 15}, &ReferenceLocation{Start: 5, End: 11}}, + {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, &ReferenceLocation{Start: 11, End: 26}, &ReferenceLocation{Start: 7, End: 10}}, + {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, &ReferenceLocation{Start: 8, End: 13}, nil}, + {1235, "", "", "1235", XRefActionNone, &RefSpan{Start: 8, End: 13}, nil}, }, }, { "Which abc. #9434 same as above", []result{ - {9434, "", "", "9434", XRefActionNone, &ReferenceLocation{Start: 11, End: 16}, nil}, + {9434, "", "", "9434", XRefActionNone, &RefSpan{Start: 11, End: 16}, nil}, }, }, { "This closes #600 and reopens #599", []result{ - {600, "", "", "600", XRefActionCloses, &ReferenceLocation{Start: 12, End: 16}, &ReferenceLocation{Start: 5, End: 11}}, - {599, "", "", "599", XRefActionReopens, &ReferenceLocation{Start: 29, End: 33}, &ReferenceLocation{Start: 21, End: 28}}, + {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}}, }, }, } @@ -171,21 +171,21 @@ func TestFindAllIssueReferences(t *testing.T) { type alnumFixture struct { input string issue string - refLocation *ReferenceLocation + refLocation *RefSpan action XRefAction - actionLocation *ReferenceLocation + actionLocation *RefSpan } alnumFixtures := []alnumFixture{ { "This ref ABC-123 is alphanumeric", - "ABC-123", &ReferenceLocation{Start: 9, End: 16}, + "ABC-123", &RefSpan{Start: 9, End: 16}, XRefActionNone, nil, }, { "This closes ABCD-1234 alphanumeric", - "ABCD-1234", &ReferenceLocation{Start: 12, End: 21}, - XRefActionCloses, &ReferenceLocation{Start: 5, End: 11}, + "ABCD-1234", &RefSpan{Start: 12, End: 21}, + XRefActionCloses, &RefSpan{Start: 5, End: 11}, }, } @@ -195,10 +195,10 @@ func TestFindAllIssueReferences(t *testing.T) { 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) + 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) } } } From 3f9cd8083c55d1189d14722129f53d22ed8ca43b Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 26 Sep 2019 22:21:11 -0300 Subject: [PATCH 29/32] Add CSS style for action keywords --- public/css/index.css | 1 + public/less/_repository.less | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/public/css/index.css b/public/css/index.css index 1da2399c461b8..bc4e70385fc1e 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} .CodeMirror{font:14px 'SF Mono',Consolas,Menlo,'Liberation Mono',Monaco,'Lucida Console',monospace} .CodeMirror.cm-s-default{border-radius:3px;padding:0!important} .CodeMirror .cm-comment{background:inherit!important} diff --git a/public/less/_repository.less b/public/less/_repository.less index fde11f7a4d619..492c9194bbd9c 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -2384,3 +2384,8 @@ tbody.commit-list { padding-top: 8px; padding-bottom: 8px; } + +.issue-keyword { + border-bottom: 1px dotted #959da5; + display: inline-block; +} From 02b6eb21942c72ef0a093458faee27943247bf1f Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Sun, 29 Sep 2019 22:23:19 -0300 Subject: [PATCH 30/32] Fix permission for admin and owner --- models/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/action.go b/models/action.go index f5d9a13ecea20..5cfb79d1bb5ac 100644 --- a/models/action.go +++ b/models/action.go @@ -578,7 +578,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra } // only close issues in another repo if user has push access - if perm.CanWrite(UnitTypeCode) { + if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeCode) { if err := changeIssueStatus(refRepo, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil { return err } From 1faece025e0cc434dfeb6f62724b679b2d69bb6d Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 9 Oct 2019 03:46:37 -0300 Subject: [PATCH 31/32] Fix golangci-lint --- models/action.go | 21 --------------------- models/action_test.go | 20 -------------------- 2 files changed, 41 deletions(-) diff --git a/models/action.go b/models/action.go index 5cfb79d1bb5ac..b5069941216c3 100644 --- a/models/action.go +++ b/models/action.go @@ -10,7 +10,6 @@ import ( "fmt" "html" "path" - "regexp" "strconv" "strings" "time" @@ -54,29 +53,9 @@ 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. diff --git a/models/action_test.go b/models/action_test.go index 6e1cb3676ce0d..df41556850d0c 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -180,26 +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 TestUpdateIssuesCommit(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) pushCommits := []*PushCommit{ From b0735c3acb32b70b426e0671e0e278a7ba57bff4 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 9 Oct 2019 03:56:52 -0300 Subject: [PATCH 32/32] Fix golangci-lint --- models/action.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/models/action.go b/models/action.go index b5069941216c3..2d2999f8809a4 100644 --- a/models/action.go +++ b/models/action.go @@ -53,9 +53,6 @@ const ( ActionMirrorSyncDelete // 20 ) -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|$))` - // Action represents user operation type and other information to // repository. It implemented interface base.Actioner so that can be // used in template render.