From 84841c8c4b257bd627fcc033112237d40e3a329a Mon Sep 17 00:00:00 2001 From: Cosmin Stroe Date: Wed, 29 Jun 2016 10:07:39 -0500 Subject: [PATCH] Stricter parsing of issue URLs and commit URLs. (#3121) --- modules/markdown/markdown.go | 44 ++++---- modules/markdown/markdown_test.go | 162 +++++++++++++++++++++--------- 2 files changed, 140 insertions(+), 66 deletions(-) diff --git a/modules/markdown/markdown.go b/modules/markdown/markdown.go index 899d877455..99a4323d99 100644 --- a/modules/markdown/markdown.go +++ b/modules/markdown/markdown.go @@ -23,7 +23,7 @@ import ( ) const ( - ISSUE_NAME_STYLE_NUMERIC = "numeric" + ISSUE_NAME_STYLE_NUMERIC = "numeric" ISSUE_NAME_STYLE_ALPHANUMERIC = "alphanumeric" ) @@ -120,28 +120,30 @@ func (r *Renderer) AutoLink(out *bytes.Buffer, link []byte, kind int) { // Since this method could only possibly serve one link at a time, // we do not need to find all. - m := CommitPattern.Find(link) - if m != nil { - m = bytes.TrimSpace(m) - i := strings.Index(string(m), "commit/") - j := strings.Index(string(m), "#") - if j == -1 { - j = len(m) + if bytes.HasPrefix(link, []byte(setting.AppUrl)) { + m := CommitPattern.Find(link) + if m != nil { + m = bytes.TrimSpace(m) + i := strings.Index(string(m), "commit/") + j := strings.Index(string(m), "#") + if j == -1 { + j = len(m) + } + out.WriteString(fmt.Sprintf(` %s`, m, base.ShortSha(string(m[i+7:j])))) + return } - out.WriteString(fmt.Sprintf(` %s`, m, base.ShortSha(string(m[i+7:j])))) - return - } - m = IssueFullPattern.Find(link) - if m != nil { - m = bytes.TrimSpace(m) - i := strings.Index(string(m), "issues/") - j := strings.Index(string(m), "#") - if j == -1 { - j = len(m) + m = IssueFullPattern.Find(link) + if m != nil { + m = bytes.TrimSpace(m) + i := strings.Index(string(m), "issues/") + j := strings.Index(string(m), "#") + if j == -1 { + j = len(m) + } + out.WriteString(fmt.Sprintf(`#%s`, m, base.ShortSha(string(m[i+7:j])))) + return } - out.WriteString(fmt.Sprintf(` #%s`, m, base.ShortSha(string(m[i+7:j])))) - return } r.Renderer.AutoLink(out, link, kind) @@ -223,7 +225,7 @@ func RenderIssueIndexPattern(rawBytes []byte, urlPrefix string, metas map[string ms := pattern.FindAll(rawBytes, -1) for _, m := range ms { - if m[0] == ' ' || m[0] == '(' { + if m[0] == ' ' || m[0] == '(' { m = m[1:] // ignore leading space or opening parentheses } var link string diff --git a/modules/markdown/markdown_test.go b/modules/markdown/markdown_test.go index f39565b810..d4071ba356 100644 --- a/modules/markdown/markdown_test.go +++ b/modules/markdown/markdown_test.go @@ -1,24 +1,26 @@ package markdown_test import ( - "testing" - . "github.com/smartystreets/goconvey/convey" . "github.com/gogits/gogs/modules/markdown" + . "github.com/smartystreets/goconvey/convey" + "testing" + "bytes" "github.com/gogits/gogs/modules/setting" + "github.com/russross/blackfriday" ) func TestMarkdown(t *testing.T) { - var ( - urlPrefix = "/prefix" - metas map[string]string = nil - ) - setting.AppSubUrlDepth = 0 + Convey("Rendering an issue mention", t, func() { + var ( + urlPrefix = "/prefix" + metas map[string]string = nil + ) + setting.AppSubUrlDepth = 0 - Convey("Rendering an issue link", t, func () { - Convey("To the internal issue tracker", func () { - Convey("It should not render anything when there are no issues", func () { - testCases := []string { + Convey("To the internal issue tracker", func() { + Convey("It should not render anything when there are no mentions", func() { + testCases := []string{ "", "this is a test", "test 123 123 1234", @@ -36,8 +38,8 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i]) } }) - Convey("It should render freestanding issue mentions", func () { - testCases := []string { + Convey("It should render freestanding mentions", func() { + testCases := []string{ "#1234 test", "#1234 test", "test #1234 issue", "test #1234 issue", "test issue #1234", "test issue #1234", @@ -50,18 +52,18 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i+1]) } }) - Convey("It should not render issue without leading space", func () { + Convey("It should not render issue mention without leading space", func() { input := []byte("test#54321 issue") expected := "test#54321 issue" So(string(RenderIssueIndexPattern(input, urlPrefix, metas)), ShouldEqual, expected) }) - Convey("It should not render issue without trailing space", func () { + Convey("It should not render issue mention without trailing space", func() { input := []byte("test #54321issue") expected := "test #54321issue" So(string(RenderIssueIndexPattern(input, urlPrefix, metas)), ShouldEqual, expected) }) - Convey("It should render issue in parentheses", func () { - testCases := []string { + Convey("It should render issue mention in parentheses", func() { + testCases := []string{ "(#54321 issue)", "(#54321 issue)", "test (#54321) issue", "test (#54321) issue", "test (#54321 extra) issue", "test (#54321 extra) issue", @@ -73,8 +75,8 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i+1]) } }) - Convey("It should render multiple issues in the same line", func () { - testCases := []string { + Convey("It should render multiple issue mentions in the same line", func() { + testCases := []string{ "#54321 #1243", "#54321 #1243", "test #54321 #1243", "test #54321 #1243", "(#54321 #1243)", "(#54321 #1243)", @@ -88,15 +90,15 @@ func TestMarkdown(t *testing.T) { } }) }) - Convey("To an external issue tracker with numeric style", func () { + Convey("To an external issue tracker with numeric style", func() { metas = make(map[string]string) metas["format"] = "https://someurl.com/{user}/{repo}/{index}" metas["user"] = "someuser" metas["repo"] = "somerepo" metas["style"] = ISSUE_NAME_STYLE_NUMERIC - Convey("should not render anything when there are no issues", func () { - testCases := []string { + Convey("should not render anything when there are no mentions", func() { + testCases := []string{ "this is a test", "test 123 123 1234", "#", @@ -109,8 +111,8 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i]) } }) - Convey("It should render freestanding issue", func () { - testCases := []string { + Convey("It should render freestanding issue mentions", func() { + testCases := []string{ "#1234 test", "#1234 test", "test #1234 issue", "test #1234 issue", "test issue #1234", "test issue #1234", @@ -122,18 +124,18 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i+1]) } }) - Convey("It should not render issue mention without leading space", func () { + Convey("It should not render issue mention without leading space", func() { input := []byte("test#54321 issue") expected := "test#54321 issue" So(string(RenderIssueIndexPattern(input, urlPrefix, metas)), ShouldEqual, expected) }) - Convey("It should not render issue mention without trailing space", func () { + Convey("It should not render issue mention without trailing space", func() { input := []byte("test #54321issue") expected := "test #54321issue" So(string(RenderIssueIndexPattern(input, urlPrefix, metas)), ShouldEqual, expected) }) - Convey("It should render mention in parentheses", func () { - testCases := []string { + Convey("It should render issue mention in parentheses", func() { + testCases := []string{ "(#54321 issue)", "(#54321 issue)", "test (#54321) issue", "test (#54321) issue", "test (#54321 extra) issue", "test (#54321 extra) issue", @@ -145,8 +147,8 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i+1]) } }) - Convey("It should render multiple mentions in the same line", func () { - testCases := []string { + Convey("It should render multiple issue mentions in the same line", func() { + testCases := []string{ "#54321 #1243", "#54321 #1243", "test #54321 #1243", "test #54321 #1243", "(#54321 #1243)", "(#54321 #1243)", @@ -160,14 +162,14 @@ func TestMarkdown(t *testing.T) { } }) }) - Convey("To an external issue tracker with alphanumeric style", func () { + Convey("To an external issue tracker with alphanumeric style", func() { metas = make(map[string]string) metas["format"] = "https://someurl.com/{user}/{repo}/?b={index}" metas["user"] = "someuser" metas["repo"] = "somerepo" metas["style"] = ISSUE_NAME_STYLE_ALPHANUMERIC - Convey("It should not render anything when there are no issues", func () { - testCases := []string { + Convey("It should not render anything when there are no mentions", func() { + testCases := []string{ "", "this is a test", "test 123 123 1234", @@ -176,23 +178,23 @@ func TestMarkdown(t *testing.T) { "# 123", "#abcd", "test #123", - "abc-1234", // issue prefix must be capital - "ABc-1234", // issue prefix must be _all_ capital + "abc-1234", // issue prefix must be capital + "ABc-1234", // issue prefix must be _all_ capital "ABCDEFGHIJK-1234", // the limit is 10 characters in the prefix - "ABC1234", // dash is required - "test ABC- test", // number is required - "test -1234 test", // prefix is required + "ABC1234", // dash is required + "test ABC- test", // number is required + "test -1234 test", // prefix is required "testABC-123 test", // leading space is required "test ABC-123test", // trailing space is required - "ABC-0123", // no leading zero + "ABC-0123", // no leading zero } for i := 0; i < len(testCases); i += 2 { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i]) } }) - Convey("It should render freestanding issue", func () { - testCases := []string { + Convey("It should render freestanding issue mention", func() { + testCases := []string{ "OTT-1234 test", "OTT-1234 test", "test T-12 issue", "test T-12 issue", "test issue ABCDEFGHIJ-1234567890", "test issue ABCDEFGHIJ-1234567890", @@ -204,8 +206,8 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i+1]) } }) - Convey("It should render mention in parentheses", func () { - testCases := []string { + Convey("It should render issue mention in parentheses", func() { + testCases := []string{ "(ABG-124 issue)", "(ABG-124 issue)", "test (ABG-124) issue", "test (ABG-124) issue", "test (ABG-124 extra) issue", "test (ABG-124 extra) issue", @@ -217,8 +219,8 @@ func TestMarkdown(t *testing.T) { So(string(RenderIssueIndexPattern([]byte(testCases[i]), urlPrefix, metas)), ShouldEqual, testCases[i+1]) } }) - Convey("It should render multiple mentions in the same line", func () { - testCases := []string { + Convey("It should render multiple issue mentions in the same line", func() { + testCases := []string{ "ABG-124 OTT-4321", "ABG-124 OTT-4321", "test ABG-124 OTT-4321", "test ABG-124 OTT-4321", "(ABG-124 OTT-4321)", "(ABG-124 OTT-4321)", @@ -233,4 +235,74 @@ func TestMarkdown(t *testing.T) { }) }) }) + + Convey("Rendering an issue URL", t, func() { + setting.AppUrl = "http://localhost:3000/" + htmlFlags := 0 + htmlFlags |= blackfriday.HTML_SKIP_STYLE + htmlFlags |= blackfriday.HTML_OMIT_CONTENTS + renderer := &Renderer{ + Renderer: blackfriday.HtmlRenderer(htmlFlags, "", ""), + } + buffer := new(bytes.Buffer) + Convey("To the internal issue tracker", func() { + Convey("It should render valid issue URLs", func() { + testCases := []string{ + "http://localhost:3000/user/repo/issues/3333", "#3333", + } + + for i := 0; i < len(testCases); i += 2 { + renderer.AutoLink(buffer, []byte(testCases[i]), blackfriday.LINK_TYPE_NORMAL) + + line, _ := buffer.ReadString(0) + So(line, ShouldEqual, testCases[i+1]) + } + }) + Convey("It should render but not change non-issue URLs", func() { + testCases := []string{ + "http://1111/2222/ssss-issues/3333?param=blah&blahh=333", "http://1111/2222/ssss-issues/3333?param=blah&blahh=333", + "http://test.com/issues/33333", "http://test.com/issues/33333", + "http://test.com/issues/3", "http://test.com/issues/3", + "http://issues/333", "http://issues/333", + "https://issues/333", "https://issues/333", + "http://tissues/0", "http://tissues/0", + } + + for i := 0; i < len(testCases); i += 2 { + renderer.AutoLink(buffer, []byte(testCases[i]), blackfriday.LINK_TYPE_NORMAL) + + line, _ := buffer.ReadString(0) + So(line, ShouldEqual, testCases[i+1]) + } + }) + }) + }) + + Convey("Rendering a commit URL", t, func() { + setting.AppUrl = "http://localhost:3000/" + htmlFlags := 0 + htmlFlags |= blackfriday.HTML_SKIP_STYLE + htmlFlags |= blackfriday.HTML_OMIT_CONTENTS + renderer := &Renderer{ + Renderer: blackfriday.HtmlRenderer(htmlFlags, "", ""), + } + buffer := new(bytes.Buffer) + Convey("To the internal issue tracker", func() { + Convey("It should correctly convert URLs", func() { + testCases := []string{ + "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae", " d8a994ef24", + "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2", " d8a994ef24", + "https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2", "https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2", + "https://commit/d8a994ef243349f321568f9e36d5c3f444b99cae", "https://commit/d8a994ef243349f321568f9e36d5c3f444b99cae", + } + + for i := 0; i < len(testCases); i += 2 { + renderer.AutoLink(buffer, []byte(testCases[i]), blackfriday.LINK_TYPE_NORMAL) + + line, _ := buffer.ReadString(0) + So(line, ShouldEqual, testCases[i+1]) + } + }) + }) + }) }