Repository: endojs/endo
Pull request #2417 (2024-08-19 19:02:13)

feat(ses): shim ArrayBuffer.prototype.transfer
Staged on #2419 

Closes: #XXXX
Refs: https://github.com/endojs/endo/pull/2414 #2418 #2419 

## Description

#2414  by itself does not work on Node 18 and Node 20 because
- those platforms do not have `Array.prototype.transfer`, so #2414 must use `structuredClone` instead
- `structuredClone` does exist on Node >= 18, so it should be on supported platforms (though see #2418 ). However, `structuredClone` itself is dangerous and so must not be added to the shared intrinsics. As a result, in #2414 , when `@endo/pass-style` is initialized in a created compartment, it fails to find either `Array.prototype.transfer` and `structuredClone

To solve this, @kriskowal suggested that we also shim `Array.prototype.transfer` if needed during `lockdown`, along with other repairs. We are avoiding similarly shimming `Array.prototype.transferToImmutable` because it is not yet standard. But `Array.prototype.transfer` is standard, and so `lockdown` can globally shim it before hardening the shared intrinsics.

This PR implements @kriskowal 's suggestion.

### Security Considerations

none
### Scaling Considerations

by itself, none
### Documentation Considerations

nothing signicant.
### Testing Considerations

See #2418 . Aside from that, none
### Compatibility and Upgrade Considerations

On platforms with neither `Array.prototype.transfer` nor a global `structuredClone`, the ses-shim will *currently* not install an emulation of `Array.prototype.transfer`. However, once we verify that endo is not intended to support platforms without both, we may change lockdown to throw, failing to lock down.

erights: See https://github.com/endojs/endo/issues/2418

The platform-compatability-test tests are not marked as required, so solving #2418 does not actually block this PR. But it would be nice to get that fixed first.
erights: PTAL
erights: > Please consider removing the unnecessary `slice` call as brand check, and improve support for when the new length doesn't match.

Done

Patch:
diff --git a/packages/ses/NEWS.md b/packages/ses/NEWS.md
index 730956fbf..535ebbbaf 100644
--- a/packages/ses/NEWS.md
+++ b/packages/ses/NEWS.md
@@ -1,5 +1,18 @@
 User-visible changes in `ses`:
 
+# Next release
+
+- On platforms without
+  [`Array.prototype.transfer`](https://github.com/tc39/proposal-resizablearraybuffer)
+  but with a global `structuredClone`, the ses-shim's `lockdown` will now
+  install an emulation of `Array.prototype.transfer`. On platforms with neither,
+  the ses-shim will *currently* not install such an emulation.
+  However, once we verify that endo is not intended to support platforms
+  without both, we may change `lockdown` to throw, failing to lock down.
+  - XS and Node >= 22 already have `Array.prototype.transfer`.
+  - Node 18, Node 20, and all browsers have `structuredClone`
+  - Node <= 16 have neither, but are also no longer supported by Endo.
+
 # v1.8.0 (2024-08-27)
 
 - New `legacyRegeneratorRuntimeTaming: 'unsafe-ignore'` lockdown option to tame
diff --git a/packages/ses/src/commons.js b/packages/ses/src/commons.js
index 3d696f3a7..b4f8f6f39 100644
--- a/packages/ses/src/commons.js
+++ b/packages/ses/src/commons.js
@@ -19,6 +19,7 @@ export { universalThis as globalThis };
 
 export const {
   Array,
+  ArrayBuffer,
   Date,
   FinalizationRegistry,
   Float32Array,
@@ -34,6 +35,7 @@ export const {
   Set,
   String,
   Symbol,
+  Uint8Array,
   WeakMap,
   WeakSet,
 } = globalThis;
@@ -124,6 +126,7 @@ export const {
 } = Reflect;
 
 export const { isArray, prototype: arrayPrototype } = Array;
+export const { prototype: arrayBufferPrototype } = ArrayBuffer;
 export const { prototype: mapPrototype } = Map;
 export const { revocable: proxyRevocable } = Proxy;
 export const { prototype: regexpPrototype } = RegExp;
@@ -178,6 +181,15 @@ export const arraySome = uncurryThis(arrayPrototype.some);
 export const arraySort = uncurryThis(arrayPrototype.sort);
 export const iterateArray = uncurryThis(arrayPrototype[iteratorSymbol]);
 //
+export const arrayBufferSlice = uncurryThis(arrayBufferPrototype.slice);
+/** @type {(b: ArrayBuffer) => number} */
+export const arrayBufferGetByteLength = uncurryThis(
+  // @ts-expect-error we know it is there on all conforming platforms
+  getOwnPropertyDescriptor(arrayBufferPrototype, 'byteLength').get,
+);
+//
+export const typedArraySet = uncurryThis(typedArrayPrototype.set);
+//
 export const mapSet = uncurryThis(mapPrototype.set);
 export const mapGet = uncurryThis(mapPrototype.get);
 export const mapHas = uncurryThis(mapPrototype.has);
diff --git a/packages/ses/src/lockdown.js b/packages/ses/src/lockdown.js
index 1a2c6b272..170c341fb 100644
--- a/packages/ses/src/lockdown.js
+++ b/packages/ses/src/lockdown.js
@@ -56,6 +56,7 @@ import { tameHarden } from './tame-harden.js';
 import { tameSymbolConstructor } from './tame-symbol-constructor.js';
 import { tameFauxDataProperties } from './tame-faux-data-properties.js';
 import { tameRegeneratorRuntime } from './tame-regenerator-runtime.js';
+import { shimArrayBufferTransfer } from './shim-arraybuffer-transfer.js';
 
 /** @import {LockdownOptions} from '../types.js' */
 
@@ -284,6 +285,7 @@ export const repairIntrinsics = (options = {}) => {
   addIntrinsics(tameMathObject(mathTaming));
   addIntrinsics(tameRegExpConstructor(regExpTaming));
   addIntrinsics(tameSymbolConstructor());
+  addIntrinsics(shimArrayBufferTransfer());
 
   addIntrinsics(getAnonymousIntrinsics());
 
diff --git a/packages/ses/src/shim-arraybuffer-transfer.js b/packages/ses/src/shim-arraybuffer-transfer.js
new file mode 100644
index 000000000..1c3bc4120
--- /dev/null
+++ b/packages/ses/src/shim-arraybuffer-transfer.js
@@ -0,0 +1,86 @@
+import {
+  ArrayBuffer,
+  arrayBufferPrototype,
+  arrayBufferSlice,
+  arrayBufferGetByteLength,
+  Uint8Array,
+  typedArraySet,
+  globalThis,
+  TypeError,
+  defineProperty,
+} from './commons.js';
+
+export const shimArrayBufferTransfer = () => {
+  // @ts-expect-error TODO extend ArrayBuffer type to include transfer, etc.
+  if (typeof arrayBufferPrototype.transfer === 'function') {
+    // Assume already exists so does not need to be shimmed.
+    // Such conditional shimming is ok in this case since ArrayBuffer.p.transfer
+    // is already officially part of JS.
+    //
+    // Empty object because this shim has nothing for `addIntrinsics` to add.
+    return {};
+  }
+  const clone = globalThis.structuredClone;
+  if (typeof clone !== 'function') {
+    // On a platform with neither `Array.prototype.transfer`
+    // nor `structuredClone`, this shim does nothing.
+    // For example, Node <= 16 has neither.
+    //
+    // Empty object because this shim has nothing for `addIntrinsics` to add.
+    return {};
+    // TODO Rather than doing nothing, should the endo ses-shim throw
+    // in this case?
+    // throw TypeError(
+    //   `Can only shim missing ArrayBuffer.prototype.transfer on a platform with "structuredClone"`,
+    // );
+    // For example, endo no longer supports Node <= 16. All browsers have
+    // `structuredClone`. XS has `Array.prototype.transfer`. Are there still
+    // any platforms without both that Endo should still support?
+    // What about Hermes?
+  }
+
+  /**
+   * @type {ThisType<ArrayBuffer>}
+   */
+  const methods = {
+    /**
+     * @param {number} [newLength]
+     */
+    transfer(newLength = undefined) {
+      // Using this builtin getter also ensures that `this` is a genuine
+      // ArrayBuffer.
+      const oldLength = arrayBufferGetByteLength(this);
+      if (newLength === undefined || newLength === oldLength) {
+        return clone(this, { transfer: [this] });
+      }
+      if (typeof newLength !== 'number') {
+        throw TypeError(`transfer newLength if provided must be a number`);
+      }
+      if (newLength > oldLength) {
+        const result = new ArrayBuffer(newLength);
+        const taOld = new Uint8Array(this);
+        const taNew = new Uint8Array(result);
+        typedArraySet(taNew, taOld);
+        // Using clone only to detach, and only after the copy succeeds
+        clone(this, { transfer: [this] });
+        return result;
+      } else {
+        const result = arrayBufferSlice(this, 0, newLength);
+        // Using clone only to detach, and only after the slice succeeds
+        clone(this, { transfer: [this] });
+        return result;
+      }
+    },
+  };
+
+  defineProperty(arrayBufferPrototype, 'transfer', {
+    // @ts-expect-error
+    value: methods.transfer,
+    writable: true,
+    enumerable: false,
+    configurable: true,
+  });
+
+  // Empty object because this shim has nothing for `addIntrinsics` to add.
+  return {};
+};


Test patch:
diff --git a/packages/ses/test/shim-arraybuffer-transfer.test.js b/packages/ses/test/shim-arraybuffer-transfer.test.js
new file mode 100644
index 000000000..97e686552
--- /dev/null
+++ b/packages/ses/test/shim-arraybuffer-transfer.test.js
@@ -0,0 +1,67 @@
+/* global globalThis */
+import test from 'ava';
+import '../index.js';
+
+lockdown();
+
+// The purpose of this test is to see if Array.prototype.transfer works
+// correctly enough on platforms like Node 18 or Node 20 that don't yet have
+// it natively, and so are testing the shim on those. On platforms where
+// Array.prototype.transfer is present, like Node 22,
+// we also run the same tests. Thus,
+// this test only tests the intersection behavior of the standard and
+// the shim.
+
+test('ArrayBuffer.p.transfer', t => {
+  const abX = new ArrayBuffer(3);
+  t.is(abX.byteLength, 3);
+  const taX = new Uint8Array(abX);
+  t.is(taX[2], 0);
+  t.is(taX[3], undefined);
+  taX[0] = 10;
+  taX[1] = 11;
+  taX[2] = 12;
+  t.is(taX[0], 10);
+  t.is(taX[1], 11);
+  t.is(taX[2], 12);
+  t.is(taX[3], undefined);
+
+  if (!('transfer' in ArrayBuffer.prototype)) {
+    t.false('structuredClone' in globalThis);
+    // Currently, shim-arraybuffer-transfer.shim, when run on a platform
+    // with neither `Array.prototype.transfer` nor `structuredClone` does
+    // not shim `Array.prototype.transfer`. Thus, we currently do not
+    // consider this absence to be a non-conformance to the endo ses-shim.
+    return;
+  }
+
+  // because this test must run on platforms prior to
+  // ArrayBuffer.prototype.detached, we test detachment by other means.
+
+  const abY = abX.transfer();
+  t.is(abY.byteLength, 3);
+  t.is(abX.byteLength, 0);
+  const taY = new Uint8Array(abY);
+  t.is(taX[2], undefined);
+  t.is(taY[2], 12);
+
+  const abZ = abY.transfer(2);
+  t.is(abY.byteLength, 0);
+  t.is(abZ.byteLength, 2);
+  const taZ = new Uint8Array(abZ);
+  t.is(taY[2], undefined);
+  t.is(taZ[0], 10);
+  t.is(taZ[1], 11);
+  t.is(taZ[2], undefined);
+
+  const abW = abZ.transfer(4);
+  t.is(abZ.byteLength, 0);
+  t.is(abW.byteLength, 4);
+  const taW = new Uint8Array(abW);
+  t.is(taZ[2], undefined);
+  t.is(taW[0], 10);
+  t.is(taW[1], 11);
+  t.is(taW[2], 0);
+  t.is(taW[3], 0);
+  t.is(taW[4], undefined);
+});