Make converting a cyclic error a catchable RangeError

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.        |
 

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', () => {

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.

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', () => {

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 #<Object>");
   });
 
+  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(

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<string> = 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))))
 );

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');

packages/maple/src/values/arguments.ts

@@ -33,7 +33,7 @@ export function localesArgument(value: unknown): string | Array<string> | 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;

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<unknown> = [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]');
     });
   });
 

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<ReadonlyArray<unknown>>();
+
+  #errorDepth = 0;
+
+  /** Joins `array` with `run`, or gives `''` for an array this conversion is already joining. */
+  public join(array: ReadonlyArray<unknown>, 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<T>(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<unknown>, 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<unknown>, seen: Set<object>, 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<string> = [];
-  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<unknown>, 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<string> = [];
+    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);
+  });
 }