From 70197e2261fd3d0707effda24b21b492336d239b Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Mon, 17 Aug 2026 11:37:55 +0200 Subject: [PATCH 1/2] fix(ui): support promoted JSON fields in JsVar --- .agents/skills/jaws/SKILL.md | 8 ++- SECURITY.md | 19 +++--- go.mod | 2 +- go.sum | 4 +- lib/ui/AI.md | 7 ++- lib/ui/jsvar.go | 17 ++--- lib/ui/jsvar_check_test.go | 119 +++++++++++++++++++++++++++++++++++ 7 files changed, 155 insertions(+), 21 deletions(-) diff --git a/.agents/skills/jaws/SKILL.md b/.agents/skills/jaws/SKILL.md index c728bcf9..ac69f8fc 100644 --- a/.agents/skills/jaws/SKILL.md +++ b/.agents/skills/jaws/SKILL.md @@ -316,8 +316,12 @@ The handler candidate is asked via `JawsClick` / `JawsContextMenu` / `JawsInput` - Generic jq paths require JavaScript array-index names at Go arrays and slices: `"0"` or a nonzero decimal without a leading zero, at most `4294967294` and representable as Go `int`. Other components produce an error matching - `jq.ErrPathNotFound` at an array or slice. String-keyed map entries and JSON - field names are exact; empty components are ignored. + `jq.ErrPathNotFound` at an array or slice. String-keyed map entries are exact. + Struct path components exactly match names selected by `encoding/json`'s + default field-selection rules, including JSON tag names and unambiguous + promoted fields. Generic writes do not allocate nil pointers; a path or + map-to-struct key that would traverse one fails with `jq.ErrPathNotFound`. + Empty components are ignored. - Browser JSON numbers use JavaScript `Number` values. Integers outside `-9007199254740991` through `9007199254740991` may round, and a browser write may commit the rounded value to Go. diff --git a/SECURITY.md b/SECURITY.md index db00e797..5cc617ce 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -264,11 +264,12 @@ data. The selected Go setter (`jq.Set`, `jq.SetChecked`, or an application `PathSetter`) independently controls accepted server-side paths. **Trust boundary (application responsibility):** the generic JSON path will set -*any* exported field matched by its `json` tag, or by its Go name when the tag -has no explicit name, and will append to a slice one element per `Set` message -(`json:"-"` fields remain unwritable). The 32 KiB WebSocket read limit bounds one -message, not the accumulated server state, and there is (see I6) no per-message -rate limit. With +*any* exported field selected by `encoding/json`'s default field rules, including +unambiguous promoted fields. Names come from an explicit `json` tag or the Go +field name when the tag has no explicit name; `json:"-"` fields remain +unwritable. The setter will also append to a slice one element per `Set` message. +The 32 KiB WebSocket read limit bounds one message, not the accumulated server +state, and there is (see I6) no per-message rate limit. With a nil `JsVar.ClientCheck`, a type-correct generic write has no additional state-size policy. @@ -277,8 +278,12 @@ value and the browser-supplied jq path before a generic browser write commits. The path is passed through unchanged. Empty components are ignored, so `.value.` aliases `value`, and both `""` and `"."` address the root. Generic array and slice writes require canonical JavaScript array-index names -representable as Go `int`; string-keyed map entries and JSON field names are -exact. Invalid array or slice paths are rejected before `ClientCheck` runs. +representable as Go `int`, and string-keyed map entries are exact. Struct path +components exactly match names selected by `encoding/json`'s default +field-selection rules, including JSON tag names and unambiguous promoted fields. +Generic writes do not allocate nil pointers; a path or map-to-struct key that +would traverse one fails with `jq.ErrPathNotFound`. Invalid array or slice paths +are rejected before `ClientCheck` runs. Treat the raw path as an inspection hint, not an authorization key; use `ui.PathSetter` to allow-list paths. A returned error rolls the write back without broadcasting it. If the error matches diff --git a/go.mod b/go.mod index cb474ff1..de32210d 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.25.0 require ( github.com/coder/websocket v1.8.15 github.com/linkdata/deadlock v0.5.5 - github.com/linkdata/jq v0.3.0 + github.com/linkdata/jq v0.4.0 github.com/linkdata/secureheaders v1.5.0 github.com/linkdata/staticserve v1.1.8 golang.org/x/net v0.58.0 diff --git a/go.sum b/go.sum index a9ee4b55..e75f8712 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/coder/websocket v1.8.15 h1:6B2JPeOGlpff2Uz6vOEH1Vzpi0iUz20A+lPVhPHtNU github.com/coder/websocket v1.8.15/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg= github.com/linkdata/deadlock v0.5.5 h1:d6O+rzEqasSfamGDA8u7bjtaq7hOX8Ha4Zn36Wxrkvo= github.com/linkdata/deadlock v0.5.5/go.mod h1:tXb28stzAD3trzEEK0UJWC+rZKuobCoPktPYzebb1u0= -github.com/linkdata/jq v0.3.0 h1:GKDcydShVH63mLUvZJwoEvSFO9HS7M2iYxp7HOEsEfw= -github.com/linkdata/jq v0.3.0/go.mod h1:b76MMuWybyXrVEKSHJkv+r5IEattnghPf1bNRJ5PNlE= +github.com/linkdata/jq v0.4.0 h1:8YkqVsiAmbOJQ8benmEx22zs21L91sQWTjQryQiMurY= +github.com/linkdata/jq v0.4.0/go.mod h1:b76MMuWybyXrVEKSHJkv+r5IEattnghPf1bNRJ5PNlE= github.com/linkdata/secureheaders v1.5.0 h1:4kI290hrVUYqluxF8pQfRYpbZuorSd9+8GlMgrLcrK4= github.com/linkdata/secureheaders v1.5.0/go.mod h1:50TiKmPaWki8gIZXmBxrLUinyJst12YuN3EbsXZMO+4= github.com/linkdata/staticserve v1.1.8 h1:qpm0dAaAsik1YrETSiMoS7ciBL9qPVNUFcfdio/ol84= diff --git a/lib/ui/AI.md b/lib/ui/AI.md index 42096eb5..009ed07a 100644 --- a/lib/ui/AI.md +++ b/lib/ui/AI.md @@ -320,8 +320,11 @@ value before an actual generic write commits. It receives the browser-supplied jq path unchanged. Empty components are ignored, so `.value.` aliases `value`; treat the raw path as an inspection hint, not an authorization key. Array and slice components must be canonical JavaScript array-index names representable -as Go `int`; string-keyed map entries and JSON field names are exact. Use -`PathSetter` to allow-list paths and operations. +as Go `int`, and string-keyed map entries are exact. Struct path components +exactly match names selected by `encoding/json`'s default field-selection rules, +including JSON tag names and unambiguous promoted fields. Generic writes do not +allocate nil pointers; a path or map-to-struct key that would traverse one fails +with `jq.ErrPathNotFound`. Use `PathSetter` to allow-list paths and operations. A check runs while the application locker is held. It must inspect only: do not mutate or retain tentative state, re-enter the JsVar, call a path setter, acquire diff --git a/lib/ui/jsvar.go b/lib/ui/jsvar.go index 0f7a10e3..fabe0ad9 100644 --- a/lib/ui/jsvar.go +++ b/lib/ui/jsvar.go @@ -204,8 +204,13 @@ func JSONSizeCheck[T any](maxBytes int) (check JsVarCheck[T]) { // indices representable as int: "0" or ASCII decimal digits without a leading // zero, at most 4294967294. A component that violates these rules produces an // error matching [github.com/linkdata/jq.ErrPathNotFound] when traversal reaches -// an array or slice. String-keyed map entries and JSON field names are matched -// exactly. Empty components are ignored, and both "" and "." address the root. +// an array or slice. String-keyed map entries are matched exactly. Struct path +// components exactly match names selected by the default field-selection rules +// of [encoding/json], including JSON tag names and unambiguous promoted fields. +// Generic writes do not allocate nil pointers; a path or map-to-struct key that +// would traverse one produces an error matching +// [github.com/linkdata/jq.ErrPathNotFound]. Empty components are ignored, and +// both "" and "." address the root. // // While the WebSocket is open, jawsVar sends one complete message per matching // live binding, subject to [jaws.Request.ServeHTTP]'s inbound limit. @@ -258,11 +263,9 @@ func JSONSizeCheck[T any](maxBytes int) (check JsVarCheck[T]) { // // SECURITY: a JsVar is client-writable. Incoming browser "set" messages are // applied by path to the bound value. If the bound value implements [PathSetter], -// its JawsSetPath validates and applies the change. Otherwise the generic path -// setter ([github.com/linkdata/jq.Set]) can set any exported field — matched by its -// json tag, or by the Go field name when its json tag has no explicit name (a -// json:"-" tag is never writable) — and append to slices one element per -// message. +// its JawsSetPath validates and applies the change. Otherwise +// [github.com/linkdata/jq.Set] can write any exported field addressable by the +// generic path rules above and append one element per message to a slice. // // There is no default cumulative size bound. Set [JsVar.ClientCheck] before first // use to validate each tentative generic browser update. [JSONSizeCheck] provides diff --git a/lib/ui/jsvar_check_test.go b/lib/ui/jsvar_check_test.go index 886378df..2556af50 100644 --- a/lib/ui/jsvar_check_test.go +++ b/lib/ui/jsvar_check_test.go @@ -43,6 +43,18 @@ func (state *jsVarArrayIndexState) JawsPathSet(_ *jaws.Element, jsPath string, _ state.lastPath = jsPath } +type jsVarPromotedFields struct { + Value int `json:"value"` +} + +type jsVarPromotedState struct { + jsVarPromotedFields +} + +type jsVarNilPromotedState struct { + *jsVarPromotedFields +} + type jsVarConcurrentData struct { Left string `json:"left"` Right string `json:"right"` @@ -368,6 +380,113 @@ func TestJsVarGenericPathsUseJavaScriptArrayIndices(t *testing.T) { } } +func TestJsVarPromotedJSONFieldPaths(t *testing.T) { + state := jsVarPromotedState{jsVarPromotedFields{Value: 1}} + data, err := json.Marshal(state) + if err != nil { + t.Fatal(err) + } + if got, want := string(data), `{"value":1}`; got != want { + t.Fatalf("json.Marshal = %s, want %s", got, want) + } + + jsvar := NewJsVar(new(sync.Mutex), &state) + var checkPaths []string + var checkedValues []int + jsvar.ClientCheck = func(value *jsVarPromotedState, jsPath string) error { + checkPaths = append(checkPaths, jsPath) + checkedValues = append(checkedValues, value.Value) + return nil + } + if got := jsvar.JawsGetPath(nil, "value"); got != 1 { + t.Fatalf("JawsGetPath = %#v, want 1", got) + } + if err = jsvar.JawsSetPath(nil, "value", 2); err != nil { + t.Fatal(err) + } + if state.Value != 2 { + t.Fatalf("programmatic path write left Value = %d, want 2", state.Value) + } + if len(checkPaths) != 0 { + t.Fatalf("programmatic operations made %d ClientCheck calls, want 0", len(checkPaths)) + } + + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + t.Cleanup(jw.Close) + go jw.Serve() + + tr := jawstest.NewTestRequest(jw, nil) + if tr == nil { + t.Fatal("nil test request") + } + cleanupJsVarTestRequest(t, tr) + <-tr.ReadyCh + + rw := RequestWriter{Request: tr.Request, Writer: io.Discard} + if err = rw.JsVar("promoted", jsvar); err != nil { + t.Fatal(err) + } + elements := tr.GetElements(&state) + if len(elements) != 1 { + t.Fatalf("rendered elements = %d, want 1", len(elements)) + } + elem := elements[0] + + if err = clientSetFrame(t, jsvar, elem, "value", 3); err != nil { + t.Fatal(err) + } + if state.Value != 3 { + t.Fatalf("browser path write left Value = %d, want 3", state.Value) + } + msg := awaitJsVarOperation(t, "promoted path broadcast", tr.OutCh) + if msg.What != what.Set || msg.Data != "value=3" { + t.Fatalf("promoted path broadcast = %#v, want value=3 Set", msg) + } + + if err = clientSetFrame(t, jsvar, elem, "", map[string]any{"value": 4}); err != nil { + t.Fatal(err) + } + if state.Value != 4 { + t.Fatalf("browser whole-object write left Value = %d, want 4", state.Value) + } + msg = awaitJsVarOperation(t, "promoted whole-object broadcast", tr.OutCh) + if msg.What != what.Set || msg.Data != `={"value":4}` { + t.Fatalf(`promoted whole-object broadcast = %#v, want ={"value":4} Set`, msg) + } + if len(checkPaths) != 2 || checkPaths[0] != "value" || checkPaths[1] != "" { + t.Fatalf("ClientCheck paths = %q, want [value, root]", checkPaths) + } + if len(checkedValues) != 2 || checkedValues[0] != 3 || checkedValues[1] != 4 { + t.Fatalf("ClientCheck values = %v, want [3 4]", checkedValues) + } +} + +func TestJsVarPromotedJSONFieldNilPointer(t *testing.T) { + state := jsVarNilPromotedState{} + jsvar := NewJsVar(new(sync.Mutex), &state) + checkCalls := 0 + jsvar.ClientCheck = func(*jsVarNilPromotedState, string) error { + checkCalls++ + return nil + } + + if got := jsvar.JawsGetPath(nil, "value"); got != nil { + t.Fatalf("JawsGetPath = %#v, want nil", got) + } + if err := jsvar.JawsSetPath(nil, "value", 1); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("JawsSetPath error = %v, want ErrPathNotFound", err) + } + if err := jsvar.JawsInput(nil, `={"value":1}`); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("browser whole-object write error = %v, want ErrPathNotFound", err) + } + if state.jsVarPromotedFields != nil || checkCalls != 0 { + t.Fatalf("failed writes left state %#v and made %d checks", state, checkCalls) + } +} + func TestJsVarClientCheckRejectRestoresZeroValueAppendAliases(t *testing.T) { backing := []string{"sentinel"} state := jsVarSliceData{Items: backing[:0]} From 3c9f8a0cd1fc690678e4764aa6d871a90445f4b4 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Mon, 17 Aug 2026 15:08:47 +0200 Subject: [PATCH 2/2] fix(ui): align JsVar with jq v0.5 --- .agents/skills/jaws/SKILL.md | 24 ++- SECURITY.md | 44 +++-- go.mod | 2 +- go.sum | 4 +- lib/ui/AI.md | 26 ++- lib/ui/jsvar.go | 32 +++- lib/ui/jsvar_check_test.go | 354 +++++++++++++++++++++++++++-------- 7 files changed, 371 insertions(+), 115 deletions(-) diff --git a/.agents/skills/jaws/SKILL.md b/.agents/skills/jaws/SKILL.md index ac69f8fc..45b48906 100644 --- a/.agents/skills/jaws/SKILL.md +++ b/.agents/skills/jaws/SKILL.md @@ -316,12 +316,24 @@ The handler candidate is asked via `JawsClick` / `JawsContextMenu` / `JawsInput` - Generic jq paths require JavaScript array-index names at Go arrays and slices: `"0"` or a nonzero decimal without a leading zero, at most `4294967294` and representable as Go `int`. Other components produce an error matching - `jq.ErrPathNotFound` at an array or slice. String-keyed map entries are exact. - Struct path components exactly match names selected by `encoding/json`'s - default field-selection rules, including JSON tag names and unambiguous - promoted fields. Generic writes do not allocate nil pointers; a path or - map-to-struct key that would traverse one fails with `jq.ErrPathNotFound`. - Empty components are ignored. + `jq.ErrPathNotFound` at an array or slice. String-keyed map entries are exact; + empty components are ignored. +- Struct path components and map-to-struct keys follow `encoding/json`'s default + field-selection rules. An exact `json:"-"` tag excludes an otherwise selected + exported field. For a non-promoting field, a valid nonempty tag name is used + verbatim, while an absent, empty, or invalid name falls back to the Go field + name; `json:"-,"` names the field `-`. Ambiguous fields are absent. +- An anonymous struct without a valid explicit JSON name contributes promoted + fields directly without a Go-type-name component: use `value`, not + `Inner.value`, or add an explicit tag for a nested path. Promotion reaches + exported fields through unexported embedded structs. An explicitly named + unexported anonymous struct is not itself a readable or writable endpoint or + writable map-to-struct key, but longer paths can reach its exported fields. A + `ClientCheck` using `jq.Get` must inspect a longer path or the tentative Go + value. +- Reads and generic writes that traverse a nil pointer fail with + `jq.ErrPathNotFound`; generic writes do not allocate it. `JsVar.JawsGetPath` + returns nil on lookup failure, indistinguishable from a resolved nil value. - Browser JSON numbers use JavaScript `Number` values. Integers outside `-9007199254740991` through `9007199254740991` may round, and a browser write may commit the rounded value to Go. diff --git a/SECURITY.md b/SECURITY.md index 5cc617ce..ff6daeb3 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -263,27 +263,41 @@ access. It does not scan values for that name; own `"__proto__"` members remain data. The selected Go setter (`jq.Set`, `jq.SetChecked`, or an application `PathSetter`) independently controls accepted server-side paths. -**Trust boundary (application responsibility):** the generic JSON path will set -*any* exported field selected by `encoding/json`'s default field rules, including -unambiguous promoted fields. Names come from an explicit `json` tag or the Go -field name when the tag has no explicit name; `json:"-"` fields remain -unwritable. The setter will also append to a slice one element per `Set` message. -The 32 KiB WebSocket read limit bounds one message, not the accumulated server -state, and there is (see I6) no per-message rate limit. With -a nil `JsVar.ClientCheck`, a type-correct generic write has no additional -state-size policy. +**Trust boundary (application responsibility):** the generic JSON path can set +every exported field selected by `encoding/json`'s default field rules and can +append one slice element per `Set` message. Struct path components and +map-to-struct keys use a valid nonempty JSON tag name verbatim for non-promoting +fields; an absent, empty, or invalid name falls back to the Go field name. An +exact `json:"-"` tag excludes an otherwise selected exported field, while +`json:"-,"` names that field `-`. Ambiguous fields are absent from the path +namespace. + +An anonymous struct without a valid explicit JSON name contributes its promoted +fields directly without a Go-type-name component: use `value`, not +`Inner.value`, or tag the anonymous field to create a nested path. Promotion +reaches exported fields through unexported embedded structs. An explicitly +named unexported anonymous struct is not itself a readable or writable endpoint +or writable map-to-struct key, but longer paths can reach its exported fields. +Reads and generic writes that traverse a nil pointer fail with +`jq.ErrPathNotFound`; writes do not allocate it, and `JsVar.JawsGetPath` returns +nil, which cannot be distinguished from a successfully resolved nil value. + +The 32 KiB WebSocket read limit bounds one message, not accumulated server state, +and there is (see I6) no per-message rate limit. With a nil +`JsVar.ClientCheck`, a type-correct generic write has no additional state-size +policy. Applications can set `JsVar.ClientCheck` to inspect the complete tentative value and the browser-supplied jq path before a generic browser write commits. The path is passed through unchanged. Empty components are ignored, so `.value.` aliases `value`, and both `""` and `"."` address the root. Generic array and slice writes require canonical JavaScript array-index names -representable as Go `int`, and string-keyed map entries are exact. Struct path -components exactly match names selected by `encoding/json`'s default -field-selection rules, including JSON tag names and unambiguous promoted fields. -Generic writes do not allocate nil pointers; a path or map-to-struct key that -would traverse one fails with `jq.ErrPathNotFound`. Invalid array or slice paths -are rejected before `ClientCheck` runs. +representable as Go `int`, and string-keyed map entries are exact. Struct paths +and map-to-struct keys follow the selection rules above. Invalid writes do not +reach `ClientCheck`; ignored map-to-struct keys do not by themselves cause a +check. A check using `jq.Get` cannot inspect an explicitly named unexported +anonymous struct at its own endpoint after a tentative write beneath it; inspect +a longer exported-field path or the Go value directly. Treat the raw path as an inspection hint, not an authorization key; use `ui.PathSetter` to allow-list paths. A returned error rolls the write back without broadcasting it. If the error matches diff --git a/go.mod b/go.mod index de32210d..bf98c3b9 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.25.0 require ( github.com/coder/websocket v1.8.15 github.com/linkdata/deadlock v0.5.5 - github.com/linkdata/jq v0.4.0 + github.com/linkdata/jq v0.5.0 github.com/linkdata/secureheaders v1.5.0 github.com/linkdata/staticserve v1.1.8 golang.org/x/net v0.58.0 diff --git a/go.sum b/go.sum index e75f8712..f3991a3d 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/coder/websocket v1.8.15 h1:6B2JPeOGlpff2Uz6vOEH1Vzpi0iUz20A+lPVhPHtNU github.com/coder/websocket v1.8.15/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg= github.com/linkdata/deadlock v0.5.5 h1:d6O+rzEqasSfamGDA8u7bjtaq7hOX8Ha4Zn36Wxrkvo= github.com/linkdata/deadlock v0.5.5/go.mod h1:tXb28stzAD3trzEEK0UJWC+rZKuobCoPktPYzebb1u0= -github.com/linkdata/jq v0.4.0 h1:8YkqVsiAmbOJQ8benmEx22zs21L91sQWTjQryQiMurY= -github.com/linkdata/jq v0.4.0/go.mod h1:b76MMuWybyXrVEKSHJkv+r5IEattnghPf1bNRJ5PNlE= +github.com/linkdata/jq v0.5.0 h1:JtTlQKBsH4VlKde2md0iNp69ebXedBTY6C9aNXo0LeY= +github.com/linkdata/jq v0.5.0/go.mod h1:b76MMuWybyXrVEKSHJkv+r5IEattnghPf1bNRJ5PNlE= github.com/linkdata/secureheaders v1.5.0 h1:4kI290hrVUYqluxF8pQfRYpbZuorSd9+8GlMgrLcrK4= github.com/linkdata/secureheaders v1.5.0/go.mod h1:50TiKmPaWki8gIZXmBxrLUinyJst12YuN3EbsXZMO+4= github.com/linkdata/staticserve v1.1.8 h1:qpm0dAaAsik1YrETSiMoS7ciBL9qPVNUFcfdio/ol84= diff --git a/lib/ui/AI.md b/lib/ui/AI.md index 009ed07a..6aec62a1 100644 --- a/lib/ui/AI.md +++ b/lib/ui/AI.md @@ -320,18 +320,32 @@ value before an actual generic write commits. It receives the browser-supplied jq path unchanged. Empty components are ignored, so `.value.` aliases `value`; treat the raw path as an inspection hint, not an authorization key. Array and slice components must be canonical JavaScript array-index names representable -as Go `int`, and string-keyed map entries are exact. Struct path components -exactly match names selected by `encoding/json`'s default field-selection rules, -including JSON tag names and unambiguous promoted fields. Generic writes do not -allocate nil pointers; a path or map-to-struct key that would traverse one fails -with `jq.ErrPathNotFound`. Use `PathSetter` to allow-list paths and operations. +as Go `int`, and string-keyed map entries are exact. Struct path components and +map-to-struct keys follow `encoding/json`'s default field-selection rules. An +exact `json:"-"` tag excludes an otherwise selected exported field. For a +non-promoting field, a valid nonempty tag name is used verbatim, while an absent, +empty, or invalid name falls back to the Go field name; `json:"-,"` therefore +names the field `-`. Ambiguous fields are absent from the path namespace. + +An anonymous struct without a valid explicit JSON name contributes its promoted +fields directly without a Go-type-name component: use `value`, not +`Inner.value`, or tag the anonymous field to create a nested path. Promotion +reaches exported fields through unexported embedded structs. An explicitly +named unexported anonymous struct is not itself a readable or writable endpoint +or writable map-to-struct key, but longer paths can reach its exported fields. +Reads and generic writes that traverse a nil pointer fail with +`jq.ErrPathNotFound`, and generic writes do not allocate it. `JawsGetPath` +returns nil on lookup failure, so nil does not distinguish failure from a +resolved nil value. Use `PathSetter` to allow-list paths and operations. A check runs while the application locker is held. It must inspect only: do not mutate or retain tentative state, re-enter the JsVar, call a path setter, acquire the same locker, or return/wrap `jaws.ErrEventUnhandled`. A nil result commits; an error rolls back without a broadcast. The browser already changed locally, so an ordinary rejection can leave it divergent until application -resynchronization. +resynchronization. A check using `jq.Get` cannot inspect an explicitly named +unexported anonymous struct at its own endpoint after a tentative write beneath +it; inspect a longer exported-field path or the Go value directly. The check sees tentative Go state, not necessarily the decoded value later used for a peer broadcast. jq conversions and ignored map-to-struct fields can make diff --git a/lib/ui/jsvar.go b/lib/ui/jsvar.go index fabe0ad9..52f08cae 100644 --- a/lib/ui/jsvar.go +++ b/lib/ui/jsvar.go @@ -205,12 +205,25 @@ func JSONSizeCheck[T any](maxBytes int) (check JsVarCheck[T]) { // zero, at most 4294967294. A component that violates these rules produces an // error matching [github.com/linkdata/jq.ErrPathNotFound] when traversal reaches // an array or slice. String-keyed map entries are matched exactly. Struct path -// components exactly match names selected by the default field-selection rules -// of [encoding/json], including JSON tag names and unambiguous promoted fields. -// Generic writes do not allocate nil pointers; a path or map-to-struct key that -// would traverse one produces an error matching -// [github.com/linkdata/jq.ErrPathNotFound]. Empty components are ignored, and -// both "" and "." address the root. +// components and map-to-struct keys follow the default field-selection rules of +// [encoding/json]. An exact json:"-" tag excludes an otherwise selected exported +// field. For a non-promoting field, a valid nonempty JSON tag name is used +// verbatim, while an absent, empty, or invalid name falls back to the Go field +// name; json:"-," therefore names the field "-". Ambiguous fields are absent +// from the path namespace. +// +// An anonymous struct without a valid explicit JSON name contributes its +// promoted fields directly and does not add its Go type name as a component: +// use "value", not "Inner.value", or give the anonymous field an explicit tag +// to create a nested path. Promotion reaches exported fields through unexported +// embedded structs. An explicitly named unexported anonymous struct is not +// itself a readable or writable endpoint or writable map-to-struct key, but +// longer paths can reach its exported fields. +// +// Reads and generic writes that traverse a nil pointer produce an error matching +// [github.com/linkdata/jq.ErrPathNotFound], and generic writes do not allocate +// the pointer. Empty components are ignored, and both "" and "." address the +// root. // // While the WebSocket is open, jawsVar sends one complete message per matching // live binding, subject to [jaws.Request.ServeHTTP]'s inbound limit. @@ -276,6 +289,9 @@ func JSONSizeCheck[T any](maxBytes int) (check JsVarCheck[T]) { // A ClientCheck does not run for rendering, programmatic writes, invalid or // unchanged writes, or values implementing PathSetter. It is an acceptance gate, // not a monitor that proves the current value always satisfies an invariant. +// A check that uses [github.com/linkdata/jq.Get] cannot inspect an explicitly +// named unexported anonymous struct at its own endpoint; it must inspect a +// longer exported-field path or the tentative Go value directly. // // A size check does not prevent a client from setting individual exported fields. // When only some fields or paths should be client-writable, implement [PathSetter] @@ -299,7 +315,9 @@ type JsVar[T any] struct { // JawsGetPath returns the value at jsPath. // // A path containing only empty components returns the same logical root value -// as [JsVar.JawsGet]. Lookup errors are logged on elem when possible. +// as [JsVar.JawsGet]. Lookup errors return nil and are logged on elem when +// possible. A nil result therefore does not distinguish a lookup failure from a +// successfully resolved nil value. func (jsvar *JsVar[T]) JawsGetPath(elem *jaws.Element, jsPath string) (value any) { if strings.Trim(jsPath, ".") == "" { return jsvar.JawsGet(elem) diff --git a/lib/ui/jsvar_check_test.go b/lib/ui/jsvar_check_test.go index 2556af50..0bf01861 100644 --- a/lib/ui/jsvar_check_test.go +++ b/lib/ui/jsvar_check_test.go @@ -15,6 +15,7 @@ import ( "github.com/linkdata/jaws" "github.com/linkdata/jaws/jawstest" "github.com/linkdata/jaws/lib/what" + "github.com/linkdata/jaws/lib/wire" "github.com/linkdata/jq" ) @@ -43,18 +44,6 @@ func (state *jsVarArrayIndexState) JawsPathSet(_ *jaws.Element, jsPath string, _ state.lastPath = jsPath } -type jsVarPromotedFields struct { - Value int `json:"value"` -} - -type jsVarPromotedState struct { - jsVarPromotedFields -} - -type jsVarNilPromotedState struct { - *jsVarPromotedFields -} - type jsVarConcurrentData struct { Left string `json:"left"` Right string `json:"right"` @@ -134,12 +123,29 @@ func clientSetFrame(t *testing.T, jsvar IsJsVar, elem *jaws.Element, jsPath stri return jaws.CallEventHandlers(jsvar, elem, what.Set, jsPath+"="+string(data)) } -func cleanupJsVarTestRequest(t *testing.T, tr *jawstest.TestRequest) { +func newJsVarTestJaws(t *testing.T) (jw *jaws.Jaws) { + t.Helper() + var err error + if jw, err = jaws.New(); err != nil { + t.Fatal(err) + } + t.Cleanup(jw.Close) + go jw.Serve() + return +} + +func newJsVarTestRequest(t *testing.T, jw *jaws.Jaws) (tr *jawstest.TestRequest) { t.Helper() + tr = jawstest.NewTestRequest(jw, nil) + if tr == nil { + t.Fatal("nil test request") + } t.Cleanup(func() { tr.Close() <-tr.DoneCh }) + <-tr.ReadyCh + return } func TestJSONSizeCheck(t *testing.T) { @@ -175,19 +181,8 @@ func TestJSONSizeCheck(t *testing.T) { func TestJsVarClientCheckContract(t *testing.T) { type clientCheck func(*jsVarCheckedState, string) error - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - t.Cleanup(jw.Close) - go jw.Serve() - - tr := jawstest.NewTestRequest(jw, nil) - if tr == nil { - t.Fatal("nil test request") - } - cleanupJsVarTestRequest(t, tr) - <-tr.ReadyCh + tr := newJsVarTestRequest(t, newJsVarTestJaws(t)) + var err error var mu sync.Mutex state := jsVarCheckedState{Value: "initial"} @@ -284,19 +279,8 @@ func TestJsVarClientCheckContract(t *testing.T) { } func TestJsVarGenericPathsUseJavaScriptArrayIndices(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - t.Cleanup(jw.Close) - go jw.Serve() - - tr := jawstest.NewTestRequest(jw, nil) - if tr == nil { - t.Fatal("nil test request") - } - cleanupJsVarTestRequest(t, tr) - <-tr.ReadyCh + tr := newJsVarTestRequest(t, newJsVarTestJaws(t)) + var err error state := jsVarArrayIndexState{ Items: []int{1, 2}, @@ -381,19 +365,18 @@ func TestJsVarGenericPathsUseJavaScriptArrayIndices(t *testing.T) { } func TestJsVarPromotedJSONFieldPaths(t *testing.T) { - state := jsVarPromotedState{jsVarPromotedFields{Value: 1}} - data, err := json.Marshal(state) - if err != nil { - t.Fatal(err) + type Inner struct { + Value int `json:"value"` } - if got, want := string(data), `{"value":1}`; got != want { - t.Fatalf("json.Marshal = %s, want %s", got, want) + type stateType struct { + Inner } + state := stateType{Inner{Value: 1}} jsvar := NewJsVar(new(sync.Mutex), &state) var checkPaths []string var checkedValues []int - jsvar.ClientCheck = func(value *jsVarPromotedState, jsPath string) error { + jsvar.ClientCheck = func(value *stateType, jsPath string) error { checkPaths = append(checkPaths, jsPath) checkedValues = append(checkedValues, value.Value) return nil @@ -401,6 +384,13 @@ func TestJsVarPromotedJSONFieldPaths(t *testing.T) { if got := jsvar.JawsGetPath(nil, "value"); got != 1 { t.Fatalf("JawsGetPath = %#v, want 1", got) } + if got := jsvar.JawsGetPath(nil, "Inner.value"); got != nil { + t.Fatalf("JawsGetPath through Go type name = %#v, want nil", got) + } + if err := jsvar.JawsSetPath(nil, "Inner.value", 2); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("JawsSetPath through Go type name error = %v, want ErrPathNotFound", err) + } + var err error if err = jsvar.JawsSetPath(nil, "value", 2); err != nil { t.Fatal(err) } @@ -411,19 +401,7 @@ func TestJsVarPromotedJSONFieldPaths(t *testing.T) { t.Fatalf("programmatic operations made %d ClientCheck calls, want 0", len(checkPaths)) } - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - t.Cleanup(jw.Close) - go jw.Serve() - - tr := jawstest.NewTestRequest(jw, nil) - if tr == nil { - t.Fatal("nil test request") - } - cleanupJsVarTestRequest(t, tr) - <-tr.ReadyCh + tr := newJsVarTestRequest(t, newJsVarTestJaws(t)) rw := RequestWriter{Request: tr.Request, Writer: io.Discard} if err = rw.JsVar("promoted", jsvar); err != nil { @@ -464,11 +442,176 @@ func TestJsVarPromotedJSONFieldPaths(t *testing.T) { } } +func TestJsVarPromotedJSONFieldPointer(t *testing.T) { + type embedded struct { + Value int `json:"value"` + } + type stateType struct { + *embedded + } + + pointee := &embedded{Value: 1} + state := stateType{embedded: pointee} + jsvar := NewJsVar(new(sync.Mutex), &state) + errRejected := errors.New("rejected") + accept := false + checkCalls := 0 + jsvar.ClientCheck = func(value *stateType, jsPath string) error { + checkCalls++ + if value.embedded != pointee || value.Value != 2 || jsPath != "" { + t.Errorf("ClientCheck saw pointer/value/path = %p/%d/%q, want %p/2/root", value.embedded, value.Value, jsPath, pointee) + } + if !accept { + return errRejected + } + return nil + } + + if err := jsvar.JawsInput(nil, `={"value":2}`); err != errRejected { + t.Fatalf("rejected pointer write error = %v, want exact rejection", err) + } + if state.embedded != pointee || state.Value != 1 { + t.Fatalf("rollback left pointer/value = %p/%d, want %p/1", state.embedded, state.Value, pointee) + } + + accept = true + if err := jsvar.JawsInput(nil, `={"value":2}`); err != nil { + t.Fatal(err) + } + if state.embedded != pointee || state.Value != 2 || checkCalls != 2 { + t.Fatalf("accepted write left pointer/value/checks = %p/%d/%d, want %p/2/2", state.embedded, state.Value, checkCalls, pointee) + } +} + +func TestJsVarPromotedJSONFieldSelection(t *testing.T) { + t.Run("ignored and untagged", func(t *testing.T) { + type fields struct { + Fallback int + Hidden int `json:"-"` + } + type stateType struct { + fields + } + state := stateType{fields{Fallback: 1, Hidden: 3}} + jsvar := NewJsVar(new(sync.Mutex), &state) + checkCalls := 0 + jsvar.ClientCheck = func(value *stateType, jsPath string) error { + checkCalls++ + if value.Fallback != 6 || value.Hidden != 3 || jsPath != "" { + t.Errorf("ClientCheck saw state/path = %#v/%q", value, jsPath) + } + return nil + } + + if got := jsvar.JawsGetPath(nil, "Fallback"); got != 1 { + t.Fatalf("untagged fallback Get = %#v, want 1", got) + } + if got := jsvar.JawsGetPath(nil, "Hidden"); got != nil { + t.Fatalf("ignored promoted Get = %#v, want nil", got) + } + if err := jsvar.JawsSetPath(nil, "Fallback", 4); err != nil { + t.Fatal(err) + } + if err := jsvar.JawsSetPath(nil, "Hidden", 8); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("ignored promoted Set error = %v, want ErrPathNotFound", err) + } + if err := jsvar.JawsInput(nil, `={"Fallback":6,"Hidden":8}`); err != nil { + t.Fatal(err) + } + if state.Fallback != 6 || state.Hidden != 3 || checkCalls != 1 { + t.Fatalf("root write left state/checks = %#v/%d", state, checkCalls) + } + }) + + t.Run("ambiguous", func(t *testing.T) { + type left struct { + Value int + } + type right struct { + Value int + } + type stateType struct { + left + right + } + state := stateType{left: left{Value: 1}, right: right{Value: 2}} + jsvar := NewJsVar(new(sync.Mutex), &state) + checkCalls := 0 + jsvar.ClientCheck = func(*stateType, string) error { + checkCalls++ + return nil + } + + if got := jsvar.JawsGetPath(nil, "Value"); got != nil { + t.Fatalf("ambiguous Get = %#v, want nil", got) + } + if err := jsvar.JawsSetPath(nil, "Value", 3); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("ambiguous programmatic Set error = %v, want ErrPathNotFound", err) + } + if err := jsvar.JawsInput(nil, `Value=3`); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("ambiguous browser Set error = %v, want ErrPathNotFound", err) + } + if err := jsvar.JawsInput(nil, `={"Value":3}`); err != nil { + t.Fatal(err) + } + if state.left.Value != 1 || state.right.Value != 2 || checkCalls != 0 { + t.Fatalf("ambiguous writes left state/checks = %#v/%d", state, checkCalls) + } + }) + + t.Run("tagged unexported endpoint", func(t *testing.T) { + type inner struct { + Value int `json:"value"` + } + type stateType struct { + inner `json:"inner"` + } + state := stateType{inner: inner{Value: 1}} + jsvar := NewJsVar(new(sync.Mutex), &state) + checkCalls := 0 + jsvar.ClientCheck = func(value *stateType, _ string) error { + checkCalls++ + if _, err := jq.Get(value, "inner"); !errors.Is(err, jq.ErrPathNotFound) { + t.Errorf("ClientCheck endpoint Get error = %v, want ErrPathNotFound", err) + } + if got, err := jq.Get(value, "inner.value"); err != nil || got != 2 { + t.Errorf("ClientCheck descendant Get = %#v, %v; want 2, nil", got, err) + } + return nil + } + + if got := jsvar.JawsGetPath(nil, "inner"); got != nil { + t.Fatalf("tagged unexported endpoint Get = %#v, want nil", got) + } + if got := jsvar.JawsGetPath(nil, "inner.value"); got != 1 { + t.Fatalf("tagged unexported descendant Get = %#v, want 1", got) + } + if err := jsvar.JawsSetPath(nil, "inner", inner{Value: 2}); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("tagged unexported endpoint Set error = %v, want ErrPathNotFound", err) + } + if err := jsvar.JawsInput(nil, `inner.value=2`); err != nil { + t.Fatal(err) + } + if err := jsvar.JawsInput(nil, `={"inner":{"value":3}}`); !errors.Is(err, jq.ErrPathNotFound) { + t.Fatalf("tagged unexported map key error = %v, want ErrPathNotFound", err) + } + if state.Value != 2 || checkCalls != 1 { + t.Fatalf("tagged unexported writes left state/checks = %#v/%d", state, checkCalls) + } + }) +} + func TestJsVarPromotedJSONFieldNilPointer(t *testing.T) { - state := jsVarNilPromotedState{} + type embedded struct { + Value int `json:"value"` + } + type stateType struct { + *embedded + } + state := stateType{} jsvar := NewJsVar(new(sync.Mutex), &state) checkCalls := 0 - jsvar.ClientCheck = func(*jsVarNilPromotedState, string) error { + jsvar.ClientCheck = func(*stateType, string) error { checkCalls++ return nil } @@ -482,11 +625,78 @@ func TestJsVarPromotedJSONFieldNilPointer(t *testing.T) { if err := jsvar.JawsInput(nil, `={"value":1}`); !errors.Is(err, jq.ErrPathNotFound) { t.Fatalf("browser whole-object write error = %v, want ErrPathNotFound", err) } - if state.jsVarPromotedFields != nil || checkCalls != 0 { + if state.embedded != nil || checkCalls != 0 { t.Fatalf("failed writes left state %#v and made %d checks", state, checkCalls) } } +func TestJsVarNilPromotedFieldDoesNotSuppressSharedNameSibling(t *testing.T) { + type embedded struct { + Value int `json:"value"` + } + type stateType struct { + *embedded + } + + nilState := stateType{} + liveState := stateType{embedded: &embedded{Value: 1}} + nilJsVar := NewJsVar(new(sync.Mutex), &nilState) + liveJsVar := NewJsVar(new(sync.Mutex), &liveState) + nilChecks := 0 + liveChecks := 0 + nilJsVar.ClientCheck = func(*stateType, string) error { + nilChecks++ + return nil + } + liveJsVar.ClientCheck = func(*stateType, string) error { + liveChecks++ + return nil + } + + tr := newJsVarTestRequest(t, newJsVarTestJaws(t)) + rw := RequestWriter{Request: tr.Request, Writer: io.Discard} + if err := rw.JsVar("shared", nilJsVar); err != nil { + t.Fatal(err) + } + if err := rw.JsVar("shared", liveJsVar); err != nil { + t.Fatal(err) + } + nilElements := tr.GetElements(&nilState) + liveElements := tr.GetElements(&liveState) + if len(nilElements) != 1 || len(liveElements) != 1 { + t.Fatalf("rendered elements = %d, %d; want 1, 1", len(nilElements), len(liveElements)) + } + + // jawsVar sends one frame per live binding in attachment order. The request + // loop handles each frame independently, so the first error cannot suppress + // the second binding. + tr.InCh <- wire.WsMsg{Jid: nilElements[0].Jid(), What: what.Set, Data: "value=2"} + tr.InCh <- wire.WsMsg{Jid: liveElements[0].Jid(), What: what.Set, Data: "value=2"} + + gotAlert := false + gotSet := false + for range 2 { + msg := awaitJsVarOperation(t, "shared-name promoted write", tr.OutCh) + switch msg.What { + case what.Alert: + gotAlert = true + case what.Set: + if msg.Jid != liveElements[0].Jid() || msg.Data != "value=2" { + t.Fatalf("sibling broadcast = %#v, want live sibling value=2 Set", msg) + } + gotSet = true + default: + t.Fatalf("shared-name write produced unexpected message %#v", msg) + } + } + if !gotAlert || !gotSet { + t.Fatalf("shared-name messages: alert=%t set=%t, want both", gotAlert, gotSet) + } + if nilState.embedded != nil || liveState.Value != 2 || nilChecks != 0 || liveChecks != 1 { + t.Fatalf("shared-name states/checks = %#v, %#v, %d, %d", nilState, liveState, nilChecks, liveChecks) + } +} + func TestJsVarClientCheckRejectRestoresZeroValueAppendAliases(t *testing.T) { backing := []string{"sentinel"} state := jsVarSliceData{Items: backing[:0]} @@ -681,22 +891,10 @@ func TestJsVarJSONSizeCheckMarshalFailureRollsBackAndCancels(t *testing.T) { } func TestJsVarJSONSizeCheckSharedBackingState(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - t.Cleanup(jw.Close) - go jw.Serve() - - tr1 := jawstest.NewTestRequest(jw, nil) - tr2 := jawstest.NewTestRequest(jw, nil) - if tr1 == nil || tr2 == nil { - t.Fatal("nil test request") - } - cleanupJsVarTestRequest(t, tr1) - cleanupJsVarTestRequest(t, tr2) - <-tr1.ReadyCh - <-tr2.ReadyCh + jw := newJsVarTestJaws(t) + tr1 := newJsVarTestRequest(t, jw) + tr2 := newJsVarTestRequest(t, jw) + var err error var mu sync.Mutex state := jsVarSliceData{}