From 2892aaab0258a606719ad7f71a5087aba3549325 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. (cherry picked from commit fb42972c057364a1dc99dfb528554e7a94415be7) Conflicts: docs/content/administration/mail-templates.en-us.md docs/content/administration/mail-templates.zh-cn.md prefer their version always --- .../administration/mail-templates.en-us.md | 16 ++++++------- .../administration/mail-templates.zh-cn.md | 24 +++++++++---------- 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, 52 insertions(+), 47 deletions(-) diff --git a/docs/content/administration/mail-templates.en-us.md b/docs/content/administration/mail-templates.en-us.md index 32b352da4..9077f97ae 100644 --- a/docs/content/administration/mail-templates.en-us.md +++ b/docs/content/administration/mail-templates.en-us.md @@ -222,9 +222,9 @@ Please check [Gitea's logs](administration/logging-config.md) for error messages {{.Repo}}#{{.Issue.Index}}.

{{if not (eq .Body "")}} -

Message content:

+

Message content

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

@@ -245,7 +245,7 @@ This template produces something along these lines: > [@rhonda](#) (Rhonda Myers) updated [mike/stuff#38](#). > -> #### Message content: +> #### Message content > > \_********************************\_******************************** > @@ -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. | -| `Safe` | string | Body only | Takes the input as HTML; can be used for `.ReviewComments.RenderedContent`. | +| `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 588f0b2cc..d58f9dc17 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}}

@@ -228,7 +228,7 @@ _主题_ 和 _邮件正文_ 由 [Golang的模板引擎](https://go.dev/pkg/text/ > [@rhonda](#)（Rhonda Myers）更新了 [mike/stuff#38](#)。 > -> #### 消息内容： +> #### 消息内容 > > \_********************************\_******************************** > @@ -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 标签对文本进行清理 | -| `Safe` | 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 708de57c9..9afcb96fc 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, @@ -210,8 +210,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 8f5d633d4..3365278ac 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 f4079f188..743465082 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -65,7 +65,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 } @@ -244,7 +244,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio } } if len(content) == 0 { - content = templates.Str2html(desc) + content = templates.SanitizeHTML(desc) } // It's a common practice for feed generators to use plain text titles. diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index f2db4a457..82cd91997 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 2ba40fcd5..e74049810 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1770,7 +1770,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 160584f76..760d3bfa2 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 1d7ec15dc..6801c8240 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 49887e1e1..4d467041e 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 115ee9295..7892a5716 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 9f3b8dbad..179df5bb3 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -166,7 +166,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 5a3fc0e7b..00f9a48ba 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 d0c874614..8589020a5 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