diff --git a/models/fixtures/label.yml b/models/fixtures/label.yml index acfac74968..84c2a7f418 100644 --- a/models/fixtures/label.yml +++ b/models/fixtures/label.yml @@ -3,6 +3,7 @@ repo_id: 1 org_id: 0 name: label1 + description: 'First label' color: '#abcdef' exclusive: false num_issues: 2 @@ -107,3 +108,26 @@ num_issues: 0 num_closed_issues: 0 archived_unix: 0 + +- + id: 11 + repo_id: 3 + org_id: 0 + name: " /'?&" + description: "Malicious label ' " + color: '#000000' + exclusive: true + num_issues: 0 + num_closed_issues: 0 + archived_unix: 0 + +- + id: 12 + repo_id: 3 + org_id: 0 + name: 'archived label<>' + color: '#000000' + exclusive: false + num_issues: 0 + num_closed_issues: 0 + archived_unix: 2991092130 diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 02b175e6f6..42b4bad83c 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -188,6 +188,7 @@ func NewFuncMap() template.FuncMap { "RenderMarkdownToHtml": RenderMarkdownToHtml, "RenderLabel": RenderLabel, "RenderLabels": RenderLabels, + "RenderUser": RenderUser, "RenderReviewRequest": RenderReviewRequest, // ----------------------------------------------------------------- diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index a4d7a82eea..48f4eb04a3 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -7,6 +7,7 @@ import ( "context" "encoding/hex" "fmt" + "html" "html/template" "math" "net/url" @@ -15,6 +16,7 @@ import ( "unicode" issues_model "forgejo.org/models/issues" + user_model "forgejo.org/models/user" "forgejo.org/modules/emoji" "forgejo.org/modules/log" "forgejo.org/modules/markup" @@ -26,7 +28,7 @@ import ( // RenderCommitMessage renders commit message with XSS-safe and special links. func RenderCommitMessage(ctx context.Context, msg string, metas map[string]string) template.HTML { - cleanMsg := template.HTMLEscapeString(msg) + cleanMsg := html.EscapeString(msg) // we can safely assume that it will not return any error, since there // shouldn't be any special HTML. fullMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ @@ -63,7 +65,7 @@ func RenderCommitMessageLinkSubject(ctx context.Context, msg, urlDefault string, Ctx: ctx, DefaultLink: urlDefault, Metas: metas, - }, template.HTMLEscapeString(msgLine)) + }, html.EscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessageSubject: %v", err) return template.HTML("") @@ -88,7 +90,7 @@ func RenderCommitBody(ctx context.Context, msg string, metas map[string]string) renderedMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ Ctx: ctx, Metas: metas, - }, template.HTMLEscapeString(msgLine)) + }, html.EscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessage: %v", err) return "" @@ -122,7 +124,7 @@ func RenderIssueTitle(ctx context.Context, text string, metas map[string]string) renderedText, err := markup.RenderIssueTitle(&markup.RenderContext{ Ctx: ctx, Metas: metas, - }, template.HTMLEscapeString(text)) + }, html.EscapeString(text)) if err != nil { log.Error("RenderIssueTitle: %v", err) return template.HTML("") @@ -132,7 +134,7 @@ func RenderIssueTitle(ctx context.Context, text string, metas map[string]string) // RenderRefIssueTitle renders referenced issue/pull title with defined post processors func RenderRefIssueTitle(ctx context.Context, text string) template.HTML { - renderedText, err := markup.RenderRefIssueTitle(&markup.RenderContext{Ctx: ctx}, template.HTMLEscapeString(text)) + renderedText, err := markup.RenderRefIssueTitle(&markup.RenderContext{Ctx: ctx}, html.EscapeString(text)) if err != nil { log.Error("RenderRefIssueTitle: %v", err) return "" @@ -150,7 +152,7 @@ func RenderLabel(ctx context.Context, locale translation.Locale, label *issues_m labelScope = label.ExclusiveScope() ) - description := emoji.ReplaceAliases(template.HTMLEscapeString(label.Description)) + description := emoji.ReplaceAliases(html.EscapeString(label.Description)) if label.IsArchived() { archivedCSSClass = "archived-label" @@ -212,7 +214,7 @@ func RenderLabel(ctx context.Context, locale translation.Locale, label *issues_m // RenderEmoji renders html text with emoji post processors func RenderEmoji(ctx context.Context, text string) template.HTML { renderedText, err := markup.RenderEmoji(&markup.RenderContext{Ctx: ctx}, - template.HTMLEscapeString(text)) + html.EscapeString(text)) if err != nil { log.Error("RenderEmoji: %v", err) return template.HTML("") @@ -263,10 +265,20 @@ func RenderLabels(ctx context.Context, locale translation.Locale, labels []*issu return template.HTML(htmlCode) } +func RenderUser(ctx context.Context, user user_model.User) template.HTML { + if user.ID > 0 { + return template.HTML(fmt.Sprintf( + "%s", + user.HomeLink(), html.EscapeString(user.GetDisplayName()))) + } + return template.HTML(fmt.Sprintf("%s", + html.EscapeString(user.GetDisplayName()))) +} + func RenderReviewRequest(users []issues_model.RequestReviewTarget) template.HTML { usernames := make([]string, 0, len(users)) for _, user := range users { - usernames = append(usernames, template.HTMLEscapeString(user.Name())) + usernames = append(usernames, html.EscapeString(user.Name())) } htmlCode := `` diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index 62e063213c..a5fb18642a 100644 --- a/modules/templates/util_render_test.go +++ b/modules/templates/util_render_test.go @@ -11,6 +11,9 @@ import ( "forgejo.org/models/db" issues_model "forgejo.org/models/issues" "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/setting" + "forgejo.org/modules/test" "forgejo.org/modules/translation" "github.com/stretchr/testify/assert" @@ -215,9 +218,51 @@ func TestRenderLabels(t *testing.T) { tr := &translation.MockLocale{} label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) + labelScoped := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 7}) + labelMalicious := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 11}) + labelArchived := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 12}) - assert.Contains(t, RenderLabels(db.DefaultContext, tr, []*issues_model.Label{label}, "user2/repo1", false), - "user2/repo1/issues?labels=1") - assert.Contains(t, RenderLabels(db.DefaultContext, tr, []*issues_model.Label{label}, "user2/repo1", true), - "user2/repo1/pulls?labels=1") + rendered := RenderLabels(db.DefaultContext, tr, []*issues_model.Label{label}, "user2/repo1", false) + assert.Contains(t, rendered, "user2/repo1/issues?labels=1") + assert.Contains(t, rendered, ">label1<") + assert.Contains(t, rendered, "title='First label'") + rendered = RenderLabels(db.DefaultContext, tr, []*issues_model.Label{label}, "user2/repo1", true) + assert.Contains(t, rendered, "user2/repo1/pulls?labels=1") + assert.Contains(t, rendered, ">label1<") + rendered = RenderLabels(db.DefaultContext, tr, []*issues_model.Label{labelScoped}, "user2/repo1", false) + assert.Contains(t, rendered, "user2/repo1/issues?labels=7") + assert.Contains(t, rendered, ">scope<") + assert.Contains(t, rendered, ">label1<") + rendered = RenderLabels(db.DefaultContext, tr, []*issues_model.Label{labelMalicious}, "user2/repo1", false) + assert.Contains(t, rendered, "user2/repo1/issues?labels=11") + assert.Contains(t, rendered, "> <script>malicious</script> <") + assert.Contains(t, rendered, ">'?&<") + assert.Contains(t, rendered, "title='Malicious label ' <script>malicious</script>'") + rendered = RenderLabels(db.DefaultContext, tr, []*issues_model.Label{labelArchived}, "user2/repo1", false) + assert.Contains(t, rendered, "user2/repo1/issues?labels=12") + assert.Contains(t, rendered, ">archived label<><") + assert.Contains(t, rendered, "title='repo.issues.archived_label_description'") +} + +func TestRenderUser(t *testing.T) { + unittest.PrepareTestEnv(t) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + ghost := user_model.NewGhostUser() + + assert.Contains(t, RenderUser(db.DefaultContext, *user), + "user2") + assert.Contains(t, RenderUser(db.DefaultContext, *org), + "org3") + assert.Contains(t, RenderUser(db.DefaultContext, *ghost), + "Ghost") + + defer test.MockVariableValue(&setting.UI.DefaultShowFullName, true)() + assert.Contains(t, RenderUser(db.DefaultContext, *user), + "< U<se>r Tw<o > ><") + assert.Contains(t, RenderUser(db.DefaultContext, *org), + "<<<< >> >> > >> > >>> >>") + assert.Contains(t, RenderUser(db.DefaultContext, *ghost), + "Ghost") } diff --git a/services/convert/issue_test.go b/services/convert/issue_test.go index 97bacfb229..ea8ad9b7ef 100644 --- a/services/convert/issue_test.go +++ b/services/convert/issue_test.go @@ -24,10 +24,11 @@ func TestLabel_ToLabel(t *testing.T) { label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: label.RepoID}) assert.Equal(t, &api.Label{ - ID: label.ID, - Name: label.Name, - Color: "abcdef", - URL: fmt.Sprintf("%sapi/v1/repos/user2/repo1/labels/%d", setting.AppURL, label.ID), + ID: label.ID, + Name: label.Name, + Color: "abcdef", + Description: label.Description, + URL: fmt.Sprintf("%sapi/v1/repos/user2/repo1/labels/%d", setting.AppURL, label.ID), }, ToLabel(label, repo, nil)) } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 3bc4cd0773..4ab1fa7584 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -221,27 +221,23 @@ {{else if and (eq .Type 9) (gt .AssigneeID 0)}}
{{svg "octicon-person"}} - {{if .RemovedAssignee}} - {{template "shared/user/avatarlink" dict "user" .Assignee}} - - {{template "shared/user/authorlink" .Assignee}} + {{template "shared/user/avatarlink" dict "user" .Assignee}} + + {{template "shared/user/authorlink" .Assignee}} + {{if .RemovedAssignee}} {{if eq .Poster.ID .Assignee.ID}} {{ctx.Locale.Tr "repo.issues.remove_self_assignment" $createdStr}} {{else}} - {{ctx.Locale.Tr "repo.issues.remove_assignee_at" .Poster.GetDisplayName $createdStr}} + {{ctx.Locale.Tr "repo.issues.remove_assignee_at" (RenderUser $.Context .Poster) $createdStr}} {{end}} - - {{else}} - {{template "shared/user/avatarlink" dict "user" .Assignee}} - - {{template "shared/user/authorlink" .Assignee}} + {{else}} {{if eq .Poster.ID .AssigneeID}} {{ctx.Locale.Tr "repo.issues.self_assign_at" $createdStr}} {{else}} - {{ctx.Locale.Tr "repo.issues.add_assignee_at" .Poster.GetDisplayName $createdStr}} + {{ctx.Locale.Tr "repo.issues.add_assignee_at" (RenderUser $.Context .Poster) $createdStr}} {{end}} - - {{end}} + {{end}} +
{{else if eq .Type 10}}
diff --git a/tests/integration/issue_comment_test.go b/tests/integration/issue_comment_test.go index 0c53c3028b..6c4a514eba 100644 --- a/tests/integration/issue_comment_test.go +++ b/tests/integration/issue_comment_test.go @@ -223,15 +223,13 @@ func TestIssueCommentChangeAssignee(t *testing.T) { testIssueCommentChangeEvent(t, htmlDoc, "2041", "octicon-person", "User One", "/user1", []string{"user1 was unassigned by user2"}, - []string{"/user1"}) - // []string{"/user1", "/user2"}) + []string{"/user1", "/user2"}) // Add other testIssueCommentChangeEvent(t, htmlDoc, "2042", "octicon-person", "< Ur Tw ><", "/user2", []string{"user2 was assigned by user1"}, - []string{"/user2"}) - // []string{"/user2", "/user1"}) + []string{"/user2", "/user1"}) // Self-remove testIssueCommentChangeEvent(t, htmlDoc, "2043",