Skip to content

Commit bfe13a2

Browse files
lafrikslunny
lafriks
authored and
lunny
committed
Fix escaping changed title in comments (#3530) (#3535)
* Fix escaping of wiki page titile Signed-off-by: Lauris Bukšis-Haberkorns <[email protected]>
1 parent ed27da4 commit bfe13a2

File tree

5 files changed

+58
-23
lines changed

5 files changed

+58
-23
lines changed

‎integrations/pull_create_test.go

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"github.com/stretchr/testify/assert"
1414
)
1515

16-
func testPullCreate(t *testing.T, session *TestSession, user, repo, branch string) *TestResponse {
16+
func testPullCreate(t *testing.T, session *TestSession, user, repo, branch, title string) *TestResponse {
1717
req := NewRequest(t, "GET", path.Join(user, repo))
1818
resp := session.MakeRequest(t, req, http.StatusOK)
1919

@@ -34,7 +34,7 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch strin
3434
assert.True(t, exists, "The template has changed")
3535
req = NewRequestWithValues(t, "POST", link, map[string]string{
3636
"_csrf": htmlDoc.GetCSRF(),
37-
"title": "This is a pull title",
37+
"title": title,
3838
})
3939
resp = session.MakeRequest(t, req, http.StatusFound)
4040

@@ -48,5 +48,40 @@ func TestPullCreate(t *testing.T) {
4848
session := loginUser(t, "user1")
4949
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
5050
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
51-
testPullCreate(t, session, "user1", "repo1", "master")
51+
testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
52+
}
53+
54+
func TestPullCreate_TitleEscape(t *testing.T) {
55+
prepareTestEnv(t)
56+
session := loginUser(t, "user1")
57+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
58+
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
59+
resp := testPullCreate(t, session, "user1", "repo1", "master", "<i>XSS PR</i>")
60+
61+
// check the redirected URL
62+
url := RedirectURL(t, resp)
63+
assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
64+
65+
// Edit title
66+
req := NewRequest(t, "GET", url)
67+
resp = session.MakeRequest(t, req, http.StatusOK)
68+
htmlDoc := NewHTMLParser(t, resp.Body)
69+
editTestTitleURL, exists := htmlDoc.doc.Find("#save-edit-title").First().Attr("data-update-url")
70+
assert.True(t, exists, "The template has changed")
71+
72+
req = NewRequestWithValues(t, "POST", editTestTitleURL, map[string]string{
73+
"_csrf": htmlDoc.GetCSRF(),
74+
"title": "<u>XSS PR</u>",
75+
})
76+
session.MakeRequest(t, req, http.StatusOK)
77+
78+
req = NewRequest(t, "GET", url)
79+
resp = session.MakeRequest(t, req, http.StatusOK)
80+
htmlDoc = NewHTMLParser(t, resp.Body)
81+
titleHTML, err := htmlDoc.doc.Find(".comments .event .text b").First().Html()
82+
assert.NoError(t, err)
83+
assert.Equal(t, "&lt;i&gt;XSS PR&lt;/i&gt;", titleHTML)
84+
titleHTML, err = htmlDoc.doc.Find(".comments .event .text b").Next().Html()
85+
assert.NoError(t, err)
86+
assert.Equal(t, "&lt;u&gt;XSS PR&lt;/u&gt;", titleHTML)
5287
}

‎integrations/pull_merge_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func TestPullMerge(t *testing.T) {
5151
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
5252
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
5353

54-
resp := testPullCreate(t, session, "user1", "repo1", "master")
54+
resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
5555

5656
elem := strings.Split(RedirectURL(t, resp), "/")
5757
assert.EqualValues(t, "pulls", elem[3])
@@ -64,7 +64,7 @@ func TestPullCleanUpAfterMerge(t *testing.T) {
6464
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
6565
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n")
6666

67-
resp := testPullCreate(t, session, "user1", "repo1", "feature/test")
67+
resp := testPullCreate(t, session, "user1", "repo1", "feature/test", "This is a pull title")
6868

6969
elem := strings.Split(RedirectURL(t, resp), "/")
7070
assert.EqualValues(t, "pulls", elem[3])

‎integrations/repo_activity_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,16 @@ func TestRepoActivity(t *testing.T) {
1919
// Create PRs (1 merged & 2 proposed)
2020
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
2121
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
22-
resp := testPullCreate(t, session, "user1", "repo1", "master")
22+
resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
2323
elem := strings.Split(RedirectURL(t, resp), "/")
2424
assert.EqualValues(t, "pulls", elem[3])
2525
testPullMerge(t, session, elem[1], elem[2], elem[4])
2626

2727
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/better_readme", "README.md", "Hello, World (Edited Again)\n")
28-
testPullCreate(t, session, "user1", "repo1", "feat/better_readme")
28+
testPullCreate(t, session, "user1", "repo1", "feat/better_readme", "This is a pull title")
2929

3030
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/much_better_readme", "README.md", "Hello, World (Edited More)\n")
31-
testPullCreate(t, session, "user1", "repo1", "feat/much_better_readme")
31+
testPullCreate(t, session, "user1", "repo1", "feat/much_better_readme", "This is a pull title")
3232

3333
// Create issues (3 new issues)
3434
testNewIssue(t, session, "user2", "repo1", "Issue 1", "Description 1")

‎templates/repo/issue/view_content/comments.tmpl

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
<img src="{{.Poster.RelAvatarLink}}">
9797
</a>
9898
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
99-
{{if .Content}}{{$.i18n.Tr "repo.issues.add_label_at" .Label.ForegroundColor .Label.Color .Label.Name $createdStr | UnescapeLocale | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_label_at" .Label.ForegroundColor .Label.Color .Label.Name $createdStr | UnescapeLocale | Safe}}{{end}}</span>
99+
{{if .Content}}{{$.i18n.Tr "repo.issues.add_label_at" .Label.ForegroundColor .Label.Color (.Label.Name|Escape) $createdStr | UnescapeLocale | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_label_at" .Label.ForegroundColor .Label.Color (.Label.Name|Escape) $createdStr | UnescapeLocale | Safe}}{{end}}</span>
100100
</div>
101101
{{end}}
102102
{{else if eq .Type 8}}
@@ -106,7 +106,7 @@
106106
<img src="{{.Poster.RelAvatarLink}}">
107107
</a>
108108
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
109-
{{if gt .OldMilestoneID 0}}{{if gt .MilestoneID 0}}{{$.i18n.Tr "repo.issues.change_milestone_at" .OldMilestone.Name .Milestone.Name $createdStr | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_milestone_at" .OldMilestone.Name $createdStr | Safe}}{{end}}{{else if gt .MilestoneID 0}}{{$.i18n.Tr "repo.issues.add_milestone_at" .Milestone.Name $createdStr | Safe}}{{end}}</span>
109+
{{if gt .OldMilestoneID 0}}{{if gt .MilestoneID 0}}{{$.i18n.Tr "repo.issues.change_milestone_at" (.OldMilestone.Name|Escape) (.Milestone.Name|Escape) $createdStr | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_milestone_at" (.OldMilestone.Name|Escape) $createdStr | Safe}}{{end}}{{else if gt .MilestoneID 0}}{{$.i18n.Tr "repo.issues.add_milestone_at" (.Milestone.Name|Escape) $createdStr | Safe}}{{end}}</span>
110110
</div>
111111
{{else if eq .Type 9}}
112112
<div class="event">
@@ -124,23 +124,23 @@
124124
{{else if eq .Type 10}}
125125
<div class="event">
126126
<span class="octicon octicon-primitive-dot"></span>
127+
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
128+
<img src="{{.Poster.RelAvatarLink}}">
129+
</a>
130+
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
131+
{{$.i18n.Tr "repo.issues.change_title_at" (.OldTitle|Escape) (.NewTitle|Escape) $createdStr | Safe}}
132+
</span>
127133
</div>
128-
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
129-
<img src="{{.Poster.RelAvatarLink}}">
130-
</a>
131-
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
132-
{{$.i18n.Tr "repo.issues.change_title_at" .OldTitle .NewTitle $createdStr | Safe}}
133-
</span>
134134
{{else if eq .Type 11}}
135135
<div class="event">
136136
<span class="octicon octicon-primitive-dot"></span>
137+
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
138+
<img src="{{.Poster.RelAvatarLink}}">
139+
</a>
140+
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
141+
{{$.i18n.Tr "repo.issues.delete_branch_at" .CommitSHA $createdStr | Safe}}
142+
</span>
137143
</div>
138-
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
139-
<img src="{{.Poster.RelAvatarLink}}">
140-
</a>
141-
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
142-
{{$.i18n.Tr "repo.issues.delete_branch_at" .CommitSHA $createdStr | Safe}}
143-
</span>
144144
{{else if eq .Type 12}}
145145
<div class="event">
146146
<span class="octicon octicon-primitive-dot"></span>

‎templates/repo/wiki/view.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@
9898
{{.i18n.Tr "repo.wiki.delete_page_button"}}
9999
</div>
100100
<div class="content">
101-
<p>{{.i18n.Tr "repo.wiki.delete_page_notice_1" $title | Safe}}</p>
101+
<p>{{.i18n.Tr "repo.wiki.delete_page_notice_1" ($title|Escape) | Safe}}</p>
102102
</div>
103103
{{template "base/delete_modal_actions" .}}
104104
</div>

0 commit comments

Comments
 (0)