From 5cc8a1a2ba95c5819b42ce7413c5b60ecb3cb916 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 3 Dec 2022 16:24:40 +0000 Subject: [PATCH 1/3] Ensure that plain files are rendered correctly even when containing ambiguous characters As recognised in #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 `
` 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 #21841

Signed-off-by: Andrew Thornton 
---
 modules/charset/escape.go             | 30 +++++++++++++++++++++++++++
 routers/web/repo/view.go              | 20 ++++--------------
 templates/repo/settings/lfs_file.tmpl |  6 +++---
 templates/repo/view_file.tmpl         |  6 +++---
 4 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/modules/charset/escape.go b/modules/charset/escape.go
index ce2eb1446dbb3..31b2dc693fd5c 100644
--- a/modules/charset/escape.go
+++ b/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{}
diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go
index fa4eb6d61f2cd..5ae696a8a210e 100644
--- a/routers/web/repo/view.go
+++ b/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", `
`,
-			)
+			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", `
`,
-			)
 		} else {
 			buf, _ := io.ReadAll(rd)
 
diff --git a/templates/repo/settings/lfs_file.tmpl b/templates/repo/settings/lfs_file.tmpl
index ce3c39eac28a6..2e14bda840f64 100644
--- a/templates/repo/settings/lfs_file.tmpl
+++ b/templates/repo/settings/lfs_file.tmpl
@@ -17,11 +17,11 @@
 			
 			

 				{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
-				

+				

 					{{if .IsMarkup}}
 						{{if .FileContent}}{{.FileContent | Safe}}{{end}}
-					{{else if .IsRenderedHTML}}
-						
{{if .FileContent}}{{.FileContent | Str2html}}{{end}}

+					{{else if .IsRenderedPlainText}}
+						
{{if .FileContent}}{{.FileContent | Safe}}{{end}}

 					{{else if not .IsTextFile}}
 						

 							{{if .IsImageFile}}
diff --git a/templates/repo/view_file.tmpl b/templates/repo/view_file.tmpl
index 9d82cc018c792..7ac65057e9fcd 100644
--- a/templates/repo/view_file.tmpl
+++ b/templates/repo/view_file.tmpl
@@ -61,11 +61,11 @@
 		{{if not (or .IsMarkup .IsRenderedHTML)}}
 			{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
 		{{end}}
-		

+		

 			{{if .IsMarkup}}
 				{{if .FileContent}}{{.FileContent | Safe}}{{end}}
-			{{else if .IsRenderedHTML}}
-				
{{if .FileContent}}{{.FileContent | Str2html}}{{end}}

+			{{else if .IsRenderedPlainText}}
+				
{{if .FileContent}}{{.FileContent | Safe}}{{end}}

 			{{else if not .IsTextSource}}
 				

 					{{if .IsImageFile}}

From 7ba05dc140350130f99d64b861356b034b2b8952 Mon Sep 17 00:00:00 2001
From: Andrew Thornton 
Date: Sat, 3 Dec 2022 19:44:46 +0000
Subject: [PATCH 2/3] adjust comment

Signed-off-by: Andrew Thornton 
---
 modules/charset/escape.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules/charset/escape.go b/modules/charset/escape.go
index 31b2dc693fd5c..3b1c20697793b 100644
--- a/modules/charset/escape.go
+++ b/modules/charset/escape.go
@@ -32,7 +32,7 @@ func EscapeControlHTML(text string, locale translation.Locale, allowed ...rune)
 	return streamer.escaped, sb.String()
 }
 
-// EscapeControlReaders escapes the unicode control sequences in a provider reader and writer in a locale and returns the findings as an EscapeStatus and the escaped []byte
+// EscapeControlReaders escapes the unicode control sequences in a provided reader of HTML content and writer in a locale and returns the findings as an EscapeStatus and the escaped []byte
 func EscapeControlReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
 	outputStream := &HTMLStreamerWriter{Writer: writer}
 	streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
@@ -44,7 +44,7 @@ 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
+// EscapeControlStringReader escapes the unicode control sequences in a provided reader of string content and writer in a locale and returns the findings as an EscapeStatus and the escaped []byte
 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}

From 2381f0a92dfc5b45f38e12298ebe961cacc8e5d7 Mon Sep 17 00:00:00 2001
From: Andrew Thornton 
Date: Sat, 3 Dec 2022 19:50:27 +0000
Subject: [PATCH 3/3] IsRenderedPlainText -> IsPlainText

Signed-off-by: Andrew Thornton 
---
 routers/web/repo/view.go              | 2 +-
 templates/repo/settings/lfs_file.tmpl | 4 ++--
 templates/repo/view_file.tmpl         | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go
index 5ae696a8a210e..79b2cb15799c7 100644
--- a/routers/web/repo/view.go
+++ b/routers/web/repo/view.go
@@ -344,7 +344,7 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin
 			ctx.Data["FileContent"] = buf.String()
 		}
 	} else {
-		ctx.Data["IsRenderedPlainText"] = true
+		ctx.Data["IsPlainText"] = true
 		buf := &bytes.Buffer{}
 		ctx.Data["EscapeStatus"], err = charset.EscapeControlStringReader(rd, buf, ctx.Locale)
 		if err != nil {
diff --git a/templates/repo/settings/lfs_file.tmpl b/templates/repo/settings/lfs_file.tmpl
index 2e14bda840f64..6d20aaddef121 100644
--- a/templates/repo/settings/lfs_file.tmpl
+++ b/templates/repo/settings/lfs_file.tmpl
@@ -17,10 +17,10 @@
 			
 			

 				{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
-				

+				

 					{{if .IsMarkup}}
 						{{if .FileContent}}{{.FileContent | Safe}}{{end}}
-					{{else if .IsRenderedPlainText}}
+					{{else if .IsPlainText}}
 						
{{if .FileContent}}{{.FileContent | Safe}}{{end}}

 					{{else if not .IsTextFile}}
 						

diff --git a/templates/repo/view_file.tmpl b/templates/repo/view_file.tmpl
index 7ac65057e9fcd..711a37461ecb4 100644
--- a/templates/repo/view_file.tmpl
+++ b/templates/repo/view_file.tmpl
@@ -61,10 +61,10 @@
 		{{if not (or .IsMarkup .IsRenderedHTML)}}
 			{{template "repo/unicode_escape_prompt" dict "EscapeStatus" .EscapeStatus "root" $}}
 		{{end}}
-		

+		

 			{{if .IsMarkup}}
 				{{if .FileContent}}{{.FileContent | Safe}}{{end}}
-			{{else if .IsRenderedPlainText}}
+			{{else if .IsPlainText}}
 				
{{if .FileContent}}{{.FileContent | Safe}}{{end}}

 			{{else if not .IsTextSource}}