Ensure that plain files are rendered correctly even when containing ambiguous characters (#22017)

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 `<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 IsPlainText
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 <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
zeripath 2022-12-17 20:22:25 +00:00 committed by GitHub
parent f3370eeaee
commit 6e22605793
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 41 additions and 23 deletions

View file

@ -8,6 +8,7 @@
package charset package charset
import ( import (
"bufio"
"io" "io"
"strings" "strings"
@ -31,7 +32,7 @@ func EscapeControlHTML(text string, locale translation.Locale, allowed ...rune)
return streamer.escaped, sb.String() 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) { func EscapeControlReader(reader io.Reader, writer io.Writer, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, err error) {
outputStream := &HTMLStreamerWriter{Writer: writer} outputStream := &HTMLStreamerWriter{Writer: writer}
streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer) streamer := NewEscapeStreamer(locale, outputStream, allowed...).(*escapeStreamer)
@ -43,6 +44,35 @@ func EscapeControlReader(reader io.Reader, writer io.Writer, locale translation.
return streamer.escaped, err return streamer.escaped, err
} }
// 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}
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 // 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) { func EscapeControlString(text string, locale translation.Locale, allowed ...rune) (escaped *EscapeStatus, output string) {
sb := &strings.Builder{} sb := &strings.Builder{}

View file

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

View file

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

View file

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