-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix(stage-ui): sanitize context snapshots before clone #1928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 8 commits
3c40f80
89b0230
af7a960
ed2c6ac
c2119db
0e82e18
16003f1
319f87c
d679631
ff75194
4fb9335
3a3a713
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
|
|
||
| import { sanitizeCloneable } from './context-bridge-sanitize' | ||
|
|
||
| describe('sanitizeCloneable', () => { | ||
| it('preserves cloneable bigint values', () => { | ||
| const input = { | ||
| metadata: { | ||
| count: 3n, | ||
| nested: [1n, { latest: 5n }], | ||
| }, | ||
| } | ||
|
|
||
| const sanitized = sanitizeCloneable(input) | ||
| const cloned = structuredClone(sanitized) | ||
|
|
||
| expect(cloned).toEqual({ | ||
| metadata: { | ||
| count: 3n, | ||
| nested: [1n, { latest: 5n }], | ||
| }, | ||
| }) | ||
| }) | ||
|
|
||
| it('removes nested non-cloneable values and stays structuredClone-safe', () => { | ||
| const input = { | ||
| text: 'vision update', | ||
| metadata: { | ||
| safe: 'ok', | ||
| nested: { | ||
| window: globalThis, | ||
| fn: () => 'bad', | ||
| arr: [1, { keep: true, drop: globalThis }], | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| const sanitized = sanitizeCloneable(input) | ||
| const cloned = structuredClone(sanitized) | ||
|
|
||
| expect(cloned).toEqual({ | ||
| text: 'vision update', | ||
| metadata: { | ||
| safe: 'ok', | ||
| nested: { | ||
| arr: [1, { keep: true }], | ||
| }, | ||
| }, | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| export function sanitizeCloneable(value: unknown, seen = new WeakSet<object>()): unknown { | ||
| if (value == null) | ||
| return value | ||
| if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') | ||
| return value | ||
| if (typeof value === 'symbol' || typeof value === 'function') | ||
| return undefined | ||
|
|
||
| const rawValue = value | ||
|
|
||
| if (Array.isArray(rawValue)) { | ||
| return rawValue | ||
| .map(item => sanitizeCloneable(item, seen)) | ||
| .filter(item => item !== undefined) | ||
|
Comment on lines
+11
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a context value contains a cycle through an array, such as Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| if (rawValue instanceof Date) | ||
| return rawValue.toISOString() | ||
|
|
||
| if (rawValue instanceof RegExp) | ||
| return rawValue.toString() | ||
|
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fresh evidence in this revision is that Useful? React with 👍 / 👎. |
||
|
|
||
| if (typeof rawValue !== 'object') | ||
| return rawValue | ||
|
|
||
| if (seen.has(rawValue)) | ||
| return undefined | ||
|
Comment on lines
+26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a single context message reuses the same object in two places, such as Useful? React with 👍 / 👎. |
||
| seen.add(rawValue) | ||
|
|
||
| if (rawValue instanceof Map) { | ||
| return new Map( | ||
| [...rawValue.entries()] | ||
| .map(([key, nestedValue]) => [ | ||
| sanitizeCloneable(key, seen), | ||
| sanitizeCloneable(nestedValue, seen), | ||
| ] as const) | ||
| .filter(([key, nestedValue]) => key !== undefined && nestedValue !== undefined), | ||
| ) | ||
| } | ||
|
|
||
| if (rawValue instanceof Set) { | ||
| return new Set( | ||
| [...rawValue.values()] | ||
| .map(item => sanitizeCloneable(item, seen)) | ||
| .filter(item => item !== undefined), | ||
| ) | ||
| } | ||
|
|
||
| if (ArrayBuffer.isView(rawValue) || rawValue instanceof ArrayBuffer) | ||
| return rawValue | ||
|
|
||
| const proto = Object.getPrototypeOf(rawValue) | ||
| if (proto !== Object.prototype && proto !== null) | ||
| return undefined | ||
|
Comment on lines
+53
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a local hook or context provider puts a structured-cloneable non-plain object such as Useful? React with 👍 / 👎. |
||
|
|
||
| return Object.fromEntries( | ||
| Object.entries(rawValue) | ||
| .map(([key, nestedValue]) => [key, sanitizeCloneable(nestedValue, seen)] as const) | ||
| .filter(([, nestedValue]) => nestedValue !== undefined), | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { defineConfig } from 'vitest/config' | ||
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| environment: 'node', | ||
| include: ['packages/stage-ui/src/stores/mods/api/context-bridge-sanitize.test.ts'], | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When context metadata or content intentionally contains
undefined(for example as an array placeholder or Map value), it is already structured-cloneable, but usingundefinedas the removal sentinel makes this filter and the analogous Map/object filters drop it. For positional arrays this also shifts later entries, so broadcast stream snapshots can no longer match the sender's context even though the original value was clone-safe.Useful? React with 👍 / 👎.