markup: migrate from blackfriday to goldmark

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019c3baf-c434-7794-9efd-084363bad1a2

go.mod

@@ -36,7 +36,6 @@ require (
 	github.com/olekukonko/tablewriter v1.1.3
 	github.com/pquerna/otp v1.5.0
 	github.com/prometheus/client_golang v1.23.0
-	github.com/russross/blackfriday v1.6.0
 	github.com/sergi/go-diff v1.4.0
 	github.com/sourcegraph/run v0.12.0
 	github.com/stretchr/testify v1.11.1
@@ -45,6 +44,7 @@ require (
 	github.com/unknwon/i18n v0.0.0-20190805065654-5c6446a380b6
 	github.com/unknwon/paginater v0.0.0-20170405233947-45e5d631308e
 	github.com/urfave/cli v1.22.17
+	github.com/yuin/goldmark v1.7.16
 	golang.org/x/crypto v0.47.0
 	golang.org/x/image v0.35.0
 	golang.org/x/net v0.48.0

go.sum

@@ -399,8 +399,6 @@ github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qq
 github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
 github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
 github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog=
-github.com/russross/blackfriday v1.6.0 h1:KqfZb0pUVN2lYqZUYRddxF4OR8ZMURnJIG5Y3VRLtww=
-github.com/russross/blackfriday v1.6.0/go.mod h1:ti0ldHuxg49ri4ksnFxlkCfN+hvslNlmVHqNRXXJNAY=
 github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/saintfish/chardet v0.0.0-20120816061221-3af4cd4741ca h1:NugYot0LIVPxTvN8n+Kvkn6TrbMyxQiuvKdEwFdR9vI=
@@ -457,6 +455,8 @@ github.com/urfave/cli v1.22.17 h1:SYzXoiPfQjHBbkYxbew5prZHS1TOLT3ierW8SYLqtVQ=
 github.com/urfave/cli v1.22.17/go.mod h1:b0ht0aqgH/6pBYzzxURyrM4xXNgsoT/n2ZzwQiEhNVo=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
+github.com/yuin/goldmark v1.7.16 h1:n+CJdUxaFMiDUNnWC3dMWCIQJSkxH4uz3ZwQBkAlVNE=
+github.com/yuin/goldmark v1.7.16/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg=
 github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs=
 github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
 go.bobheadxi.dev/streamline v1.2.1 h1:IqKSA1TbeuDqCzYNAwtlh8sqf3tsQus8XgJdkCWFT8c=

internal/markup/markdown.go

@@ -3,11 +3,21 @@ package markup
 import (
 	"bytes"
 	"fmt"
+	"html"
+	"log"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 
-	"github.com/russross/blackfriday"
+	"github.com/yuin/goldmark"
+	"github.com/yuin/goldmark/ast"
+	"github.com/yuin/goldmark/extension"
+	"github.com/yuin/goldmark/parser"
+	"github.com/yuin/goldmark/renderer"
+	goldmarkhtml "github.com/yuin/goldmark/renderer/html"
+	"github.com/yuin/goldmark/text"
+	"github.com/yuin/goldmark/util"
 
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/lazyregexp"
@@ -25,40 +35,55 @@ func IsMarkdownFile(name string) bool {
 	return false
 }
 
-// MarkdownRenderer is a extended version of underlying Markdown render object.
-type MarkdownRenderer struct {
-	blackfriday.Renderer
-	urlPrefix string
-}
-
 var validLinksPattern = lazyregexp.New(`^[a-z][\w-]+://|^mailto:`)
+var linkifyURLRegexp = regexp.MustCompile(`^(?:http|https|ftp)://[-a-zA-Z0-9@:%._+~#=]{1,256}(?:\.[a-z]+)?(?::\d+)?(?:[/#?][-a-zA-Z0-9@:%_+.~#$!?&/=();,'\^{}\[\]` + "`" + `]*)?`)
 
-// isLink reports whether link fits valid format.
 func isLink(link []byte) bool {
 	return validLinksPattern.Match(link)
 }
 
-// Link defines how formal links should be processed to produce corresponding HTML elements.
+type linkTransformer struct {
-func (r *MarkdownRenderer) Link(out *bytes.Buffer, link, title, content []byte) {
+	urlPrefix string
-	if len(link) > 0 && !isLink(link) {
-		if link[0] != '#' {
-			link = []byte(path.Join(r.urlPrefix, string(link)))
-		}
-	}
-
-	r.Renderer.Link(out, link, title, content)
 }
 
-// AutoLink defines how auto-detected links should be processed to produce corresponding HTML elements.
+func (t *linkTransformer) Transform(node *ast.Document, reader text.Reader, _ parser.Context) {
-// Reference for kind: https://github.com/russross/blackfriday/blob/master/markdown.go#L69-L76
+	_ = ast.Walk(node, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
-func (r *MarkdownRenderer) AutoLink(out *bytes.Buffer, link []byte, kind int) {
+		if !entering {
-	if kind != blackfriday.LINK_TYPE_NORMAL {
+			return ast.WalkContinue, nil
-		r.Renderer.AutoLink(out, link, kind)
+		}
-		return
+		if link, ok := n.(*ast.Link); ok {
+			dest := link.Destination
+			if len(dest) > 0 && !isLink(dest) && dest[0] != '#' {
+				link.Destination = []byte(path.Join(t.urlPrefix, string(dest)))
+			}
+		}
+		return ast.WalkContinue, nil
+	})
+}
+
+type gogsRenderer struct {
+	urlPrefix string
+}
+
+func (r *gogsRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) {
+	reg.Register(ast.KindAutoLink, r.renderAutoLink)
+}
+
+func (r *gogsRenderer) renderAutoLink(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
+	n := node.(*ast.AutoLink)
+	if !entering {
+		return ast.WalkContinue, nil
 	}
 
-	// Since this method could only possibly serve one link at a time,
+	if n.AutoLinkType != ast.AutoLinkURL {
-	// we do not need to find all.
+		url := n.URL(source)
+		escaped := html.EscapeString(string(url))
+		_, _ = fmt.Fprintf(w, `<a href="mailto:%s">%s</a>`, escaped, escaped)
+		return ast.WalkContinue, nil
+	}
+
+	link := n.URL(source)
+
 	if bytes.HasPrefix(link, []byte(conf.Server.ExternalURL)) {
 		m := CommitPattern.Find(link)
 		if m != nil {
@@ -68,8 +93,9 @@ func (r *MarkdownRenderer) AutoLink(out *bytes.Buffer, link []byte, kind int) {
 			if j == -1 {
 				j = len(m)
 			}
-			_, _ = fmt.Fprintf(out, ` <code><a href="%s">%s</a></code>`, m, tool.ShortSHA1(string(m[i+7:j])))
+			escapedURL := html.EscapeString(string(m))
-			return
+			_, _ = fmt.Fprintf(w, ` <code><a href="%s">%s</a></code>`, escapedURL, tool.ShortSHA1(string(m[i+7:j])))
+			return ast.WalkContinue, nil
 		}
 
 		m = IssueFullPattern.Find(link)
@@ -82,78 +108,65 @@ func (r *MarkdownRenderer) AutoLink(out *bytes.Buffer, link []byte, kind int) {
 			}
 
 			index := string(m[i+7 : j])
+			escapedURL := html.EscapeString(string(m))
 			fullRepoURL := conf.Server.ExternalURL + strings.TrimPrefix(r.urlPrefix, "/")
-			var link string
+			var href string
 			if strings.HasPrefix(string(m), fullRepoURL) {
-				// Use a short issue reference if the URL refers to this repository
+				href = fmt.Sprintf(`<a href="%s">#%s</a>`, escapedURL, html.EscapeString(index))
-				link = fmt.Sprintf(`<a href="%s">#%s</a>`, m, index)
 			} else {
-				// Use a cross-repository issue reference if the URL refers to a different repository
+				repo := html.EscapeString(string(m[len(conf.Server.ExternalURL) : i-1]))
-				repo := string(m[len(conf.Server.ExternalURL) : i-1])
+				href = fmt.Sprintf(`<a href="%s">%s#%s</a>`, escapedURL, repo, html.EscapeString(index))
-				link = fmt.Sprintf(`<a href="%s">%s#%s</a>`, m, repo, index)
 			}
-			out.WriteString(link)
+			_, _ = w.WriteString(href)
-			return
+			return ast.WalkContinue, nil
 		}
 	}
 
-	r.Renderer.AutoLink(out, link, kind)
+	escapedLink := html.EscapeString(string(link))
-}
+	_, _ = fmt.Fprintf(w, `<a href="%s">%s</a>`, escapedLink, escapedLink)
-
+	return ast.WalkContinue, nil
-// ListItem defines how list items should be processed to produce corresponding HTML elements.
-func (r *MarkdownRenderer) ListItem(out *bytes.Buffer, text []byte, flags int) {
-	// Detect procedures to draw checkboxes.
-	switch {
-	case bytes.HasPrefix(text, []byte("[ ] ")):
-		text = append([]byte(`<input type="checkbox" disabled="" />`), text[3:]...)
-	case bytes.HasPrefix(text, []byte("[x] ")):
-		text = append([]byte(`<input type="checkbox" disabled="" checked="" />`), text[3:]...)
-	}
-	r.Renderer.ListItem(out, text, flags)
 }
 
 // RawMarkdown renders content in Markdown syntax to HTML without handling special links.
 func RawMarkdown(body []byte, urlPrefix string) []byte {
-	htmlFlags := 0
+	extensions := []goldmark.Extender{
-	htmlFlags |= blackfriday.HTML_SKIP_STYLE
+		extension.Table,
-	htmlFlags |= blackfriday.HTML_OMIT_CONTENTS
+		extension.Strikethrough,
+		extension.TaskList,
+		extension.NewLinkify(extension.WithLinkifyURLRegexp(linkifyURLRegexp)),
+	}
 
 	if conf.Smartypants.Enabled {
-		htmlFlags |= blackfriday.HTML_USE_SMARTYPANTS
+		extensions = append(extensions, extension.Typographer)
-		if conf.Smartypants.Fractions {
-			htmlFlags |= blackfriday.HTML_SMARTYPANTS_FRACTIONS
-		}
-		if conf.Smartypants.Dashes {
-			htmlFlags |= blackfriday.HTML_SMARTYPANTS_DASHES
-		}
-		if conf.Smartypants.LatexDashes {
-			htmlFlags |= blackfriday.HTML_SMARTYPANTS_LATEX_DASHES
-		}
-		if conf.Smartypants.AngledQuotes {
-			htmlFlags |= blackfriday.HTML_SMARTYPANTS_ANGLED_QUOTES
-		}
 	}
 
-	renderer := &MarkdownRenderer{
+	rendererOpts := []renderer.Option{
-		Renderer:  blackfriday.HtmlRenderer(htmlFlags, "", ""),
+		goldmarkhtml.WithUnsafe(),
-		urlPrefix: urlPrefix,
+		renderer.WithNodeRenderers(
+			util.Prioritized(&gogsRenderer{urlPrefix: urlPrefix}, 0),
+		),
 	}
 
-	// set up the parser
-	extensions := 0
-	extensions |= blackfriday.EXTENSION_NO_INTRA_EMPHASIS
-	extensions |= blackfriday.EXTENSION_TABLES
-	extensions |= blackfriday.EXTENSION_FENCED_CODE
-	extensions |= blackfriday.EXTENSION_AUTOLINK
-	extensions |= blackfriday.EXTENSION_STRIKETHROUGH
-	extensions |= blackfriday.EXTENSION_SPACE_HEADERS
-	extensions |= blackfriday.EXTENSION_NO_EMPTY_LINE_BEFORE_BLOCK
-
 	if conf.Markdown.EnableHardLineBreak {
-		extensions |= blackfriday.EXTENSION_HARD_LINE_BREAK
+		rendererOpts = append(rendererOpts, goldmarkhtml.WithHardWraps())
 	}
 
-	return blackfriday.Markdown(body, renderer, extensions)
+	md := goldmark.New(
+		goldmark.WithExtensions(extensions...),
+		goldmark.WithParserOptions(
+			parser.WithASTTransformers(
+				util.Prioritized(&linkTransformer{urlPrefix: urlPrefix}, 0),
+			),
+		),
+		goldmark.WithRendererOptions(rendererOpts...),
+	)
+
+	var buf bytes.Buffer
+	if err := md.Convert(body, &buf); err != nil {
+		log.Printf("markup: failed to convert Markdown: %v", err)
+		return nil
+	}
+	return buf.Bytes()
 }
 
 // Markdown takes a string or []byte and renders to HTML in Markdown syntax with special links.

internal/markup/markdown_test.go

@@ -1,19 +1,20 @@
 package markup_test
 
 import (
-	"bytes"
 	"strings"
 	"testing"
 
-	"github.com/russross/blackfriday"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"gogs.io/gogs/internal/conf"
 	. "gogs.io/gogs/internal/markup"
 )
 
 func Test_IsMarkdownFile(t *testing.T) {
-	// TODO: Refactor to accept a list of extensions
+	oldExts := conf.Markdown.FileExtensions
+	defer func() { conf.Markdown.FileExtensions = oldExts }()
+
 	conf.Markdown.FileExtensions = strings.Split(".md,.markdown,.mdown,.mkd", ",")
 	tests := []struct {
 		ext    string
@@ -32,41 +33,53 @@ func Test_IsMarkdownFile(t *testing.T) {
 	}
 }
 
-func Test_Markdown(t *testing.T) {
+func Test_RawMarkdown_AutoLink(t *testing.T) {
-	// TODO: Refactor to accept URL
+	oldURL := conf.Server.ExternalURL
+	defer func() { conf.Server.ExternalURL = oldURL }()
+
 	conf.Server.ExternalURL = "http://localhost:3000/"
 
-	htmlFlags := 0
-	htmlFlags |= blackfriday.HTML_SKIP_STYLE
-	htmlFlags |= blackfriday.HTML_OMIT_CONTENTS
-	renderer := &MarkdownRenderer{
-		Renderer: blackfriday.HtmlRenderer(htmlFlags, "", ""),
-	}
-
 	tests := []struct {
-		input  string
+		name  string
-		expVal string
+		input string
+		want  string
 	}{
-		// Issue URL
+		{
-		{input: "http://localhost:3000/user/repo/issues/3333", expVal: "<a href=\"http://localhost:3000/user/repo/issues/3333\">#3333</a>"},
+			name:  "issue URL from same instance",
-		{input: "http://1111/2222/ssss-issues/3333?param=blah&blahh=333", expVal: "<a href=\"http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333\">http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333</a>"},
+			input: "http://localhost:3000/user/repo/issues/3333",
-		{input: "http://test.com/issues/33333", expVal: "<a href=\"http://test.com/issues/33333\">http://test.com/issues/33333</a>"},
+			want:  `<a href="http://localhost:3000/user/repo/issues/3333">#3333</a>`,
-		{input: "http://test.com/issues/3", expVal: "<a href=\"http://test.com/issues/3\">http://test.com/issues/3</a>"},
+		},
-		{input: "http://issues/333", expVal: "<a href=\"http://issues/333\">http://issues/333</a>"},
+		{
-		{input: "https://issues/333", expVal: "<a href=\"https://issues/333\">https://issues/333</a>"},
+			name:  "non-matching issue-like URL",
-		{input: "http://tissues/0", expVal: "<a href=\"http://tissues/0\">http://tissues/0</a>"},
+			input: "http://1111/2222/ssss-issues/3333?param=blah&blahh=333",
-
+			want:  `<a href="http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333">http://1111/2222/ssss-issues/3333?param=blah&amp;blahh=333</a>`,
-		// Commit URL
+		},
-		{input: "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae", expVal: " <code><a href=\"http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae\">d8a994ef24</a></code>"},
+		{
-		{input: "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2", expVal: " <code><a href=\"http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2\">d8a994ef24</a></code>"},
+			name:  "external issue URL",
-		{input: "https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2", expVal: "<a href=\"https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2\">https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2</a>"},
+			input: "http://test.com/issues/33333",
-		{input: "https://commit/d8a994ef243349f321568f9e36d5c3f444b99cae", expVal: "<a href=\"https://commit/d8a994ef243349f321568f9e36d5c3f444b99cae\">https://commit/d8a994ef243349f321568f9e36d5c3f444b99cae</a>"},
+			want:  `http://test.com/issues/33333`,
+		},
+		{
+			name:  "commit URL from same instance",
+			input: "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae",
+			want:  `<code><a href="http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae">d8a994ef24</a></code>`,
+		},
+		{
+			name:  "commit URL with fragment from same instance",
+			input: "http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2",
+			want:  `<code><a href="http://localhost:3000/user/project/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2">d8a994ef24</a></code>`,
+		},
+		{
+			name:  "external commit URL",
+			input: "https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2",
+			want:  `https://external-link.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2`,
+		},
 	}
 	for _, test := range tests {
-		t.Run("", func(t *testing.T) {
+		t.Run(test.name, func(t *testing.T) {
-			buf := new(bytes.Buffer)
+			result := string(RawMarkdown([]byte(test.input), ""))
-			renderer.AutoLink(buf, []byte(test.input), blackfriday.LINK_TYPE_NORMAL)
+			require.NotEmpty(t, result)
-			assert.Equal(t, test.expVal, buf.String())
+			assert.Contains(t, result, test.want)
 		})
 	}
 }