Refactor renders (#15175)

* Refactor renders

* Some performance optimization

* Fix comment

* Transform reader

* Fix csv test

* Fix test

* Fix tests

* Improve optimaziation

* Fix test

* Fix test

* Detect file encoding with reader

* Improve optimaziation

* reduce memory usage

* improve code

* fix build

* Fix test

* Fix for go1.15

* Fix render

* Fix comment

* Fix lint

* Fix test

* Don't use NormalEOF when unnecessary

* revert change on util.go

* Apply suggestions from code review

Co-authored-by: zeripath <art27@cantab.net>

* rename function

* Take NormalEOF back

Co-authored-by: zeripath <art27@cantab.net>

2 files changed, 112 insertions, 57 deletions

diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go
index 5bb0fbd652..87fae2a23b 100644
--- a/modules/markup/markdown/markdown.go
+++ b/modules/markup/markdown/markdown.go
@@ -8,6 +8,7 @@ package markdown
 import (
 	"fmt"
 	"io"
+	"io/ioutil"
 	"strings"
 	"sync"
 
@@ -73,17 +74,17 @@ func (l *limitWriter) CloseWithError(err error) error {
 	return l.w.CloseWithError(err)
 }
 
-// NewGiteaParseContext creates a parser.Context with the gitea context set
-func NewGiteaParseContext(urlPrefix string, metas map[string]string, isWiki bool) parser.Context {
+// newParserContext creates a parser.Context with the render context set
+func newParserContext(ctx *markup.RenderContext) parser.Context {
 	pc := parser.NewContext(parser.WithIDs(newPrefixedIDs()))
-	pc.Set(urlPrefixKey, urlPrefix)
-	pc.Set(isWikiKey, isWiki)
-	pc.Set(renderMetasKey, metas)
+	pc.Set(urlPrefixKey, ctx.URLPrefix)
+	pc.Set(isWikiKey, ctx.IsWiki)
+	pc.Set(renderMetasKey, ctx.Metas)
 	return pc
 }
 
 // actualRender renders Markdown to HTML without handling special links.
-func actualRender(body []byte, urlPrefix string, metas map[string]string, wikiMarkdown bool) []byte {
+func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
 	once.Do(func() {
 		converter = goldmark.New(
 			goldmark.WithExtensions(extension.Table,
@@ -169,7 +170,7 @@ func actualRender(body []byte, urlPrefix string, metas map[string]string, wikiMa
 		limit: setting.UI.MaxDisplayFileSize * 3,
 	}
 
-	// FIXME: should we include a timeout that closes the pipe to abort the parser and sanitizer if it takes too long?
+	// FIXME: should we include a timeout that closes the pipe to abort the renderer and sanitizer if it takes too long?
 	go func() {
 		defer func() {
 			err := recover()
@@ -184,18 +185,26 @@ func actualRender(body []byte, urlPrefix string, metas map[string]string, wikiMa
 			_ = lw.CloseWithError(fmt.Errorf("%v", err))
 		}()
 
-		pc := NewGiteaParseContext(urlPrefix, metas, wikiMarkdown)
-		if err := converter.Convert(giteautil.NormalizeEOL(body), lw, parser.WithContext(pc)); err != nil {
+		// FIXME: Don't read all to memory, but goldmark doesn't support
+		pc := newParserContext(ctx)
+		buf, err := ioutil.ReadAll(input)
+		if err != nil {
+			log.Error("Unable to ReadAll: %v", err)
+			return
+		}
+		if err := converter.Convert(giteautil.NormalizeEOL(buf), lw, parser.WithContext(pc)); err != nil {
 			log.Error("Unable to render: %v", err)
 			_ = lw.CloseWithError(err)
 			return
 		}
 		_ = lw.Close()
 	}()
-	return markup.SanitizeReader(rd).Bytes()
+	buf := markup.SanitizeReader(rd)
+	_, err := io.Copy(output, buf)
+	return err
 }
 
-func render(body []byte, urlPrefix string, metas map[string]string, wikiMarkdown bool) (ret []byte) {
+func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
 	defer func() {
 		err := recover()
 		if err == nil {
@@ -206,9 +215,13 @@ func render(body []byte, urlPrefix string, metas map[string]string, wikiMarkdown
 		if log.IsDebug() {
 			log.Debug("Panic in markdown: %v\n%s", err, string(log.Stack(2)))
 		}
-		ret = markup.SanitizeBytes(body)
+		ret := markup.SanitizeReader(input)
+		_, err = io.Copy(output, ret)
+		if err != nil {
+			log.Error("SanitizeReader failed: %v", err)
+		}
 	}()
-	return actualRender(body, urlPrefix, metas, wikiMarkdown)
+	return actualRender(ctx, input, output)
 }
 
 var (
@@ -217,48 +230,59 @@ var (
 )
 
 func init() {
-	markup.RegisterParser(Parser{})
+	markup.RegisterRenderer(Renderer{})
 }
 
-// Parser implements markup.Parser
-type Parser struct{}
+// Renderer implements markup.Renderer
+type Renderer struct{}
 
-// Name implements markup.Parser
-func (Parser) Name() string {
+// Name implements markup.Renderer
+func (Renderer) Name() string {
 	return MarkupName
 }
 
-// NeedPostProcess implements markup.Parser
-func (Parser) NeedPostProcess() bool { return true }
+// NeedPostProcess implements markup.Renderer
+func (Renderer) NeedPostProcess() bool { return true }
 
-// Extensions implements markup.Parser
-func (Parser) Extensions() []string {
+// Extensions implements markup.Renderer
+func (Renderer) Extensions() []string {
 	return setting.Markdown.FileExtensions
 }
 
-// Render implements markup.Parser
-func (Parser) Render(rawBytes []byte, urlPrefix string, metas map[string]string, isWiki bool) []byte {
-	return render(rawBytes, urlPrefix, metas, isWiki)
+// Render implements markup.Renderer
+func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
+	return render(ctx, input, output)
 }
 
 // Render renders Markdown to HTML with all specific handling stuff.
-func Render(rawBytes []byte, urlPrefix string, metas map[string]string) []byte {
-	return markup.Render("a.md", rawBytes, urlPrefix, metas)
+func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
+	if ctx.Filename == "" {
+		ctx.Filename = "a.md"
+	}
+	return markup.Render(ctx, input, output)
 }
 
-// RenderRaw renders Markdown to HTML without handling special links.
-func RenderRaw(body []byte, urlPrefix string, wikiMarkdown bool) []byte {
-	return render(body, urlPrefix, map[string]string{}, wikiMarkdown)
+// RenderString renders Markdown string to HTML with all specific handling stuff and return string
+func RenderString(ctx *markup.RenderContext, content string) (string, error) {
+	var buf strings.Builder
+	if err := Render(ctx, strings.NewReader(content), &buf); err != nil {
+		return "", err
+	}
+	return buf.String(), nil
 }
 
-// RenderString renders Markdown to HTML with special links and returns string type.
-func RenderString(raw, urlPrefix string, metas map[string]string) string {
-	return markup.RenderString("a.md", raw, urlPrefix, metas)
+// RenderRaw renders Markdown to HTML without handling special links.
+func RenderRaw(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
+	return render(ctx, input, output)
 }
 
-// RenderWiki renders markdown wiki page to HTML and return HTML string
-func RenderWiki(rawBytes []byte, urlPrefix string, metas map[string]string) string {
-	return markup.RenderWiki("a.md", rawBytes, urlPrefix, metas)
+// RenderRawString renders Markdown to HTML without handling special links and return string
+func RenderRawString(ctx *markup.RenderContext, content string) (string, error) {
+	var buf strings.Builder
+	if err := RenderRaw(ctx, strings.NewReader(content), &buf); err != nil {
+		return "", err
+	}
+	return buf.String(), nil
 }
 
 // IsMarkdownFile reports whether name looks like a Markdown file
diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go
index 0e340763ae..5997dbccdc 100644
--- a/modules/markup/markdown/markdown_test.go
+++ b/modules/markup/markdown/markdown_test.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/modules/markup"
 	. "code.gitea.io/gitea/modules/markup/markdown"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
@@ -31,10 +32,17 @@ func TestRender_StandardLinks(t *testing.T) {
 	setting.AppSubURL = AppSubURL
 
 	test := func(input, expected, expectedWiki string) {
-		buffer := RenderString(input, setting.AppSubURL, nil)
+		buffer, err := RenderString(&markup.RenderContext{
+			URLPrefix: setting.AppSubURL,
+		}, input)
+		assert.NoError(t, err)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
-		bufferWiki := RenderWiki([]byte(input), setting.AppSubURL, nil)
-		assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(bufferWiki))
+
+		buffer, err = RenderString(&markup.RenderContext{
+			URLPrefix: setting.AppSubURL,
+			IsWiki:    true,
+		}, input)
+		assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
 	}
 
 	googleRendered := `<p><a href="https://google.com/" rel="nofollow">https://google.com/</a></p>`
@@ -74,7 +82,10 @@ func TestRender_Images(t *testing.T) {
 	setting.AppSubURL = AppSubURL
 
 	test := func(input, expected string) {
-		buffer := RenderString(input, setting.AppSubURL, nil)
+		buffer, err := RenderString(&markup.RenderContext{
+			URLPrefix: setting.AppSubURL,
+		}, input)
+		assert.NoError(t, err)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
 	}
 
@@ -261,7 +272,12 @@ func TestTotal_RenderWiki(t *testing.T) {
 	answers := testAnswers(util.URLJoin(AppSubURL, "wiki/"), util.URLJoin(AppSubURL, "wiki", "raw/"))
 
 	for i := 0; i < len(sameCases); i++ {
-		line := RenderWiki([]byte(sameCases[i]), AppSubURL, localMetas)
+		line, err := RenderString(&markup.RenderContext{
+			URLPrefix: AppSubURL,
+			Metas:     localMetas,
+			IsWiki:    true,
+		}, sameCases[i])
+		assert.NoError(t, err)
 		assert.Equal(t, answers[i], line)
 	}
 
@@ -279,7 +295,11 @@ func TestTotal_RenderWiki(t *testing.T) {
 	}
 
 	for i := 0; i < len(testCases); i += 2 {
-		line := RenderWiki([]byte(testCases[i]), AppSubURL, nil)
+		line, err := RenderString(&markup.RenderContext{
+			URLPrefix: AppSubURL,
+			IsWiki:    true,
+		}, testCases[i])
+		assert.NoError(t, err)
 		assert.Equal(t, testCases[i+1], line)
 	}
 }
@@ -288,31 +308,40 @@ func TestTotal_RenderString(t *testing.T) {
 	answers := testAnswers(util.URLJoin(AppSubURL, "src", "master/"), util.URLJoin(AppSubURL, "raw", "master/"))
 
 	for i := 0; i < len(sameCases); i++ {
-		line := RenderString(sameCases[i], util.URLJoin(AppSubURL, "src", "master/"), localMetas)
+		line, err := RenderString(&markup.RenderContext{
+			URLPrefix: util.URLJoin(AppSubURL, "src", "master/"),
+			Metas:     localMetas,
+		}, sameCases[i])
+		assert.NoError(t, err)
 		assert.Equal(t, answers[i], line)
 	}
 
 	testCases := []string{}
 
 	for i := 0; i < len(testCases); i += 2 {
-		line := RenderString(testCases[i], AppSubURL, nil)
+		line, err := RenderString(&markup.RenderContext{
+			URLPrefix: AppSubURL,
+		}, testCases[i])
+		assert.NoError(t, err)
 		assert.Equal(t, testCases[i+1], line)
 	}
 }
 
 func TestRender_RenderParagraphs(t *testing.T) {
 	test := func(t *testing.T, str string, cnt int) {
-		unix := []byte(str)
-		res := string(RenderRaw(unix, "", false))
-		assert.Equal(t, strings.Count(res, "<p"), cnt, "Rendered result for unix should have %d paragraph(s) but has %d:\n%s\n", cnt, strings.Count(res, "<p"), res)
-
-		mac := []byte(strings.ReplaceAll(str, "\n", "\r"))
-		res = string(RenderRaw(mac, "", false))
-		assert.Equal(t, strings.Count(res, "<p"), cnt, "Rendered result for mac should have %d paragraph(s) but has %d:\n%s\n", cnt, strings.Count(res, "<p"), res)
-
-		dos := []byte(strings.ReplaceAll(str, "\n", "\r\n"))
-		res = string(RenderRaw(dos, "", false))
-		assert.Equal(t, strings.Count(res, "<p"), cnt, "Rendered result for windows should have %d paragraph(s) but has %d:\n%s\n", cnt, strings.Count(res, "<p"), res)
+		res, err := RenderRawString(&markup.RenderContext{}, str)
+		assert.NoError(t, err)
+		assert.Equal(t, cnt, strings.Count(res, "<p"), "Rendered result for unix should have %d paragraph(s) but has %d:\n%s\n", cnt, strings.Count(res, "<p"), res)
+
+		mac := strings.ReplaceAll(str, "\n", "\r")
+		res, err = RenderRawString(&markup.RenderContext{}, mac)
+		assert.NoError(t, err)
+		assert.Equal(t, cnt, strings.Count(res, "<p"), "Rendered result for mac should have %d paragraph(s) but has %d:\n%s\n", cnt, strings.Count(res, "<p"), res)
+
+		dos := strings.ReplaceAll(str, "\n", "\r\n")
+		res, err = RenderRawString(&markup.RenderContext{}, dos)
+		assert.NoError(t, err)
+		assert.Equal(t, cnt, strings.Count(res, "<p"), "Rendered result for windows should have %d paragraph(s) but has %d:\n%s\n", cnt, strings.Count(res, "<p"), res)
 	}
 
 	test(t, "\nOne\nTwo\nThree", 1)
@@ -337,7 +366,8 @@ func TestMarkdownRenderRaw(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
-		_ = RenderRaw(testcase, "", false)
+		_, err := RenderRawString(&markup.RenderContext{}, string(testcase))
+		assert.NoError(t, err)
 	}
 }
 
@@ -348,7 +378,8 @@ func TestRenderSiblingImages_Issue12925(t *testing.T) {
 	expected := `<p><a href="/image1" rel="nofollow"><img src="/image1" alt="image1"></a><br>
 <a href="/image2" rel="nofollow"><img src="/image2" alt="image2"></a></p>
 `
-	res := string(RenderRaw([]byte(testcase), "", false))
+	res, err := RenderRawString(&markup.RenderContext{}, testcase)
+	assert.NoError(t, err)
 	assert.Equal(t, expected, res)
 
 }