From 45b7624280c7d7b57f0f5fab3ad17568977a18a2 Mon Sep 17 00:00:00 2001 From: PJ Date: Sat, 15 Aug 2026 12:45:25 +0530 Subject: [PATCH] fix(web): keep an undefined reading's index through JSON json has no undefined, so an extractor whose getter returned one had its whole index dropped by JSON.stringify. that index then kept goja's dump-derived value while its neighbours held the page's, and a property comparing previous to current across the split fires on a healthy app. folio has nine on(route, tag) extractors, so this was most extractors on most steps. each reading is wrapped in a {value} envelope: the drop now happens inside the entry, and an absent value means the getter returned undefined, which is what the goja host records for the same getter. a json null would instead claim it returned null and x.current === undefined would answer differently on the two hosts. --- internal/driver/chrome/driver.go | 27 ++++++++- internal/driver/chrome/driver_test.go | 82 ++++++++++++++++++++++++++- pkg/spec/src/web-runtime.ts | 15 ++++- pkg/spec/test/web-runtime.test.ts | 48 +++++++++++++--- 4 files changed, 159 insertions(+), 13 deletions(-) diff --git a/internal/driver/chrome/driver.go b/internal/driver/chrome/driver.go index a819351..97498ee 100644 --- a/internal/driver/chrome/driver.go +++ b/internal/driver/chrome/driver.go @@ -787,16 +787,39 @@ func (d *Driver) EvaluateExtractors(ctx context.Context) (map[int]json.RawMessag return nil, fmt.Errorf("decode extractor map: %w", err) } result := make(map[int]json.RawMessage, len(stringMap)) - for key, value := range stringMap { + for key, entry := range stringMap { index, err := strconv.Atoi(key) if err != nil { return nil, fmt.Errorf("non-integer extractor key %q", key) } - result[index] = value + reading, err := extractorReading(entry) + if err != nil { + return nil, fmt.Errorf("extractor %d: %w", index, err) + } + result[index] = reading } return result, nil } +// extractorReading unwraps one entry of the page's extractor table. The page +// wraps every reading in a {"value": ...} envelope (evaluateExtractors in +// pkg/spec/src/web-runtime.ts) because JSON has no undefined: an absent `value` +// is the getter returning undefined, and returning it as an empty payload is +// what makes the goja host record undefined too. Reading it as JSON null would +// claim the getter returned null, so `x.current === undefined` would answer one +// thing on native and another on web. +func extractorReading(entry json.RawMessage) (json.RawMessage, error) { + var envelope struct { + Value json.RawMessage `json:"value"` + } + if err := json.Unmarshal(entry, &envelope); err != nil { + return nil, fmt.Errorf( + "reading %s is not a {\"value\"} envelope; the page and the host are "+ + "running different bundles: %w", entry, err) + } + return envelope.Value, nil +} + // SetLastAction installs the previous step's action as state.lastAction inside // the page runtime. The page cannot derive it: only the runner knows which // action was actually applied. Without this call every web state.lastAction is diff --git a/internal/driver/chrome/driver_test.go b/internal/driver/chrome/driver_test.go index 234223f..36ef3fb 100644 --- a/internal/driver/chrome/driver_test.go +++ b/internal/driver/chrome/driver_test.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -772,7 +773,7 @@ func TestEvaluateExtractors_WaitsOutARouteTransition(t *testing.T) { const root = document.getElementById("app").attachShadow({mode: "open"}); root.innerHTML = '
ledger
'; window.__sanderlingExtractors__ = function () { - return {0: Array.from(root.querySelectorAll('[id$="Screen"]')).map(e => e.id).join(",")}; + return {0: {value: Array.from(root.querySelectorAll('[id$="Screen"]')).map(e => e.id).join(",")}}; }; window.startTransition = function () { const incoming = document.createElement("div"); @@ -885,3 +886,82 @@ func TestEvaluateExtractors_ReportsAMissingTable(t *testing.T) { "extractor table; a page that cannot be read must not read as empty", values) } } + +// TestEvaluateExtractors_KeepsUndefinedApartFromNull covers the wire the page's +// readings cross. JSON has no undefined, so the web runtime wraps each reading +// in a {value} envelope: written straight into the table, an extractor that +// returned undefined lost its whole index to JSON.stringify and the host kept +// goja's dump-derived reading for it while the rest held the page's. An absent +// value has to arrive as an empty payload, which is what makes the verifier +// record undefined (the value the native host records for the same getter); +// arriving as JSON null would claim the getter returned null. +func TestEvaluateExtractors_KeepsUndefinedApartFromNull(t *testing.T) { + const page = `` + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte(page)) + })) + defer server.Close() + + d := New() + defer d.Terminate(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := d.Launch(ctx, server.URL, false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + values, err := d.EvaluateExtractors(ctx) + if err != nil { + t.Fatalf("EvaluateExtractors: %v", err) + } + if len(values) != 3 { + t.Fatalf("the page reported 3 readings, %d survived the wire: %v", len(values), values) + } + if got := values[0]; len(got) != 0 { + t.Errorf("the undefined reading arrived as %s, want an empty payload; "+ + "the verifier records anything else as a value the getter never returned", got) + } + if got := string(values[1]); got != "null" { + t.Errorf("the null reading arrived as %s, want null", got) + } + if got := string(values[2]); got != `{"balance":7}` { + t.Errorf("the object reading arrived as %s, want {\"balance\":7}", got) + } +} + +// TestEvaluateExtractors_RejectsAnUnenvelopedReading is the loud failure a page +// running an older @sanderling/spec produces. Its readings are bare values, and +// a bare value is indistinguishable from a reading whose getter returned that +// value, so accepting them silently puts the two engines on different bundles. +func TestEvaluateExtractors_RejectsAnUnenvelopedReading(t *testing.T) { + const page = `` + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte(page)) + })) + defer server.Close() + + d := New() + defer d.Terminate(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := d.Launch(ctx, server.URL, false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + values, err := d.EvaluateExtractors(ctx) + if err == nil { + t.Fatalf("EvaluateExtractors accepted %v from a page whose readings are not "+ + "enveloped; the page and the host are running different bundles", values) + } + if !strings.Contains(err.Error(), "different bundles") { + t.Errorf("EvaluateExtractors failed with %q, want it to name the bundle mismatch", err) + } +} diff --git a/pkg/spec/src/web-runtime.ts b/pkg/spec/src/web-runtime.ts index eac9ff3..244bc34 100644 --- a/pkg/spec/src/web-runtime.ts +++ b/pkg/spec/src/web-runtime.ts @@ -501,9 +501,18 @@ function defineLockedGlobal(name: string, value: unknown): void { }); } -function evaluateExtractors(): Record { +// Each reading is wrapped in a {value} envelope because JSON has no undefined. +// Written straight into the map, an extractor whose getter returned undefined +// (folio's on(route, tag) off its own screen, which is most extractors on most +// steps) had its whole INDEX dropped by JSON.stringify, and the host kept goja's +// dump-derived reading for it while the rest held the page's. Inside the +// envelope the same drop means "this getter returned undefined", which is what +// the goja host records for the same getter; a JSON null would instead claim it +// returned null, and `x.current === undefined` would answer differently on the +// two hosts. +function evaluateExtractors(): Record { const state = buildState(); - const result: Record = {}; + const result: Record = {}; for (let i = 0; i < extractors.length; i++) { const entry = extractors[i]; if (!entry) continue; @@ -520,7 +529,7 @@ function evaluateExtractors(): Record { extracting = false; } entry.currentValue = value; - result[i] = sanitize(value); + result[i] = { value: sanitize(value) }; } return result; } diff --git a/pkg/spec/test/web-runtime.test.ts b/pkg/spec/test/web-runtime.test.ts index 0e80c5b..a5ac7e6 100644 --- a/pkg/spec/test/web-runtime.test.ts +++ b/pkg/spec/test/web-runtime.test.ts @@ -206,6 +206,13 @@ function withState(run: () => void) { } } +// Every reading leaves the runtime inside a {value} envelope, so an extractor +// whose getter returned undefined keeps its index instead of being dropped by +// JSON.stringify. +function readingOf(values: Record, index: number): unknown { + return values[index]!.value; +} + test("named() sets the extractor's display name", () => { const handle = __testing__.runtime.extract(() => "home").named("route"); const entry = __testing__.extractors.find((e) => e.handle === handle); @@ -251,6 +258,33 @@ test("an uncaught cross-extractor read aborts evaluateExtractors", () => { ); }); +// JSON.stringify drops an undefined-valued key, so a reading written straight +// into the table took the extractor's whole INDEX with it when the getter +// returned undefined - folio's on(route, tag) off its own screen, which is most +// of its extractors on most steps. The host then kept goja's dump-derived value +// for those and the page's for the rest, and a property comparing previous to +// current across that split convicts an app that did nothing wrong. +test("an extractor that returned undefined keeps its index through JSON", () => { + __testing__.extractors.length = 0; + __testing__.runtime.extract(() => undefined); + __testing__.runtime.extract(() => null); + __testing__.runtime.extract(() => 5); + let table: Record = {}; + withState(() => { + table = __testing__.evaluateExtractors(); + }); + + const overTheWire = JSON.parse(JSON.stringify(table)) as Record; + assert.deepEqual(Object.keys(overTheWire), ["0", "1", "2"]); + // undefined and null have to stay distinguishable across the wire: the goja + // host records undefined for a getter that returned undefined, so reporting + // null instead would make `x.current === undefined` answer one thing on + // native and another on web. + assert.equal("value" in overTheWire["0"]!, false); + assert.equal(overTheWire["1"]!.value, null); + assert.equal(overTheWire["2"]!.value, 5); +}); + // state.lastAction is the one piece of state the page cannot observe for // itself: only the runner knows which action it actually applied. While the web // runtime hardcoded null there, a spec property gated on the last action (e.g. @@ -262,12 +296,12 @@ function lastActionSeenByASpec(pushed: unknown): unknown { .__sanderlingSetLastAction__ as (value: unknown) => void; __testing__.extractors.length = 0; __testing__.runtime.extract((state) => (state as { lastAction: unknown }).lastAction); - let out: Record = {}; + let out: Record = {}; withState(() => { setLastAction(pushed); out = __testing__.evaluateExtractors(); }); - return out[0]; + return readingOf(out, 0); } test("state.lastAction carries the action the host pushed", () => { @@ -289,11 +323,11 @@ test("state.lastAction is null when the host pushed nothing", () => { function sanitizeViaExtract(value: unknown): unknown { __testing__.extractors.length = 0; __testing__.runtime.extract(() => value); - let out: Record = {}; + let out: Record = {}; withState(() => { out = __testing__.evaluateExtractors(); }); - return out[0]; + return readingOf(out, 0); } test("sanitize breaks a self-referential cycle instead of overflowing", () => { @@ -471,7 +505,7 @@ test("ax.findAll resolves a selector path segment by segment", () => { const values = __testing__.evaluateExtractors(); // Scoped to the head match: the cards come from the HomeScreen node, not // from a document-wide sweep for AccountCard. - assert.deepEqual(values[0], ["first", "second"]); + assert.deepEqual(readingOf(values, 0), ["first", "second"]); } finally { g.document = originalDocument; g.window = originalWindow; @@ -506,11 +540,11 @@ test("ax.find and ax.findAll label the element with its selector", () => { return ax.findAll({ testTag: "TxnSubmit" }); }); const values = __testing__.evaluateExtractors(); - const found = values[0] as Record; + const found = readingOf(values, 0) as Record; assert.equal(found.__sanderlingSelector, "testTag:TxnSubmit"); // findAll passes each element through map(); passing the callback by // reference would hand the array INDEX to the runtime as the selector. - const all = values[1] as Record[]; + const all = readingOf(values, 1) as Record[]; assert.equal(all[0]!.__sanderlingSelector, "testTag:TxnSubmit"); } finally { g.document = originalDocument;