diff --git a/internal/runner/element_extractor_trace_test.go b/internal/runner/element_extractor_trace_test.go new file mode 100644 index 0000000..4eb7611 --- /dev/null +++ b/internal/runner/element_extractor_trace_test.go @@ -0,0 +1,121 @@ +package runner + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/priyanshujain/sanderling/internal/trace" +) + +// elementExtractorSpec reads a live ax element, the shape every field and +// button in examples/folio/sanderling/spec.ts is extracted with. The property +// is false the moment the field is on screen, so the run records a witness +// whose only interesting content is that element. +const elementExtractorSpec = ` +import { actions, always, extract } from "@sanderling/spec"; +const amountField = extract("amountField", s => s.ax.find({ "resource-id": "TxnAmountField" })); +globalThis.properties = { + noAmountField: always(() => amountField.current === undefined), +}; +globalThis.actions = actions(() => []); +` + +const amountFieldTreeJSON = `{ + "attributes": {"resource-id": "root", "bounds": "[0,0,400,800]"}, + "enabled": true, + "children": [ + {"attributes": {"resource-id": "TxnAmountField", "text": "199", "bounds": "[0,100,400,160]"}, + "editable": true, "enabled": true, "children": []} + ] +}` + +// TestRunner_TraceRecordsElementValuedExtractors is the guard on the artifact a +// person opens to decide whether a conviction is real. An element-valued +// extractor used to reach the trace as null on the goja hosts (ios, android): +// its exported value carries the element's find/findAll host functions, which +// json.Marshal refuses, so the encoding failed and both the per-step diff and +// the witness recorded nothing. A witness that reads null for the field the +// property fired on describes a state the property could not have fired in, +// which is worse than a blank. +func TestRunner_TraceRecordsElementValuedExtractors(t *testing.T) { + state := newHarnessWithSpec(t, elementExtractorSpec) + state.mock.HierarchyJSON = amountFieldTreeJSON + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 2, + Driver: state.mock, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if !containsProperty(summary.Violations, "noAmountField") { + t.Fatalf("noAmountField did not violate, so the element never reached a predicate: %v", + summary.Violations) + } + + file, err := os.Open(filepath.Join(state.writer.Directory(), "trace.jsonl")) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + type traceLine struct { + Step int `json:"step"` + ExtractorChanges map[string]trace.ExtractorChange `json:"extractor_changes"` + Witnesses map[string]trace.Witness `json:"witnesses"` + } + changes, witnesses := 0, 0 + scanner := bufio.NewScanner(file) + scanner.Buffer(make([]byte, 0, 64*1024), 8*1024*1024) + for scanner.Scan() { + var line traceLine + if err := json.Unmarshal(scanner.Bytes(), &line); err != nil { + t.Fatalf("trace line decode: %v", err) + } + if change, ok := line.ExtractorChanges["amountField"]; ok { + changes++ + assertAmountField(t, fmt.Sprintf("step %d extractor_changes", line.Step), change.Curr) + } + for name, witness := range line.Witnesses { + witnesses++ + assertAmountField(t, fmt.Sprintf("step %d %s witness", line.Step, name), + witness.Extractors["amountField"]) + } + } + if err := scanner.Err(); err != nil { + t.Fatalf("scan trace: %v", err) + } + if changes == 0 { + t.Error("amountField never appears in extractor_changes; the element the run read is not in the trace") + } + if witnesses == 0 { + t.Error("no witness reached the trace; nothing was compared") + } +} + +// assertAmountField reads the recorded element the way a person opening the +// trace would: the field's text is the number the property was judged on. +func assertAmountField(t *testing.T, where string, recorded json.RawMessage) { + t.Helper() + var element struct { + Text string `json:"text"` + } + if err := json.Unmarshal(recorded, &element); err != nil { + t.Fatalf("%s: decode %s: %v", where, recorded, err) + } + if element.Text != "199" { + t.Errorf("%s: recorded element is %s, want its text to read 199", where, recorded) + } +} diff --git a/internal/verifier/extractor_encoding_test.go b/internal/verifier/extractor_encoding_test.go new file mode 100644 index 0000000..a693dbf --- /dev/null +++ b/internal/verifier/extractor_encoding_test.go @@ -0,0 +1,179 @@ +package verifier + +import ( + "bytes" + "encoding/json" + "testing" +) + +const elementTreeJSON = `{ + "attributes": {"resource-id": "root", "bounds": "[0,0,400,800]"}, + "enabled": true, + "children": [ + {"attributes": {"resource-id": "TxnAmountField", "text": "199", "bounds": "[0,100,400,160]"}, + "editable": true, "enabled": true, "children": []} + ] +}` + +const elementExtractorSpec = ` +const field = __sanderling__.extract(state => state.ax.find({ "resource-id": "TxnAmountField" }), "field"); +globalThis.properties = {}; +` + +// canonicalElement is the trace's record of one ax element, written out in the +// key order encoding/json emits. It is the contract both hosts owe the replay +// UI: an element the reader can read, with no host-function members and nothing +// dropped. Keys the two hosts disagree on (a DOM has no `checked`, a native +// tree has no `dataset`) are each host's own business; the ENCODING is not. +const canonicalElement = `{ + "__sanderlingSelector": "resource-id:TxnAmountField", + "attrs": { + "bounds": "[0,100,400,160]", + "editable": "true", + "enabled": "true", + "resource-id": "TxnAmountField", + "text": "199" + }, + "bounds": {"bottom": 160, "left": 0, "right": 400, "top": 100}, + "checked": false, + "class": "", + "clickable": false, + "desc": "", + "editable": true, + "enabled": true, + "focused": false, + "id": "TxnAmountField", + "selected": false, + "text": "199", + "x": 200, + "y": 130 +}` + +// TestExtractorEncoding_ElementIsIdenticalOnBothHosts holds the two extractor +// paths to one encoding of one element. The goja hosts (ios, android) run the +// getter in-process and encode the value it returned; the web host runs it in +// V8 and injects the page's reading through OverrideExtractorValues. A reader +// opening a trace does not know which host wrote it, so the same element has to +// land as the same bytes either way. +// +// The goja side used to write null here: an ax element carries find/findAll as +// host functions and json.Marshal refuses the whole object over them. +func TestExtractorEncoding_ElementIsIdenticalOnBothHosts(t *testing.T) { + want := compactJSON(t, canonicalElement) + + native := newVerifier(t) + mustLoad(t, native, elementExtractorSpec) + pushTree(t, native, elementTreeJSON) + fromGoja := string(native.extractors[0].curr) + if fromGoja != want { + t.Errorf("goja host encoded the element as\n %s\nwant\n %s", fromGoja, want) + } + + web := newVerifier(t) + mustLoad(t, web, elementExtractorSpec) + if err := web.PushSnapshot(SnapshotInput{}); err != nil { + t.Fatal(err) + } + if _, err := web.OverrideExtractorValues(map[int]json.RawMessage{0: json.RawMessage(want)}); err != nil { + t.Fatal(err) + } + fromWeb := string(web.extractors[0].curr) + if fromWeb != fromGoja { + t.Errorf("the same element reaches the trace as\n %s\non the web host and\n %s\non goja", + fromWeb, fromGoja) + } +} + +// TestExtractorEncoding_MirrorsTheWebSanitizeRule pins the goja host to the +// rule the web host applies before a reading leaves the page (sanitize in +// pkg/spec/src/web-runtime.ts, asserted there by the "sanitize ..." tests in +// pkg/spec/test/web-runtime.test.ts). Two hosts encoding one value two ways is +// the same defect as encoding it not at all: the reader cannot line the traces +// up. +func TestExtractorEncoding_MirrorsTheWebSanitizeRule(t *testing.T) { + for _, test := range []struct { + name string + expression string + want string + }{ + { + name: "function-valued properties are dropped", + expression: `({ keep: 1, fn: () => 7 })`, + want: `{"keep":1}`, + }, + { + name: "a top-level function is not a value", + expression: `(() => 7)`, + want: `null`, + }, + { + name: "a self-referential cycle breaks instead of overflowing", + expression: `(() => { const a = { name: "root" }; a.self = a; return a; })()`, + want: `{"name":"root","self":null}`, + }, + { + name: "arrays and nested plain values are preserved", + expression: `({ items: [1, "two", { ok: true }] })`, + want: `{"items":[1,"two",{"ok":true}]}`, + }, + { + name: "a non-finite number is not a value", + expression: `Number("nope")`, + want: `null`, + }, + } { + t.Run(test.name, func(t *testing.T) { + if got := encodeSpecValue(t, test.expression); got != test.want { + t.Errorf("encoded as %s, want %s", got, test.want) + } + }) + } +} + +// TestExtractorEncoding_BoundsRecursionPastTheDepthLimit mirrors the web host's +// depth cap. state.ax hands out no cyclic element, but a spec returning a value +// it built itself can nest without end, and a walk with no bound takes the run +// down with a stack overflow. +func TestExtractorEncoding_BoundsRecursionPastTheDepthLimit(t *testing.T) { + encoded := encodeSpecValue(t, `(() => { + let deep = { leaf: true }; + for (let i = 0; i < 40; i++) deep = { next: deep }; + return deep; + })()`) + + var node any + if err := json.Unmarshal([]byte(encoded), &node); err != nil { + t.Fatalf("decode %s: %v", encoded, err) + } + for depth := 0; depth < recordableMaxDepth; depth++ { + object, ok := node.(map[string]any) + if !ok { + t.Fatalf("depth %d: recursion stopped early at %v", depth, node) + } + node = object["next"] + } + if node != nil { + t.Errorf("depth %d is %v, want null", recordableMaxDepth, node) + } +} + +// encodeSpecValue returns what the trace records for an extractor whose getter +// returned the given expression. +func encodeSpecValue(t *testing.T, expression string) string { + t.Helper() + verifier := newVerifier(t) + mustLoad(t, verifier, "__sanderling__.extract(state => "+expression+", \"value\");\nglobalThis.properties = {};") + if err := verifier.PushSnapshot(SnapshotInput{}); err != nil { + t.Fatal(err) + } + return string(verifier.extractors[0].curr) +} + +func compactJSON(t *testing.T, source string) string { + t.Helper() + var compact bytes.Buffer + if err := json.Compact(&compact, []byte(source)); err != nil { + t.Fatal(err) + } + return compact.String() +} diff --git a/internal/verifier/worker.go b/internal/verifier/worker.go index e891969..69bd9b8 100644 --- a/internal/verifier/worker.go +++ b/internal/verifier/worker.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "maps" + "math" + "reflect" "sort" "time" @@ -356,21 +358,76 @@ func (v *Verifier) runExtractor(extractor *extractorState, state goja.Value) (go } // encodeExtractorValue produces a stable JSON encoding of an extractor's -// current value for diff comparison. goja values that don't survive Export -// (e.g. wrapped host functions) yield nil; callers treat nil as "unknown" and -// emit no diff entry. +// current value for diff comparison. Values that still don't survive encoding +// yield nil; callers treat nil as "unknown" and emit no diff entry. func encodeExtractorValue(value goja.Value) []byte { if value == nil || goja.IsUndefined(value) || goja.IsNull(value) { return []byte("null") } - exported := value.Export() - body, err := json.Marshal(exported) + body, err := json.Marshal(recordableValue(value.Export(), 0, map[uintptr]bool{})) if err != nil { return nil } return body } +// recordableMaxDepth mirrors SANITIZE_MAX_DEPTH in pkg/spec/src/web-runtime.ts. +const recordableMaxDepth = 32 + +// recordableValue applies the web host's sanitize rule (web-runtime.ts) to an +// exported goja value: function members are dropped, a cycle or a branch past +// the depth cap becomes null, and a non-finite number becomes null. One rule on +// both hosts is what lets the replay UI render a trace without the reader +// having to know which host produced it. An ax element carries its find and +// findAll host functions, and json.Marshal rejects the whole element over them, +// so without this an element-valued extractor reached the trace as null. +func recordableValue(value any, depth int, seen map[uintptr]bool) any { + switch typed := value.(type) { + case map[string]any: + address := reflect.ValueOf(typed).Pointer() + if depth >= recordableMaxDepth || seen[address] { + return nil + } + seen[address] = true + members := make(map[string]any, len(typed)) + for key, member := range typed { + if reflect.ValueOf(member).Kind() == reflect.Func { + continue + } + members[key] = recordableValue(member, depth+1, seen) + } + return members + case []any: + if depth >= recordableMaxDepth { + return nil + } + // Every zero-length allocation shares one address, so tracking an empty + // array would identify it as every other empty array. It cannot close a + // cycle either way. + if len(typed) > 0 { + address := reflect.ValueOf(typed).Pointer() + if seen[address] { + return nil + } + seen[address] = true + } + members := make([]any, len(typed)) + for index, member := range typed { + members[index] = recordableValue(member, depth+1, seen) + } + return members + case float64: + if math.IsNaN(typed) || math.IsInf(typed, 0) { + return nil + } + return typed + } + if reflect.ValueOf(value).Kind() == reflect.Func { + return nil + } + return value +} + // ChangedExtractors returns the named extractors whose value changed between // the prior PushSnapshot and the current one. The map is keyed by extractor // name; unnamed extractors (extractor_N fallback) are included so the replay