fix: terminate the Request on non-finite floats and non-reflexive UI values

.agents/skills/jaws/SKILL.md

@@ -35,17 +35,25 @@ JaWS is an immediate-mode, server-driven UI framework, not an MVC framework.
 
 ## Hard framework constraints
 
-- Every JaWS `UI` value must be comparable.
+- Every JaWS `UI` value must be a non-nil interface, comparable at runtime, **and
+  equal to itself**, since it is used as a map key. A value that is a nil interface,
+  only statically comparable (an interface field holding a slice/map/func), or that
+  holds a `NaN` (so `v != v`) is rejected: the container widgets — the only place a raw
+  `UI` value is used as a map key — cancel the `Request` in all builds (cause matches
+  `tag.ErrNotUsableAsTag`), and `Request.NewElement` asserts runtime comparability in
+  debug builds. A typed nil (e.g. `(*Widget)(nil)`) is accepted as usable; its methods
+  must tolerate a nil receiver.
 - Every JaWS `UI` value is request-scoped. Once used by one Request, never use
   that value with another Request; construct fresh widgets per request. The
   widgets may still refer to shared, synchronized application state, binders,
   handlers, and tags.
 - Within its Request, a `UI` value normally backs one live `Element`. Reuse one
   value for multiple live Elements only when its concrete type documents that
   support and retains no state that can differ between those Elements.
-- `Container.JawsContains` must return hashable `UI` items, and the returned
-  slice must not be mutated after return. A UI value may occur more than once in
-  one returned slice only when its type supports multiple live Elements.
+- `Container.JawsContains` must return `UI` items that are comparable and equal to
+  themselves (see above); returning one that is not cancels the `Request`. The
+  returned slice must not be mutated after return. A UI value may occur more than
+  once in one returned slice only when its type supports multiple live Elements.
 - Treat the package documentation shown by
   `go doc github.com/linkdata/jaws/lib/ui` as the canonical standard-widget
   multiplicity summary, and consult each concrete type's docs for its conditions.

click.go

@@ -1,6 +1,7 @@
 package jaws
 
 import (
+	"errors"
 	"fmt"
 	"math"
 	"strconv"
@@ -113,10 +114,18 @@ func runFormatFloat(value float64) string {
 func runAtof(value string) (n float64, ok bool) {
 	var err error
 	n, err = strconv.ParseFloat(value, 64)
-	ok = err == nil && !math.IsInf(n, 0) && !math.IsNaN(n)
+	// A range error (e.g. "1e999") still yields a usable ±Inf; only a syntax error is
+	// a malformed frame. Accept the value either way so the event dispatch's finiteness
+	// check can terminate the Request on a non-finite coordinate.
+	ok = err == nil || errors.Is(err, strconv.ErrRange)
 	return
 }
 
+// finite reports whether f is neither NaN nor infinite.
+func finite(f float64) bool {
+	return !math.IsNaN(f) && !math.IsInf(f, 0)
+}
+
 func runAtoi(value string) (n int, ok bool) {
 	var err error
 	n, err = strconv.Atoi(value)

click_test.go

@@ -1,6 +1,7 @@
 package jaws
 
 import (
+	"math"
 	"strings"
 	"testing"
 )
@@ -63,31 +64,11 @@ func TestParseClickData(t *testing.T) {
 			in:     "bad 20 0 save",
 			wantOK: false,
 		},
-		{
-			name:   "nan x",
-			in:     "NaN 20 0 save",
-			wantOK: false,
-		},
-		{
-			name:   "infinite x",
-			in:     "+Inf 20 0 save",
-			wantOK: false,
-		},
 		{
 			name:   "invalid y",
 			in:     "10 bad 0 save",
 			wantOK: false,
 		},
-		{
-			name:   "nan y",
-			in:     "10 NaN 0 save",
-			wantOK: false,
-		},
-		{
-			name:   "infinite y",
-			in:     "10 -Inf 0 save",
-			wantOK: false,
-		},
 		{
 			name:   "invalid keystate",
 			in:     "10 20 bad save",
@@ -123,6 +104,53 @@ func TestParseClickData(t *testing.T) {
 	}
 }
 
+func TestParseClickDataAcceptsNonFinite(t *testing.T) {
+	// runAtof no longer rejects non-finite coordinates; the event dispatch terminates
+	// the Request instead (see TestCallEventHandlerTerminatesOnNonFiniteClick).
+	tests := []struct {
+		name  string
+		in    string
+		check func(Click) bool
+	}{
+		{"nan x", "NaN 20 0 save", func(c Click) bool { return math.IsNaN(c.X) && c.Y == 20 }},
+		{"infinite x", "+Inf 20 0 save", func(c Click) bool { return math.IsInf(c.X, 1) && c.Y == 20 }},
+		{"nan y", "10 NaN 0 save", func(c Click) bool { return c.X == 10 && math.IsNaN(c.Y) }},
+		{"infinite y", "10 -Inf 0 save", func(c Click) bool { return c.X == 10 && math.IsInf(c.Y, -1) }},
+		{"overflow x", "1e999 20 0 save", func(c Click) bool { return math.IsInf(c.X, 1) && c.Y == 20 }},
+		{"overflow y", "10 -1e999 0 save", func(c Click) bool { return c.X == 10 && math.IsInf(c.Y, -1) }},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			clk, _, ok := parseClickData(tt.in)
+			if !ok {
+				t.Fatalf("ok = false, want true")
+			}
+			if !tt.check(clk) {
+				t.Fatalf("unexpected click %+v", clk)
+			}
+		})
+	}
+}
+
+func TestFinite(t *testing.T) {
+	tests := []struct {
+		f    float64
+		want bool
+	}{
+		{0, true},
+		{-1.5, true},
+		{math.MaxFloat64, true},
+		{math.NaN(), false},
+		{math.Inf(1), false},
+		{math.Inf(-1), false},
+	}
+	for _, tt := range tests {
+		if got := finite(tt.f); got != tt.want {
+			t.Errorf("finite(%v) = %v, want %v", tt.f, got, tt.want)
+		}
+	}
+}
+
 func TestClickString(t *testing.T) {
 	got := (Click{Name: "x", X: 1.25, Y: 2.5, Shift: true, Control: true, Alt: true}).String()
 	want := "1.25 2.5 7 x"
@@ -142,6 +170,12 @@ func Fuzz_parseClickData(f *testing.F) {
 		if !ok {
 			return
 		}
+		// parseClickData accepts non-finite coordinates (the event dispatch terminates
+		// the Request on them); Click.String round-trips only finite values, since
+		// NaN != NaN defeats the equality check below.
+		if !finite(clk.X) || !finite(clk.Y) {
+			return
+		}
 
 		encoded := clk.String()
 		clk2, after2, ok := parseClickData(encoded)

contracts.go

@@ -11,13 +11,15 @@ import (
 type Container interface {
 	// JawsContains returns the current child [UI] values contained by elem.
 	//
-	// The returned [UI] values must be comparable, since they are used as map keys
-	// (see [UI] for the comparability requirement), and the slice contents must not
-	// be modified after returning it. Returning a child UI again from a later call
-	// lets the container reuse its existing live [Element]. The same UI may occur
-	// more than once in one returned slice only when its type documents support for
-	// backing multiple live Elements. A child UI must not be shared with a different
-	// [Request].
+	// The returned [UI] values must be comparable and equal to themselves, since they
+	// are used as map keys (see [UI] for the requirement); a child that is a nil
+	// interface, not comparable at runtime, or not equal to itself (such as one holding
+	// NaN) cancels the [Request] instead of being reconciled. A typed nil is usable.
+	// The slice contents must not be modified after returning it. Returning a usable
+	// child UI again from a later call lets the container reuse its existing live
+	// [Element]. The same UI may occur more than once in one returned slice only when
+	// its type documents support for backing multiple live Elements. A child UI must
+	// not be shared with a different [Request].
 	JawsContains(elem *Element) (contents []UI)
 }
 
@@ -67,14 +69,18 @@ type TemplateLookuper interface {
 // Application state referenced by UI values may be shared across Requests when
 // synchronized as required.
 //
-// In addition, all UI objects must be comparable so they can be used as map keys.
-// The compile-time type must be comparable; debug builds additionally perform a
-// runtime value-level check in [Request.NewElement] and panic on a value that is
-// statically comparable but not comparable at runtime (for example a comparable
-// struct holding a func in an interface field). Production builds rely on the
-// static check alone, so such a value is accepted and instead panics when first
-// used as a map key; callers must therefore ensure UI values are genuinely
-// comparable.
+// In addition, all UI objects must be comparable and equal to themselves, so they
+// can be used as map keys. The compile-time type must be comparable; the container
+// widgets additionally check each child value at runtime and cancel the Request, in
+// every build, when it is a nil interface, not comparable at runtime (for example a
+// comparable struct holding a func in an interface field), or not equal to itself (a
+// value holding NaN), with a cause matching
+// [github.com/linkdata/jaws/lib/tag.ErrNotUsableAsTag] under errors.Is. That is the
+// only place a raw UI value is used as a map key; outside a container
+// [Request.NewElement] asserts runtime comparability in debug builds. A typed nil (a
+// non-nil interface holding a nil pointer) is usable, and tolerating a nil receiver
+// is the concrete type's responsibility. Callers must ensure UI values are genuinely
+// comparable and reflexive.
 type UI interface {
 	Renderer
 	Updater

element.go

@@ -148,9 +148,13 @@ var debugCommentSanitizer = strings.NewReplacer("-->", "==>", "--!>", "==>")
 // JawsRender calls [Renderer.JawsRender] for this [Element].
 //
 // Do not call this yourself unless it is from within another JawsRender implementation.
+//
+// A nil [UI] interface renders as a no-op; this arises only from [Request.NewElement]
+// given a nil interface. A typed nil (a non-nil interface holding a nil pointer) still
+// dispatches to its [Renderer], which is responsible for tolerating a nil receiver.
 func (elem *Element) JawsRender(w io.Writer, params []any) (err error) {
-	if !elem.deleted.Load() {
-		if err = elem.UI().JawsRender(elem, w, params); err == nil {
+	if ui := elem.UI(); ui != nil && !elem.deleted.Load() {
+		if err = ui.JawsRender(elem, w, params); err == nil {
 			if elem.Jaws.Debug {
 				err = elem.renderDebug(w)
 			}
@@ -166,9 +170,12 @@ func (elem *Element) JawsRender(w io.Writer, params []any) (err error) {
 // JawsUpdate calls [Updater.JawsUpdate] for this [Element].
 //
 // Do not call this yourself unless it is from within another JawsUpdate implementation.
+//
+// A nil [UI] interface is a no-op; a typed nil dispatches to its [Updater] (see
+// [Element.JawsRender]).
 func (elem *Element) JawsUpdate() {
-	if !elem.deleted.Load() {
-		elem.UI().JawsUpdate(elem)
+	if ui := elem.UI(); ui != nil && !elem.deleted.Load() {
+		ui.JawsUpdate(elem)
 	}
 }
 

element_test.go

@@ -736,15 +736,6 @@ func TestElement_ApplyGetterDebugBranches(t *testing.T) {
 	if _, _, err := elem.ApplyGetter(agErr); err != tag.ErrNotComparable {
 		t.Fatalf("expected init err, got %v", err)
 	}
-
-	if deadlock.Debug {
-		defer func() {
-			if recover() == nil {
-				t.Fatal("expected panic for non-comparable UI in debug mode")
-			}
-		}()
-		rq.NewElement(testUnhashableUI{m: map[string]int{"x": 1}})
-	}
 }
 
 type testClickHandler struct{}
@@ -1230,3 +1221,10 @@ func TestElement_JawsInit(t *testing.T) {
 		t.Error(err)
 	}
 }
+
+// nonReflexiveUI is a comparable UI value that is not equal to itself when it holds
+// NaN; it backs TestNewErrUnusableUI.
+type nonReflexiveUI struct{ f float64 }
+
+func (nonReflexiveUI) JawsRender(*Element, io.Writer, []any) error { return nil }
+func (nonReflexiveUI) JawsUpdate(*Element)                         {}

errors.go

@@ -32,6 +32,17 @@ var ErrValueUnchanged = errors.New("value unchanged")
 // with [errors.Is]; the wrapped text identifies which channel overflowed.
 var ErrRequestOverloaded = errors.New("request overloaded")
 
+// ErrValueNotFinite indicates a [Request] was torn down because a NaN or infinite
+// float64 reached the UI.
+//
+// A non-finite value has no valid rendering or wire representation and, in the case
+// of NaN, is not even equal to itself, so it cannot be coerced safely. Rather than
+// silently blanking or dropping it, the Request that produced it is cancelled. The
+// cancellation cause reachable via [context.Cause] on [Request.Context] wraps this
+// sentinel, so it can be matched with [errors.Is]; the wrapped text identifies the
+// offending value.
+var ErrValueNotFinite = errors.New("float value is not finite")
+
 // ErrRequestAlreadyClaimed is returned when [Jaws.UseRequest] is called more than once for a [Request].
 var ErrRequestAlreadyClaimed = errors.New("request already claimed")
 

errunusableui.go

@@ -0,0 +1,55 @@
+package jaws
+
+import (
+	"reflect"
+
+	"github.com/linkdata/jaws/lib/tag"
+)
+
+// errUnusableUI reports that a value cannot be used as a [UI] value because it is a
+// nil interface, not comparable at runtime, or not equal to itself as a value holding
+// NaN is.
+//
+// It matches [tag.ErrNotUsableAsTag] and [tag.ErrNotComparable] with errors.Is, but
+// carries UI-specific text: the tag package's advice to implement JawsGetTag cannot
+// make a raw UI value usable as a map[UI] key, so it must not be surfaced here.
+type errUnusableUI struct {
+	t reflect.Type // nil when the offending value was a nil UI
+}
+
+func (e errUnusableUI) Error() (s string) {
+	s = "nil"
+	if e.t != nil {
+		s = e.t.String()
+	}
+	return s + " is not usable as a jaws.UI value: it must be non-nil, comparable, and equal to itself"
+}
+
+// Is reports whether target is a tag sentinel this error stands in for, so callers
+// can match it with errors.Is.
+func (errUnusableUI) Is(target error) bool {
+	return target == tag.ErrNotUsableAsTag || target == tag.ErrNotComparable
+}
+
+// NewErrUnusableUI returns a non-nil error when ui cannot be used as a [UI] value,
+// and nil when it can.
+//
+// A UI value is unusable when it is a nil interface, not comparable at runtime, or not
+// equal to itself as a value holding NaN is. Containers use it both as a map key and
+// to render children: a non-comparable value panics when hashed and a NaN-bearing one
+// never matches itself, while a nil interface is a legal map key but has no methods to
+// render. A typed nil — a non-nil interface holding a nil pointer whose type
+// implements [UI] — is comparable and equal to itself, so it is reported usable;
+// whether its [Renderer] tolerates a nil receiver is the concrete type's
+// responsibility.
+//
+// The returned error matches both [tag.ErrNotUsableAsTag] and [tag.ErrNotComparable]
+// under errors.Is. The container widgets use it to terminate a Request handed such a
+// child; a nil interface passed directly to [Request.NewElement] is instead tolerated
+// as a no-op Element, so this reports it unusable only for the container's benefit.
+func NewErrUnusableUI(ui UI) error {
+	if ui == nil || tag.NewErrNotUsableAsTag(ui) != nil {
+		return errUnusableUI{t: reflect.TypeOf(ui)}
+	}
+	return nil
+}