Skip to content

Commit

Permalink
Ensure that plain files are rendered correctly even when containing a…
Browse files Browse the repository at this point in the history
…mbiguous 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]>
  • Loading branch information
zeripath committed Dec 3, 2022
1 parent 0e46499 commit 5cc8a1a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 22 deletions.
30 changes: 30 additions & 0 deletions modules/charset/escape.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package charset

import (
"bufio"
"io"
"strings"

Expand Down Expand Up @@ -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{}
Expand Down
20 changes: 4 additions & 16 deletions routers/web/repo/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
gocontext "context"
"encoding/base64"
"fmt"
gotemplate "html/template"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions templates/repo/settings/lfs_file.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
6 changes: 3 additions & 3 deletions templates/repo/view_file.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down

0 comments on commit 5cc8a1a

Please sign in to comment.