Skip to content

Commit 7274357

Browse files
authored
Fix incorrect tests in 1.21 (#29366)
The submitted tests in the patch for the XSS fix is not right. To test, it should test "what should happen", but not "what doesn't exist" or "what is processed/decoded".
1 parent 829b807 commit 7274357

File tree

1 file changed

+26
-25
lines changed

1 file changed

+26
-25
lines changed

tests/integration/xss_test.go

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ package integration
55

66
import (
77
"context"
8-
"fmt"
8+
"html"
99
"net/http"
1010
"net/url"
1111
"os"
@@ -27,7 +27,7 @@ import (
2727
func TestXSSUserFullName(t *testing.T) {
2828
defer tests.PrepareTestEnv(t)()
2929
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
30-
const fullName = `name & <script class="evil">alert('Oh no!');</script>`
30+
const fullName = `name & <script class="evil">alert('xss');</script>`
3131

3232
session := loginUser(t, user.Name)
3333
req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
@@ -43,58 +43,55 @@ func TestXSSUserFullName(t *testing.T) {
4343
resp := session.MakeRequest(t, req, http.StatusOK)
4444
htmlDoc := NewHTMLParser(t, resp.Body)
4545
assert.EqualValues(t, 0, htmlDoc.doc.Find("script.evil").Length())
46-
assert.EqualValues(t, fullName,
47-
htmlDoc.doc.Find("div.content").Find(".header.text.center").Text(),
48-
)
46+
htmlCode, err := htmlDoc.doc.Find("div.content").Find(".header.text.center").Html()
47+
assert.NoError(t, err)
48+
assert.EqualValues(t, html.EscapeString(fullName), htmlCode)
4949
}
5050

5151
func TestXSSWikiLastCommitInfo(t *testing.T) {
5252
onGiteaRun(t, func(t *testing.T, u *url.URL) {
53-
// Prepare the environment.
5453
dstPath := t.TempDir()
55-
r := fmt.Sprintf("%suser2/repo1.wiki.git", u.String())
56-
u, err := url.Parse(r)
54+
cloneWikiURL, err := url.Parse(u.String() + "user2/repo1.wiki.git")
5755
assert.NoError(t, err)
58-
u.User = url.UserPassword("user2", userPassword)
59-
assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), u.String(), dstPath, git.CloneRepoOptions{}))
56+
cloneWikiURL.User = url.UserPassword("user2", userPassword)
57+
assert.NoError(t, git.CloneWithArgs(context.Background(), git.AllowLFSFiltersArgs(), cloneWikiURL.String(), dstPath, git.CloneRepoOptions{}))
6058

6159
// Use go-git here, because using git wouldn't work, it has code to remove
6260
// `<`, `>` and `\n` in user names. Even though this is permitted and
6361
// wouldn't result in a error by a Git server.
6462
gitRepo, err := gogit.PlainOpen(dstPath)
65-
if err != nil {
66-
panic(err)
63+
if !assert.NoError(t, err) {
64+
return
6765
}
68-
6966
w, err := gitRepo.Worktree()
70-
if err != nil {
71-
panic(err)
67+
if !assert.NoError(t, err) {
68+
return
7269
}
7370

7471
filename := filepath.Join(dstPath, "Home.md")
75-
err = os.WriteFile(filename, []byte("Oh, a XSS attack?"), 0o644)
72+
err = os.WriteFile(filename, []byte("dummy content"), 0o644)
7673
if !assert.NoError(t, err) {
77-
t.FailNow()
74+
return
7875
}
7976

8077
_, err = w.Add("Home.md")
8178
if !assert.NoError(t, err) {
82-
t.FailNow()
79+
return
8380
}
8481

85-
_, err = w.Commit("Yay XSS", &gogit.CommitOptions{
82+
_, err = w.Commit("dummy message", &gogit.CommitOptions{
8683
Author: &object.Signature{
87-
Name: `Gusted <script class="evil">alert('Oh no!');</script>`,
84+
Name: `foo<script class="evil">alert('xss');</script>bar`,
8885
89-
When: time.Date(2024, time.January, 31, 0, 0, 0, 0, time.UTC),
86+
When: time.Date(2001, time.January, 31, 0, 0, 0, 0, time.UTC),
9087
},
9188
})
9289
if !assert.NoError(t, err) {
93-
t.FailNow()
90+
return
9491
}
9592

9693
// Push.
97-
_, _, err = git.NewCommand(git.DefaultContext, "push").AddArguments(git.ToTrustedCmdArgs([]string{"origin", "master"})...).RunStdString(&git.RunOpts{Dir: dstPath})
94+
_, _, err = git.NewCommand(git.DefaultContext, "push").AddArguments("origin", "master").RunStdString(&git.RunOpts{Dir: dstPath})
9895
assert.NoError(t, err)
9996

10097
// Check on page view.
@@ -106,7 +103,9 @@ func TestXSSWikiLastCommitInfo(t *testing.T) {
106103
htmlDoc := NewHTMLParser(t, resp.Body)
107104

108105
htmlDoc.AssertElement(t, "script.evil", false)
109-
assert.EqualValues(t, `Gusted edited this page 0001-01-01 00:00:00 +00:00`, strings.TrimSpace(htmlDoc.Find(".ui.sub.header").Text()))
106+
htmlCode, err := htmlDoc.Find(".ui.sub.header").Html()
107+
assert.NoError(t, err)
108+
assert.EqualValues(t, `foo&lt;script class=&#34;evil&#34;&gt;alert(&#39;xss&#39;);&lt;/script&gt;bar edited this page <relative-time class="time-since" prefix="" tense="past" datetime="2001-01-31T00:00:00Z" data-tooltip-content="" data-tooltip-interactive="true">2001-01-31 00:00:00 +00:00</relative-time>`, strings.TrimSpace(htmlCode))
110109
})
111110

112111
// Check on revisions page.
@@ -118,7 +117,9 @@ func TestXSSWikiLastCommitInfo(t *testing.T) {
118117
htmlDoc := NewHTMLParser(t, resp.Body)
119118

120119
htmlDoc.AssertElement(t, "script.evil", false)
121-
assert.EqualValues(t, `Gusted edited this page 0001-01-01 00:00:00 +00:00`, strings.TrimSpace(htmlDoc.Find(".ui.sub.header").Text()))
120+
htmlCode, err := htmlDoc.Find(".ui.sub.header").Html()
121+
assert.NoError(t, err)
122+
assert.EqualValues(t, `foo&lt;script class=&#34;evil&#34;&gt;alert(&#39;xss&#39;);&lt;/script&gt;bar edited this page <relative-time class="time-since" prefix="" tense="past" datetime="2001-01-31T00:00:00Z" data-tooltip-content="" data-tooltip-interactive="true">2001-01-31 00:00:00 +00:00</relative-time>`, strings.TrimSpace(htmlCode))
122123
})
123124
})
124125
}

0 commit comments

Comments
 (0)