test(web): cover the merge request pages, writes, events and text rules
- commit
- 8523225779761a50f455ffe1d8c1b4fd1d76ffbf
- author
- brice · · 2026-10-10 18:29 UTC
- parent
- a889e05
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +620 -0
A cmd/ermite/web_mr_test.go +548 -0
| @@ -0,0 +1,548 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package main | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "html" | |
| 7 | + | "net/http" | |
| 8 | + | "net/url" | |
| 9 | + | "regexp" | |
| 10 | + | "strings" | |
| 11 | + | "testing" | |
| 12 | + | ||
| 13 | + | "github.com/stretchr/testify/assert" | |
| 14 | + | "github.com/stretchr/testify/require" | |
| 15 | + | ||
| 16 | + | "bnema.dev/ermite/internal/app" | |
| 17 | + | "bnema.dev/ermite/internal/domain" | |
| 18 | + | ) | |
| 19 | + | ||
| 20 | + | // mrRepo is a public repository "proj" with README.md on main and a branch | |
| 21 | + | // feat/search that is one commit ahead of it (search.go, three lines). | |
| 22 | + | type mrRepo struct { | |
| 23 | + | *cliHarness | |
| 24 | + | wt, url string | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | func newMRRepo(t *testing.T, vis domain.Visibility) *mrRepo { | |
| 28 | + | t.Helper() | |
| 29 | + | c := newCLIHarness(t) | |
| 30 | + | _, err := c.forge.Repos.Create(c.ctx, domain.Root(), createParams(c.harness, "proj", vis)) | |
| 31 | + | require.NoError(t, err) | |
| 32 | + | r := &mrRepo{cliHarness: c, wt: c.workTree("main", "README.md"), url: c.urlAuth(adminToken, "/proj.git")} | |
| 33 | + | c.mustGit(r.wt, "push", "-q", r.url, "main") | |
| 34 | + | c.mustGit(r.wt, "checkout", "-q", "-b", "feat/search") | |
| 35 | + | c.writeFile(r.wt, "search.go", []byte("package search\n\nfunc Find() {}\n")) | |
| 36 | + | c.mustGit(r.wt, "push", "-q", r.url, "feat/search") | |
| 37 | + | return r | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | func (r *mrRepo) headOf(branch string) string { return r.bareGit("proj", "rev-parse", branch) } | |
| 41 | + | ||
| 42 | + | // form posts url-encoded values as the signed-in owner, from this site. | |
| 43 | + | func (b browser) form(path, token string, values url.Values) page { | |
| 44 | + | b.h.t.Helper() | |
| 45 | + | return b.do("POST", path, post(token), values.Encode()) | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | var reHeadField = regexp.MustCompile(`name="head_id" value="([0-9a-f]{40})"`) | |
| 49 | + | ||
| 50 | + | // formHead reads the head the merge form of a page carries. | |
| 51 | + | func formHead(t *testing.T, body string) string { | |
| 52 | + | t.Helper() | |
| 53 | + | m := reHeadField.FindStringSubmatch(body) | |
| 54 | + | require.Len(t, m, 2, "the page has a merge form") | |
| 55 | + | return m[1] | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | // AC7: the pages as an anonymous visitor sees them on a public repository: | |
| 59 | + | // read-only, no forms, public cache headers; the same pages signed in carry | |
| 60 | + | // the forms and the private headers. | |
| 61 | + | func TestWebMergeRequestPagesRender(t *testing.T) { | |
| 62 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 63 | + | b := browser{r.harness} | |
| 64 | + | token := r.signIn(t) | |
| 65 | + | ||
| 66 | + | // The compare page: anonymous reads, the owner gets the form. | |
| 67 | + | anon := b.get("/proj/compare/feat/search", nil) | |
| 68 | + | require.Equal(t, 200, anon.status, anon.body) | |
| 69 | + | assert.Equal(t, "no-cache", anon.hdr.Get("Cache-Control")) | |
| 70 | + | assert.Empty(t, anon.hdr.Values("Vary")) | |
| 71 | + | assert.Contains(t, anon.body, "feat/search") | |
| 72 | + | assert.Contains(t, anon.body, "package search</td>", "the diff of the single commit") | |
| 73 | + | assert.NotContains(t, anon.body, "<form class=\"cform create\"") | |
| 74 | + | assert.NotContains(t, anon.body, `<form class="cform create"`) | |
| 75 | + | assertNoInline(t, anon.body) | |
| 76 | + | owner := b.get("/proj/compare/feat/search", as(token)) | |
| 77 | + | require.Equal(t, 200, owner.status) | |
| 78 | + | assert.Equal(t, "private, no-store", owner.hdr.Get("Cache-Control")) | |
| 79 | + | assert.Contains(t, owner.hdr.Values("Vary"), "Cookie") | |
| 80 | + | assert.Contains(t, owner.body, `<form class="cform create" method="post" action="/proj/merge-requests">`) | |
| 81 | + | assert.Contains(t, owner.body, `name="source" value="feat/search"`) | |
| 82 | + | assert.Contains(t, owner.body, `name="target" value="main"`) | |
| 83 | + | assert.Equal(t, 404, b.get("/proj/compare/nope", nil).status) | |
| 84 | + | assert.Equal(t, 404, b.get("/proj/compare/main/extra", nil).status) | |
| 85 | + | main := b.get("/proj/compare/main", nil) | |
| 86 | + | assert.Equal(t, 200, main.status) | |
| 87 | + | assert.Contains(t, main.body, "nothing to compare") | |
| 88 | + | ||
| 89 | + | // An empty list, with the way to start one. | |
| 90 | + | list := b.get("/proj/merge-requests", nil) | |
| 91 | + | require.Equal(t, 200, list.status) | |
| 92 | + | assert.Contains(t, list.body, "no open merge requests") | |
| 93 | + | assert.Contains(t, list.body, `href="/proj/compare/feat/search"`) | |
| 94 | + | assert.Contains(t, list.body, `data-key="m"`, "the section of the nav") | |
| 95 | + | assert.NotContains(t, strings.ToLower(list.body), "proposal") | |
| 96 | + | ||
| 97 | + | // Open one from the compare page. | |
| 98 | + | res := b.form("/proj/merge-requests", token, url.Values{ | |
| 99 | + | "source": {"feat/search"}, "target": {"main"}, "title": {"Add <b>search</b>"}, | |
| 100 | + | "description": {"Finds things.\r\nSee https://example.org/spec) and javascript:alert(1)"}, | |
| 101 | + | }) | |
| 102 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 103 | + | assert.Equal(t, "/proj/merge-requests/1", res.hdr.Get("Location")) | |
| 104 | + | assert.Equal(t, "no-store", res.hdr.Get("Cache-Control")) | |
| 105 | + | head := r.headOf("feat/search") | |
| 106 | + | ||
| 107 | + | // The same pair twice is refused with a page, and nothing is created. | |
| 108 | + | dup := b.form("/proj/merge-requests", token, url.Values{"source": {"feat/search"}, "target": {"main"}, "title": {"dup"}}) | |
| 109 | + | assert.Equal(t, http.StatusConflict, dup.status) | |
| 110 | + | assert.Equal(t, "no-store", dup.hdr.Get("Cache-Control")) | |
| 111 | + | assert.Contains(t, dup.body, "already exists") | |
| 112 | + | bad := b.form("/proj/merge-requests", token, url.Values{"source": {"feat/search"}, "target": {"main"}, "title": {"bidi\u202etitle"}}) | |
| 113 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |
| 114 | + | assert.NotContains(t, bad.body, "\u202e") | |
| 115 | + | ||
| 116 | + | // The compare page now points at it. | |
| 117 | + | assert.Contains(t, b.get("/proj/compare/feat/search", as(token)).body, `href="/proj/merge-requests/1"`) | |
| 118 | + | assert.NotContains(t, b.get("/proj/compare/feat/search", as(token)).body, `<form class="cform create"`) | |
| 119 | + | ||
| 120 | + | // The list. | |
| 121 | + | list = b.get("/proj/merge-requests", nil) | |
| 122 | + | assert.Contains(t, list.body, `<a href="/proj/merge-requests/1">Add <b>search</b></a>`) | |
| 123 | + | assert.Contains(t, list.body, "feat/search → main") | |
| 124 | + | assert.Equal(t, "no-cache", list.hdr.Get("Cache-Control")) | |
| 125 | + | assertNoInline(t, list.body) | |
| 126 | + | assert.Equal(t, "private, no-store", b.get("/proj/merge-requests", as(token)).hdr.Get("Cache-Control")) | |
| 127 | + | ||
| 128 | + | // The page, anonymous: read-only. | |
| 129 | + | mr := b.get("/proj/merge-requests/1", nil) | |
| 130 | + | require.Equal(t, 200, mr.status, mr.body) | |
| 131 | + | assert.Equal(t, "no-cache", mr.hdr.Get("Cache-Control")) | |
| 132 | + | assert.Empty(t, mr.hdr.Values("Vary")) | |
| 133 | + | assert.Contains(t, mr.body, "<h1><span class=\"mute\">!1</span> Add <b>search</b></h1>") | |
| 134 | + | assert.NotContains(t, mr.body, "<b>search</b>") | |
| 135 | + | assert.Contains(t, mr.body, "feat/search") | |
| 136 | + | assert.Contains(t, mr.body, "package search</td>") | |
| 137 | + | assert.Contains(t, mr.body, "no checks") | |
| 138 | + | assert.Contains(t, mr.body, "fast-forward, squash") | |
| 139 | + | assert.Contains(t, mr.body, `<a href="https://example.org/spec" rel="noopener nofollow ugc">https://example.org/spec</a>)`, "the address is linked, not its closing bracket") | |
| 140 | + | assert.NotContains(t, mr.body, `href="javascript:`) | |
| 141 | + | assert.Contains(t, mr.body, "Finds things.\nSee ", "line breaks are kept") | |
| 142 | + | assert.NotContains(t, mr.body, "<form", "an anonymous visitor gets no form") | |
| 143 | + | assert.NotContains(t, mr.body, "ermite mr merge", "nor the merge line") | |
| 144 | + | assert.NotContains(t, mr.body, "[merge]") | |
| 145 | + | assert.Contains(t, mr.body, `href="/proj/commit/`+head+`"`, "the commits are in the rail") | |
| 146 | + | assert.NotContains(t, strings.ToLower(mr.body), "proposal") | |
| 147 | + | assertNoInline(t, mr.body) | |
| 148 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/1/since", nil).status, "the read position is the owner's") | |
| 149 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/9", nil).status) | |
| 150 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/01", nil).status) | |
| 151 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/x", nil).status) | |
| 152 | + | ||
| 153 | + | // Signed in: forms, the merge box with the head and the CLI lines. | |
| 154 | + | own := b.get("/proj/merge-requests/1", as(token)) | |
| 155 | + | require.Equal(t, 200, own.status) | |
| 156 | + | assert.Equal(t, "private, no-store", own.hdr.Get("Cache-Control")) | |
| 157 | + | assert.Contains(t, own.hdr.Values("Vary"), "Cookie") | |
| 158 | + | assert.Equal(t, head, formHead(t, own.body)) | |
| 159 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/merge"`) | |
| 160 | + | assert.Contains(t, own.body, `<option value="fast-forward">`) | |
| 161 | + | assert.Contains(t, own.body, `<option value="squash">`) | |
| 162 | + | assert.Contains(t, own.body, "ermite mr merge proj 1 --head "+head) | |
| 163 | + | assert.Contains(t, own.body, "ermite mr merge proj 1 --head "+head+" --squash") | |
| 164 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/comments"`) | |
| 165 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/close"`) | |
| 166 | + | assert.Contains(t, own.body, `href="/proj/merge-requests/1?on=new:1:search.go#f`, "line numbers link to their form") | |
| 167 | + | assertNoInline(t, own.body) | |
| 168 | + | ||
| 169 | + | // Reading is safe: GET marked nothing as read. | |
| 170 | + | assert.NotContains(t, own.body, "since your last read") | |
| 171 | + | show := r.cli(t, adminToken, "mr", "show", "proj", "1", "--json").ok(t) | |
| 172 | + | assert.Contains(t, show.stdout, `"last_read_id": ""`) | |
| 173 | + | } | |
| 174 | + | ||
| 175 | + | // A private repository is invisible to the anonymous visitor on every merge | |
| 176 | + | // request page, and the owner sees it. | |
| 177 | + | func TestWebMergeRequestPrivate(t *testing.T) { | |
| 178 | + | r := newMRRepo(t, domain.VisibilityPrivate) | |
| 179 | + | b := browser{r.harness} | |
| 180 | + | token := r.signIn(t) | |
| 181 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Secret plan").ok(t) | |
| 182 | + | ||
| 183 | + | missing := b.get("/nothere/merge-requests/1", nil) | |
| 184 | + | for _, p := range []string{ | |
| 185 | + | "/proj/merge-requests", "/proj/merge-requests/1", "/proj/merge-requests/1/since", "/proj/compare/feat/search", | |
| 186 | + | } { | |
| 187 | + | got := b.get(p, nil) | |
| 188 | + | assert.Equal(t, 404, got.status, p) | |
| 189 | + | assert.NotContains(t, got.body, "Secret plan", p) | |
| 190 | + | assert.NotContains(t, got.body, "search.go", p) | |
| 191 | + | } | |
| 192 | + | assert.Equal(t, missing.body, b.get("/proj/merge-requests/1", nil).body, "same page as a missing repository") | |
| 193 | + | assert.Equal(t, 404, b.get("/-/events?topic=repo:proj&view=mrs", nil).status) | |
| 194 | + | ||
| 195 | + | for _, p := range []string{"/proj/merge-requests", "/proj/merge-requests/1", "/proj/merge-requests/1/since", "/proj/compare/feat/search"} { | |
| 196 | + | got := b.get(p, as(token)) | |
| 197 | + | assert.Equal(t, 200, got.status, p) | |
| 198 | + | assert.Equal(t, "private, no-store", got.hdr.Get("Cache-Control"), p) | |
| 199 | + | assert.Contains(t, got.hdr.Values("Vary"), "Cookie", p) | |
| 200 | + | } | |
| 201 | + | assert.Contains(t, b.get("/proj/merge-requests", as(token)).body, "Secret plan") | |
| 202 | + | } | |
| 203 | + | ||
| 204 | + | // AC2: every write needs the session and an Origin of this site. | |
| 205 | + | func TestWebMergeRequestWritesNeedSessionAndOrigin(t *testing.T) { | |
| 206 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 207 | + | b := browser{r.harness} | |
| 208 | + | token := r.signIn(t) | |
| 209 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "t").ok(t) | |
| 210 | + | head := r.headOf("feat/search") | |
| 211 | + | ||
| 212 | + | paths := map[string]string{ | |
| 213 | + | "/proj/merge-requests": "source=feat/search&target=main&title=x", | |
| 214 | + | "/proj/merge-requests/1/comments": "text=hello", | |
| 215 | + | "/proj/merge-requests/1/comments/1/resolve": "resolved=true", | |
| 216 | + | "/proj/merge-requests/1/merge": "mode=fast-forward&head_id=" + head, | |
| 217 | + | "/proj/merge-requests/1/close": "", | |
| 218 | + | "/proj/merge-requests/1/read": "head_id=" + head, | |
| 219 | + | } | |
| 220 | + | for path, body := range paths { | |
| 221 | + | noOrigin := b.do("POST", path, as(token), body) | |
| 222 | + | assert.Equal(t, 403, noOrigin.status, path+" without Origin") | |
| 223 | + | assert.Equal(t, "no-store", noOrigin.hdr.Get("Cache-Control")) | |
| 224 | + | foreign := b.do("POST", path, map[string]string{"Cookie": sessionCookie + "=" + token, "Origin": "https://evil.example"}, body) | |
| 225 | + | assert.Equal(t, 403, foreign.status, path+" foreign Origin") | |
| 226 | + | anon := b.do("POST", path, map[string]string{"Origin": publicOrigin}, body) | |
| 227 | + | assert.Equal(t, 403, anon.status, path+" without a session") | |
| 228 | + | } | |
| 229 | + | // Nothing happened. | |
| 230 | + | out := r.cli(t, "", "mr", "show", "proj", "1", "--json").ok(t).stdout | |
| 231 | + | assert.Contains(t, out, `"state": "open"`) | |
| 232 | + | assert.NotContains(t, out, "hello") | |
| 233 | + | assert.Equal(t, "", strings.TrimSpace(r.bareGit("proj", "rev-list", "main..feat/search", "--not", "feat/search"))) | |
| 234 | + | ||
| 235 | + | // A GET on a write route is not a page. | |
| 236 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/1/merge", as(token)).status) | |
| 237 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/1/close", as(token)).status) | |
| 238 | + | } | |
| 239 | + | ||
| 240 | + | var reNoteID = regexp.MustCompile(`id="c(\d+)"`) | |
| 241 | + | ||
| 242 | + | // The review of one request, in the browser's words: a note on a line, a | |
| 243 | + | // general one, resolve, since the last read after a push, a stale head, the | |
| 244 | + | // fast-forward merge. | |
| 245 | + | func TestWebMergeRequestReviewAndFastForward(t *testing.T) { | |
| 246 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 247 | + | b := browser{r.harness} | |
| 248 | + | token := r.signIn(t) | |
| 249 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Add search").ok(t) | |
| 250 | + | head1 := r.headOf("feat/search") | |
| 251 | + | base := "/proj/merge-requests/1" | |
| 252 | + | ||
| 253 | + | // The form of a line, then the note under that line. | |
| 254 | + | line := b.get(base+"?on=new:3:search.go", as(token)) | |
| 255 | + | require.Equal(t, 200, line.status) | |
| 256 | + | assert.Contains(t, line.body, `<form class="cform"`) | |
| 257 | + | assert.Contains(t, line.body, `name="path" value="search.go"`) | |
| 258 | + | assert.Contains(t, line.body, `name="line" value="3"`) | |
| 259 | + | assert.Contains(t, line.body, `name="side" value="new"`) | |
| 260 | + | assert.Equal(t, 0, strings.Count(b.get(base+"?on=new:99:search.go", as(token)).body, `name="line" value="99"`), "a line the diff lacks has no form") | |
| 261 | + | assert.Equal(t, 0, strings.Count(b.get(base+"?on=new:3:search.go", nil).body, `<form`), "nor for a visitor") | |
| 262 | + | ||
| 263 | + | res := b.form(base+"/comments", token, url.Values{"text": {"Why <i>empty</i>?\r\nSee https://example.org/a."}, "path": {"search.go"}, "line": {"3"}, "side": {"new"}}) | |
| 264 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 265 | + | assert.Equal(t, base+"#c1", res.hdr.Get("Location")) | |
| 266 | + | assert.Equal(t, "no-store", res.hdr.Get("Cache-Control")) | |
| 267 | + | res = b.form(base+"/comments", token, url.Values{"text": {"Looks fine overall."}}) | |
| 268 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 269 | + | assert.Equal(t, base+"#c2", res.hdr.Get("Location")) | |
| 270 | + | ||
| 271 | + | anon := b.get(base, nil) | |
| 272 | + | i := strings.Index(anon.body, `<tr class="fnote">`) | |
| 273 | + | require.GreaterOrEqual(t, i, 0, "a note row under the line") | |
| 274 | + | note := anon.body[i:] | |
| 275 | + | assert.Less(t, strings.Index(note, "Why <i>empty</i>?"), strings.Index(note, "</tr>"), "the note sits in the row under its line") | |
| 276 | + | assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="noopener nofollow ugc">https://example.org/a</a>.`) | |
| 277 | + | assert.NotContains(t, anon.body, "<i>empty</i>") | |
| 278 | + | assert.Contains(t, anon.body, "Looks fine overall.") | |
| 279 | + | assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1))) | |
| 280 | + | assert.NotContains(t, anon.body, "[resolve]", "a visitor cannot resolve") | |
| 281 | + | ||
| 282 | + | // A comment that breaks a rule is refused with the rule, nothing is lost. | |
| 283 | + | bad := b.form(base+"/comments", token, url.Values{"text": {"ok"}, "path": {"search.go"}, "line": {"99"}, "side": {"new"}}) | |
| 284 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |
| 285 | + | assert.Equal(t, "no-store", bad.hdr.Get("Cache-Control")) | |
| 286 | + | assert.Contains(t, bad.body, "search.go") | |
| 287 | + | bad = b.form(base+"/comments", token, url.Values{"text": {"x"}, "path": {"search.go"}, "line": {"zz"}, "side": {"new"}}) | |
| 288 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |
| 289 | + | bad = b.form(base+"/comments", token, url.Values{"text": {"x"}, "path": {"search.go"}, "line": {"3"}, "side": {"sideways"}}) | |
| 290 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |
| 291 | + | bad = b.form(base+"/comments", token, url.Values{"text": {"evil\u202etext"}}) | |
| 292 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |
| 293 | + | assert.NotContains(t, bad.body, "\u202e") | |
| 294 | + | assert.Equal(t, 404, b.form("/proj/merge-requests/9/comments", token, url.Values{"text": {"x"}}).status) | |
| 295 | + | ||
| 296 | + | // Resolve and unresolve. | |
| 297 | + | own := b.get(base, as(token)) | |
| 298 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/comments/1/resolve"`) | |
| 299 | + | res = b.form(base+"/comments/1/resolve", token, url.Values{"resolved": {"true"}}) | |
| 300 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 301 | + | assert.Equal(t, base+"#c1", res.hdr.Get("Location")) | |
| 302 | + | own = b.get(base, as(token)) | |
| 303 | + | assert.Contains(t, own.body, `data-resolved="true"`) | |
| 304 | + | assert.Contains(t, own.body, "[unresolve]") | |
| 305 | + | assert.Equal(t, http.StatusSeeOther, b.form(base+"/comments/1/resolve", token, url.Values{"resolved": {"false"}}).status) | |
| 306 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/comments/1/resolve", token, url.Values{"resolved": {"maybe"}}).status) | |
| 307 | + | assert.Equal(t, 404, b.form(base+"/comments/42/resolve", token, url.Values{"resolved": {"true"}}).status) | |
| 308 | + | ||
| 309 | + | // Mark read, then push: the since page shows the new change only. | |
| 310 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/read"`) | |
| 311 | + | res = b.form(base+"/read", token, url.Values{"head_id": {head1}}) | |
| 312 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 313 | + | assert.NotContains(t, b.get(base, as(token)).body, "[mark read]", "the head is the one that was read") | |
| 314 | + | r.writeFile(r.wt, "extra.txt", []byte("extra\n")) | |
| 315 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/search") | |
| 316 | + | head2 := r.headOf("feat/search") | |
| 317 | + | ||
| 318 | + | all := b.get(base, as(token)) | |
| 319 | + | assert.Contains(t, all.body, `href="/proj/merge-requests/1/since"`) | |
| 320 | + | assert.Contains(t, all.body, "since your last read") | |
| 321 | + | assert.Contains(t, all.body, "<li data-new>", "the commit pushed since is marked") | |
| 322 | + | assert.Contains(t, all.body, "[mark read]") | |
| 323 | + | assert.Contains(t, all.body, "outdated", "the notes were written on the earlier head") | |
| 324 | + | since := b.get(base+"/since", as(token)) | |
| 325 | + | require.Equal(t, 200, since.status, since.body) | |
| 326 | + | assert.Equal(t, "private, no-store", since.hdr.Get("Cache-Control")) | |
| 327 | + | assert.Contains(t, since.body, "changes since "+head1[:7]) | |
| 328 | + | assert.Contains(t, since.body, "extra</td>") | |
| 329 | + | assert.NotContains(t, since.body, "package search</td>", "only what changed after the read head") | |
| 330 | + | assert.Contains(t, since.body, "Why <i>empty</i>?", "the notes are listed below, their lines are not in this diff") | |
| 331 | + | assert.Contains(t, since.body, "search.go:3 (new)") | |
| 332 | + | assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff") | |
| 333 | + | ||
| 334 | + | // The merge form of the page the owner read carries that head: the push | |
| 335 | + | // that came after is not merged unseen. | |
| 336 | + | stale := b.form(base+"/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {head1}}) | |
| 337 | + | assert.Equal(t, http.StatusConflict, stale.status) | |
| 338 | + | assert.Equal(t, "no-store", stale.hdr.Get("Cache-Control")) | |
| 339 | + | assert.Contains(t, stale.body, "branch moved") | |
| 340 | + | assert.NotEqual(t, head2, r.headOf("main")) | |
| 341 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/merge", token, url.Values{"mode": {"rebase"}, "head_id": {head2}}).status) | |
| 342 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/merge", token, url.Values{"mode": {"squash"}, "head_id": {"abc"}}).status) | |
| 343 | + | ||
| 344 | + | // Fast-forward with the head that was read. | |
| 345 | + | fresh := b.get(base, as(token)) | |
| 346 | + | require.Equal(t, head2, formHead(t, fresh.body)) | |
| 347 | + | res = b.form(base+"/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, fresh.body)}}) | |
| 348 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 349 | + | assert.Equal(t, base, res.hdr.Get("Location")) | |
| 350 | + | assert.Equal(t, head2, r.headOf("main"), "the target moved to the head") | |
| 351 | + | ||
| 352 | + | done := b.get(base, nil) | |
| 353 | + | assert.Contains(t, done.body, "merged") | |
| 354 | + | assert.Contains(t, done.body, "fast-forward") | |
| 355 | + | assert.Contains(t, done.body, `href="/proj/commit/`+head2+`"`) | |
| 356 | + | assert.NotContains(t, done.body, "<form") | |
| 357 | + | assert.NotContains(t, b.get(base, as(token)).body, "[merge]", "nothing to merge any more") | |
| 358 | + | // Writes on a merged request are refused with a page. | |
| 359 | + | late := b.form(base+"/comments", token, url.Values{"text": {"too late"}}) | |
| 360 | + | assert.Equal(t, http.StatusConflict, late.status) | |
| 361 | + | assert.Equal(t, http.StatusConflict, b.form(base+"/close", token, nil).status) | |
| 362 | + | list := b.get("/proj/merge-requests", nil) | |
| 363 | + | assert.Contains(t, list.body, "merged and closed") | |
| 364 | + | assert.Regexp(t, `(?s)merged and closed.*Add search`, list.body) | |
| 365 | + | } | |
| 366 | + | ||
| 367 | + | // Squash and close, and the order of the list: open first, then the decided. | |
| 368 | + | func TestWebMergeRequestSquashCloseAndList(t *testing.T) { | |
| 369 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 370 | + | b := browser{r.harness} | |
| 371 | + | token := r.signIn(t) | |
| 372 | + | head := r.headOf("feat/search") | |
| 373 | + | main0 := r.headOf("main") | |
| 374 | + | ||
| 375 | + | // Open from compare with a description; the merge form squashes. | |
| 376 | + | res := b.form("/proj/merge-requests", token, url.Values{ | |
| 377 | + | "source": {"feat/search"}, "target": {"main"}, "title": {"Squash me"}, "description": {""}, | |
| 378 | + | }) | |
| 379 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 380 | + | res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}}) | |
| 381 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 382 | + | merged := r.headOf("main") | |
| 383 | + | assert.NotEqual(t, head, merged, "squash makes a new commit") | |
| 384 | + | assert.Equal(t, main0, r.bareGit("proj", "rev-parse", "main^"), "on top of the old target") | |
| 385 | + | assert.Contains(t, r.bareGit("proj", "cat-file", "-p", "main"), "gpgsig", "signed by the instance key") | |
| 386 | + | page := b.get("/proj/merge-requests/1", nil) | |
| 387 | + | assert.Contains(t, page.body, "squash") | |
| 388 | + | assert.Contains(t, page.body, `href="/proj/commit/`+merged+`"`) | |
| 389 | + | ||
| 390 | + | // A second request: closed. | |
| 391 | + | r.mustGit(r.wt, "checkout", "-q", "-b", "feat/other", "main") | |
| 392 | + | r.mustGit(r.wt, "pull", "-q", r.url, "main") | |
| 393 | + | r.writeFile(r.wt, "other.go", []byte("package other\n")) | |
| 394 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/other") | |
| 395 | + | r.mustGit(r.wt, "checkout", "-q", "-b", "feat/third", "feat/other") | |
| 396 | + | r.writeFile(r.wt, "third.go", []byte("package third\n")) | |
| 397 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/third") | |
| 398 | + | for _, title := range []struct{ src, title string }{{"feat/other", "Other change"}, {"feat/third", "Third change"}} { | |
| 399 | + | res = b.form("/proj/merge-requests", token, url.Values{"source": {title.src}, "target": {"main"}, "title": {title.title}}) | |
| 400 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 401 | + | } | |
| 402 | + | res = b.form("/proj/merge-requests/2/close", token, nil) | |
| 403 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 404 | + | assert.Equal(t, "/proj/merge-requests/2", res.hdr.Get("Location")) | |
| 405 | + | assert.Equal(t, "no-store", res.hdr.Get("Cache-Control")) | |
| 406 | + | closed := b.get("/proj/merge-requests/2", nil) | |
| 407 | + | assert.Contains(t, closed.body, `data-state="closed"`) | |
| 408 | + | assert.NotContains(t, closed.body, "<form") | |
| 409 | + | assert.Equal(t, http.StatusConflict, b.form("/proj/merge-requests/2/close", token, nil).status, "closing twice says so") | |
| 410 | + | ||
| 411 | + | // Open one first, then merged and closed ones, newest first within each. | |
| 412 | + | list := b.get("/proj/merge-requests", nil).body | |
| 413 | + | third, squash, other := strings.Index(list, "Third change"), strings.Index(list, "Squash me"), strings.Index(list, "Other change") | |
| 414 | + | require.True(t, third >= 0 && squash >= 0 && other >= 0, list) | |
| 415 | + | assert.Less(t, third, squash, "the open request comes first") | |
| 416 | + | assert.Less(t, third, other) | |
| 417 | + | assert.Less(t, strings.Index(list, ">open<"), strings.Index(list, "merged and closed")) | |
| 418 | + | assert.Less(t, other, squash, "newest first among the decided") | |
| 419 | + | ||
| 420 | + | // A compare page of a branch that is behind says so. | |
| 421 | + | r.mustGit(r.wt, "checkout", "-q", "-b", "feat/stale", main0) | |
| 422 | + | r.writeFile(r.wt, "stale.go", []byte("package stale\n")) | |
| 423 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/stale") | |
| 424 | + | stale := b.get("/proj/compare/feat/stale", nil) | |
| 425 | + | assert.Equal(t, 200, stale.status) | |
| 426 | + | assert.Contains(t, stale.body, "has moved on") | |
| 427 | + | } | |
| 428 | + | ||
| 429 | + | // The comment limit of the API is a message, not a crash. | |
| 430 | + | func TestWebMergeRequestLimitIsFriendly(t *testing.T) { | |
| 431 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 432 | + | b := browser{r.harness} | |
| 433 | + | token := r.signIn(t) | |
| 434 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "t").ok(t) | |
| 435 | + | for i := 0; i < 1000; i++ { | |
| 436 | + | _, err := r.forge.MergeRequests.AddComment(r.ctx, domain.Root(), r.name("proj"), 1, app.AddCommentParams{Text: "n"}) | |
| 437 | + | require.NoError(t, err) | |
| 438 | + | } | |
| 439 | + | full := b.form("/proj/merge-requests/1/comments", token, url.Values{"text": {"one more"}}) | |
| 440 | + | assert.Equal(t, http.StatusConflict, full.status) | |
| 441 | + | assert.Contains(t, full.body, "limit reached") | |
| 442 | + | assert.Contains(t, full.body, "1000 comments") | |
| 443 | + | assert.Equal(t, "no-store", full.hdr.Get("Cache-Control")) | |
| 444 | + | } | |
| 445 | + | ||
| 446 | + | // Text is escaped everywhere and invisible characters are shown as markers. | |
| 447 | + | func TestWebMergeRequestEscapesText(t *testing.T) { | |
| 448 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 449 | + | b := browser{r.harness} | |
| 450 | + | token := r.signIn(t) | |
| 451 | + | res := b.form("/proj/merge-requests", token, url.Values{ | |
| 452 | + | "source": {"feat/search"}, "target": {"main"}, "title": {`"><script>alert(1)</script>`}, | |
| 453 | + | "description": {"<img src=x onerror=alert(1)>\nhttp://x.test/\"onmouseover=\"alert(1)"}, | |
| 454 | + | }) | |
| 455 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |
| 456 | + | r.cli(t, adminToken, "mr", "comment", "proj", "1", "-m", "<script>x</script> & more").ok(t) | |
| 457 | + | for _, tok := range []string{"", token} { | |
| 458 | + | var h map[string]string | |
| 459 | + | if tok != "" { | |
| 460 | + | h = as(tok) | |
| 461 | + | } | |
| 462 | + | body := b.get("/proj/merge-requests/1", h).body | |
| 463 | + | assert.NotContains(t, body, "<script>alert") | |
| 464 | + | assert.NotContains(t, body, "<img src=x") | |
| 465 | + | assert.NotContains(t, body, "<script>x</script>") | |
| 466 | + | assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`) | |
| 467 | + | assert.Contains(t, body, "<script>x</script> & more") | |
| 468 | + | assertNoInline(t, body) | |
| 469 | + | assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote") | |
| 470 | + | } | |
| 471 | + | } | |
| 472 | + | ||
| 473 | + | // The merge request pages learn of changes over the event stream: a fixed | |
| 474 | + | // sentence and a link, never the text of a comment or a title. | |
| 475 | + | func TestWebMergeRequestEvents(t *testing.T) { | |
| 476 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 477 | + | list := r.sse("topic=repo:proj&view=mrs", "") | |
| 478 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Secret <title>").ok(t) | |
| 479 | + | ev := list.next(t, watchWait) | |
| 480 | + | assert.Equal(t, "mr-notice", ev.name) | |
| 481 | + | assert.Contains(t, ev.data, `<div id="mr-notice" class="live-banner" role="status">`) | |
| 482 | + | assert.Contains(t, ev.data, `href="/proj/merge-requests"`) | |
| 483 | + | assert.NotContains(t, ev.data, "Secret") | |
| 484 | + | ||
| 485 | + | one := r.sse("topic=repo:proj&view=mr&n=1&ref=feat/search", "") | |
| 486 | + | other := r.sse("topic=repo:proj&view=mr&n=2&ref=feat/other", "") | |
| 487 | + | r.cli(t, adminToken, "mr", "comment", "proj", "1", "-m", "private words").ok(t) | |
| 488 | + | ev = one.next(t, watchWait) | |
| 489 | + | assert.Equal(t, "mr-notice", ev.name) | |
| 490 | + | assert.Contains(t, ev.data, "a comment was added") | |
| 491 | + | assert.Contains(t, ev.data, `href="/proj/merge-requests/1"`) | |
| 492 | + | assert.NotContains(t, ev.data, "private words") | |
| 493 | + | ||
| 494 | + | r.writeFile(r.wt, "more.txt", []byte("more\n")) | |
| 495 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/search") | |
| 496 | + | ev = one.next(t, watchWait) | |
| 497 | + | assert.Contains(t, ev.data, "the branch changed") | |
| 498 | + | ||
| 499 | + | r.cli(t, adminToken, "mr", "close", "proj", "1").ok(t) | |
| 500 | + | assert.Contains(t, one.next(t, watchWait).data, "was closed") | |
| 501 | + | ||
| 502 | + | // A page of another request hears nothing of this one. | |
| 503 | + | for drained := false; !drained; { | |
| 504 | + | select { | |
| 505 | + | case ev := <-other.events: | |
| 506 | + | if !ev.comment && ev.name != "" { | |
| 507 | + | t.Fatalf("unexpected event %+v", ev) | |
| 508 | + | } | |
| 509 | + | default: | |
| 510 | + | drained = true | |
| 511 | + | } | |
| 512 | + | } | |
| 513 | + | assert.Contains(t, r.get1("/proj/merge-requests/1"), `hx-sse:connect="/-/events?topic=repo%3Aproj&view=mr&n=1&ref=feat%2Fsearch"`) | |
| 514 | + | } | |
| 515 | + | ||
| 516 | + | func (r *mrRepo) get1(path string) string { | |
| 517 | + | r.t.Helper() | |
| 518 | + | _, _, body := r.get(path) | |
| 519 | + | return body | |
| 520 | + | } | |
| 521 | + | ||
| 522 | + | // A rewritten branch that dropped the commit the owner read says so, and shows | |
| 523 | + | // the whole change. | |
| 524 | + | func TestWebMergeRequestSinceAfterForcePush(t *testing.T) { | |
| 525 | + | r := newMRRepo(t, domain.VisibilityPublic) | |
| 526 | + | b := browser{r.harness} | |
| 527 | + | token := r.signIn(t) | |
| 528 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "t").ok(t) | |
| 529 | + | base := "/proj/merge-requests/1" | |
| 530 | + | ||
| 531 | + | nothing := b.get(base+"/since", as(token)) | |
| 532 | + | assert.Equal(t, 200, nothing.status) | |
| 533 | + | assert.Contains(t, nothing.body, "nothing was marked read yet") | |
| 534 | + | ||
| 535 | + | head1 := r.headOf("feat/search") | |
| 536 | + | require.Equal(t, http.StatusSeeOther, b.form(base+"/read", token, url.Values{"head_id": {head1}}).status) | |
| 537 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/read", token, url.Values{"head_id": {strings.Repeat("0", 40)}}).status, "not a commit of the repository") | |
| 538 | + | r.mustGit(r.wt, "reset", "-q", "--hard", "main") | |
| 539 | + | r.writeFile(r.wt, "other.txt", []byte("o\n")) | |
| 540 | + | r.mustGit(r.wt, "push", "-q", "--force", r.url, "feat/search") | |
| 541 | + | r.bareGit("proj", "reflog", "expire", "--expire=now", "--all") | |
| 542 | + | r.bareGit("proj", "gc", "-q", "--prune=now") | |
| 543 | + | ||
| 544 | + | lost := b.get(base+"/since", as(token)) | |
| 545 | + | require.Equal(t, 200, lost.status) | |
| 546 | + | assert.Contains(t, lost.body, "is gone, the branch was rewritten") | |
| 547 | + | assert.Contains(t, lost.body, "other.txt", "the whole change is shown") | |
| 548 | + | } | |
| @@ -0,0 +1,548 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package main | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "html" | |||
| 7 | + | "net/http" | |||
| 8 | + | "net/url" | |||
| 9 | + | "regexp" | |||
| 10 | + | "strings" | |||
| 11 | + | "testing" | |||
| 12 | + | ||||
| 13 | + | "github.com/stretchr/testify/assert" | |||
| 14 | + | "github.com/stretchr/testify/require" | |||
| 15 | + | ||||
| 16 | + | "bnema.dev/ermite/internal/app" | |||
| 17 | + | "bnema.dev/ermite/internal/domain" | |||
| 18 | + | ) | |||
| 19 | + | ||||
| 20 | + | // mrRepo is a public repository "proj" with README.md on main and a branch | |||
| 21 | + | // feat/search that is one commit ahead of it (search.go, three lines). | |||
| 22 | + | type mrRepo struct { | |||
| 23 | + | *cliHarness | |||
| 24 | + | wt, url string | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | func newMRRepo(t *testing.T, vis domain.Visibility) *mrRepo { | |||
| 28 | + | t.Helper() | |||
| 29 | + | c := newCLIHarness(t) | |||
| 30 | + | _, err := c.forge.Repos.Create(c.ctx, domain.Root(), createParams(c.harness, "proj", vis)) | |||
| 31 | + | require.NoError(t, err) | |||
| 32 | + | r := &mrRepo{cliHarness: c, wt: c.workTree("main", "README.md"), url: c.urlAuth(adminToken, "/proj.git")} | |||
| 33 | + | c.mustGit(r.wt, "push", "-q", r.url, "main") | |||
| 34 | + | c.mustGit(r.wt, "checkout", "-q", "-b", "feat/search") | |||
| 35 | + | c.writeFile(r.wt, "search.go", []byte("package search\n\nfunc Find() {}\n")) | |||
| 36 | + | c.mustGit(r.wt, "push", "-q", r.url, "feat/search") | |||
| 37 | + | return r | |||
| 38 | + | } | |||
| 39 | + | ||||
| 40 | + | func (r *mrRepo) headOf(branch string) string { return r.bareGit("proj", "rev-parse", branch) } | |||
| 41 | + | ||||
| 42 | + | // form posts url-encoded values as the signed-in owner, from this site. | |||
| 43 | + | func (b browser) form(path, token string, values url.Values) page { | |||
| 44 | + | b.h.t.Helper() | |||
| 45 | + | return b.do("POST", path, post(token), values.Encode()) | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | var reHeadField = regexp.MustCompile(`name="head_id" value="([0-9a-f]{40})"`) | |||
| 49 | + | ||||
| 50 | + | // formHead reads the head the merge form of a page carries. | |||
| 51 | + | func formHead(t *testing.T, body string) string { | |||
| 52 | + | t.Helper() | |||
| 53 | + | m := reHeadField.FindStringSubmatch(body) | |||
| 54 | + | require.Len(t, m, 2, "the page has a merge form") | |||
| 55 | + | return m[1] | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | // AC7: the pages as an anonymous visitor sees them on a public repository: | |||
| 59 | + | // read-only, no forms, public cache headers; the same pages signed in carry | |||
| 60 | + | // the forms and the private headers. | |||
| 61 | + | func TestWebMergeRequestPagesRender(t *testing.T) { | |||
| 62 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 63 | + | b := browser{r.harness} | |||
| 64 | + | token := r.signIn(t) | |||
| 65 | + | ||||
| 66 | + | // The compare page: anonymous reads, the owner gets the form. | |||
| 67 | + | anon := b.get("/proj/compare/feat/search", nil) | |||
| 68 | + | require.Equal(t, 200, anon.status, anon.body) | |||
| 69 | + | assert.Equal(t, "no-cache", anon.hdr.Get("Cache-Control")) | |||
| 70 | + | assert.Empty(t, anon.hdr.Values("Vary")) | |||
| 71 | + | assert.Contains(t, anon.body, "feat/search") | |||
| 72 | + | assert.Contains(t, anon.body, "package search</td>", "the diff of the single commit") | |||
| 73 | + | assert.NotContains(t, anon.body, "<form class=\"cform create\"") | |||
| 74 | + | assert.NotContains(t, anon.body, `<form class="cform create"`) | |||
| 75 | + | assertNoInline(t, anon.body) | |||
| 76 | + | owner := b.get("/proj/compare/feat/search", as(token)) | |||
| 77 | + | require.Equal(t, 200, owner.status) | |||
| 78 | + | assert.Equal(t, "private, no-store", owner.hdr.Get("Cache-Control")) | |||
| 79 | + | assert.Contains(t, owner.hdr.Values("Vary"), "Cookie") | |||
| 80 | + | assert.Contains(t, owner.body, `<form class="cform create" method="post" action="/proj/merge-requests">`) | |||
| 81 | + | assert.Contains(t, owner.body, `name="source" value="feat/search"`) | |||
| 82 | + | assert.Contains(t, owner.body, `name="target" value="main"`) | |||
| 83 | + | assert.Equal(t, 404, b.get("/proj/compare/nope", nil).status) | |||
| 84 | + | assert.Equal(t, 404, b.get("/proj/compare/main/extra", nil).status) | |||
| 85 | + | main := b.get("/proj/compare/main", nil) | |||
| 86 | + | assert.Equal(t, 200, main.status) | |||
| 87 | + | assert.Contains(t, main.body, "nothing to compare") | |||
| 88 | + | ||||
| 89 | + | // An empty list, with the way to start one. | |||
| 90 | + | list := b.get("/proj/merge-requests", nil) | |||
| 91 | + | require.Equal(t, 200, list.status) | |||
| 92 | + | assert.Contains(t, list.body, "no open merge requests") | |||
| 93 | + | assert.Contains(t, list.body, `href="/proj/compare/feat/search"`) | |||
| 94 | + | assert.Contains(t, list.body, `data-key="m"`, "the section of the nav") | |||
| 95 | + | assert.NotContains(t, strings.ToLower(list.body), "proposal") | |||
| 96 | + | ||||
| 97 | + | // Open one from the compare page. | |||
| 98 | + | res := b.form("/proj/merge-requests", token, url.Values{ | |||
| 99 | + | "source": {"feat/search"}, "target": {"main"}, "title": {"Add <b>search</b>"}, | |||
| 100 | + | "description": {"Finds things.\r\nSee https://example.org/spec) and javascript:alert(1)"}, | |||
| 101 | + | }) | |||
| 102 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 103 | + | assert.Equal(t, "/proj/merge-requests/1", res.hdr.Get("Location")) | |||
| 104 | + | assert.Equal(t, "no-store", res.hdr.Get("Cache-Control")) | |||
| 105 | + | head := r.headOf("feat/search") | |||
| 106 | + | ||||
| 107 | + | // The same pair twice is refused with a page, and nothing is created. | |||
| 108 | + | dup := b.form("/proj/merge-requests", token, url.Values{"source": {"feat/search"}, "target": {"main"}, "title": {"dup"}}) | |||
| 109 | + | assert.Equal(t, http.StatusConflict, dup.status) | |||
| 110 | + | assert.Equal(t, "no-store", dup.hdr.Get("Cache-Control")) | |||
| 111 | + | assert.Contains(t, dup.body, "already exists") | |||
| 112 | + | bad := b.form("/proj/merge-requests", token, url.Values{"source": {"feat/search"}, "target": {"main"}, "title": {"bidi\u202etitle"}}) | |||
| 113 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |||
| 114 | + | assert.NotContains(t, bad.body, "\u202e") | |||
| 115 | + | ||||
| 116 | + | // The compare page now points at it. | |||
| 117 | + | assert.Contains(t, b.get("/proj/compare/feat/search", as(token)).body, `href="/proj/merge-requests/1"`) | |||
| 118 | + | assert.NotContains(t, b.get("/proj/compare/feat/search", as(token)).body, `<form class="cform create"`) | |||
| 119 | + | ||||
| 120 | + | // The list. | |||
| 121 | + | list = b.get("/proj/merge-requests", nil) | |||
| 122 | + | assert.Contains(t, list.body, `<a href="/proj/merge-requests/1">Add <b>search</b></a>`) | |||
| 123 | + | assert.Contains(t, list.body, "feat/search → main") | |||
| 124 | + | assert.Equal(t, "no-cache", list.hdr.Get("Cache-Control")) | |||
| 125 | + | assertNoInline(t, list.body) | |||
| 126 | + | assert.Equal(t, "private, no-store", b.get("/proj/merge-requests", as(token)).hdr.Get("Cache-Control")) | |||
| 127 | + | ||||
| 128 | + | // The page, anonymous: read-only. | |||
| 129 | + | mr := b.get("/proj/merge-requests/1", nil) | |||
| 130 | + | require.Equal(t, 200, mr.status, mr.body) | |||
| 131 | + | assert.Equal(t, "no-cache", mr.hdr.Get("Cache-Control")) | |||
| 132 | + | assert.Empty(t, mr.hdr.Values("Vary")) | |||
| 133 | + | assert.Contains(t, mr.body, "<h1><span class=\"mute\">!1</span> Add <b>search</b></h1>") | |||
| 134 | + | assert.NotContains(t, mr.body, "<b>search</b>") | |||
| 135 | + | assert.Contains(t, mr.body, "feat/search") | |||
| 136 | + | assert.Contains(t, mr.body, "package search</td>") | |||
| 137 | + | assert.Contains(t, mr.body, "no checks") | |||
| 138 | + | assert.Contains(t, mr.body, "fast-forward, squash") | |||
| 139 | + | assert.Contains(t, mr.body, `<a href="https://example.org/spec" rel="noopener nofollow ugc">https://example.org/spec</a>)`, "the address is linked, not its closing bracket") | |||
| 140 | + | assert.NotContains(t, mr.body, `href="javascript:`) | |||
| 141 | + | assert.Contains(t, mr.body, "Finds things.\nSee ", "line breaks are kept") | |||
| 142 | + | assert.NotContains(t, mr.body, "<form", "an anonymous visitor gets no form") | |||
| 143 | + | assert.NotContains(t, mr.body, "ermite mr merge", "nor the merge line") | |||
| 144 | + | assert.NotContains(t, mr.body, "[merge]") | |||
| 145 | + | assert.Contains(t, mr.body, `href="/proj/commit/`+head+`"`, "the commits are in the rail") | |||
| 146 | + | assert.NotContains(t, strings.ToLower(mr.body), "proposal") | |||
| 147 | + | assertNoInline(t, mr.body) | |||
| 148 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/1/since", nil).status, "the read position is the owner's") | |||
| 149 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/9", nil).status) | |||
| 150 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/01", nil).status) | |||
| 151 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/x", nil).status) | |||
| 152 | + | ||||
| 153 | + | // Signed in: forms, the merge box with the head and the CLI lines. | |||
| 154 | + | own := b.get("/proj/merge-requests/1", as(token)) | |||
| 155 | + | require.Equal(t, 200, own.status) | |||
| 156 | + | assert.Equal(t, "private, no-store", own.hdr.Get("Cache-Control")) | |||
| 157 | + | assert.Contains(t, own.hdr.Values("Vary"), "Cookie") | |||
| 158 | + | assert.Equal(t, head, formHead(t, own.body)) | |||
| 159 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/merge"`) | |||
| 160 | + | assert.Contains(t, own.body, `<option value="fast-forward">`) | |||
| 161 | + | assert.Contains(t, own.body, `<option value="squash">`) | |||
| 162 | + | assert.Contains(t, own.body, "ermite mr merge proj 1 --head "+head) | |||
| 163 | + | assert.Contains(t, own.body, "ermite mr merge proj 1 --head "+head+" --squash") | |||
| 164 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/comments"`) | |||
| 165 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/close"`) | |||
| 166 | + | assert.Contains(t, own.body, `href="/proj/merge-requests/1?on=new:1:search.go#f`, "line numbers link to their form") | |||
| 167 | + | assertNoInline(t, own.body) | |||
| 168 | + | ||||
| 169 | + | // Reading is safe: GET marked nothing as read. | |||
| 170 | + | assert.NotContains(t, own.body, "since your last read") | |||
| 171 | + | show := r.cli(t, adminToken, "mr", "show", "proj", "1", "--json").ok(t) | |||
| 172 | + | assert.Contains(t, show.stdout, `"last_read_id": ""`) | |||
| 173 | + | } | |||
| 174 | + | ||||
| 175 | + | // A private repository is invisible to the anonymous visitor on every merge | |||
| 176 | + | // request page, and the owner sees it. | |||
| 177 | + | func TestWebMergeRequestPrivate(t *testing.T) { | |||
| 178 | + | r := newMRRepo(t, domain.VisibilityPrivate) | |||
| 179 | + | b := browser{r.harness} | |||
| 180 | + | token := r.signIn(t) | |||
| 181 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Secret plan").ok(t) | |||
| 182 | + | ||||
| 183 | + | missing := b.get("/nothere/merge-requests/1", nil) | |||
| 184 | + | for _, p := range []string{ | |||
| 185 | + | "/proj/merge-requests", "/proj/merge-requests/1", "/proj/merge-requests/1/since", "/proj/compare/feat/search", | |||
| 186 | + | } { | |||
| 187 | + | got := b.get(p, nil) | |||
| 188 | + | assert.Equal(t, 404, got.status, p) | |||
| 189 | + | assert.NotContains(t, got.body, "Secret plan", p) | |||
| 190 | + | assert.NotContains(t, got.body, "search.go", p) | |||
| 191 | + | } | |||
| 192 | + | assert.Equal(t, missing.body, b.get("/proj/merge-requests/1", nil).body, "same page as a missing repository") | |||
| 193 | + | assert.Equal(t, 404, b.get("/-/events?topic=repo:proj&view=mrs", nil).status) | |||
| 194 | + | ||||
| 195 | + | for _, p := range []string{"/proj/merge-requests", "/proj/merge-requests/1", "/proj/merge-requests/1/since", "/proj/compare/feat/search"} { | |||
| 196 | + | got := b.get(p, as(token)) | |||
| 197 | + | assert.Equal(t, 200, got.status, p) | |||
| 198 | + | assert.Equal(t, "private, no-store", got.hdr.Get("Cache-Control"), p) | |||
| 199 | + | assert.Contains(t, got.hdr.Values("Vary"), "Cookie", p) | |||
| 200 | + | } | |||
| 201 | + | assert.Contains(t, b.get("/proj/merge-requests", as(token)).body, "Secret plan") | |||
| 202 | + | } | |||
| 203 | + | ||||
| 204 | + | // AC2: every write needs the session and an Origin of this site. | |||
| 205 | + | func TestWebMergeRequestWritesNeedSessionAndOrigin(t *testing.T) { | |||
| 206 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 207 | + | b := browser{r.harness} | |||
| 208 | + | token := r.signIn(t) | |||
| 209 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "t").ok(t) | |||
| 210 | + | head := r.headOf("feat/search") | |||
| 211 | + | ||||
| 212 | + | paths := map[string]string{ | |||
| 213 | + | "/proj/merge-requests": "source=feat/search&target=main&title=x", | |||
| 214 | + | "/proj/merge-requests/1/comments": "text=hello", | |||
| 215 | + | "/proj/merge-requests/1/comments/1/resolve": "resolved=true", | |||
| 216 | + | "/proj/merge-requests/1/merge": "mode=fast-forward&head_id=" + head, | |||
| 217 | + | "/proj/merge-requests/1/close": "", | |||
| 218 | + | "/proj/merge-requests/1/read": "head_id=" + head, | |||
| 219 | + | } | |||
| 220 | + | for path, body := range paths { | |||
| 221 | + | noOrigin := b.do("POST", path, as(token), body) | |||
| 222 | + | assert.Equal(t, 403, noOrigin.status, path+" without Origin") | |||
| 223 | + | assert.Equal(t, "no-store", noOrigin.hdr.Get("Cache-Control")) | |||
| 224 | + | foreign := b.do("POST", path, map[string]string{"Cookie": sessionCookie + "=" + token, "Origin": "https://evil.example"}, body) | |||
| 225 | + | assert.Equal(t, 403, foreign.status, path+" foreign Origin") | |||
| 226 | + | anon := b.do("POST", path, map[string]string{"Origin": publicOrigin}, body) | |||
| 227 | + | assert.Equal(t, 403, anon.status, path+" without a session") | |||
| 228 | + | } | |||
| 229 | + | // Nothing happened. | |||
| 230 | + | out := r.cli(t, "", "mr", "show", "proj", "1", "--json").ok(t).stdout | |||
| 231 | + | assert.Contains(t, out, `"state": "open"`) | |||
| 232 | + | assert.NotContains(t, out, "hello") | |||
| 233 | + | assert.Equal(t, "", strings.TrimSpace(r.bareGit("proj", "rev-list", "main..feat/search", "--not", "feat/search"))) | |||
| 234 | + | ||||
| 235 | + | // A GET on a write route is not a page. | |||
| 236 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/1/merge", as(token)).status) | |||
| 237 | + | assert.Equal(t, 404, b.get("/proj/merge-requests/1/close", as(token)).status) | |||
| 238 | + | } | |||
| 239 | + | ||||
| 240 | + | var reNoteID = regexp.MustCompile(`id="c(\d+)"`) | |||
| 241 | + | ||||
| 242 | + | // The review of one request, in the browser's words: a note on a line, a | |||
| 243 | + | // general one, resolve, since the last read after a push, a stale head, the | |||
| 244 | + | // fast-forward merge. | |||
| 245 | + | func TestWebMergeRequestReviewAndFastForward(t *testing.T) { | |||
| 246 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 247 | + | b := browser{r.harness} | |||
| 248 | + | token := r.signIn(t) | |||
| 249 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Add search").ok(t) | |||
| 250 | + | head1 := r.headOf("feat/search") | |||
| 251 | + | base := "/proj/merge-requests/1" | |||
| 252 | + | ||||
| 253 | + | // The form of a line, then the note under that line. | |||
| 254 | + | line := b.get(base+"?on=new:3:search.go", as(token)) | |||
| 255 | + | require.Equal(t, 200, line.status) | |||
| 256 | + | assert.Contains(t, line.body, `<form class="cform"`) | |||
| 257 | + | assert.Contains(t, line.body, `name="path" value="search.go"`) | |||
| 258 | + | assert.Contains(t, line.body, `name="line" value="3"`) | |||
| 259 | + | assert.Contains(t, line.body, `name="side" value="new"`) | |||
| 260 | + | assert.Equal(t, 0, strings.Count(b.get(base+"?on=new:99:search.go", as(token)).body, `name="line" value="99"`), "a line the diff lacks has no form") | |||
| 261 | + | assert.Equal(t, 0, strings.Count(b.get(base+"?on=new:3:search.go", nil).body, `<form`), "nor for a visitor") | |||
| 262 | + | ||||
| 263 | + | res := b.form(base+"/comments", token, url.Values{"text": {"Why <i>empty</i>?\r\nSee https://example.org/a."}, "path": {"search.go"}, "line": {"3"}, "side": {"new"}}) | |||
| 264 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 265 | + | assert.Equal(t, base+"#c1", res.hdr.Get("Location")) | |||
| 266 | + | assert.Equal(t, "no-store", res.hdr.Get("Cache-Control")) | |||
| 267 | + | res = b.form(base+"/comments", token, url.Values{"text": {"Looks fine overall."}}) | |||
| 268 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 269 | + | assert.Equal(t, base+"#c2", res.hdr.Get("Location")) | |||
| 270 | + | ||||
| 271 | + | anon := b.get(base, nil) | |||
| 272 | + | i := strings.Index(anon.body, `<tr class="fnote">`) | |||
| 273 | + | require.GreaterOrEqual(t, i, 0, "a note row under the line") | |||
| 274 | + | note := anon.body[i:] | |||
| 275 | + | assert.Less(t, strings.Index(note, "Why <i>empty</i>?"), strings.Index(note, "</tr>"), "the note sits in the row under its line") | |||
| 276 | + | assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="noopener nofollow ugc">https://example.org/a</a>.`) | |||
| 277 | + | assert.NotContains(t, anon.body, "<i>empty</i>") | |||
| 278 | + | assert.Contains(t, anon.body, "Looks fine overall.") | |||
| 279 | + | assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1))) | |||
| 280 | + | assert.NotContains(t, anon.body, "[resolve]", "a visitor cannot resolve") | |||
| 281 | + | ||||
| 282 | + | // A comment that breaks a rule is refused with the rule, nothing is lost. | |||
| 283 | + | bad := b.form(base+"/comments", token, url.Values{"text": {"ok"}, "path": {"search.go"}, "line": {"99"}, "side": {"new"}}) | |||
| 284 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |||
| 285 | + | assert.Equal(t, "no-store", bad.hdr.Get("Cache-Control")) | |||
| 286 | + | assert.Contains(t, bad.body, "search.go") | |||
| 287 | + | bad = b.form(base+"/comments", token, url.Values{"text": {"x"}, "path": {"search.go"}, "line": {"zz"}, "side": {"new"}}) | |||
| 288 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |||
| 289 | + | bad = b.form(base+"/comments", token, url.Values{"text": {"x"}, "path": {"search.go"}, "line": {"3"}, "side": {"sideways"}}) | |||
| 290 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |||
| 291 | + | bad = b.form(base+"/comments", token, url.Values{"text": {"evil\u202etext"}}) | |||
| 292 | + | assert.Equal(t, http.StatusBadRequest, bad.status) | |||
| 293 | + | assert.NotContains(t, bad.body, "\u202e") | |||
| 294 | + | assert.Equal(t, 404, b.form("/proj/merge-requests/9/comments", token, url.Values{"text": {"x"}}).status) | |||
| 295 | + | ||||
| 296 | + | // Resolve and unresolve. | |||
| 297 | + | own := b.get(base, as(token)) | |||
| 298 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/comments/1/resolve"`) | |||
| 299 | + | res = b.form(base+"/comments/1/resolve", token, url.Values{"resolved": {"true"}}) | |||
| 300 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 301 | + | assert.Equal(t, base+"#c1", res.hdr.Get("Location")) | |||
| 302 | + | own = b.get(base, as(token)) | |||
| 303 | + | assert.Contains(t, own.body, `data-resolved="true"`) | |||
| 304 | + | assert.Contains(t, own.body, "[unresolve]") | |||
| 305 | + | assert.Equal(t, http.StatusSeeOther, b.form(base+"/comments/1/resolve", token, url.Values{"resolved": {"false"}}).status) | |||
| 306 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/comments/1/resolve", token, url.Values{"resolved": {"maybe"}}).status) | |||
| 307 | + | assert.Equal(t, 404, b.form(base+"/comments/42/resolve", token, url.Values{"resolved": {"true"}}).status) | |||
| 308 | + | ||||
| 309 | + | // Mark read, then push: the since page shows the new change only. | |||
| 310 | + | assert.Contains(t, own.body, `action="/proj/merge-requests/1/read"`) | |||
| 311 | + | res = b.form(base+"/read", token, url.Values{"head_id": {head1}}) | |||
| 312 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 313 | + | assert.NotContains(t, b.get(base, as(token)).body, "[mark read]", "the head is the one that was read") | |||
| 314 | + | r.writeFile(r.wt, "extra.txt", []byte("extra\n")) | |||
| 315 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/search") | |||
| 316 | + | head2 := r.headOf("feat/search") | |||
| 317 | + | ||||
| 318 | + | all := b.get(base, as(token)) | |||
| 319 | + | assert.Contains(t, all.body, `href="/proj/merge-requests/1/since"`) | |||
| 320 | + | assert.Contains(t, all.body, "since your last read") | |||
| 321 | + | assert.Contains(t, all.body, "<li data-new>", "the commit pushed since is marked") | |||
| 322 | + | assert.Contains(t, all.body, "[mark read]") | |||
| 323 | + | assert.Contains(t, all.body, "outdated", "the notes were written on the earlier head") | |||
| 324 | + | since := b.get(base+"/since", as(token)) | |||
| 325 | + | require.Equal(t, 200, since.status, since.body) | |||
| 326 | + | assert.Equal(t, "private, no-store", since.hdr.Get("Cache-Control")) | |||
| 327 | + | assert.Contains(t, since.body, "changes since "+head1[:7]) | |||
| 328 | + | assert.Contains(t, since.body, "extra</td>") | |||
| 329 | + | assert.NotContains(t, since.body, "package search</td>", "only what changed after the read head") | |||
| 330 | + | assert.Contains(t, since.body, "Why <i>empty</i>?", "the notes are listed below, their lines are not in this diff") | |||
| 331 | + | assert.Contains(t, since.body, "search.go:3 (new)") | |||
| 332 | + | assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff") | |||
| 333 | + | ||||
| 334 | + | // The merge form of the page the owner read carries that head: the push | |||
| 335 | + | // that came after is not merged unseen. | |||
| 336 | + | stale := b.form(base+"/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {head1}}) | |||
| 337 | + | assert.Equal(t, http.StatusConflict, stale.status) | |||
| 338 | + | assert.Equal(t, "no-store", stale.hdr.Get("Cache-Control")) | |||
| 339 | + | assert.Contains(t, stale.body, "branch moved") | |||
| 340 | + | assert.NotEqual(t, head2, r.headOf("main")) | |||
| 341 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/merge", token, url.Values{"mode": {"rebase"}, "head_id": {head2}}).status) | |||
| 342 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/merge", token, url.Values{"mode": {"squash"}, "head_id": {"abc"}}).status) | |||
| 343 | + | ||||
| 344 | + | // Fast-forward with the head that was read. | |||
| 345 | + | fresh := b.get(base, as(token)) | |||
| 346 | + | require.Equal(t, head2, formHead(t, fresh.body)) | |||
| 347 | + | res = b.form(base+"/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, fresh.body)}}) | |||
| 348 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 349 | + | assert.Equal(t, base, res.hdr.Get("Location")) | |||
| 350 | + | assert.Equal(t, head2, r.headOf("main"), "the target moved to the head") | |||
| 351 | + | ||||
| 352 | + | done := b.get(base, nil) | |||
| 353 | + | assert.Contains(t, done.body, "merged") | |||
| 354 | + | assert.Contains(t, done.body, "fast-forward") | |||
| 355 | + | assert.Contains(t, done.body, `href="/proj/commit/`+head2+`"`) | |||
| 356 | + | assert.NotContains(t, done.body, "<form") | |||
| 357 | + | assert.NotContains(t, b.get(base, as(token)).body, "[merge]", "nothing to merge any more") | |||
| 358 | + | // Writes on a merged request are refused with a page. | |||
| 359 | + | late := b.form(base+"/comments", token, url.Values{"text": {"too late"}}) | |||
| 360 | + | assert.Equal(t, http.StatusConflict, late.status) | |||
| 361 | + | assert.Equal(t, http.StatusConflict, b.form(base+"/close", token, nil).status) | |||
| 362 | + | list := b.get("/proj/merge-requests", nil) | |||
| 363 | + | assert.Contains(t, list.body, "merged and closed") | |||
| 364 | + | assert.Regexp(t, `(?s)merged and closed.*Add search`, list.body) | |||
| 365 | + | } | |||
| 366 | + | ||||
| 367 | + | // Squash and close, and the order of the list: open first, then the decided. | |||
| 368 | + | func TestWebMergeRequestSquashCloseAndList(t *testing.T) { | |||
| 369 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 370 | + | b := browser{r.harness} | |||
| 371 | + | token := r.signIn(t) | |||
| 372 | + | head := r.headOf("feat/search") | |||
| 373 | + | main0 := r.headOf("main") | |||
| 374 | + | ||||
| 375 | + | // Open from compare with a description; the merge form squashes. | |||
| 376 | + | res := b.form("/proj/merge-requests", token, url.Values{ | |||
| 377 | + | "source": {"feat/search"}, "target": {"main"}, "title": {"Squash me"}, "description": {""}, | |||
| 378 | + | }) | |||
| 379 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 380 | + | res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}}) | |||
| 381 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 382 | + | merged := r.headOf("main") | |||
| 383 | + | assert.NotEqual(t, head, merged, "squash makes a new commit") | |||
| 384 | + | assert.Equal(t, main0, r.bareGit("proj", "rev-parse", "main^"), "on top of the old target") | |||
| 385 | + | assert.Contains(t, r.bareGit("proj", "cat-file", "-p", "main"), "gpgsig", "signed by the instance key") | |||
| 386 | + | page := b.get("/proj/merge-requests/1", nil) | |||
| 387 | + | assert.Contains(t, page.body, "squash") | |||
| 388 | + | assert.Contains(t, page.body, `href="/proj/commit/`+merged+`"`) | |||
| 389 | + | ||||
| 390 | + | // A second request: closed. | |||
| 391 | + | r.mustGit(r.wt, "checkout", "-q", "-b", "feat/other", "main") | |||
| 392 | + | r.mustGit(r.wt, "pull", "-q", r.url, "main") | |||
| 393 | + | r.writeFile(r.wt, "other.go", []byte("package other\n")) | |||
| 394 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/other") | |||
| 395 | + | r.mustGit(r.wt, "checkout", "-q", "-b", "feat/third", "feat/other") | |||
| 396 | + | r.writeFile(r.wt, "third.go", []byte("package third\n")) | |||
| 397 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/third") | |||
| 398 | + | for _, title := range []struct{ src, title string }{{"feat/other", "Other change"}, {"feat/third", "Third change"}} { | |||
| 399 | + | res = b.form("/proj/merge-requests", token, url.Values{"source": {title.src}, "target": {"main"}, "title": {title.title}}) | |||
| 400 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 401 | + | } | |||
| 402 | + | res = b.form("/proj/merge-requests/2/close", token, nil) | |||
| 403 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 404 | + | assert.Equal(t, "/proj/merge-requests/2", res.hdr.Get("Location")) | |||
| 405 | + | assert.Equal(t, "no-store", res.hdr.Get("Cache-Control")) | |||
| 406 | + | closed := b.get("/proj/merge-requests/2", nil) | |||
| 407 | + | assert.Contains(t, closed.body, `data-state="closed"`) | |||
| 408 | + | assert.NotContains(t, closed.body, "<form") | |||
| 409 | + | assert.Equal(t, http.StatusConflict, b.form("/proj/merge-requests/2/close", token, nil).status, "closing twice says so") | |||
| 410 | + | ||||
| 411 | + | // Open one first, then merged and closed ones, newest first within each. | |||
| 412 | + | list := b.get("/proj/merge-requests", nil).body | |||
| 413 | + | third, squash, other := strings.Index(list, "Third change"), strings.Index(list, "Squash me"), strings.Index(list, "Other change") | |||
| 414 | + | require.True(t, third >= 0 && squash >= 0 && other >= 0, list) | |||
| 415 | + | assert.Less(t, third, squash, "the open request comes first") | |||
| 416 | + | assert.Less(t, third, other) | |||
| 417 | + | assert.Less(t, strings.Index(list, ">open<"), strings.Index(list, "merged and closed")) | |||
| 418 | + | assert.Less(t, other, squash, "newest first among the decided") | |||
| 419 | + | ||||
| 420 | + | // A compare page of a branch that is behind says so. | |||
| 421 | + | r.mustGit(r.wt, "checkout", "-q", "-b", "feat/stale", main0) | |||
| 422 | + | r.writeFile(r.wt, "stale.go", []byte("package stale\n")) | |||
| 423 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/stale") | |||
| 424 | + | stale := b.get("/proj/compare/feat/stale", nil) | |||
| 425 | + | assert.Equal(t, 200, stale.status) | |||
| 426 | + | assert.Contains(t, stale.body, "has moved on") | |||
| 427 | + | } | |||
| 428 | + | ||||
| 429 | + | // The comment limit of the API is a message, not a crash. | |||
| 430 | + | func TestWebMergeRequestLimitIsFriendly(t *testing.T) { | |||
| 431 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 432 | + | b := browser{r.harness} | |||
| 433 | + | token := r.signIn(t) | |||
| 434 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "t").ok(t) | |||
| 435 | + | for i := 0; i < 1000; i++ { | |||
| 436 | + | _, err := r.forge.MergeRequests.AddComment(r.ctx, domain.Root(), r.name("proj"), 1, app.AddCommentParams{Text: "n"}) | |||
| 437 | + | require.NoError(t, err) | |||
| 438 | + | } | |||
| 439 | + | full := b.form("/proj/merge-requests/1/comments", token, url.Values{"text": {"one more"}}) | |||
| 440 | + | assert.Equal(t, http.StatusConflict, full.status) | |||
| 441 | + | assert.Contains(t, full.body, "limit reached") | |||
| 442 | + | assert.Contains(t, full.body, "1000 comments") | |||
| 443 | + | assert.Equal(t, "no-store", full.hdr.Get("Cache-Control")) | |||
| 444 | + | } | |||
| 445 | + | ||||
| 446 | + | // Text is escaped everywhere and invisible characters are shown as markers. | |||
| 447 | + | func TestWebMergeRequestEscapesText(t *testing.T) { | |||
| 448 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 449 | + | b := browser{r.harness} | |||
| 450 | + | token := r.signIn(t) | |||
| 451 | + | res := b.form("/proj/merge-requests", token, url.Values{ | |||
| 452 | + | "source": {"feat/search"}, "target": {"main"}, "title": {`"><script>alert(1)</script>`}, | |||
| 453 | + | "description": {"<img src=x onerror=alert(1)>\nhttp://x.test/\"onmouseover=\"alert(1)"}, | |||
| 454 | + | }) | |||
| 455 | + | require.Equal(t, http.StatusSeeOther, res.status, res.body) | |||
| 456 | + | r.cli(t, adminToken, "mr", "comment", "proj", "1", "-m", "<script>x</script> & more").ok(t) | |||
| 457 | + | for _, tok := range []string{"", token} { | |||
| 458 | + | var h map[string]string | |||
| 459 | + | if tok != "" { | |||
| 460 | + | h = as(tok) | |||
| 461 | + | } | |||
| 462 | + | body := b.get("/proj/merge-requests/1", h).body | |||
| 463 | + | assert.NotContains(t, body, "<script>alert") | |||
| 464 | + | assert.NotContains(t, body, "<img src=x") | |||
| 465 | + | assert.NotContains(t, body, "<script>x</script>") | |||
| 466 | + | assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`) | |||
| 467 | + | assert.Contains(t, body, "<script>x</script> & more") | |||
| 468 | + | assertNoInline(t, body) | |||
| 469 | + | assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote") | |||
| 470 | + | } | |||
| 471 | + | } | |||
| 472 | + | ||||
| 473 | + | // The merge request pages learn of changes over the event stream: a fixed | |||
| 474 | + | // sentence and a link, never the text of a comment or a title. | |||
| 475 | + | func TestWebMergeRequestEvents(t *testing.T) { | |||
| 476 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 477 | + | list := r.sse("topic=repo:proj&view=mrs", "") | |||
| 478 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "Secret <title>").ok(t) | |||
| 479 | + | ev := list.next(t, watchWait) | |||
| 480 | + | assert.Equal(t, "mr-notice", ev.name) | |||
| 481 | + | assert.Contains(t, ev.data, `<div id="mr-notice" class="live-banner" role="status">`) | |||
| 482 | + | assert.Contains(t, ev.data, `href="/proj/merge-requests"`) | |||
| 483 | + | assert.NotContains(t, ev.data, "Secret") | |||
| 484 | + | ||||
| 485 | + | one := r.sse("topic=repo:proj&view=mr&n=1&ref=feat/search", "") | |||
| 486 | + | other := r.sse("topic=repo:proj&view=mr&n=2&ref=feat/other", "") | |||
| 487 | + | r.cli(t, adminToken, "mr", "comment", "proj", "1", "-m", "private words").ok(t) | |||
| 488 | + | ev = one.next(t, watchWait) | |||
| 489 | + | assert.Equal(t, "mr-notice", ev.name) | |||
| 490 | + | assert.Contains(t, ev.data, "a comment was added") | |||
| 491 | + | assert.Contains(t, ev.data, `href="/proj/merge-requests/1"`) | |||
| 492 | + | assert.NotContains(t, ev.data, "private words") | |||
| 493 | + | ||||
| 494 | + | r.writeFile(r.wt, "more.txt", []byte("more\n")) | |||
| 495 | + | r.mustGit(r.wt, "push", "-q", r.url, "feat/search") | |||
| 496 | + | ev = one.next(t, watchWait) | |||
| 497 | + | assert.Contains(t, ev.data, "the branch changed") | |||
| 498 | + | ||||
| 499 | + | r.cli(t, adminToken, "mr", "close", "proj", "1").ok(t) | |||
| 500 | + | assert.Contains(t, one.next(t, watchWait).data, "was closed") | |||
| 501 | + | ||||
| 502 | + | // A page of another request hears nothing of this one. | |||
| 503 | + | for drained := false; !drained; { | |||
| 504 | + | select { | |||
| 505 | + | case ev := <-other.events: | |||
| 506 | + | if !ev.comment && ev.name != "" { | |||
| 507 | + | t.Fatalf("unexpected event %+v", ev) | |||
| 508 | + | } | |||
| 509 | + | default: | |||
| 510 | + | drained = true | |||
| 511 | + | } | |||
| 512 | + | } | |||
| 513 | + | assert.Contains(t, r.get1("/proj/merge-requests/1"), `hx-sse:connect="/-/events?topic=repo%3Aproj&view=mr&n=1&ref=feat%2Fsearch"`) | |||
| 514 | + | } | |||
| 515 | + | ||||
| 516 | + | func (r *mrRepo) get1(path string) string { | |||
| 517 | + | r.t.Helper() | |||
| 518 | + | _, _, body := r.get(path) | |||
| 519 | + | return body | |||
| 520 | + | } | |||
| 521 | + | ||||
| 522 | + | // A rewritten branch that dropped the commit the owner read says so, and shows | |||
| 523 | + | // the whole change. | |||
| 524 | + | func TestWebMergeRequestSinceAfterForcePush(t *testing.T) { | |||
| 525 | + | r := newMRRepo(t, domain.VisibilityPublic) | |||
| 526 | + | b := browser{r.harness} | |||
| 527 | + | token := r.signIn(t) | |||
| 528 | + | r.cli(t, adminToken, "mr", "create", "proj", "--source", "feat/search", "--title", "t").ok(t) | |||
| 529 | + | base := "/proj/merge-requests/1" | |||
| 530 | + | ||||
| 531 | + | nothing := b.get(base+"/since", as(token)) | |||
| 532 | + | assert.Equal(t, 200, nothing.status) | |||
| 533 | + | assert.Contains(t, nothing.body, "nothing was marked read yet") | |||
| 534 | + | ||||
| 535 | + | head1 := r.headOf("feat/search") | |||
| 536 | + | require.Equal(t, http.StatusSeeOther, b.form(base+"/read", token, url.Values{"head_id": {head1}}).status) | |||
| 537 | + | assert.Equal(t, http.StatusBadRequest, b.form(base+"/read", token, url.Values{"head_id": {strings.Repeat("0", 40)}}).status, "not a commit of the repository") | |||
| 538 | + | r.mustGit(r.wt, "reset", "-q", "--hard", "main") | |||
| 539 | + | r.writeFile(r.wt, "other.txt", []byte("o\n")) | |||
| 540 | + | r.mustGit(r.wt, "push", "-q", "--force", r.url, "feat/search") | |||
| 541 | + | r.bareGit("proj", "reflog", "expire", "--expire=now", "--all") | |||
| 542 | + | r.bareGit("proj", "gc", "-q", "--prune=now") | |||
| 543 | + | ||||
| 544 | + | lost := b.get(base+"/since", as(token)) | |||
| 545 | + | require.Equal(t, 200, lost.status) | |||
| 546 | + | assert.Contains(t, lost.body, "is gone, the branch was rewritten") | |||
| 547 | + | assert.Contains(t, lost.body, "other.txt", "the whole change is shown") | |||
| 548 | + | } | |||
A internal/adapters/web/plaintext_test.go +72 -0
| @@ -0,0 +1,72 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package web | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "bytes" | |
| 7 | + | "context" | |
| 8 | + | "testing" | |
| 9 | + | ||
| 10 | + | "github.com/stretchr/testify/assert" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // Plain text stays text; only http(s) addresses become links, without the | |
| 15 | + | // punctuation that ends a sentence; invisible characters are marked. | |
| 16 | + | func TestPlainParts(t *testing.T) { | |
| 17 | + | tests := []struct { | |
| 18 | + | name, in string | |
| 19 | + | want []TextPart | |
| 20 | + | }{ | |
| 21 | + | {"plain", "nothing to see", []TextPart{{Text: "nothing to see"}}}, | |
| 22 | + | {"empty", "", nil}, | |
| 23 | + | {"link", "see https://example.org/a/b?x=1#y now", []TextPart{{Text: "see "}, {Text: "https://example.org/a/b?x=1#y", Href: "https://example.org/a/b?x=1#y"}, {Text: " now"}}}, | |
| 24 | + | {"sentence end", "go to http://x.test/a.", []TextPart{{Text: "go to "}, {Text: "http://x.test/a", Href: "http://x.test/a"}, {Text: "."}}}, | |
| 25 | + | {"closing bracket", "(see https://x.test/a)", []TextPart{{Text: "(see "}, {Text: "https://x.test/a", Href: "https://x.test/a"}, {Text: ")"}}}, | |
| 26 | + | {"balanced bracket", "https://x.test/a_(b)", []TextPart{{Text: "https://x.test/a_(b)", Href: "https://x.test/a_(b)"}}}, | |
| 27 | + | {"other schemes stay text", "javascript:alert(1) ftp://x.test data:text/html,x", []TextPart{{Text: "javascript:alert(1) ftp://x.test data:text/html,x"}}}, | |
| 28 | + | {"no host", "https:// and http:///x", []TextPart{{Text: "https:// and http:///x"}}}, | |
| 29 | + | {"quote ends the address", `http://x.test/"onmouseover="y`, []TextPart{{Text: "http://x.test/", Href: "http://x.test/"}, {Text: `"onmouseover="y`}}}, | |
| 30 | + | {"crlf", "a\r\nb", []TextPart{{Text: "a\nb"}}}, | |
| 31 | + | {"invisible", "a\u202eb", []TextPart{{Text: "a<U+202E>b"}}}, | |
| 32 | + | } | |
| 33 | + | for _, tt := range tests { | |
| 34 | + | t.Run(tt.name, func(t *testing.T) { | |
| 35 | + | assert.Equal(t, tt.want, plainParts(tt.in)) | |
| 36 | + | }) | |
| 37 | + | } | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | // userText escapes the text and the address, and marks the link as untrusted. | |
| 41 | + | func TestUserTextEscapes(t *testing.T) { | |
| 42 | + | var b bytes.Buffer | |
| 43 | + | require.NoError(t, userText(plainParts(`<b>x</b> & http://x.test/?a=1&b=2`)).Render(context.Background(), &b)) | |
| 44 | + | assert.Equal(t, `<b>x</b> & <a href="http://x.test/?a=1&b=2" rel="noopener nofollow ugc">http://x.test/?a=1&b=2</a>`, b.String()) | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func TestParseOn(t *testing.T) { | |
| 48 | + | assert.Equal(t, &lineRef{side: "new", line: 12, path: "a/b:c.go"}, parseOn("new:12:a/b:c.go")) | |
| 49 | + | assert.Equal(t, &lineRef{side: "old", line: 1, path: "x"}, parseOn("old:1:x")) | |
| 50 | + | for _, s := range []string{"", "new", "new:12", "new:12:", "new:0:x", "new:-1:x", "new:x:y", "up:1:x", "new::x"} { | |
| 51 | + | assert.Nil(t, parseOn(s), s) | |
| 52 | + | } | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | func TestShellWord(t *testing.T) { | |
| 56 | + | assert.Equal(t, "feat/search", shellWord("feat/search")) | |
| 57 | + | assert.Equal(t, `'a b'`, shellWord("a b")) | |
| 58 | + | assert.Equal(t, `'it'\''s'`, shellWord("it's")) | |
| 59 | + | assert.Equal(t, `'$(x)'`, shellWord("$(x)")) | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | // The notes of a split row: a context line counts once. | |
| 63 | + | func TestSplitRowNotes(t *testing.T) { | |
| 64 | + | a, b := CommentView{ID: "1"}, CommentView{ID: "2"} | |
| 65 | + | ctx := &DiffLine{Notes: []CommentView{a}} | |
| 66 | + | assert.Equal(t, []CommentView{a}, SplitRow{ctx, ctx}.Notes()) | |
| 67 | + | del, add := &DiffLine{Notes: []CommentView{a}}, &DiffLine{Notes: []CommentView{b}} | |
| 68 | + | assert.Equal(t, []CommentView{a, b}, SplitRow{del, add}.Notes()) | |
| 69 | + | assert.Equal(t, []CommentView{a}, SplitRow{del, nil}.Notes()) | |
| 70 | + | assert.Equal(t, []CommentView{b}, SplitRow{nil, add}.Notes()) | |
| 71 | + | assert.Nil(t, SplitRow{}.Notes()) | |
| 72 | + | } | |
| @@ -0,0 +1,72 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package web | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "bytes" | |||
| 7 | + | "context" | |||
| 8 | + | "testing" | |||
| 9 | + | ||||
| 10 | + | "github.com/stretchr/testify/assert" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // Plain text stays text; only http(s) addresses become links, without the | |||
| 15 | + | // punctuation that ends a sentence; invisible characters are marked. | |||
| 16 | + | func TestPlainParts(t *testing.T) { | |||
| 17 | + | tests := []struct { | |||
| 18 | + | name, in string | |||
| 19 | + | want []TextPart | |||
| 20 | + | }{ | |||
| 21 | + | {"plain", "nothing to see", []TextPart{{Text: "nothing to see"}}}, | |||
| 22 | + | {"empty", "", nil}, | |||
| 23 | + | {"link", "see https://example.org/a/b?x=1#y now", []TextPart{{Text: "see "}, {Text: "https://example.org/a/b?x=1#y", Href: "https://example.org/a/b?x=1#y"}, {Text: " now"}}}, | |||
| 24 | + | {"sentence end", "go to http://x.test/a.", []TextPart{{Text: "go to "}, {Text: "http://x.test/a", Href: "http://x.test/a"}, {Text: "."}}}, | |||
| 25 | + | {"closing bracket", "(see https://x.test/a)", []TextPart{{Text: "(see "}, {Text: "https://x.test/a", Href: "https://x.test/a"}, {Text: ")"}}}, | |||
| 26 | + | {"balanced bracket", "https://x.test/a_(b)", []TextPart{{Text: "https://x.test/a_(b)", Href: "https://x.test/a_(b)"}}}, | |||
| 27 | + | {"other schemes stay text", "javascript:alert(1) ftp://x.test data:text/html,x", []TextPart{{Text: "javascript:alert(1) ftp://x.test data:text/html,x"}}}, | |||
| 28 | + | {"no host", "https:// and http:///x", []TextPart{{Text: "https:// and http:///x"}}}, | |||
| 29 | + | {"quote ends the address", `http://x.test/"onmouseover="y`, []TextPart{{Text: "http://x.test/", Href: "http://x.test/"}, {Text: `"onmouseover="y`}}}, | |||
| 30 | + | {"crlf", "a\r\nb", []TextPart{{Text: "a\nb"}}}, | |||
| 31 | + | {"invisible", "a\u202eb", []TextPart{{Text: "a<U+202E>b"}}}, | |||
| 32 | + | } | |||
| 33 | + | for _, tt := range tests { | |||
| 34 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 35 | + | assert.Equal(t, tt.want, plainParts(tt.in)) | |||
| 36 | + | }) | |||
| 37 | + | } | |||
| 38 | + | } | |||
| 39 | + | ||||
| 40 | + | // userText escapes the text and the address, and marks the link as untrusted. | |||
| 41 | + | func TestUserTextEscapes(t *testing.T) { | |||
| 42 | + | var b bytes.Buffer | |||
| 43 | + | require.NoError(t, userText(plainParts(`<b>x</b> & http://x.test/?a=1&b=2`)).Render(context.Background(), &b)) | |||
| 44 | + | assert.Equal(t, `<b>x</b> & <a href="http://x.test/?a=1&b=2" rel="noopener nofollow ugc">http://x.test/?a=1&b=2</a>`, b.String()) | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | func TestParseOn(t *testing.T) { | |||
| 48 | + | assert.Equal(t, &lineRef{side: "new", line: 12, path: "a/b:c.go"}, parseOn("new:12:a/b:c.go")) | |||
| 49 | + | assert.Equal(t, &lineRef{side: "old", line: 1, path: "x"}, parseOn("old:1:x")) | |||
| 50 | + | for _, s := range []string{"", "new", "new:12", "new:12:", "new:0:x", "new:-1:x", "new:x:y", "up:1:x", "new::x"} { | |||
| 51 | + | assert.Nil(t, parseOn(s), s) | |||
| 52 | + | } | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | func TestShellWord(t *testing.T) { | |||
| 56 | + | assert.Equal(t, "feat/search", shellWord("feat/search")) | |||
| 57 | + | assert.Equal(t, `'a b'`, shellWord("a b")) | |||
| 58 | + | assert.Equal(t, `'it'\''s'`, shellWord("it's")) | |||
| 59 | + | assert.Equal(t, `'$(x)'`, shellWord("$(x)")) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | // The notes of a split row: a context line counts once. | |||
| 63 | + | func TestSplitRowNotes(t *testing.T) { | |||
| 64 | + | a, b := CommentView{ID: "1"}, CommentView{ID: "2"} | |||
| 65 | + | ctx := &DiffLine{Notes: []CommentView{a}} | |||
| 66 | + | assert.Equal(t, []CommentView{a}, SplitRow{ctx, ctx}.Notes()) | |||
| 67 | + | del, add := &DiffLine{Notes: []CommentView{a}}, &DiffLine{Notes: []CommentView{b}} | |||
| 68 | + | assert.Equal(t, []CommentView{a, b}, SplitRow{del, add}.Notes()) | |||
| 69 | + | assert.Equal(t, []CommentView{a}, SplitRow{del, nil}.Notes()) | |||
| 70 | + | assert.Equal(t, []CommentView{b}, SplitRow{nil, add}.Notes()) | |||
| 71 | + | assert.Nil(t, SplitRow{}.Notes()) | |||
| 72 | + | } | |||