Commit 467699a1ed

467699a1ed1e29f30e110aa41eefc8c1d59138a0

parent: bc49092e23

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

cmc <hello@cleberg.net> · 2026-09-04 00:06 UTC

web: form actions answer by exit code

The action handlers discarded a command's exit code and redirected
with the message whatever the cause, so an issue that does not exist
came back as the page with a note in the URL. runControlCode keeps
the code, and done finishes the action by it: the 404 page for
not-found, and back to the page with the message for anything else,
since a refusal is feedback on the page a person was looking at,
whether a merge gate, a permission they lack, or a field they got
wrong. Twelve handlers converted.

Closes #106

Layout: unified · split

e2e/webwrites_test.go +6
@@ -50,6 +50,12 @@ func TestWebWritesGoThroughRegistry(t *testing.T) {
5050 t.Fatalf("repo created past the quota from the web: exit %d, want 3", code)
5151 }
5252
53 // A form action on an issue that does not exist is the 404 page, not a
54 // redirect carrying a message (#106).
55 if status, _ := browserPost(t, browser, inst.base()+"/alice/first/issues/999/state", url.Values{"action": {"close"}}); status != 404 {
56 t.Fatalf("closing a missing issue from the web: %d, want 404", status)
57 }
58
5359 // An archived repository refuses a comment from the web as it does
5460 // over ssh.
5561 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "archive", "alice/first"); code != 0 {
internal/httpd/control.go +35 −4
@@ -20,6 +20,14 @@ import (
2020// ViaAPI is set, which refuses SSHOnly commands: anything whose input is a
2121// credential (secrets, mirror tokens, session minting) stays on SSH.
2222func (s *Server) runControl(u store.User, argv []string) (out string, msg string, ok bool) {
23 out, msg, code := s.runControlCode(u, argv)
24 return out, msg, code == protocol.ExitOK
25}
26
27// runControlCode is runControl with the exit code, for handlers that
28// answer a form: not-found and denied deserve their own statuses rather
29// than a redirect carrying the message (#106).
30func (s *Server) runControlCode(u store.User, argv []string) (out string, msg string, code int) {
2331 var stdout, stderr bytes.Buffer
2432 ctx := &control.Ctx{
2533 User: u,
@@ -32,12 +40,30 @@ func (s *Server) runControl(u store.User, argv []string) (out string, msg string
3240 Stderr: &stderr,
3341 ViaAPI: true,
3442 }
35 code := control.Dispatch(ctx, argv)
43 code = control.Dispatch(ctx, argv)
3644 m := strings.TrimSpace(stderr.String())
3745 if m == "" {
3846 m = strings.TrimSpace(stdout.String())
3947 }
40 return stdout.String(), m, code == protocol.ExitOK
48 return stdout.String(), m, code
49}
50
51// done finishes a form action by exit code: back to the page on success,
52// the 404 page when the thing does not exist, and back to the page with
53// the message for anything else. A refusal is feedback on the page a
54// person was looking at, whether it is a merge gate, a permission they
55// lack, or a field they got wrong; only a thing that does not exist has
56// no page to go back to.
57func (s *Server) done(w http.ResponseWriter, r *http.Request, code int, msg string,
58 redirect func(http.ResponseWriter, *http.Request, string)) {
59 switch code {
60 case protocol.ExitOK:
61 redirect(w, r, "")
62 case protocol.ExitNotFound:
63 s.notFound(w, r)
64 default:
65 redirect(w, r, msg)
66 }
4167}
4268
4369// runControlStdin is runControl for the handful of commands whose input
@@ -46,6 +72,11 @@ func (s *Server) runControl(u store.User, argv []string) (out string, msg string
4672// tokens and mirror credentials remain SSHOnly and are refused by the
4773// dispatcher.
4874func (s *Server) runControlStdin(u store.User, argv []string, stdin string) (msg string, ok bool) {
75 msg, code := s.runControlStdinCode(u, argv, stdin)
76 return msg, code == protocol.ExitOK
77}
78
79func (s *Server) runControlStdinCode(u store.User, argv []string, stdin string) (msg string, code int) {
4980 var stdout, stderr bytes.Buffer
5081 ctx := &control.Ctx{
5182 User: u,
@@ -58,12 +89,12 @@ func (s *Server) runControlStdin(u store.User, argv []string, stdin string) (msg
5889 Stderr: &stderr,
5990 ViaAPI: true,
6091 }
61 code := control.Dispatch(ctx, argv)
92 code = control.Dispatch(ctx, argv)
6293 m := strings.TrimSpace(stderr.String())
6394 if m == "" {
6495 m = strings.TrimSpace(stdout.String())
6596 }
66 return m, code == protocol.ExitOK
97 return m, code
6798}
6899
69100// runControlInto runs a command in JSON mode and decodes its data into
internal/httpd/issueactions.go +8 −20
@@ -36,11 +36,8 @@ func (s *Server) issueStateSubmit(w http.ResponseWriter, r *http.Request, u stor
3636 if r.FormValue("action") == "reopen" {
3737 verb = "reopen"
3838 }
39 _, msg, ok := s.runControl(u, issueArgs(r, verb))
40 if ok {
41 msg = ""
42 }
43 s.issueRedirect(w, r, msg)
39 _, msg, code := s.runControlCode(u, issueArgs(r, verb))
40 s.done(w, r, code, msg, s.issueRedirect)
4441}
4542
4643// fieldArgs turns a space-separated form value into repeated flags, the
@@ -59,11 +56,8 @@ func (s *Server) issueLabelSubmit(w http.ResponseWriter, r *http.Request, u stor
5956 s.issueRedirect(w, r, "name at least one label")
6057 return
6158 }
62 _, msg, ok := s.runControl(u, issueArgs(r, "label", args...))
63 if ok {
64 msg = ""
65 }
66 s.issueRedirect(w, r, msg)
59 _, msg, code := s.runControlCode(u, issueArgs(r, "label", args...))
60 s.done(w, r, code, msg, s.issueRedirect)
6761}
6862
6963func (s *Server) issueAssignSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
@@ -72,11 +66,8 @@ func (s *Server) issueAssignSubmit(w http.ResponseWriter, r *http.Request, u sto
7266 s.issueRedirect(w, r, "name at least one person")
7367 return
7468 }
75 _, msg, ok := s.runControl(u, issueArgs(r, "assign", args...))
76 if ok {
77 msg = ""
78 }
79 s.issueRedirect(w, r, msg)
69 _, msg, code := s.runControlCode(u, issueArgs(r, "assign", args...))
70 s.done(w, r, code, msg, s.issueRedirect)
8071}
8172
8273func (s *Server) issueMilestoneSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
@@ -84,9 +75,6 @@ func (s *Server) issueMilestoneSubmit(w http.ResponseWriter, r *http.Request, u
8475 if title == "" {
8576 title = "none"
8677 }
87 _, msg, ok := s.runControl(u, issueArgs(r, "milestone", title))
88 if ok {
89 msg = ""
90 }
91 s.issueRedirect(w, r, msg)
78 _, msg, code := s.runControlCode(u, issueArgs(r, "milestone", title))
79 s.done(w, r, code, msg, s.issueRedirect)
9280}
internal/httpd/mractions.go +12 −30
@@ -59,11 +59,8 @@ func (s *Server) mrReviewSubmit(w http.ResponseWriter, r *http.Request, u store.
5959 s.mrRedirect(w, r, "pick approve, request changes, or comment")
6060 return
6161 }
62 _, msg, ok := s.runControl(u, mrArgs(r, "review", flag))
63 if ok {
64 msg = ""
65 }
66 s.mrRedirect(w, r, msg)
62 _, msg, code := s.runControlCode(u, mrArgs(r, "review", flag))
63 s.done(w, r, code, msg, s.mrRedirect)
6764}
6865
6966func (s *Server) mrMergeSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
@@ -71,19 +68,13 @@ func (s *Server) mrMergeSubmit(w http.ResponseWriter, r *http.Request, u store.U
7168 if st := strings.TrimSpace(r.FormValue("strategy")); st != "" && st != "auto" {
7269 args = append(args, "--strategy", st)
7370 }
74 _, msg, ok := s.runControl(u, mrArgs(r, "merge", args...))
75 if ok {
76 msg = ""
77 }
78 s.mrRedirect(w, r, msg)
71 _, msg, code := s.runControlCode(u, mrArgs(r, "merge", args...))
72 s.done(w, r, code, msg, s.mrRedirect)
7973}
8074
8175func (s *Server) mrCloseSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
82 _, msg, ok := s.runControl(u, mrArgs(r, "close"))
83 if ok {
84 msg = ""
85 }
86 s.mrRedirect(w, r, msg)
76 _, msg, code := s.runControlCode(u, mrArgs(r, "close"))
77 s.done(w, r, code, msg, s.mrRedirect)
8778}
8879
8980// mrDiffCommentSubmit opens a review thread on a diff line, or replies to
@@ -114,11 +105,8 @@ func (s *Server) mrDiffCommentSubmit(w http.ResponseWriter, r *http.Request, u s
114105 extra = append(extra, "--old")
115106 }
116107 }
117 msg, ok := s.runControlStdin(u, mrArgs(r, "diff-comment", append(extra, "--file", "-")...), body)
118 if ok {
119 msg = ""
120 }
121 s.mrDiffRedirect(w, r, msg)
108 msg, code := s.runControlStdinCode(u, mrArgs(r, "diff-comment", append(extra, "--file", "-")...), body)
109 s.done(w, r, code, msg, s.mrDiffRedirect)
122110}
123111
124112// mrRetargetSubmit moves the merge request onto another branch.
@@ -128,11 +116,8 @@ func (s *Server) mrRetargetSubmit(w http.ResponseWriter, r *http.Request, u stor
128116 s.mrRedirect(w, r, "pick a branch to retarget onto")
129117 return
130118 }
131 _, msg, ok := s.runControl(u, mrArgs(r, "retarget", target))
132 if ok {
133 msg = ""
134 }
135 s.mrRedirect(w, r, msg)
119 _, msg, code := s.runControlCode(u, mrArgs(r, "retarget", target))
120 s.done(w, r, code, msg, s.mrRedirect)
136121}
137122
138123// mrThreadSubmit resolves or reopens one review thread.
@@ -146,11 +131,8 @@ func (s *Server) mrThreadSubmit(w http.ResponseWriter, r *http.Request, u store.
146131 s.mrRedirect(w, r, "bad thread id")
147132 return
148133 }
149 _, msg, ok := s.runControl(u, mrArgs(r, verb, id))
150 if ok {
151 msg = ""
152 }
153 s.mrRedirect(w, r, msg)
134 _, msg, code := s.runControlCode(u, mrArgs(r, verb, id))
135 s.done(w, r, code, msg, s.mrRedirect)
154136}
155137
156138// mrNewPage is the create form: branches to choose from, plus whatever
internal/httpd/releaseactions.go +4 −10
@@ -45,11 +45,8 @@ func (s *Server) releaseSubmit(w http.ResponseWriter, r *http.Request, u store.U
4545 if notes != "" || verb == "edit" {
4646 argv = append(argv, "--notes", notes)
4747 }
48 _, msg, ok := s.runControl(u, argv)
49 if ok {
50 msg = ""
51 }
52 s.backTo(w, r, "releases", msg)
48 _, msg, code := s.runControlCode(u, argv)
49 s.done(w, r, code, msg, func(w http.ResponseWriter, r *http.Request, msg string) { s.backTo(w, r, "releases", msg) })
5350}
5451
5552func (s *Server) buildTriggerSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
@@ -59,9 +56,6 @@ func (s *Server) buildTriggerSubmit(w http.ResponseWriter, r *http.Request, u st
5956 s.backTo(w, r, "builds", "pick a job")
6057 return
6158 }
62 _, msg, ok := s.runControl(u, []string{"build", "trigger", repo, job})
63 if ok {
64 msg = ""
65 }
66 s.backTo(w, r, "builds", msg)
59 _, msg, code := s.runControlCode(u, []string{"build", "trigger", repo, job})
60 s.done(w, r, code, msg, func(w http.ResponseWriter, r *http.Request, msg string) { s.backTo(w, r, "builds", msg) })
6761}