Ensure that plain files are rendered correctly even when containing ambiguous characters

As recognised in go-gitea#21841 the rendering of plain text files is somewhat
incorrect when there are ambiguous characters as the html code is double
escaped. In fact there are several more problems here.

We have a residual isRenderedHTML which is actually simply escaping the
file - not rendering it. This is badly named and gives the wrong
impression.

There is also unusual behaviour whether the file is called a Readme or
not and there is no way to get to the source code if the file is called
README.

In reality what should happen is different depending on whether the
file is being rendered a README at the bottom of the directory view or
not.

1. If it is rendered as a README on a directory - it should simply be
   escaped and rendered as `<pre>` text.
2. If it is rendered as a file then it should be rendered as source
   code.

This PR therefore does:
1. Rename IsRenderedHTML to IsRenderedPlainText
2. Readme files rendered at the bottom of the directory are rendered without line numbers
3. Otherwise plain text files are rendered as source code.

Replace go-gitea#21841

Signed-off-by: Andrew Thornton <[email protected]>

Author: zeripath

modules/charset/escape.go

@@ -8,6 +8,7 @@
 package charset
 
 import (
+	"bufio"
 	"io"
 	"strings"
 
@@ -43,6 +44,35 @@ func EscapeControlReader(reader io.Reader, writer io.Writer, locale translation.
 	return streamer.escaped, err
 }
 
+// EscapeControlStringReader escapes the unicode control sequences in a provided string and returns the findings as an EscapeStatus and the escaped string
+func EscapeControlStringReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
+	bufRd := bufio.NewReader(reader)
+	outputStream := &HTMLStreamerWriter{Writer: writer}
+	streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
+
+	for {
+		line, rdErr := bufRd.ReadString('\n')
+		if len(line) > 0 {
+			if err := streamer.Text(line); err != nil {
+				streamer.escaped.HasError = true
+				log.Error("Error whilst escaping: %v", err)
+				return streamer.escaped, err
+			}
+		}
+		if rdErr != nil {
+			if rdErr != io.EOF {
+				err = rdErr
+			}
+			break
+		}
+		if err := streamer.SelfClosingTag("br"); err != nil {
+			streamer.escaped.HasError = true
+			return streamer.escaped, err
+		}
+	}
+	return streamer.escaped, err
+}
+
 // EscapeControlString escapes the unicode control sequences in a provided string and returns the findings as an EscapeStatus and the escaped string
 func EscapeControlString(text string, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, output string) {
 	sb := &strings.Builder{}

routers/web/repo/view.go

@@ -9,7 +9,6 @@ import (
 	gocontext "context"
 	"encoding/base64"
 	"fmt"
-	gotemplate "html/template"
 	"io"
 	"net/http"
 	"net/url"
@@ -341,15 +340,13 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin
 		if err != nil {
 			log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.name, ctx.Repo.Repository, err)
 			buf := &bytes.Buffer{}
-			ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, buf, ctx.Locale)
-			ctx.Data["FileContent"] = strings.ReplaceAll(
-				gotemplate.HTMLEscapeString(buf.String()), "\n", `<br>`,
-			)
+			ctx.Data["EscapeStatus"], _ = charset.EscapeControlStringReader(rd, buf, ctx.Locale)
+			ctx.Data["FileContent"] = buf.String()
 		}
 	} else {
-		ctx.Data["IsRenderedHTML"] = true
+		ctx.Data["IsRenderedPlainText"] = true
 		buf := &bytes.Buffer{}
-		ctx.Data["EscapeStatus"], err = charset.EscapeControlReader(rd, &charset.BreakWriter{Writer: buf}, ctx.Locale, charset.RuneNBSP)
+		ctx.Data["EscapeStatus"], err = charset.EscapeControlStringReader(rd, buf, ctx.Locale)
 		if err != nil {
 			log.Error("Read failed: %v", err)
 		}
@@ -527,15 +524,6 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
 			}
 			// to prevent iframe load third-party url
 			ctx.Resp.Header().Add("Content-Security-Policy", "frame-src 'self'")
-		} else if readmeExist && !shouldRenderSource {
-			buf := &bytes.Buffer{}
-			ctx.Data["IsRenderedHTML"] = true
-
-			ctx.Data["EscapeStatus"], _ = charset.EscapeControlReader(rd, buf, ctx.Locale)
-
-			ctx.Data["FileContent"] = strings.ReplaceAll(
-				gotemplate.HTMLEscapeString(buf.String()), "\n", `<br>`,
-			)
 		} else {
 			buf, _ := io.ReadAll(rd)
 

templates/repo/settings/lfs_file.tmpl

@@ -17,11 +17,11 @@
 			</h4>
 			<div class="ui attached table unstackable segment">
 				{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
-				<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsRenderedHTML}} plain-text{{else if .IsTextFile}} code-view{{end}}">
+				<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsRenderedPlainText}} plain-text{{else if .IsTextFile}} code-view{{end}}">
 					{{if .IsMarkup}}
 						{{if .FileContent}}{{.FileContent | Safe}}{{end}}
-					{{else if .IsRenderedHTML}}
-						<pre>{{if .FileContent}}{{.FileContent | Str2html}}{{end}}</pre>
+					{{else if .IsRenderedPlainText}}
+						<pre>{{if .FileContent}}{{.FileContent | Safe}}{{end}}</pre>
 					{{else if not .IsTextFile}}
 						<div class="view-raw ui center">
 							{{if .IsImageFile}}

templates/repo/view_file.tmpl

@@ -61,11 +61,11 @@
 		{{if not (or .IsMarkup .IsRenderedHTML)}}
 			{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
 		{{end}}
-		<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsRenderedHTML}} plain-text{{else if .IsTextSource}} code-view{{end}}">
+		<div class="file-view{{if .IsMarkup}} markup {{.MarkupType}}{{else if .IsRenderedPlainText}} plain-text{{else if .IsTextSource}} code-view{{end}}">
 			{{if .IsMarkup}}
 				{{if .FileContent}}{{.FileContent | Safe}}{{end}}
-			{{else if .IsRenderedHTML}}
-				<pre>{{if .FileContent}}{{.FileContent | Str2html}}{{end}}</pre>
+			{{else if .IsRenderedPlainText}}
+				<pre>{{if .FileContent}}{{.FileContent | Safe}}{{end}}</pre>
 			{{else if not .IsTextSource}}
 				<div class="view-raw ui center">
 					{{if .IsImageFile}}