Skip to content

Commit

Permalink
Refactor locale&string&template related code (go-gitea#29165)
Browse files Browse the repository at this point in the history
Clarify when "string" should be used (and be escaped), and when
"template.HTML" should be used (no need to escape)

And help PRs like  go-gitea#29059 , to render the error messages correctly.
  • Loading branch information
wxiaoguang authored and silverwind committed Feb 15, 2024
1 parent 6581d58 commit 21c428f
Show file tree
Hide file tree
Showing 77 changed files with 356 additions and 284 deletions.
2 changes: 1 addition & 1 deletion models/actions/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (r *ActionRunner) StatusName() string {
}

func (r *ActionRunner) StatusLocaleName(lang translation.Locale) string {
return lang.Tr("actions.runners.status." + r.StatusName())
return lang.TrString("actions.runners.status." + r.StatusName())
}

func (r *ActionRunner) IsOnline() bool {
Expand Down
2 changes: 1 addition & 1 deletion models/actions/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (s Status) String() string {

// LocaleString returns the locale string name of the Status
func (s Status) LocaleString(lang translation.Locale) string {
return lang.Tr("actions.status." + s.String())
return lang.TrString("actions.status." + s.String())
}

// IsDone returns whether the Status is final
Expand Down
2 changes: 1 addition & 1 deletion models/git/commit_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (status *CommitStatus) APIURL(ctx context.Context) string {

// LocaleString returns the locale string name of the Status
func (status *CommitStatus) LocaleString(lang translation.Locale) string {
return lang.Tr("repo.commitstatus." + status.State.String())
return lang.TrString("repo.commitstatus." + status.State.String())
}

// CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc
Expand Down
4 changes: 2 additions & 2 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,12 @@ const (

// LocaleString returns the locale string name of the role
func (r RoleInRepo) LocaleString(lang translation.Locale) string {
return lang.Tr("repo.issues.role." + string(r))
return lang.TrString("repo.issues.role." + string(r))
}

// LocaleHelper returns the locale tooltip of the role
func (r RoleInRepo) LocaleHelper(lang translation.Locale) string {
return lang.Tr("repo.issues.role." + string(r) + "_helper")
return lang.TrString("repo.issues.role." + string(r) + "_helper")
}

// Comment represents a comment in commit and issue page.
Expand Down
10 changes: 5 additions & 5 deletions models/shared/types/ownertype.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ const (
func (o OwnerType) LocaleString(locale translation.Locale) string {
switch o {
case OwnerTypeSystemGlobal:
return locale.Tr("concept_system_global")
return locale.TrString("concept_system_global")
case OwnerTypeIndividual:
return locale.Tr("concept_user_individual")
return locale.TrString("concept_user_individual")
case OwnerTypeRepository:
return locale.Tr("concept_code_repository")
return locale.TrString("concept_code_repository")
case OwnerTypeOrganization:
return locale.Tr("concept_user_organization")
return locale.TrString("concept_user_organization")
}
return locale.Tr("unknown")
return locale.TrString("unknown")
}
9 changes: 5 additions & 4 deletions modules/auth/password/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"crypto/rand"
"errors"
"html/template"
"math/big"
"strings"
"sync"
Expand Down Expand Up @@ -121,15 +122,15 @@ func Generate(n int) (string, error) {
}

// BuildComplexityError builds the error message when password complexity checks fail
func BuildComplexityError(locale translation.Locale) string {
func BuildComplexityError(locale translation.Locale) template.HTML {
var buffer bytes.Buffer
buffer.WriteString(locale.Tr("form.password_complexity"))
buffer.WriteString(locale.TrString("form.password_complexity"))
buffer.WriteString("<ul>")
for _, c := range requiredList {
buffer.WriteString("<li>")
buffer.WriteString(locale.Tr(c.TrNameOne))
buffer.WriteString(locale.TrString(c.TrNameOne))
buffer.WriteString("</li>")
}
buffer.WriteString("</ul>")
return buffer.String()
return template.HTML(buffer.String())
}
2 changes: 1 addition & 1 deletion modules/charset/escape_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (e *escapeStreamer) ambiguousRune(r, c rune) error {
Val: "ambiguous-code-point",
}, html.Attribute{
Key: "data-tooltip-content",
Val: e.locale.Tr("repo.ambiguous_character", r, c),
Val: e.locale.TrString("repo.ambiguous_character", r, c),
}); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion modules/context/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func APIContexter() func(http.Handler) http.Handler {
// NotFound handles 404s for APIContext
// String will replace message, errors will be added to a slice
func (ctx *APIContext) NotFound(objs ...any) {
message := ctx.Tr("error.not_found")
message := ctx.Locale.TrString("error.not_found")
var errors []string
for _, obj := range objs {
// Ignore nil
Expand Down
5 changes: 3 additions & 2 deletions modules/context/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package context
import (
"context"
"fmt"
"html/template"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -286,11 +287,11 @@ func (b *Base) cleanUp() {
}
}

func (b *Base) Tr(msg string, args ...any) string {
func (b *Base) Tr(msg string, args ...any) template.HTML {
return b.Locale.Tr(msg, args...)
}

func (b *Base) TrN(cnt any, key1, keyN string, args ...any) string {
func (b *Base) TrN(cnt any, key1, keyN string, args ...any) template.HTML {
return b.Locale.TrN(cnt, key1, keyN, args...)
}

Expand Down
23 changes: 10 additions & 13 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package context

import (
"context"
"html"
"fmt"
"html/template"
"io"
"net/http"
Expand Down Expand Up @@ -71,16 +71,6 @@ func init() {
})
}

// TrHTMLEscapeArgs runs ".Locale.Tr()" but pre-escapes all arguments with html.EscapeString.
// This is useful if the locale message is intended to only produce HTML content.
func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string {
trArgs := make([]any, len(args))
for i, arg := range args {
trArgs[i] = html.EscapeString(arg)
}
return ctx.Locale.Tr(msg, trArgs...)
}

type webContextKeyType struct{}

var WebContextKey = webContextKeyType{}
Expand Down Expand Up @@ -253,6 +243,13 @@ func (ctx *Context) JSONOK() {
ctx.JSON(http.StatusOK, map[string]any{"ok": true}) // this is only a dummy response, frontend seldom uses it
}

func (ctx *Context) JSONError(msg string) {
ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": msg})
func (ctx *Context) JSONError(msg any) {
switch v := msg.(type) {
case string:
ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": v, "renderFormat": "text"})
case template.HTML:
ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": v, "renderFormat": "html"})
default:
panic(fmt.Sprintf("unsupported type: %T", msg))
}
}
5 changes: 2 additions & 3 deletions modules/context/context_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,11 @@ func (ctx *Context) RenderToString(name base.TplName, data map[string]any) (stri
}

// RenderWithErr used for page has form validation but need to prompt error to users.
func (ctx *Context) RenderWithErr(msg string, tpl base.TplName, form any) {
func (ctx *Context) RenderWithErr(msg any, tpl base.TplName, form any) {
if form != nil {
middleware.AssignForm(form, ctx.Data)
}
ctx.Flash.ErrorMsg = msg
ctx.Data["Flash"] = ctx.Flash
ctx.Flash.Error(msg, true)
ctx.HTML(http.StatusOK, tpl)
}

Expand Down
3 changes: 2 additions & 1 deletion modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package context

import (
"context"
"errors"
"fmt"
"html"
"net/http"
Expand Down Expand Up @@ -85,7 +86,7 @@ func (r *Repository) CanCreateBranch() bool {
func RepoMustNotBeArchived() func(ctx *Context) {
return func(ctx *Context) {
if ctx.Repo.Repository.IsArchived {
ctx.NotFound("IsArchived", fmt.Errorf(ctx.Tr("repo.archive.title")))
ctx.NotFound("IsArchived", errors.New(ctx.Locale.TrString("repo.archive.title")))
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/csv/csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ func guessDelimiter(data []byte) rune {
func FormatError(err error, locale translation.Locale) (string, error) {
if perr, ok := err.(*stdcsv.ParseError); ok {
if perr.Err == stdcsv.ErrFieldCount {
return locale.Tr("repo.error.csv.invalid_field_count", perr.Line), nil
return locale.TrString("repo.error.csv.invalid_field_count", perr.Line), nil
}
return locale.Tr("repo.error.csv.unexpected", perr.Line, perr.Column), nil
return locale.TrString("repo.error.csv.unexpected", perr.Line, perr.Column), nil
}

return "", err
Expand Down
2 changes: 1 addition & 1 deletion modules/markup/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) {
// indicate that in the text by appending (comment)
if m[4] != -1 && m[5] != -1 {
if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok {
text += " " + locale.Tr("repo.from_comment")
text += " " + locale.TrString("repo.from_comment")
} else {
text += " (comment)"
}
Expand Down
2 changes: 1 addition & 1 deletion modules/markup/markdown/toc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func createTOCNode(toc []markup.Header, lang string, detailsAttrs map[string]str
details.SetAttributeString(k, []byte(v))
}

summary.AppendChild(summary, ast.NewString([]byte(translation.NewLocale(lang).Tr("toc"))))
summary.AppendChild(summary, ast.NewString([]byte(translation.NewLocale(lang).TrString("toc"))))
details.AppendChild(details, summary)
ul := ast.NewList('-')
details.AppendChild(details, ul)
Expand Down
2 changes: 1 addition & 1 deletion modules/migration/messenger.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

package migration

// Messenger is a formatting function similar to i18n.Tr
// Messenger is a formatting function similar to i18n.TrString
type Messenger func(key string, args ...any)

// NilMessenger represents an empty formatting function
Expand Down
46 changes: 39 additions & 7 deletions modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func NewFuncMap() template.FuncMap {
"dict": dict, // it's lowercase because this name has been widely used. Our other functions should have uppercase names.
"Eval": Eval,
"Safe": Safe,
"Escape": html.EscapeString,
"Escape": Escape,
"QueryEscape": url.QueryEscape,
"JSEscape": template.JSEscapeString,
"Str2html": Str2html, // TODO: rename it to SanitizeHTML
Expand Down Expand Up @@ -159,7 +159,7 @@ func NewFuncMap() template.FuncMap {
"RenderCodeBlock": RenderCodeBlock,
"RenderIssueTitle": RenderIssueTitle,
"RenderEmoji": RenderEmoji,
"RenderEmojiPlain": emoji.ReplaceAliases,
"RenderEmojiPlain": RenderEmojiPlain,
"ReactionToEmoji": ReactionToEmoji,

"RenderMarkdownToHtml": RenderMarkdownToHtml,
Expand All @@ -180,13 +180,45 @@ func NewFuncMap() template.FuncMap {
}

// Safe render raw as HTML
func Safe(raw string) template.HTML {
return template.HTML(raw)
func Safe(s any) template.HTML {
switch v := s.(type) {
case string:
return template.HTML(v)
case template.HTML:
return v
}
panic(fmt.Sprintf("unexpected type %T", s))
}

// Str2html sanitizes the input by pre-defined markdown rules
func Str2html(s any) template.HTML {
switch v := s.(type) {
case string:
return template.HTML(markup.Sanitize(v))
case template.HTML:
return template.HTML(markup.Sanitize(string(v)))
}
panic(fmt.Sprintf("unexpected type %T", s))
}

// Str2html render Markdown text to HTML
func Str2html(raw string) template.HTML {
return template.HTML(markup.Sanitize(raw))
func Escape(s any) template.HTML {
switch v := s.(type) {
case string:
return template.HTML(html.EscapeString(v))
case template.HTML:
return v
}
panic(fmt.Sprintf("unexpected type %T", s))
}

func RenderEmojiPlain(s any) any {
switch v := s.(type) {
case string:
return emoji.ReplaceAliases(v)
case template.HTML:
return template.HTML(emoji.ReplaceAliases(string(v)))
}
panic(fmt.Sprintf("unexpected type %T", s))
}

// DotEscape wraps a dots in names with ZWJ [U+200D] in order to prevent autolinkers from detecting these as urls
Expand Down
Loading

0 comments on commit 21c428f

Please sign in to comment.