Skip to content

Commit c503eac

Browse files
lafriksappleboy
lafriks
authored and
appleboy
committed
Fix escaping changed title in comments (#3530) (#3534)
* Fix escaping changed title in comments * Fix escaping of wiki page titile Signed-off-by: Lauris Bukšis-Haberkorns <[email protected]>
1 parent 221e502 commit c503eac

File tree

5 files changed

+60
-25
lines changed

5 files changed

+60
-25
lines changed

‎integrations/pull_create_test.go

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

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

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

@@ -47,7 +47,7 @@ func TestPullCreate(t *testing.T) {
4747
session := loginUser(t, "user1")
4848
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
4949
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
50-
resp := testPullCreate(t, session, "user1", "repo1", "master")
50+
resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
5151

5252
// check the redirected URL
5353
url := resp.HeaderMap.Get("Location")
@@ -68,3 +68,38 @@ func TestPullCreate(t *testing.T) {
6868
assert.Regexp(t, `Subject: \[PATCH\] Update 'README.md'`, resp.Body)
6969
assert.NotRegexp(t, "diff.*diff", resp.Body) // not two diffs, just one
7070
}
71+
72+
func TestPullCreate_TitleEscape(t *testing.T) {
73+
prepareTestEnv(t)
74+
session := loginUser(t, "user1")
75+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
76+
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
77+
resp := testPullCreate(t, session, "user1", "repo1", "master", "<i>XSS PR</i>")
78+
79+
// check the redirected URL
80+
url := resp.HeaderMap.Get("Location")
81+
assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url)
82+
83+
// Edit title
84+
req := NewRequest(t, "GET", url)
85+
resp = session.MakeRequest(t, req, http.StatusOK)
86+
htmlDoc := NewHTMLParser(t, resp.Body)
87+
editTestTitleURL, exists := htmlDoc.doc.Find("#save-edit-title").First().Attr("data-update-url")
88+
assert.True(t, exists, "The template has changed")
89+
90+
req = NewRequestWithValues(t, "POST", editTestTitleURL, map[string]string{
91+
"_csrf": htmlDoc.GetCSRF(),
92+
"title": "<u>XSS PR</u>",
93+
})
94+
session.MakeRequest(t, req, http.StatusOK)
95+
96+
req = NewRequest(t, "GET", url)
97+
resp = session.MakeRequest(t, req, http.StatusOK)
98+
htmlDoc = NewHTMLParser(t, resp.Body)
99+
titleHTML, err := htmlDoc.doc.Find(".comments .event .text b").First().Html()
100+
assert.NoError(t, err)
101+
assert.Equal(t, "&lt;i&gt;XSS PR&lt;/i&gt;", titleHTML)
102+
titleHTML, err = htmlDoc.doc.Find(".comments .event .text b").Next().Html()
103+
assert.NoError(t, err)
104+
assert.Equal(t, "&lt;u&gt;XSS PR&lt;/u&gt;", titleHTML)
105+
}

‎integrations/pull_merge_test.go

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

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

6161
elem := strings.Split(test.RedirectURL(resp), "/")
6262
assert.EqualValues(t, "pulls", elem[3])
@@ -69,7 +69,7 @@ func TestPullRebase(t *testing.T) {
6969
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
7070
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
7171

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

7474
elem := strings.Split(test.RedirectURL(resp), "/")
7575
assert.EqualValues(t, "pulls", elem[3])
@@ -83,7 +83,7 @@ func TestPullSquash(t *testing.T) {
8383
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
8484
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n")
8585

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

8888
elem := strings.Split(test.RedirectURL(resp), "/")
8989
assert.EqualValues(t, "pulls", elem[3])
@@ -96,7 +96,7 @@ func TestPullCleanUpAfterMerge(t *testing.T) {
9696
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
9797
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n")
9898

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

101101
elem := strings.Split(test.RedirectURL(resp), "/")
102102
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
@@ -22,16 +22,16 @@ func TestRepoActivity(t *testing.T) {
2222
// Create PRs (1 merged & 2 proposed)
2323
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
2424
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
25-
resp := testPullCreate(t, session, "user1", "repo1", "master")
25+
resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")
2626
elem := strings.Split(test.RedirectURL(resp), "/")
2727
assert.EqualValues(t, "pulls", elem[3])
2828
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleMerge)
2929

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

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

3636
// Create issues (3 new issues)
3737
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
@@ -103,7 +103,7 @@
103103
<img src="{{.Poster.RelAvatarLink}}">
104104
</a>
105105
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
106-
{{if .Content}}{{$.i18n.Tr "repo.issues.add_label_at" .Label.ForegroundColor .Label.Color .Label.Name $createdStr | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_label_at" .Label.ForegroundColor .Label.Color .Label.Name $createdStr | Safe}}{{end}}</span>
106+
{{if .Content}}{{$.i18n.Tr "repo.issues.add_label_at" .Label.ForegroundColor .Label.Color (.Label.Name|Escape) $createdStr | Safe}}{{else}}{{$.i18n.Tr "repo.issues.remove_label_at" .Label.ForegroundColor .Label.Color (.Label.Name|Escape) $createdStr | Safe}}{{end}}</span>
107107
</div>
108108
{{end}}
109109
{{else if eq .Type 8}}
@@ -113,7 +113,7 @@
113113
<img src="{{.Poster.RelAvatarLink}}">
114114
</a>
115115
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
116-
{{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>
116+
{{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>
117117
</div>
118118
{{else if eq .Type 9}}
119119
<div class="event">
@@ -131,23 +131,23 @@
131131
{{else if eq .Type 10}}
132132
<div class="event">
133133
<span class="octicon octicon-primitive-dot"></span>
134+
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
135+
<img src="{{.Poster.RelAvatarLink}}">
136+
</a>
137+
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
138+
{{$.i18n.Tr "repo.issues.change_title_at" (.OldTitle|Escape) (.NewTitle|Escape) $createdStr | Safe}}
139+
</span>
134140
</div>
135-
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
136-
<img src="{{.Poster.RelAvatarLink}}">
137-
</a>
138-
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
139-
{{$.i18n.Tr "repo.issues.change_title_at" .OldTitle .NewTitle $createdStr | Safe}}
140-
</span>
141141
{{else if eq .Type 11}}
142142
<div class="event">
143143
<span class="octicon octicon-primitive-dot"></span>
144+
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
145+
<img src="{{.Poster.RelAvatarLink}}">
146+
</a>
147+
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
148+
{{$.i18n.Tr "repo.issues.delete_branch_at" .CommitSHA $createdStr | Safe}}
149+
</span>
144150
</div>
145-
<a class="ui avatar image" href="{{.Poster.HomeLink}}">
146-
<img src="{{.Poster.RelAvatarLink}}">
147-
</a>
148-
<span class="text grey"><a href="{{.Poster.HomeLink}}">{{.Poster.Name}}</a>
149-
{{$.i18n.Tr "repo.issues.delete_branch_at" .CommitSHA $createdStr | Safe}}
150-
</span>
151151
{{else if eq .Type 12}}
152152
<div class="event">
153153
<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
@@ -104,7 +104,7 @@
104104
{{.i18n.Tr "repo.wiki.delete_page_button"}}
105105
</div>
106106
<div class="content">
107-
<p>{{.i18n.Tr "repo.wiki.delete_page_notice_1" $title | Safe}}</p>
107+
<p>{{.i18n.Tr "repo.wiki.delete_page_notice_1" ($title|Escape) | Safe}}</p>
108108
</div>
109109
{{template "base/delete_modal_actions" .}}
110110
</div>

0 commit comments

Comments
 (0)