What
Every validation error in the web config panics saveHandler instead of
re-rendering the page with the error list. The user gets a dead page, the config
is not saved, and they are never told which field was wrong.
This is not intermittent — the error path fails 100% of the time, and has since
before v1.4.1.
Why
saveHandler builds its own template FuncMap for the error re-render
(app/cmd/sdl3-clock/http.go, in the if errors != "" branch). That copy
registers checkbox, number, text, color, counter, byte, uint8 and
add — but not version and log, which indexHandler's copy does
register.
config.html uses both:
app/cmd/sdl3-clock/config.html.go:23 — <li>{{version}}</li>
app/cmd/sdl3-clock/config.html.go:722 — {{log}}
So t.Parse(configHTML) fails, and the next line is panic(err):
template: config.html:21: function "version" not defined
There is no recover() in the package, so net/http catches the panic
per-connection: the browser gets a reset/empty response rather than the
validation errors.
How to reproduce
On a running clock, open the web config and submit any value that fails
validation — e.g. a font file that does not exist, a malformed colour, a bad
OSC address, or an invalid timezone. The Save request dies instead of showing
the error list.
Reproduced without a device by parsing the real template with the exact
FuncMap that saveHandler registers:
func TestErrorPathTemplate(t *testing.T) {
tm := htmlTemplate.New("config.html")
tm.Funcs(htmlTemplate.FuncMap{
"checkbox": func(id, label string, v bool) htmlTemplate.HTML { return "" },
"number": func(id, label string, v int) htmlTemplate.HTML { return "" },
"text": func(id, label string, v string) htmlTemplate.HTML { return "" },
"color": func(id, label string, v string) htmlTemplate.HTML { return "" },
"counter": func(id, label string, v int) htmlTemplate.HTML { return "" },
"byte": func(id, label string, v int) htmlTemplate.HTML { return "" },
"uint8": func(id, label string, v uint8) htmlTemplate.HTML { return "" },
"add": func(a, b int) int { return a + b },
})
if _, err := tm.Parse(configHTML); err != nil {
t.Fatalf("saveHandler would panic here: %v", err)
}
}
Verified against 6d48e70 (master, before any of my changes), so this is not
introduced by anything in flight.
Suggested fix
Minimal: add version and log to the error-path FuncMap.
Better: the FuncMap is currently duplicated verbatim between indexHandler
and saveHandler, which is how the two drifted apart. Extracting it into one
configTemplateFuncs() used by both makes the class of bug impossible. Happy to
send a PR for either if useful.
A test that parses configHTML with the real func map would catch any future
drift at go test time — the current failure is invisible to the compiler,
go vet and staticcheck, because it only surfaces when the template is parsed
at runtime.
What
Every validation error in the web config panics
saveHandlerinstead ofre-rendering the page with the error list. The user gets a dead page, the config
is not saved, and they are never told which field was wrong.
This is not intermittent — the error path fails 100% of the time, and has since
before v1.4.1.
Why
saveHandlerbuilds its own templateFuncMapfor the error re-render(
app/cmd/sdl3-clock/http.go, in theif errors != ""branch). That copyregisters
checkbox,number,text,color,counter,byte,uint8andadd— but notversionandlog, whichindexHandler's copy doesregister.
config.htmluses both:app/cmd/sdl3-clock/config.html.go:23—<li>{{version}}</li>app/cmd/sdl3-clock/config.html.go:722—{{log}}So
t.Parse(configHTML)fails, and the next line ispanic(err):There is no
recover()in the package, sonet/httpcatches the panicper-connection: the browser gets a reset/empty response rather than the
validation errors.
How to reproduce
On a running clock, open the web config and submit any value that fails
validation — e.g. a font file that does not exist, a malformed colour, a bad
OSC address, or an invalid timezone. The Save request dies instead of showing
the error list.
Reproduced without a device by parsing the real template with the exact
FuncMapthatsaveHandlerregisters:Verified against
6d48e70(master, before any of my changes), so this is notintroduced by anything in flight.
Suggested fix
Minimal: add
versionandlogto the error-pathFuncMap.Better: the
FuncMapis currently duplicated verbatim betweenindexHandlerand
saveHandler, which is how the two drifted apart. Extracting it into oneconfigTemplateFuncs()used by both makes the class of bug impossible. Happy tosend a PR for either if useful.
A test that parses
configHTMLwith the real func map would catch any futuredrift at
go testtime — the current failure is invisible to the compiler,go vetandstaticcheck, because it only surfaces when the template is parsedat runtime.