diff --git a/internal/verifier/marshal.go b/internal/verifier/marshal.go index cb83b46..1b23546 100644 --- a/internal/verifier/marshal.go +++ b/internal/verifier/marshal.go @@ -4,6 +4,8 @@ import ( "bytes" "encoding/json" "fmt" + "math" + "reflect" "slices" "strings" "time" @@ -527,6 +529,106 @@ func exceptionsArray(runtime *goja.Runtime, exceptions []Exception) *goja.Object return array } +// traceValueMaxDepth bounds how far recordableValue walks. It mirrors +// SANITIZE_MAX_DEPTH in pkg/spec/src/web-runtime.ts, whose sanitize does this +// same job for the values the page reports, so both hosts record the same JSON +// for the same extractor. +const traceValueMaxDepth = 32 + +// recordableValue rewrites an exported goja value into one json.Marshal +// accepts. An accessibility element is a plain object carrying two host +// functions (find/findAll); marshalling it fails on those alone, so the whole +// element used to go unrecorded. Dropping them leaves the element's data (id, +// text, desc, class, the flags, bounds, attrs), which is what a trace reader +// wants and is a subset of the hierarchy the same step already records. +// +// ok is false for a value with no JSON form at all: callers drop that key from +// its object, matching the web host, where a function-valued property is +// skipped and a function inside an array stringifies to null. +func recordableValue(value any, depth int, seen map[uintptr]bool) (any, bool) { + if value == nil { + return nil, true + } + switch typed := value.(type) { + case float64: + return finiteOrNull(typed), true + case float32: + return finiteOrNull(float64(typed)), true + } + reflected := reflect.ValueOf(value) + switch reflected.Kind() { + case reflect.Func: + return nil, false + case reflect.Map: + if reflected.Type().Key().Kind() != reflect.String || !holdsAny(reflected.Type().Elem()) { + return value, true + } + if depth >= traceValueMaxDepth || !firstVisit(reflected, seen) { + return nil, true + } + out := make(map[string]any, reflected.Len()) + iterator := reflected.MapRange() + for iterator.Next() { + entry, ok := recordableValue(iterator.Value().Interface(), depth+1, seen) + if !ok { + continue + } + out[iterator.Key().String()] = entry + } + return out, true + case reflect.Slice, reflect.Array: + if !holdsAny(reflected.Type().Elem()) { + return value, true + } + if depth >= traceValueMaxDepth || !firstVisit(reflected, seen) { + return nil, true + } + out := make([]any, reflected.Len()) + for index := range out { + entry, ok := recordableValue(reflected.Index(index).Interface(), depth+1, seen) + if !ok { + entry = nil + } + out[index] = entry + } + return out, true + default: + return value, true + } +} + +// holdsAny reports whether a container's elements can hide a host function or +// a cycle. Concretely typed containers ([]string, []byte, map[string]string) +// can hold neither, and walking them would rewrite shapes json.Marshal already +// handles, such as []byte's base64 form. +func holdsAny(elem reflect.Type) bool { + return elem.Kind() == reflect.Interface +} + +// firstVisit reports whether a container has not been walked yet, so a cyclic +// value terminates. Empty containers are never recorded: they cannot close a +// cycle, and Go may hand every one of them the same address. +func firstVisit(container reflect.Value, seen map[uintptr]bool) bool { + if container.Kind() == reflect.Array || container.Len() == 0 { + return true + } + address := container.Pointer() + if seen[address] { + return false + } + seen[address] = true + return true +} + +// finiteOrNull maps NaN and the infinities to JSON null, which is what +// JSON.stringify does with them on the web host. +func finiteOrNull(value float64) any { + if math.IsNaN(value) || math.IsInf(value, 0) { + return nil + } + return value +} + func jsonToJSValue(runtime *goja.Runtime, raw json.RawMessage) (goja.Value, error) { if len(raw) == 0 { return goja.Undefined(), nil diff --git a/internal/verifier/worker.go b/internal/verifier/worker.go index b559ea7..2f68f0d 100644 --- a/internal/verifier/worker.go +++ b/internal/verifier/worker.go @@ -353,8 +353,14 @@ func (v *Verifier) PushSnapshot(input SnapshotInput) error { return fmt.Errorf("extractor %d: %w", index, err) } extractor.currentValue = newValue + encoded, err := encodeExtractorValue(newValue) + if err != nil { + return fmt.Errorf( + "extractor %q: the value cannot be recorded in the trace: %w; return plain data instead", + extractor.name, err) + } extractor.prev = extractor.curr - extractor.curr = encodeExtractorValue(newValue) + extractor.curr = encoded } return nil } @@ -369,19 +375,23 @@ 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. -func encodeExtractorValue(value goja.Value) []byte { +// current value for diff comparison. Host functions, cycles and non-finite +// numbers are projected away by recordableValue; anything still beyond JSON is +// an error, never a silently dropped value, because an extractor missing from +// the trace reads exactly like an extractor that never changed. +func encodeExtractorValue(value goja.Value) ([]byte, error) { if value == nil || goja.IsUndefined(value) || goja.IsNull(value) { - return []byte("null") + return []byte("null"), nil } - exported := value.Export() - body, err := json.Marshal(exported) + recordable, ok := recordableValue(value.Export(), 0, map[uintptr]bool{}) + if !ok { + return []byte("null"), nil + } + body, err := json.Marshal(recordable) if err != nil { - return nil + return nil, err } - return body + return body, nil } // ChangedExtractors returns the named extractors whose value changed between @@ -393,6 +403,8 @@ func encodeExtractorValue(value goja.Value) []byte { func (v *Verifier) ChangedExtractors() map[string]ExtractorChange { changes := map[string]ExtractorChange{} for _, extractor := range v.extractors { + // nil curr now means only that no snapshot has been pushed yet: an + // encoding that cannot be recorded fails PushSnapshot instead. if extractor.curr == nil { continue } @@ -453,7 +465,13 @@ func (v *Verifier) OverrideExtractorValues(overrides map[int]json.RawMessage) (s return skipped, fmt.Errorf("extractor override %d: %w", index, conversionErr) } v.extractors[index].currentValue = value - v.extractors[index].curr = encodeExtractorValue(value) + encoded, encodeErr := encodeExtractorValue(value) + if encodeErr != nil { + return skipped, fmt.Errorf( + "extractor override %d (%q): the value cannot be recorded in the trace: %w", + index, v.extractors[index].name, encodeErr) + } + v.extractors[index].curr = encoded } return skipped, nil } @@ -559,9 +577,8 @@ func (v *Verifier) captureWitness(name string) { } } -// extractorSnapshot encodes every named extractor's current value as JSON. A -// nil value (extractor never advanced or its value did not survive Export) -// is recorded as JSON null. +// extractorSnapshot encodes every named extractor's current value as JSON. An +// extractor that never advanced is recorded as JSON null. func (v *Verifier) extractorSnapshot() map[string]json.RawMessage { if len(v.extractors) == 0 { return nil