Commit 15961c4783

15961c47839f73ebaeaa1f97c575ce86107e3b3b

parent: 191da4bb52

Verified · cmc ci/build: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-04 01:44 UTC

web: a failed form action's reason rides a one-shot cookie, not the URL

Action handlers redirected back with ?e=<message>, so the reason
survived a reload, landed in history and bookmarks, and every page
that could show one read the query string. setFlash queues the message
in a short-lived HttpOnly cookie on the redirect and takeFlash reads
and clears it where the page renders. The merge request form keeps its
prefill in the query and the account page keeps ?m= for its success
note.

Closes #119

Layout: unified · split

internal/httpd/account.go +3 −5
@@ -56,7 +56,7 @@ func (s *Server) accountForm(w http.ResponseWriter, r *http.Request, u store.Use
56 Notice string 56 Notice string
57 Message string 57 Message string
58 }{s.baseFor(u), "account", keys, pgp, emails, s.cfg.SiteHost(), 58 }{s.baseFor(u), "account", keys, pgp, emails, s.cfg.SiteHost(),
59 r.URL.Query().Get("e"), r.URL.Query().Get("m")}) 59 s.takeFlash(w, r), r.URL.Query().Get("m")})
60} 60}
61 61
62// accountSubmit routes the account forms to their commands. Everything 62// accountSubmit routes the account forms to their commands. Everything
@@ -64,12 +64,10 @@ func (s *Server) accountForm(w http.ResponseWriter, r *http.Request, u store.Use
64func (s *Server) accountSubmit(w http.ResponseWriter, r *http.Request, u store.User) { 64func (s *Server) accountSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
65 back := func(msg, note string) { 65 back := func(msg, note string) {
66 q := "" 66 q := ""
67 switch { 67 if note != "" {
68 case msg != "":
69 q = "?e=" + url.QueryEscape(msg)
70 case note != "":
71 q = "?m=" + url.QueryEscape(note) 68 q = "?m=" + url.QueryEscape(note)
72 } 69 }
70 s.setFlash(w, msg)
73 http.Redirect(w, r, "/settings"+q, http.StatusSeeOther) 71 http.Redirect(w, r, "/settings"+q, http.StatusSeeOther)
74 } 72 }
75 73
internal/httpd/builds.go +1 −1
@@ -45,7 +45,7 @@ func (s *Server) builds(w http.ResponseWriter, r *http.Request) {
45 Jobs []jobView 45 Jobs []jobView
46 CanWrite bool 46 CanWrite bool
47 Notice string 47 Notice string
48 }{p, builds, jobs, s.canWriteRepo(r, p.Repo), r.URL.Query().Get("e")}) 48 }{p, builds, jobs, s.canWriteRepo(r, p.Repo), s.takeFlash(w, r)})
49} 49}
50 50
51func (s *Server) build(w http.ResponseWriter, r *http.Request) { 51func (s *Server) build(w http.ResponseWriter, r *http.Request) {
internal/httpd/flash.go added +44
@@ -0,0 +1,44 @@
1package httpd
2
3import (
4 "net/http"
5 "net/url"
6)
7
8// A form action that fails redirects back to the page it came from with
9// the reason. The reason used to ride the URL as ?e=, so it survived a
10// reload and landed in history and bookmarks. It rides a one-shot cookie
11// now: set on the redirect, read and cleared by the page that renders it
12// (#119).
13const flashCookie = "gitbay_notice"
14
15// setFlash queues msg for the next page render. An empty msg sets
16// nothing.
17func (s *Server) setFlash(w http.ResponseWriter, msg string) {
18 if msg == "" {
19 return
20 }
21 if len(msg) > 300 {
22 msg = msg[:300]
23 }
24 http.SetCookie(w, &http.Cookie{
25 Name: flashCookie, Value: url.QueryEscape(msg), Path: "/",
26 HttpOnly: true, SameSite: http.SameSiteLaxMode,
27 Secure: s.cfg.HTTP.TLS != "off",
28 MaxAge: 60,
29 })
30}
31
32// takeFlash returns the queued message, if any, and clears it.
33func (s *Server) takeFlash(w http.ResponseWriter, r *http.Request) string {
34 c, err := r.Cookie(flashCookie)
35 if err != nil || c.Value == "" {
36 return ""
37 }
38 http.SetCookie(w, &http.Cookie{Name: flashCookie, Value: "", Path: "/", MaxAge: -1})
39 msg, err := url.QueryUnescape(c.Value)
40 if err != nil {
41 return ""
42 }
43 return msg
44}
internal/httpd/issueactions.go +1 −7
@@ -3,7 +3,6 @@ package httpd
3import ( 3import (
4 "fmt" 4 "fmt"
5 "net/http" 5 "net/http"
6 "net/url"
7 "strings" 6 "strings"
8 7
9 "gitbay.org/gitbay/internal/store" 8 "gitbay.org/gitbay/internal/store"
@@ -16,12 +15,7 @@ import (
16func (s *Server) issueRedirect(w http.ResponseWriter, r *http.Request, msg string) { 15func (s *Server) issueRedirect(w http.ResponseWriter, r *http.Request, msg string) {
17 dest := fmt.Sprintf("/%s/%s/issues/%s", 16 dest := fmt.Sprintf("/%s/%s/issues/%s",
18 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n")) 17 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n"))
19 if msg != "" { 18 s.setFlash(w, msg)
20 if len(msg) > 300 {
21 msg = msg[:300]
22 }
23 dest += "?e=" + url.QueryEscape(msg)
24 }
25 http.Redirect(w, r, dest, http.StatusSeeOther) 19 http.Redirect(w, r, dest, http.StatusSeeOther)
26} 20}
27 21
internal/httpd/mractions.go +5 −14
@@ -22,12 +22,7 @@ import (
22func (s *Server) mrRedirect(w http.ResponseWriter, r *http.Request, msg string) { 22func (s *Server) mrRedirect(w http.ResponseWriter, r *http.Request, msg string) {
23 dest := fmt.Sprintf("/%s/%s/mrs/%s", 23 dest := fmt.Sprintf("/%s/%s/mrs/%s",
24 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n")) 24 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n"))
25 if msg != "" { 25 s.setFlash(w, msg)
26 if len(msg) > 300 {
27 msg = msg[:300]
28 }
29 dest += "?e=" + url.QueryEscape(msg)
30 }
31 http.Redirect(w, r, dest, http.StatusSeeOther) 26 http.Redirect(w, r, dest, http.StatusSeeOther)
32} 27}
33 28
@@ -35,12 +30,7 @@ func (s *Server) mrRedirect(w http.ResponseWriter, r *http.Request, msg string)
35func (s *Server) mrDiffRedirect(w http.ResponseWriter, r *http.Request, msg string) { 30func (s *Server) mrDiffRedirect(w http.ResponseWriter, r *http.Request, msg string) {
36 dest := fmt.Sprintf("/%s/%s/mrs/%s?view=diff", 31 dest := fmt.Sprintf("/%s/%s/mrs/%s?view=diff",
37 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n")) 32 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n"))
38 if msg != "" { 33 s.setFlash(w, msg)
39 if len(msg) > 300 {
40 msg = msg[:300]
41 }
42 dest += "&e=" + url.QueryEscape(msg)
43 }
44 http.Redirect(w, r, dest, http.StatusSeeOther) 34 http.Redirect(w, r, dest, http.StatusSeeOther)
45} 35}
46 36
@@ -163,7 +153,7 @@ func (s *Server) mrCreateForm(w http.ResponseWriter, r *http.Request, u store.Us
163 s.render(w, "mrnew.html", mrNewPage{ 153 s.render(w, "mrnew.html", mrNewPage{
164 repoPage: p, Branches: branches, 154 repoPage: p, Branches: branches,
165 Source: q.Get("source"), Target: target, 155 Source: q.Get("source"), Target: target,
166 Title: q.Get("title"), Body: q.Get("body"), Notice: q.Get("e"), 156 Title: q.Get("title"), Body: q.Get("body"), Notice: s.takeFlash(w, r),
167 }) 157 })
168} 158}
169 159
@@ -178,7 +168,8 @@ func (s *Server) mrCreateSubmit(w http.ResponseWriter, r *http.Request, u store.
178 body := strings.TrimSpace(r.FormValue("body")) 168 body := strings.TrimSpace(r.FormValue("body"))
179 169
180 back := func(msg string) { 170 back := func(msg string) {
181 q := url.Values{"source": {source}, "target": {target}, "title": {title}, "body": {body}, "e": {msg}} 171 q := url.Values{"source": {source}, "target": {target}, "title": {title}, "body": {body}}
172 s.setFlash(w, msg)
182 http.Redirect(w, r, fmt.Sprintf("/%s/mrs/new?%s", p.Repo.Path(), q.Encode()), http.StatusSeeOther) 173 http.Redirect(w, r, fmt.Sprintf("/%s/mrs/new?%s", p.Repo.Path(), q.Encode()), http.StatusSeeOther)
183 } 174 }
184 if source == "" || title == "" { 175 if source == "" || title == "" {
internal/httpd/orgweb.go +2 −6
@@ -2,7 +2,6 @@ package httpd
2 2
3import ( 3import (
4 "net/http" 4 "net/http"
5 "net/url"
6 "strings" 5 "strings"
7 6
8 "gitbay.org/gitbay/internal/store" 7 "gitbay.org/gitbay/internal/store"
@@ -45,11 +44,8 @@ func (s *Server) orgAdminView(viewer store.User, kind, name string) (teams []tea
45func (s *Server) orgSubmit(w http.ResponseWriter, r *http.Request, u store.User) { 44func (s *Server) orgSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
46 owner := r.PathValue("owner") 45 owner := r.PathValue("owner")
47 back := func(msg string) { 46 back := func(msg string) {
48 dest := "/" + owner 47 s.setFlash(w, msg)
49 if msg != "" { 48 http.Redirect(w, r, "/"+owner, http.StatusSeeOther)
50 dest += "?e=" + url.QueryEscape(msg)
51 }
52 http.Redirect(w, r, dest, http.StatusSeeOther)
53 } 49 }
54 field := r.FormValue("field") 50 field := r.FormValue("field")
55 team := strings.TrimSpace(r.FormValue("team")) 51 team := strings.TrimSpace(r.FormValue("team"))
internal/httpd/releaseactions.go +1 −7
@@ -3,7 +3,6 @@ package httpd
3import ( 3import (
4 "fmt" 4 "fmt"
5 "net/http" 5 "net/http"
6 "net/url"
7 "strings" 6 "strings"
8 7
9 "gitbay.org/gitbay/internal/store" 8 "gitbay.org/gitbay/internal/store"
@@ -15,12 +14,7 @@ import (
15 14
16func (s *Server) backTo(w http.ResponseWriter, r *http.Request, page, msg string) { 15func (s *Server) backTo(w http.ResponseWriter, r *http.Request, page, msg string) {
17 dest := fmt.Sprintf("/%s/%s/%s", r.PathValue("owner"), r.PathValue("repo"), page) 16 dest := fmt.Sprintf("/%s/%s/%s", r.PathValue("owner"), r.PathValue("repo"), page)
18 if msg != "" { 17 s.setFlash(w, msg)
19 if len(msg) > 300 {
20 msg = msg[:300]
21 }
22 dest += "?e=" + url.QueryEscape(msg)
23 }
24 http.Redirect(w, r, dest, http.StatusSeeOther) 18 http.Redirect(w, r, dest, http.StatusSeeOther)
25} 19}
26 20
internal/httpd/settings.go +2 −8
@@ -3,7 +3,6 @@ package httpd
3import ( 3import (
4 "fmt" 4 "fmt"
5 "net/http" 5 "net/http"
6 "net/url"
7 "strings" 6 "strings"
8 7
9 "gitbay.org/gitbay/internal/gitutil" 8 "gitbay.org/gitbay/internal/gitutil"
@@ -39,18 +38,13 @@ func (s *Server) settingsForm(w http.ResponseWriter, r *http.Request, u store.Us
39 s.render(w, "settings.html", settingsPage{ 38 s.render(w, "settings.html", settingsPage{
40 repoPage: p, Topics: topics, Branches: branches, 39 repoPage: p, Topics: topics, Branches: branches,
41 DepsEnabled: depsErr == nil, 40 DepsEnabled: depsErr == nil,
42 Notice: r.URL.Query().Get("e"), 41 Notice: s.takeFlash(w, r),
43 }) 42 })
44} 43}
45 44
46func (s *Server) settingsRedirect(w http.ResponseWriter, r *http.Request, msg string) { 45func (s *Server) settingsRedirect(w http.ResponseWriter, r *http.Request, msg string) {
47 dest := fmt.Sprintf("/%s/%s/settings", r.PathValue("owner"), r.PathValue("repo")) 46 dest := fmt.Sprintf("/%s/%s/settings", r.PathValue("owner"), r.PathValue("repo"))
48 if msg != "" { 47 s.setFlash(w, msg)
49 if len(msg) > 300 {
50 msg = msg[:300]
51 }
52 dest += "?e=" + url.QueryEscape(msg)
53 }
54 http.Redirect(w, r, dest, http.StatusSeeOther) 48 http.Redirect(w, r, dest, http.StatusSeeOther)
55} 49}
56 50
internal/httpd/web.go +4 −4
@@ -469,7 +469,7 @@ func (s *Server) ownerPage(w http.ResponseWriter, r *http.Request) {
469 Notice string 469 Notice string
470 }{s.baseFor(viewer), name, d.Kind, profile, aboutHTML(profile), 470 }{s.baseFor(viewer), name, d.Kind, profile, aboutHTML(profile),
471 d.Repos, d.Members, d.Orgs, 471 d.Repos, d.Members, d.Orgs,
472 weeks, activityTotal, teams, canAdmin, r.URL.Query().Get("e")}) 472 weeks, activityTotal, teams, canAdmin, s.takeFlash(w, r)})
473} 473}
474 474
475func (s *Server) repoHome(w http.ResponseWriter, r *http.Request) { 475func (s *Server) repoHome(w http.ResponseWriter, r *http.Request) {
@@ -665,7 +665,7 @@ func (s *Server) releases(w http.ResponseWriter, r *http.Request) {
665 FreeTags []string 665 FreeTags []string
666 CanWrite bool 666 CanWrite bool
667 Notice string 667 Notice string
668 }{p, views, freeTags, s.canWriteRepo(r, p.Repo), r.URL.Query().Get("e")}) 668 }{p, views, freeTags, s.canWriteRepo(r, p.Repo), s.takeFlash(w, r)})
669} 669}
670 670
671// releaseAsset streams one uploaded asset. Tags containing '/' are not 671// releaseAsset streams one uploaded asset. Tags containing '/' are not
@@ -1640,7 +1640,7 @@ func (s *Server) issue(w http.ResponseWriter, r *http.Request) {
1640 LabelColors map[string]template.CSS 1640 LabelColors map[string]template.CSS
1641 }{p, iss, md(iss.Body, iss.BodyFormat), renderComments(comments, md), 1641 }{p, iss, md(iss.Body, iss.BodyFormat), renderComments(comments, md),
1642 s.canEditItem(r, p.Repo, iss.Author), s.canWriteRepo(r, p.Repo), 1642 s.canEditItem(r, p.Repo, iss.Author), s.canWriteRepo(r, p.Repo),
1643 milestones, r.URL.Query().Get("e"), s.labelColors(p.Repo.ID)}) 1643 milestones, s.takeFlash(w, r), s.labelColors(p.Repo.ID)})
1644} 1644}
1645 1645
1646// canEditItem: the author or anyone with write access may edit. 1646// canEditItem: the author or anyone with write access may edit.
@@ -1817,7 +1817,7 @@ func (s *Server) mr(w http.ResponseWriter, r *http.Request) {
1817 Stacked []store.MR 1817 Stacked []store.MR
1818 }{p, m, view, md(m.Body, m.BodyFormat), checks, combined, renderComments(comments, md), 1818 }{p, m, view, md(m.Body, m.BodyFormat), checks, combined, renderComments(comments, md),
1819 reviews, files, diffTruncated, stat, commits, commitsTotal, branches, s.canEditItem(r, p.Repo, m.Author), 1819 reviews, files, diffTruncated, stat, commits, commitsTotal, branches, s.canEditItem(r, p.Repo, m.Author),
1820 canWrite, unresolved, r.URL.Query().Get("e"), detachedThreads, stackedOn, stacked}) 1820 canWrite, unresolved, s.takeFlash(w, r), detachedThreads, stackedOn, stacked})
1821} 1821}
1822 1822
1823func (s *Server) refs(w http.ResponseWriter, r *http.Request) { 1823func (s *Server) refs(w http.ResponseWriter, r *http.Request) {