krz/gitbay

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

merged merged by cmc on 2026-09-04 02:56 UTC · krz/gitbay:flash-notice into main

9 files changed, +63 −52

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) {