Skip to content

[bug] Web config validation errors panic saveHandler instead of showing the error list #51

Description

@Jason-Vaughan

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions