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.
go-gitea · Mar 1, 2024 · fb42972 · fb42972
1 parent cb52b17
commit fb42972
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 43 deletions.
diff --git 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 "")}}
             <h3>Message content</h3>
             <hr>
-            {{.Body | Str2html}}
+            {{.Body | SanitizeHTML}}
         {{end}}
     </p>
     <hr>
@@ -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 "Escape<my>text"}}
-Or this:           {{"Escape<my>text" | Str2html}}
+Like this:         {{SanitizeHTML "Escape<my>text"}}
+Or this:           {{"Escape<my>text" | 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
@@ -207,7 +207,7 @@ _主题_ 和 _邮件正文_ 由 [Golang的模板引擎](https://go.dev/pkg/text/
         {{if not (eq .Body "")}}
             <h3>消息内容：</h3>
             <hr>
-            {{.Body | Str2html}}
+            {{.Body | SanitizeHTML}}
         {{end}}
     </p>
     <hr>
@@ -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 "Escape<my>text"}}
-或者这样使用：       {{"Escape<my>text" | Str2html}}
+像这样使用：         {{SanitizeHTML "Escape<my>text"}}
+或者这样使用：       {{"Escape<my>text" | SanitizeHTML}}
 或者这样使用：       {{AppUrl}}
 但不要像这样使用：   {{.AppUrl}}
 ```
diff --git 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
@@ -61,3 +61,8 @@ func TestJSEscapeSafe(t *testing.T) {
 func TestHTMLFormat(t *testing.T) {
 	assert.Equal(t, template.HTML("<a>&lt; < 1</a>"), HTMLFormat("<a>%s %s %d</a>", "<", template.HTML("<"), 1))
 }
+
+func TestSanitizeHTML(t *testing.T) {
+	assert.Equal(t, template.HTML(`<a href="/" rel="nofollow">link</a> xss <div>inline</div>`), SanitizeHTML(`<a href="/">link</a> <a href="javascript:">xss</a> <div style="dangerous">inline</div>`))
+	assert.Equal(t, template.HTML(`<a href="/" rel="nofollow">link</a> xss <div>inline</div>`), SanitizeHTML(template.HTML(`<a href="/">link</a> <a href="javascript:">xss</a> <div style="dangerous">inline</div>`)))
+}
diff --git 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
@@ -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
@@ -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
@@ -1,20 +1,20 @@
 {{if .Flash.ErrorMsg}}
 	<div class="ui negative message flash-message flash-error">
-		<p>{{.Flash.ErrorMsg | Str2html}}</p>
+		<p>{{.Flash.ErrorMsg | SanitizeHTML}}</p>
 	</div>
 {{end}}
 {{if .Flash.SuccessMsg}}
 	<div class="ui positive message flash-message flash-success">
-		<p>{{.Flash.SuccessMsg | Str2html}}</p>
+		<p>{{.Flash.SuccessMsg | SanitizeHTML}}</p>
 	</div>
 {{end}}
 {{if .Flash.InfoMsg}}
 	<div class="ui info message flash-message flash-info">
-		<p>{{.Flash.InfoMsg | Str2html}}</p>
+		<p>{{.Flash.InfoMsg | SanitizeHTML}}</p>
 	</div>
 {{end}}
 {{if .Flash.WarningMsg}}
 	<div class="ui warning message flash-message flash-warning">
-		<p>{{.Flash.WarningMsg | Str2html}}</p>
+		<p>{{.Flash.WarningMsg | SanitizeHTML}}</p>
 	</div>
 {{end}}
diff --git a/templates/base/alert_details.tmpl b/templates/base/alert_details.tmpl
@@ -2,6 +2,6 @@
 <details>
 	<summary>{{.Summary}}</summary>
 	<code>
-		{{.Details | Str2html}}
+		{{.Details | SanitizeHTML}}
 	</code>
 </details>
diff --git 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}}
 			<hr>

diff --git a/templates/repo/commit_page.tmpl b/templates/repo/commit_page.tmpl
@@ -276,7 +276,7 @@
 				<span class="text grey" id="note-authored-time">{{TimeSince .NoteCommit.Author.When ctx.Locale}}</span>
 			</div>
 			<div class="ui bottom attached info segment git-notes">
-				<pre class="commit-body">{{.NoteRendered | Str2html}}</pre>
+				<pre class="commit-body">{{.NoteRendered | SanitizeHTML}}</pre>
 			</div>
 		{{end}}
 		{{template "repo/diff/box" .}}

diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl
@@ -162,7 +162,7 @@
 				</span>
 				<div class="detail">
 					{{svg "octicon-git-commit"}}
-					<span class="text grey muted-links">{{.Content | Str2html}}</span>
+					<span class="text grey muted-links">{{.Content | SanitizeHTML}}</span>
 				</div>
 			</div>
 		{{else if eq .Type 7}}

diff --git a/templates/repo/settings/webhook/base_list.tmpl b/templates/repo/settings/webhook/base_list.tmpl
@@ -10,7 +10,7 @@
 <div class="ui attached segment">
 	<div class="ui list">
 		<div class="item">
-			{{.Description | Str2html}}
+			{{.Description | SanitizeHTML}}
 		</div>
 		{{range .Webhooks}}
 			<div class="item truncated-item-container">

diff --git 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