From 8b45f9d19c5ef4544e1accf204cef95a556068e2 Mon Sep 17 00:00:00 2001 From: NetWin Date: Mon, 19 Oct 2026 21:52:03 +0200 Subject: [PATCH] fix(maple): make converting a cyclic error a catchable RangeError An error whose name or message led back to itself was converted until the host ran out of stack, ending the run past the script's catch and finally. Converting errors inside one another now stops at the nesting limit with JavaScript's RangeError, and join cuts a cycle through an array across nested conversions, as JavaScript does. --- packages/maple-parity/README.md | 2 + packages/maple-parity/src/divergences.spec.ts | 16 +++ packages/maple/docs/limits.md | 10 +- .../maple/src/builtins/array/array.spec.ts | 7 ++ .../maple/src/builtins/object/object.spec.ts | 28 ++++++ .../src/builtins/string/methods/concat.ts | 2 +- packages/maple/src/errors.spec.ts | 69 +++++++++++++ packages/maple/src/values/arguments.ts | 2 +- packages/maple/src/values/coercion.spec.ts | 26 +++++ packages/maple/src/values/coercion.ts | 98 +++++++++++++------ packages/maple/src/values/sandbox-error.ts | 30 +++--- packages/maple/src/values/sandbox-object.ts | 9 +- 12 files changed, 251 insertions(+), 48 deletions(-) diff --git a/packages/maple-parity/README.md b/packages/maple-parity/README.md index 5118b37..a69af50 100644 --- a/packages/maple-parity/README.md +++ b/packages/maple-parity/README.md @@ -62,6 +62,8 @@ js-interpreter differs from JavaScript here; Maple does what Node does. | `new Error('m', { cause: 1 }).cause`: Maple gives `1`, js-interpreter `undefined` | The `cause` option is ES2022; js-interpreter follows ES5 and ignores the second argument. | | `'abc'.split(undefined, 0)`: Maple gives `[]`, js-interpreter `['abc']` | A limit of 0 gives no parts, as in JavaScript (and ES5); js-interpreter checks the separator first. | | An error with `name` set to `''` or `undefined`: Maple prints `oops` and `Error: x` | As in JavaScript (and ES5); js-interpreter joins `name` and `message` with `: ` regardless. | +| An error whose `name` is itself: Maple throws a `RangeError` | Nothing cuts that cycle in JavaScript, so it runs out of stack; js-interpreter stops quietly. | +| An error named by an array that holds it: Maple gives `m: m` | `join` cuts the cycle at the array, as in JavaScript; js-interpreter gives `...: m`. | | `f.bind(null, 1)`: Maple names it `bound f` with the rest of `f`'s length, js-interpreter `fBound`, 0 | A bound function's `name` and `length` are ES2015; js-interpreter follows ES5. | | `f.apply(null, 1)`, `f.apply(null, 'ab')` | A primitive is no list of arguments, so it is a `TypeError`. js-interpreter ignores a number and spreads a string. | diff --git a/packages/maple-parity/src/divergences.spec.ts b/packages/maple-parity/src/divergences.spec.ts index c0ac206..8b06e56 100644 --- a/packages/maple-parity/src/divergences.spec.ts +++ b/packages/maple-parity/src/divergences.spec.ts @@ -172,6 +172,22 @@ describe('Error.prototype.toString', () => { ) ).toEqual({ maple: ['oops', 'Error: x'], jsInterpreter: [': oops', 'undefined: x'] }); }); + + it('throws a RangeError for an error that names itself', () => { + expect( + followsJavaScript( + "var e = new Error('m'); e.name = e; try { String(e); } catch (t) { return t.name; }" + ) + ).toEqual({ maple: 'RangeError', jsInterpreter: undefined }); + }); + + // join cuts a cycle through an array, nested conversions included; js-interpreter cuts it as '...' + it('converts an error named by an array that holds it', () => { + expect(followsJavaScript("var e = new Error('m'); e.name = [e]; return String(e);")).toEqual({ + maple: 'm: m', + jsInterpreter: '...: m' + }); + }); }); describe('Error:cause', () => { diff --git a/packages/maple/docs/limits.md b/packages/maple/docs/limits.md index 357ff14..1cff022 100644 --- a/packages/maple/docs/limits.md +++ b/packages/maple/docs/limits.md @@ -24,8 +24,14 @@ responsibility (see [Security](security.md#out-of-scope)). Wherever Maple walks a value recursively, it refuses one nested deeper than 1000 levels instead of overflowing the host stack: when copying values across the [boundary](boundary.md), in -`JSON.stringify` and in `JSON.parse` with a reviver, and when converting a nested array to a string -or flattening it. Inside the script this is a catchable `RangeError`. +`JSON.stringify` and in `JSON.parse` with a reviver, when converting a nested array to a string or +flattening it, and when converting an error whose `name` or `message` holds another error. Inside +the script this is a catchable `RangeError`. + +Converting an array cuts a cycle back to it with an empty string, as JavaScript does, even where +the cycle runs through an error's `name` or `message`. Nothing cuts a cycle from one error to +another, such as an error whose `name` is the error itself, so converting it fails with +JavaScript's `RangeError: Maximum call stack size exceeded`. The host itself running out of stack is not a script error: the run ends with `INTERNAL_ERROR`, and no `catch` sees it. diff --git a/packages/maple/src/builtins/array/array.spec.ts b/packages/maple/src/builtins/array/array.spec.ts index 30661b0..26a8391 100644 --- a/packages/maple/src/builtins/array/array.spec.ts +++ b/packages/maple/src/builtins/array/array.spec.ts @@ -269,6 +269,13 @@ describe('array methods', () => { expect(run('return [1, 2].join();')).toBe('1,2'); expect(run('return [{ toString: "x" }, () => 1].join();')).toBe('[object Object],[function]'); expect(run('return [1, [2]].toString();')).toBe('1,2'); + expect(run('return [1, , 3].join();')).toBe('1,,3'); + }); + + it('join an array that has its own map, which the conversion does not use', () => { + expect( + run('const a = [1, 2]; a.map = 1; return [a.join("-"), String(a), [a].toString(), `${a}`];') + ).toEqual(['1-2', '1,2', '1,2', '1,2']); }); it('mutate script arrays', () => { diff --git a/packages/maple/src/builtins/object/object.spec.ts b/packages/maple/src/builtins/object/object.spec.ts index 2a6ffec..193b44b 100644 --- a/packages/maple/src/builtins/object/object.spec.ts +++ b/packages/maple/src/builtins/object/object.spec.ts @@ -298,6 +298,26 @@ describe('Object.is, getOwnPropertyNames, freeze and isFrozen', () => { ).toBe("Cannot delete property '0' of #"); }); + it('refuses deleting from a frozen error that does not convert, as a script error', () => { + expect( + run(` + const e = new Error("m"); + e.name = e; + e.x = 1; + Object.freeze(e); + const ran = []; + try { + delete e.x; + } catch (t) { + ran.push(t.message); + } finally { + ran.push("finally"); + } + return ran; + `) + ).toEqual(["Cannot delete property 'x' of [object Error]", 'finally']); + }); + it('still lets a delete of a fixed property that freeze did not lock change nothing', () => { expect(run('const f = () => 1; return delete f.name;')).toBe(true); expect(run('return [delete Object.freeze(new Map()).size, delete /a/.source];')).toEqual([ @@ -351,6 +371,14 @@ describe('Object.seal and isSealed', () => { ); }); + it('refuse deleting from a sealed error that does not convert, as a script error', () => { + expect( + run( + 'const e = new Error("m"); e.name = e; e.x = 1; Object.seal(e); const ran = []; try { delete e.x; } catch (t) { ran.push(t.message); } finally { ran.push("finally"); } return ran;' + ) + ).toEqual(["Cannot delete property 'x' of [object Error]", 'finally']); + }); + it('seal Maple own values, which keep working', () => { expect( run( diff --git a/packages/maple/src/builtins/string/methods/concat.ts b/packages/maple/src/builtins/string/methods/concat.ts index 368b5c0..470f64b 100644 --- a/packages/maple/src/builtins/string/methods/concat.ts +++ b/packages/maple/src/builtins/string/methods/concat.ts @@ -3,5 +3,5 @@ import { guardHostErrors, method, type Method } from '../../../values/native.ts' export const STRING_METHOD_CONCAT: Method = method( String.prototype.concat.length, - (s, args, { fail }) => guardHostErrors(fail, () => s.concat(...args.map(toString))) + (s, args, { fail }) => guardHostErrors(fail, () => s.concat(...args.map((arg) => toString(arg)))) ); diff --git a/packages/maple/src/errors.spec.ts b/packages/maple/src/errors.spec.ts index c8986b0..74c40a1 100644 --- a/packages/maple/src/errors.spec.ts +++ b/packages/maple/src/errors.spec.ts @@ -8,6 +8,11 @@ function run(code: string, data?: unknown): unknown { return maple.run(code, data).value; } +/** Script that leaves `e` as the last of `length` errors, each named by the one before. */ +function errorChain(length: number): string { + return `let e = new Error("m"); for (let i = 1; i < ${length}; i++) { const n = new Error("m"); n.name = e; e = n; }`; +} + describe('errors', () => { describe('throw', () => { it('surfaces an uncaught error object as RUNTIME_ERROR', () => { @@ -282,6 +287,70 @@ describe('errors', () => { }); }); +describe('an error whose name or message leads back to it', () => { + it('fails to convert with the RangeError JavaScript gives, which the script can catch', () => { + const caught = (setup: string, conversion: string): unknown => + run( + `const e = new Error("m"); ${setup} try { ${conversion}; } catch (t) { return [t.name, t.message]; }` + ); + const rangeError = ['RangeError', 'Maximum call stack size exceeded']; + + expect(caught('e.name = e;', 'String(e)')).toEqual(rangeError); + expect(caught('e.message = e;', '"" + e')).toEqual(rangeError); + expect(caught('e.name = e;', '`${e}`')).toEqual(rangeError); + expect(caught('e.name = e;', 'e.toString()')).toEqual(rangeError); + expect(caught('e.name = e;', 'new Error(e)')).toEqual(rangeError); + expect(caught('const f = new Error("f"); e.name = f; f.name = e;', 'String(e)')).toEqual( + rangeError + ); + expect( + run( + 'const e = new Error("m"); e.name = e; try { String(e); } catch {} e.name = "X"; return String(e);' + ) + ).toBe('X: m'); + }); + + it('converts through an array, which cuts the cycle as JavaScript does', () => { + expect(run('const e = new Error("m"); e.name = [e]; return String(e);')).toBe('m: m'); + expect(run('const e = new Error("m"); e.message = [e, 1]; return String(e);')).toBe( + 'Error: Error,1' + ); + expect( + run( + 'const l = [1]; const e = new Error("m"); e.name = l; l.push(e); return [String(l), String(e)];' + ) + ).toEqual(['1,m', '1,m: m']); + }); + + it('converts a chain of errors up to the nesting limit', () => { + expect(run(`${errorChain(1000)} return String(e);`)).toBe(`Error: m${': m'.repeat(999)}`); + expect(run(`${errorChain(1001)} try { String(e); } catch (t) { return t.name; }`)).toBe( + 'RangeError' + ); + }); + + it('is named by its tag in the messages of other errors', () => { + expect( + run( + 'const e = new Error("m"); e.name = e; try { Array.from([1], e); } catch (t) { return t.message; }' + ) + ).toBe('[object Error] is not a function'); + }); + + it('still reaches the host, thrown or returned', () => { + expect( + runAndCatch(maple, 'const e = new Error("m"); e.name = e; throw e;').sandboxError + ).toMatchObject({ + name: 'Error', + message: 'm' + }); + expect(run('const e = new Error("m"); e.name = e; return e;')).toMatchObject({ + name: 'Error', + message: 'm' + }); + }); +}); + describe('error additions', () => { it('give errors a toString method', () => { expect(run('return new TypeError("bad").toString();')).toBe('TypeError: bad'); diff --git a/packages/maple/src/values/arguments.ts b/packages/maple/src/values/arguments.ts index 7c8c195..f12dd1d 100644 --- a/packages/maple/src/values/arguments.ts +++ b/packages/maple/src/values/arguments.ts @@ -33,7 +33,7 @@ export function localesArgument(value: unknown): string | Array | undefi if (value === undefined) { return undefined; } - return Array.isArray(value) ? value.map(toString) : toString(value); + return Array.isArray(value) ? value.map((item) => toString(item)) : toString(value); } type IntlOptions = Intl.CollatorOptions & Intl.DateTimeFormatOptions & Intl.NumberFormatOptions; diff --git a/packages/maple/src/values/coercion.spec.ts b/packages/maple/src/values/coercion.spec.ts index ea56795..9957931 100644 --- a/packages/maple/src/values/coercion.spec.ts +++ b/packages/maple/src/values/coercion.spec.ts @@ -61,6 +61,28 @@ describe('coercion', () => { expect(() => toString([nested])).toThrow(UnplacedError); }); + + it('cuts a cycle through an array even where it runs through an error, as the host does', () => { + const error = new SandboxError('Error', 'm'); + error.name = [error]; + expect(toString(error)).toBe('m: m'); + + const list: Array = [1]; + const named = new SandboxError('Error', 'm'); + named.name = list; + list.push(named); + expect([toString(list), toString(named)]).toEqual(['1,m', '1,m: m']); + }); + + it('refuses a cycle between errors with a RangeError for the evaluator to place', () => { + const error = new SandboxError('Error', 'm'); + error.name = error; + + expect(() => toString(error)).toThrow( + expect.objectContaining({ name: 'RangeError', message: 'Maximum call stack size exceeded' }) + ); + expect(() => toString(error)).toThrow(UnplacedError); + }); }); describe('describeValue', () => { @@ -81,6 +103,10 @@ describe('coercion', () => { expect(describeValue(Object.create(null))).toBe('[object Object]'); expect(describeValue(nested)).toBe('[object Array]'); expect(describeValue(error)).toBe('[object Error]'); + + const cyclic = new SandboxError('Error', 'm'); + cyclic.message = cyclic; + expect(describeValue(cyclic)).toBe('[object Error]'); }); }); diff --git a/packages/maple/src/values/coercion.ts b/packages/maple/src/values/coercion.ts index d0ef70d..6fd2864 100644 --- a/packages/maple/src/values/coercion.ts +++ b/packages/maple/src/values/coercion.ts @@ -21,11 +21,61 @@ export const ARGUMENTS_PLACEHOLDER = '[object Arguments]'; /** The ToPrimitive hint; only a Date converts differently for `number` than for the others. */ export type PrimitiveHint = 'default' | 'number' | 'string'; +/** + * What one conversion is in the middle of, handed down to the conversions nested in it: the + * arrays it is joining, so a cycle through one is cut as JavaScript cuts it, and how deep it + * converts errors inside one another. A conversion started from outside makes a new one. + */ +export class Conversion { + readonly #joining = new Set>(); + + #errorDepth = 0; + + /** Joins `array` with `run`, or gives `''` for an array this conversion is already joining. */ + public join(array: ReadonlyArray, run: () => string): string { + if (this.#joining.has(array)) { + return ''; + } + if (this.#joining.size >= MAX_NESTING) { + throw new UnplacedError( + 'RangeError', + `An array nested deeper than ${MAX_NESTING} levels cannot be converted to a string` + ); + } + this.#joining.add(array); + try { + return run(); + } finally { + this.#joining.delete(array); + } + } + + /** + * Converts an error with `run`. Nothing cuts a cycle through an error's `name` or `message`, so + * one runs into the limit and fails as JavaScript fails when it runs out of stack. + */ + public convertError(run: () => T): T { + if (this.#errorDepth >= MAX_NESTING) { + throw new UnplacedError('RangeError', 'Maximum call stack size exceeded'); + } + this.#errorDepth++; + try { + return run(); + } finally { + this.#errorDepth--; + } + } +} + /** * ToPrimitive without `toString` / `valueOf` hooks: a plain object is always `[object Object]`, * an array always its joined elements, so a script cannot influence a conversion. */ -export function toPrimitive(value: unknown, hint: PrimitiveHint = 'default'): Primitive { +export function toPrimitive( + value: unknown, + hint: PrimitiveHint = 'default', + conversion?: Conversion +): Primitive { if ( value === undefined || value === null || @@ -42,10 +92,10 @@ export function toPrimitive(value: unknown, hint: PrimitiveHint = 'default'): Pr throw new Error(`A ${typeof value} reached the sandbox`); } if (value instanceof SandboxObject) { - return value.toPrimitive(hint); + return value.toPrimitive(hint, conversion); } if (Array.isArray(value)) { - return joinArray(value, new Set()); + return joinArray(value, conversion ?? new Conversion()); } if (isArgumentsObject(value)) { return ARGUMENTS_PLACEHOLDER; @@ -56,8 +106,8 @@ export function toPrimitive(value: unknown, hint: PrimitiveHint = 'default'): Pr return OBJECT_PLACEHOLDER; } -export function toString(value: unknown): string { - const primitive = toPrimitive(value, 'string'); +export function toString(value: unknown, conversion?: Conversion): string { + const primitive = toPrimitive(value, 'string', conversion); return typeof primitive === 'string' ? primitive : String(primitive); } @@ -182,31 +232,21 @@ function kindOf(value: unknown): string { /** `Array.prototype.join` with the hook-free element conversion; nested arrays join with commas. */ export function joinValues(array: ReadonlyArray, separator: string): string { - return joinArray(array, new Set(), separator); + return joinArray(array, new Conversion(), separator); } -/** Nullish elements are empty and a cycle is cut with an empty string, as the host does it. */ -function joinArray(array: ReadonlyArray, seen: Set, separator = ','): string { - if (seen.has(array)) { - return ''; - } - if (seen.size >= MAX_NESTING) { - throw new UnplacedError( - 'RangeError', - `An array nested deeper than ${MAX_NESTING} levels cannot be converted to a string` - ); - } - seen.add(array); - const parts: Array = []; - for (const element of array) { - if (element === null || element === undefined) { - parts.push(''); - } else if (Array.isArray(element)) { - parts.push(joinArray(element, seen)); - } else { - parts.push(toString(element)); +/** + * Nullish elements are empty, and an array the conversion is already joining is empty too, as the + * host does it, even when the cycle runs through an error's `name` or `message`. + */ +function joinArray(array: ReadonlyArray, conversion: Conversion, separator = ','): string { + return conversion.join(array, () => { + // By index, since a script can give its array an own `map` that would shadow the host's + const parts: Array = []; + for (let index = 0; index < array.length; index++) { + const element = array[index]; + parts.push(element === null || element === undefined ? '' : toString(element, conversion)); } - } - seen.delete(array); - return parts.join(separator); + return parts.join(separator); + }); } diff --git a/packages/maple/src/values/sandbox-error.ts b/packages/maple/src/values/sandbox-error.ts index 9798326..77e3189 100644 --- a/packages/maple/src/values/sandbox-error.ts +++ b/packages/maple/src/values/sandbox-error.ts @@ -1,5 +1,5 @@ import type { Node } from 'acorn'; -import { type Primitive, toString } from './coercion.ts'; +import { Conversion, type Primitive, type PrimitiveHint, toString } from './coercion.ts'; import { type FixedProperties, SandboxObject } from './sandbox-object.ts'; /** `name` and `message` keep any value, as in JavaScript; converting the error reads them. */ @@ -63,18 +63,24 @@ export class SandboxError extends SandboxObject { /** * What JavaScript's `Error.prototype.toString` gives: an `undefined` name reads as `Error` and an * `undefined` message as empty, and an empty part leaves out the `: `. Converting an assigned - * value can throw, as it can in JavaScript. + * value can throw, as it can in JavaScript, and so can a `name` or `message` that leads back to + * the error. */ - public override toPrimitive(): Primitive { - const name = this.name === undefined ? 'Error' : toString(this.name); - const message = this.message === undefined ? '' : toString(this.message); - if (name === '') { - return message; - } - if (message === '') { - return name; - } - return `${name}: ${message}`; + public override toPrimitive( + _hint: PrimitiveHint, + conversion: Conversion = new Conversion() + ): Primitive { + return conversion.convertError(() => { + const name = this.name === undefined ? 'Error' : toString(this.name, conversion); + const message = this.message === undefined ? '' : toString(this.message, conversion); + if (name === '') { + return message; + } + if (message === '') { + return name; + } + return `${name}: ${message}`; + }); } /** The fixed part of the error as the host receives it; the boundary adds `properties`. */ diff --git a/packages/maple/src/values/sandbox-object.ts b/packages/maple/src/values/sandbox-object.ts index 2a3dd22..1ff8f19 100644 --- a/packages/maple/src/values/sandbox-object.ts +++ b/packages/maple/src/values/sandbox-object.ts @@ -1,4 +1,4 @@ -import { describeObject, type Primitive, type PrimitiveHint } from './coercion.ts'; +import { type Conversion, describeObject, type Primitive, type PrimitiveHint } from './coercion.ts'; import type { MemberContext, MethodLookup } from './members.ts'; import { type Attributes, @@ -81,8 +81,11 @@ export abstract class SandboxObject { return this.isShared ? runState.getOwnPropertiesOf(this) : this.properties; } - /** ToPrimitive without hooks, so a script cannot influence a conversion: `[object Map]`. */ - public toPrimitive(_hint: PrimitiveHint): Primitive { + /** + * ToPrimitive without hooks, so a script cannot influence a conversion: `[object Map]`. A value + * whose conversion converts others, as an error does, hands them `conversion`. + */ + public toPrimitive(_hint: PrimitiveHint, _conversion?: Conversion): Primitive { return `[object ${this.tag}]`; }