[build] internal/relui: fix URL path segment escaping for task names
1 view
Skip to first unread message
Dmitri Shuralyov (Gerrit)
Dec 23, 2025, 5:13:00 PM (2 days ago) Dec 23
to Michael Knyszek, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, golang-co...@googlegroups.com
Attention needed from Michael Knyszek
Dmitri Shuralyov has uploaded the change for review![]( "Open in Gerrit")
Dmitri Shuralyov would like Michael Knyszek to review this change.
Commit message
internal/relui: fix URL path segment escaping for task names
url.PathEscape can be used to escape a string so it can be safely placed
inside a URL path segment. Do so, which after migrating to http.ServeMux
and its pattern matching makes it possible to successfully restart tasks
with characters like "/" in their name (e.g., such as a task named "Wait
for x/tools stdlib CL submission" where we first ran into this).
Fixes golang/go#76860.
Change-Id: Ie69516e0f3b440baf6cdd60dbc4f5785cd24a822
Change diff
diff --git a/internal/relui/templates/task_list.html b/internal/relui/templates/task_list.html
index a291383..73cb594 100644
--- a/internal/relui/templates/task_list.html
+++ b/internal/relui/templates/task_list.html
@@ -76,7 +76,7 @@
{{if .Error.Valid}}
<div class="TaskList-retryTask">
<form
- action="{{baseLink (printf "/workflows/%s/tasks/%s/retry" $workflow.ID .Name)}}"
+ action="{{baseLink (printf "/workflows/%s/tasks/%s/retry" $workflow.ID (.Name|urlPathEscape))}}"
method="post">
<input type="hidden" id="workflow.id" name="workflow.id" value="{{$workflow.ID}}" />
<input
@@ -90,7 +90,7 @@
{{else if and (not .ApprovedAt.Valid) (.ReadyForApproval)}}
<div class="TaskList-approveTask">
<form
- action="{{baseLink (printf "/workflows/%s/tasks/%s/approve" $workflow.ID .Name)}}"
+ action="{{baseLink (printf "/workflows/%s/tasks/%s/approve" $workflow.ID (.Name|urlPathEscape))}}"
method="post">
<input type="hidden" id="workflow.id" name="workflow.id" value="{{$workflow.ID}}" />
<input
diff --git a/internal/relui/web.go b/internal/relui/web.go
index 8d999d1..449d0c8 100644
--- a/internal/relui/web.go
+++ b/internal/relui/web.go
@@ -88,6 +88,7 @@
"prettySize": prettySize,
"sidebarWorkflows": s.sidebarWorkflows,
"unmarshalResultDetail": unmarshalResultDetail,
+ "urlPathEscape": url.PathEscape,
}
s.templates = template.Must(template.New("").Funcs(helpers).ParseFS(templates, "templates/*.html"))
s.homeTmpl = s.mustLookup("home.html")
diff --git a/internal/relui/web_test.go b/internal/relui/web_test.go
index e5bc40d..2828052 100644
--- a/internal/relui/web_test.go
+++ b/internal/relui/web_test.go
@@ -1026,3 +1026,36 @@
}
t.Run("unacld workflow", func(t *testing.T) { testWorkflowACL(t, true, false, false) })
}
+
+// Test that it's possible to retry a task whose name includes characters
+// such as "/" that become different after URL path segment escaping.
+func TestRetryHandlerEscaping(t *testing.T) {
+ s := &Server{m: &metricsRouter{mux: http.NewServeMux()}}
+ var gotTaskName string
+ s.m.HandleFunc("POST /workflows/{id}/tasks/{name}/retry", func(w http.ResponseWriter, req *http.Request) {
+ gotTaskName = req.PathValue("name")
+ })
+ cl := &http.Client{Transport: localRoundTripper{s.m}}
+ resp, err := cl.Post("/workflows/123/tasks/update%20x%2Fbuild/retry", "", nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got, want := resp.StatusCode, http.StatusOK; got != want {
+ t.Fatalf("status code mismatch: got %d, want %d", got, want)
+ }
+ if got, want := gotTaskName, "update x/build"; got != want {
+ t.Errorf("task name mismatch: got %q, want %q", got, want)
+ }
+}
+
+// localRoundTripper is an http.RoundTripper that executes HTTP transactions
+// by using handler directly, instead of going over an HTTP connection.
+type localRoundTripper struct {
+ handler http.Handler
+}
+
+func (l localRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+ w := httptest.NewRecorder()
+ l.handler.ServeHTTP(w, req)
+ return w.Result(), nil
+}
Change information
Files:
- M internal/relui/templates/task_list.html
- M internal/relui/web.go
- M internal/relui/web_test.go
Change size: S
Delta: 3 files changed, 36 insertions(+), 2 deletions(-)
Related details
Attention is currently required from:
- Michael Knyszek
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dmitri Shuralyov (Gerrit)
Dec 23, 2025, 5:19:01 PM (2 days ago) Dec 23
to Dmitri Shuralyov, goph...@pubsubhelper.golang.org, Go LUCI, Michael Knyszek, golang-co...@googlegroups.com
Attention needed from Michael Knyszek
Dmitri Shuralyov voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Michael Knyszek
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Knyszek (Gerrit)
Dec 23, 2025, 5:26:00 PM (2 days ago) Dec 23
to Dmitri Shuralyov, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Go LUCI, golang-co...@googlegroups.com
Attention needed from Dmitri Shuralyov
Michael Knyszek voted Code-Review+2![]( "Open in Gerrit")
Attention is currently required from:
- Dmitri Shuralyov
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dmitri Shuralyov (Gerrit)
Dec 23, 2025, 5:48:11 PM (2 days ago) Dec 23
to Dmitri Shuralyov, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Dmitri Shuralyov
Dmitri Shuralyov uploaded new patchset![]( "Open in Gerrit")
Dmitri Shuralyov uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Dmitri Shuralyov
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages