From fb42972c057364a1dc99dfb528554e7a94415be7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 1 Mar 2024 18:16:19 +0800 Subject: [PATCH] Rename Str2html to SanitizeHTML and clarify its behavior (#29516) Str2html was abused a lot. So use a proper name for it: SanitizeHTML And add some tests to show its behavior. --- .../administration/mail-templates.en-us.md | 10 ++++---- .../administration/mail-templates.zh-cn.md | 22 ++++++++--------- modules/templates/helper.go | 24 +++++++++---------- modules/templates/helper_test.go | 5 ++++ routers/web/feed/convert.go | 4 ++-- routers/web/org/projects.go | 4 ++-- routers/web/repo/issue.go | 2 +- templates/base/alert.tmpl | 8 +++---- templates/base/alert_details.tmpl | 2 +- templates/mail/issue/default.tmpl | 2 +- templates/repo/commit_page.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 2 +- .../repo/settings/webhook/base_list.tmpl | 2 +- templates/status/500.tmpl | 2 +- 14 files changed, 48 insertions(+), 43 deletions(-) diff --git a/docs/content/administration/mail-templates.en-us.md b/docs/content/administration/mail-templates.en-us.md index b642ff4aa7..9077f97aea 100644 --- a/docs/content/administration/mail-templates.en-us.md +++ b/docs/content/administration/mail-templates.en-us.md @@ -224,7 +224,7 @@ Please check [Gitea's logs](administration/logging-config.md) for error messages {{if not (eq .Body "")}}

Message content

- {{.Body | Str2html}} + {{.Body | SanitizeHTML}} {{end}}

@@ -260,19 +260,19 @@ The template system contains several functions that can be used to further proce the messages. Here's a list of some of them: | Name | Parameters | Available | Usage | -| ---------------- | ----------- | --------- | --------------------------------------------------------------------------- | +| ---------------- | ----------- | --------- |-----------------------------------------------------------------------------| | `AppUrl` | - | Any | Gitea's URL | | `AppName` | - | Any | Set from `app.ini`, usually "Gitea" | | `AppDomain` | - | Any | Gitea's host name | | `EllipsisString` | string, int | Any | Truncates a string to the specified length; adds ellipsis as needed | -| `Str2html` | string | Body only | Sanitizes text by removing any HTML tags from it. | +| `SanitizeHTML` | string | Body only | Sanitizes text by removing any dangerous HTML tags from it. | | `SafeHTML` | string | Body only | Takes the input as HTML; can be used for `.ReviewComments.RenderedContent`. | These are _functions_, not metadata, so they have to be used: ```html -Like this: {{Str2html "Escapetext"}} -Or this: {{"Escapetext" | Str2html}} +Like this: {{SanitizeHTML "Escapetext"}} +Or this: {{"Escapetext" | SanitizeHTML}} Or this: {{AppUrl}} But not like this: {{.AppUrl}} ``` diff --git a/docs/content/administration/mail-templates.zh-cn.md b/docs/content/administration/mail-templates.zh-cn.md index fd455ef3a8..d58f9dc176 100644 --- a/docs/content/administration/mail-templates.zh-cn.md +++ b/docs/content/administration/mail-templates.zh-cn.md @@ -207,7 +207,7 @@ _主题_ 和 _邮件正文_ 由 [Golang的模板引擎](https://go.dev/pkg/text/ {{if not (eq .Body "")}}

消息内容：

- {{.Body | Str2html}} + {{.Body | SanitizeHTML}} {{end}}

@@ -242,20 +242,20 @@ _主题_ 和 _邮件正文_ 由 [Golang的模板引擎](https://go.dev/pkg/text/ 模板系统包含一些函数，可用于进一步处理和格式化消息。以下是其中一些函数的列表： -| 函数名 | 参数 | 可用于 | 用法 | -|------------------| ----------- | ------------ | --------------------------------------------------------------------------------- | -| `AppUrl` | - | 任何地方 | Gitea 的 URL | -| `AppName` | - | 任何地方 | 从 `app.ini` 中设置，通常为 "Gitea" | -| `AppDomain` | - | 任何地方 | Gitea 的主机名 | -| `EllipsisString` | string, int | 任何地方 | 将字符串截断为指定长度；根据需要添加省略号 | -| `Str2html` | string | 仅正文部分 | 通过删除其中的 HTML 标签对文本进行清理 | -| `SafeHTML` | string | 仅正文部分 | 将输入作为 HTML 处理；可用于 `.ReviewComments.RenderedContent` 等字段 | +| 函数名 | 参数 | 可用于 | 用法 | +|------------------| ----------- | ------------ |---------------------------------------------------------| +| `AppUrl` | - | 任何地方 | Gitea 的 URL | +| `AppName` | - | 任何地方 | 从 `app.ini` 中设置，通常为 "Gitea" | +| `AppDomain` | - | 任何地方 | Gitea 的主机名 | +| `EllipsisString` | string, int | 任何地方 | 将字符串截断为指定长度；根据需要添加省略号 | +| `SanitizeHTML` | string | 仅正文部分 | 通过删除其中的危险 HTML 标签对文本进行清理 | +| `SafeHTML` | string | 仅正文部分 | 将输入作为 HTML 处理；可用于 `.ReviewComments.RenderedContent` 等字段 | 这些都是 _函数_，而不是元数据，因此必须按以下方式使用： ```html -像这样使用： {{Str2html "Escapetext"}} -或者这样使用： {{"Escapetext" | Str2html}} +像这样使用： {{SanitizeHTML "Escapetext"}} +或者这样使用： {{"Escapetext" | SanitizeHTML}} 或者这样使用： {{AppUrl}} 但不要像这样使用： {{.AppUrl}} ``` diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 0f39767586..1487fce69d 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -33,16 +33,16 @@ func NewFuncMap() template.FuncMap { // ----------------------------------------------------------------- // html/template related functions - "dict": dict, // it's lowercase because this name has been widely used. Our other functions should have uppercase names. - "Eval": Eval, - "SafeHTML": SafeHTML, - "HTMLFormat": HTMLFormat, - "HTMLEscape": HTMLEscape, - "QueryEscape": url.QueryEscape, - "JSEscape": JSEscapeSafe, - "Str2html": Str2html, // TODO: rename it to SanitizeHTML - "URLJoin": util.URLJoin, - "DotEscape": DotEscape, + "dict": dict, // it's lowercase because this name has been widely used. Our other functions should have uppercase names. + "Eval": Eval, + "SafeHTML": SafeHTML, + "HTMLFormat": HTMLFormat, + "HTMLEscape": HTMLEscape, + "QueryEscape": url.QueryEscape, + "JSEscape": JSEscapeSafe, + "SanitizeHTML": SanitizeHTML, + "URLJoin": util.URLJoin, + "DotEscape": DotEscape, "PathEscape": url.PathEscape, "PathEscapeSegments": util.PathEscapeSegments, @@ -207,8 +207,8 @@ func SafeHTML(s any) template.HTML { panic(fmt.Sprintf("unexpected type %T", s)) } -// Str2html sanitizes the input by pre-defined markdown rules -func Str2html(s any) template.HTML { +// SanitizeHTML sanitizes the input by pre-defined markdown rules +func SanitizeHTML(s any) template.HTML { switch v := s.(type) { case string: return template.HTML(markup.Sanitize(v)) diff --git a/modules/templates/helper_test.go b/modules/templates/helper_test.go index 8f5d633d4f..3365278ac2 100644 --- a/modules/templates/helper_test.go +++ b/modules/templates/helper_test.go @@ -61,3 +61,8 @@ func TestJSEscapeSafe(t *testing.T) { func TestHTMLFormat(t *testing.T) { assert.Equal(t, template.HTML("< < 1"), HTMLFormat("%s %s %d", "<", template.HTML("<"), 1)) } + +func TestSanitizeHTML(t *testing.T) { + assert.Equal(t, template.HTML(`link xss
inline
`), SanitizeHTML(`link xss
inline
`)) + assert.Equal(t, template.HTML(`link xss
inline
`), SanitizeHTML(template.HTML(`link xss
inline
`))) +} diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index 3a2de1d9a1..3defa436a7 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -64,7 +64,7 @@ func renderMarkdown(ctx *context.Context, act *activities_model.Action, content } markdown, err := markdown.RenderString(markdownCtx, content) if err != nil { - return templates.Str2html(content) // old code did so: use Str2html to render in tmpl + return templates.SanitizeHTML(content) // old code did so: use SanitizeHTML to render in tmpl } return markdown } @@ -243,7 +243,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio } } if len(content) == 0 { - content = templates.Str2html(desc) + content = templates.SanitizeHTML(desc) } items = append(items, &feeds.Item{ diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index f2db4a4579..82cd91997a 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -105,7 +105,7 @@ func Projects(ctx *context.Context) { } for _, project := range projects { - project.RenderedContent = templates.Str2html(project.Description) // FIXME: is it right? why not render? + project.RenderedContent = templates.SanitizeHTML(project.Description) // FIXME: is it right? why not render? } err = shared_user.LoadHeaderCount(ctx) @@ -396,7 +396,7 @@ func ViewProject(ctx *context.Context) { } } - project.RenderedContent = templates.Str2html(project.Description) // FIXME: is it right? why not render? + project.RenderedContent = templates.SanitizeHTML(project.Description) // FIXME: is it right? why not render? ctx.Data["LinkedPRs"] = linkedPrsMap ctx.Data["PageIsViewProjects"] = true ctx.Data["CanWriteProjects"] = canWriteProjects(ctx) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 702aa7201b..ebaa955ac8 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1761,7 +1761,7 @@ func ViewIssue(ctx *context.Context) { // so "|" is used as delimeter to mark the new format if comment.Content[0] != '|' { // handle old time comments that have formatted text stored - comment.RenderedContent = templates.Str2html(comment.Content) + comment.RenderedContent = templates.SanitizeHTML(comment.Content) comment.Content = "" } else { // else it's just a duration in seconds to pass on to the frontend diff --git a/templates/base/alert.tmpl b/templates/base/alert.tmpl index 160584f769..760d3bfa2c 100644 --- a/templates/base/alert.tmpl +++ b/templates/base/alert.tmpl @@ -1,20 +1,20 @@ {{if .Flash.ErrorMsg}}
-

{{.Flash.ErrorMsg | Str2html}}

+

{{.Flash.ErrorMsg | SanitizeHTML}}

{{end}} {{if .Flash.SuccessMsg}}
-

{{.Flash.SuccessMsg | Str2html}}

+

{{.Flash.SuccessMsg | SanitizeHTML}}

{{end}} {{if .Flash.InfoMsg}}
-

{{.Flash.InfoMsg | Str2html}}

+

{{.Flash.InfoMsg | SanitizeHTML}}

{{end}} {{if .Flash.WarningMsg}}
-

{{.Flash.WarningMsg | Str2html}}

+

{{.Flash.WarningMsg | SanitizeHTML}}

{{end}} diff --git a/templates/base/alert_details.tmpl b/templates/base/alert_details.tmpl index 1d7ec15dc0..6801c8240f 100644 --- a/templates/base/alert_details.tmpl +++ b/templates/base/alert_details.tmpl @@ -2,6 +2,6 @@
{{.Summary}} - {{.Details | Str2html}} + {{.Details | SanitizeHTML}}
diff --git a/templates/mail/issue/default.tmpl b/templates/mail/issue/default.tmpl index 79dbe897cc..10fa0f1ffc 100644 --- a/templates/mail/issue/default.tmpl +++ b/templates/mail/issue/default.tmpl @@ -58,7 +58,7 @@ {{.locale.Tr "mail.issue.action.new" .Doer.Name .Issue.Index}} {{end}} {{else}} - {{.Body | Str2html}} + {{.Body | SanitizeHTML}} {{end -}} {{- range .ReviewComments}}
diff --git a/templates/repo/commit_page.tmpl b/templates/repo/commit_page.tmpl index 115ee92955..7892a57163 100644 --- a/templates/repo/commit_page.tmpl +++ b/templates/repo/commit_page.tmpl @@ -276,7 +276,7 @@ {{TimeSince .NoteCommit.Author.When ctx.Locale}}
- 
{{.NoteRendered | Str2html}}
+ 
{{.NoteRendered | SanitizeHTML}}
{{end}} {{template "repo/diff/box" .}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 36ef5751ae..66ecc544d2 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -162,7 +162,7 @@
{{svg "octicon-git-commit"}} - {{.Content | Str2html}} + {{.Content | SanitizeHTML}}
{{else if eq .Type 7}} diff --git a/templates/repo/settings/webhook/base_list.tmpl b/templates/repo/settings/webhook/base_list.tmpl index 5a3fc0e7b8..00f9a48ba7 100644 --- a/templates/repo/settings/webhook/base_list.tmpl +++ b/templates/repo/settings/webhook/base_list.tmpl @@ -10,7 +10,7 @@
- {{.Description | Str2html}} + {{.Description | SanitizeHTML}}
{{range .Webhooks}}
diff --git a/templates/status/500.tmpl b/templates/status/500.tmpl index d6cff28174..a92933c153 100644 --- a/templates/status/500.tmpl +++ b/templates/status/500.tmpl @@ -1,5 +1,5 @@ {{/* This page should only depend the minimal template functions/variables, to avoid triggering new panics. -* base template functions: AppName, AssetUrlPrefix, AssetVersion, AppSubUrl, ThemeName, Str2html +* base template functions: AppName, AssetUrlPrefix, AssetVersion, AppSubUrl, ThemeName, SanitizeHTML * ctx.Locale * .Flash * .ErrorMsg