Skip to content
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

feat(ses): ArrayBuffer.p.transferToImmutable #2311

Closed
wants to merge 12 commits into from
8 changes: 7 additions & 1 deletion packages/ses/NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ User-visible changes in `ses`:
All use of `Compartment` should migrate to use this option as the standard
behavior will be enabled by default with the next major version of SES.

- Adds `ArrayBuffer.p.immutable` and `ArrayBuffer.p.transferToImmutable` as a shim for a future proposal. It makes an ArrayBuffer-like object whose contents cannot be mutated. However, due to limitations of the shim
- Unlike `ArrayBuffer` and `SharedArrayBuffer` this shim's ArrayBuffer-like object cannot be transfered or cloned between JS threads.
- Unlike `ArrayBuffer` and `SharedArrayBuffer`, this shim's ArrayBuffer-like object cannot be used as the backing store of TypeArrays or DataViews.
- On Node >= 21 we use the builtin `transferToFixed` to transfer exclusive access to the array buffer contents. On Node <= 20, we emulate `transferToFixedLength` with `structuredClone`. On platforms with neither `transferToFixedLength` nor `structuredClone`, we use `slice` to copy the contents, but have no way to detach the original.
- Even after the upcoming `transferToImmutable` proposal is implemented by the platform, the current code will still replace it with the shim implementation, in accord with shim best practices. See https://github.com/endojs/endo/pull/2311#discussion_r1632607527 . It will require a later manual step to delete the shim, after manual analysis of the compat implications.

- Adds support for module descriptors better aligned with XS.
Compartments use module desriptors to load and link modules.
The importHook, importNowHook, and moduleMapHook all return module descriptors
Expand Down Expand Up @@ -60,7 +66,7 @@ User-visible changes in `ses`:
in TypeScript),
the stacktrace line-numbers point back into the original
source, as they do on Node without SES.

# v1.5.0 (2024-05-06)

- Adds `importNowHook` to the `Compartment` options. The compartment will invoke the hook whenever it encounters a missing dependency while running `compartmentInstance.importNow(specifier)`, which cannot use an asynchronous `importHook`.
Expand Down
85 changes: 85 additions & 0 deletions packages/ses/src/commons.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export { universalThis as globalThis };

export const {
Array,
erights marked this conversation as resolved.
Show resolved Hide resolved
ArrayBuffer,
Date,
FinalizationRegistry,
Float32Array,
Expand Down Expand Up @@ -124,6 +125,7 @@ export const {
} = Reflect;

export const { isArray, prototype: arrayPrototype } = Array;
export const { isView, prototype: arrayBufferPrototype } = ArrayBuffer;
export const { prototype: mapPrototype } = Map;
export const { revocable: proxyRevocable } = Proxy;
export const { prototype: regexpPrototype } = RegExp;
Expand Down Expand Up @@ -174,6 +176,89 @@ export const arraySome = uncurryThis(arrayPrototype.some);
export const arraySort = uncurryThis(arrayPrototype.sort);
export const iterateArray = uncurryThis(arrayPrototype[iteratorSymbol]);
//
export const arrayBufferSlice = uncurryThis(arrayBufferPrototype.slice);
const { asUintN: bigIntAsUintN } = BigInt;
const { structuredClone } = globalThis;
export const arrayBufferTransferToFixedLength =
// @ts-expect-error absent from Node <= 20 and not yet in TS type
// eslint-disable-next-line no-nested-ternary
arrayBufferPrototype.transferToFixedLength
? // @ts-expect-error absent from Node <= 20 and not yet in TS type
uncurryThis(arrayBufferPrototype.transferToFixedLength)
: structuredClone
? /**
* @param {ArrayBuffer} arrayBuffer
* @param {number} newLength
* @returns {ArrayBuffer}
*/
(arrayBuffer, newLength = arrayBuffer.byteLength) => {
// There is no `transferToFixedLength` on Node <= 20, but there
// is web-standard `structuredClone` on Node >= 17, on all modern
// browsers, and on many other JS platforms.
// In those cases, we first use `structuredClone` to get a fresh
// buffer with exclusive access to the underlying data, while
// detaching it from the original `arrayBuffer`.

newLength = +newLength;
bigIntAsUintN(newLength, 0n);

const newBuffer = /** @type {ArrayBuffer} */ (
structuredClone(arrayBuffer, {
transfer: [arrayBuffer],
})
);
if (newLength >= newBuffer.byteLength) {
return newBuffer;
}
// If the requested length is shorter than the length of `buffer`,
// we use `slice` to shorted the returned result, but at the cost
// of an extra copy.
//
// `slice` accepts negative arguments but `transferToFixedLength`
// does not...
// get at the underlying ToIndex operation through `BigInt.asUintN`
// (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
// and ToNumber through unary `+` (rather than `Number(newLength)`,
// which fails to reject BigInts).
//
// On platforms like Node 20
// - without`tranferToFixedLength` or `transfer`
// - with `structuredClone`
// - with `resize`
//
// It might seem like we could avoid the extra copy by
// `newBuffer.resize(newLength)`. But `structuredClone`
// makes ArrayBuffers that are not resizable.
Comment on lines +229 to +231
Copy link
Contributor

@gibson042 gibson042 Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They seem resizable to me:

$ node -e '
  console.log("Node.js", process.version);
  const buf1 = new ArrayBuffer(10, { maxByteLength: 42 });
  const buf2 = structuredClone(buf1, { transfer: [buf1] });
  console.log({ buf1, resizable: buf1.resizable });
  console.log({ buf2, resizable: buf2.resizable });
  buf2.resize(5);
  console.log({ buf2, resizable: buf2.resizable });
'
Node.js v20.14.0
{ buf1: ArrayBuffer { (detached), byteLength: 0 }, resizable: true }
{
  buf2: ArrayBuffer {
    [Uint8Contents]: <00 00 00 00 00 00 00 00 00 00>,
    byteLength: 10
  },
  resizable: true
}
{
  buf2: ArrayBuffer { [Uint8Contents]: <00 00 00 00 00>, byteLength: 5 },
  resizable: true
}

return arrayBufferSlice(newBuffer, 0, newLength);
}
Comment on lines +194 to +233
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(arrayBuffer, newLength = arrayBuffer.byteLength) => {
// There is no `transferToFixedLength` on Node <= 20, but there
// is web-standard `structuredClone` on Node >= 17, on all modern
// browsers, and on many other JS platforms.
// In those cases, we first use `structuredClone` to get a fresh
// buffer with exclusive access to the underlying data, while
// detaching it from the original `arrayBuffer`.
newLength = +newLength;
bigIntAsUintN(newLength, 0n);
const newBuffer = /** @type {ArrayBuffer} */ (
structuredClone(arrayBuffer, {
transfer: [arrayBuffer],
})
);
if (newLength >= newBuffer.byteLength) {
return newBuffer;
}
// If the requested length is shorter than the length of `buffer`,
// we use `slice` to shorted the returned result, but at the cost
// of an extra copy.
//
// `slice` accepts negative arguments but `transferToFixedLength`
// does not...
// get at the underlying ToIndex operation through `BigInt.asUintN`
// (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
// and ToNumber through unary `+` (rather than `Number(newLength)`,
// which fails to reject BigInts).
//
// On platforms like Node 20
// - without`tranferToFixedLength` or `transfer`
// - with `structuredClone`
// - with `resize`
//
// It might seem like we could avoid the extra copy by
// `newBuffer.resize(newLength)`. But `structuredClone`
// makes ArrayBuffers that are not resizable.
return arrayBufferSlice(newBuffer, 0, newLength);
}
(arrayBuffer, newLength = arrayBufferByteLength(arrayBuffer)) => {
// There is no `transferToFixedLength` on Node <= 20, but there
// is web-standard `structuredClone` on Node >= 17, on all modern
// browsers, and on many other JS platforms.
// In those cases, we first use `structuredClone` to get a fresh
// buffer with exclusive access to the underlying data, while
// detaching it from the original `arrayBuffer`.
// Before looking at actual arguments, verify that the input is an
// ArrayBuffer.
arrayBufferByteLength(arrayBuffer);
// Calculate ToIndex(newLengthAsNumber) using `BigInt.asUintN`,
// first getting newLengthAsNumber as ToNumber(newLength) using unary `+`
// (rather than `Number(newLength)`, which fails to reject BigInts).
newLength = +newLength;
bigIntAsUintN(newLength, 0n);
const newBuffer = /** @type {ArrayBuffer} */ (
structuredClone(arrayBuffer, {
transfer: [arrayBuffer],
})
);
// We might already have what we need.
// NOTE: The check for resizability is necessary to correctly emulate
// ArrayBuffer.prototype.transferToFixedLength, but a more narrow
// function specialized to e.g. transferToImmutable could skip it.
if (newLength === arrayBufferByteLength(newBuffer) && !arrayBufferResizable(newBuffer)) {
return newBuffer;
}
// We might be able to copy some or all of the contents.
if (newLength <= arrayBufferByteLength(newBuffer)) {
const copied = arrayBufferSlice(newBuffer, 0, newLength);
const copiedLength = arrayBufferByteLength(copied);
if (copiedLength !== newLength) {
throw RangeError(`internal: length ${copiedLength} should have been ${newLength}`);
}
return copied;
}
// We need a bigger boat.
// NOTE: Uint8Array must be extracted from globalThis.
const view = new Uint8Array(newLength);
typedArraySet(view, new Uint8Array(newBuffer));
return typedArrayBuffer(view);

: /**
* @param {ArrayBuffer} arrayBuffer
* @param {number} newLength
* @returns {ArrayBuffer}
*/
(arrayBuffer, newLength = arrayBuffer.byteLength) => {
// There is no `transferToFixedLength` on Node <= 20,
// and no `structuredClone` on Node <= 17 and possibly on some
// non-browser JavaScript platforms.
// In those cases,
// and assuming the absence of `transfer`, we cannot detach
// the original, but we must still produce a new fresh buffer with
// exclusive mutability of its underlying state. We use `slice`
// both to make this exclusive copy and size it appropriately.

// `slice` accepts negative arguments but `transferToFixedLength`
// does not...
// get at the underlying ToIndex operation through `BigInt.asUintN`
// (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
// and ToNumber through unary `+` (rather than `Number(newLength)`,
// which fails to reject BigInts).
newLength = +newLength;
bigIntAsUintN(newLength, 0n);

const newBuffer = arrayBufferSlice(arrayBuffer, 0, newLength);
return newBuffer;
};
Comment on lines +239 to +260
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(arrayBuffer, newLength = arrayBuffer.byteLength) => {
// There is no `transferToFixedLength` on Node <= 20,
// and no `structuredClone` on Node <= 17 and possibly on some
// non-browser JavaScript platforms.
// In those cases,
// and assuming the absence of `transfer`, we cannot detach
// the original, but we must still produce a new fresh buffer with
// exclusive mutability of its underlying state. We use `slice`
// both to make this exclusive copy and size it appropriately.
// `slice` accepts negative arguments but `transferToFixedLength`
// does not...
// get at the underlying ToIndex operation through `BigInt.asUintN`
// (avoiding the redundant allocation of e.g. `ArrayBuffer(newLength)`)
// and ToNumber through unary `+` (rather than `Number(newLength)`,
// which fails to reject BigInts).
newLength = +newLength;
bigIntAsUintN(newLength, 0n);
const newBuffer = arrayBufferSlice(arrayBuffer, 0, newLength);
return newBuffer;
};
(arrayBuffer, newLength = arrayBuffer.byteLength) => {
// There is no `transferToFixedLength` on Node <= 20,
// and no `structuredClone` on Node <= 17 and possibly on some
// non-browser JavaScript platforms.
// In those cases,
// and assuming the absence of `transfer`, we cannot detach
// the original, but we must still produce a new fresh buffer with
// exclusive mutability of its underlying state.
// Before looking at actual arguments, verify that the input is an
// ArrayBuffer.
const srcLength = arrayBufferByteLength(arrayBuffer);
// Calculate ToIndex(newLengthAsNumber) using `BigInt.asUintN`,
// first getting newLengthAsNumber as ToNumber(newLength) using unary `+`
// (rather than `Number(newLength)`, which fails to reject BigInts).
newLength = +newLength;
bigIntAsUintN(newLength, 0n);
// We might be able to copy some or all of the contents.
if (newLength <= srcLength) {
const copied = arrayBufferSlice(arrayBuffer, 0, newLength);
const copiedLength = arrayBufferByteLength(copied);
if (copiedLength !== newLength) {
throw RangeError(`internal: length ${copiedLength} should have been ${newLength}`);
}
return copied;
}
// We need a bigger boat.
// NOTE: Uint8Array must be extracted from globalThis.
const view = new Uint8Array(newLength);
typedArraySet(view, new Uint8Array(arrayBuffer));
return typedArrayBuffer(view);
};


export const mapSet = uncurryThis(mapPrototype.set);
export const mapGet = uncurryThis(mapPrototype.get);
export const mapHas = uncurryThis(mapPrototype.has);
Expand Down
11 changes: 11 additions & 0 deletions packages/ses/src/get-anonymous-intrinsics.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
matchAllSymbol,
regexpPrototype,
globalThis,
ArrayBuffer,
} from './commons.js';
import { InertCompartment } from './compartment.js';

Expand Down Expand Up @@ -157,5 +158,15 @@ export const getAnonymousIntrinsics = () => {
);
}

const ab = new ArrayBuffer(0);
// @ts-expect-error TODO How do I add transferToImmutable to ArrayBuffer type?
// eslint-disable-next-line @endo/no-polymorphic-call
const iab = ab.transferToImmutable();
const iabProto = getPrototypeOf(iab);
if (iabProto !== ArrayBuffer.prototype) {
// In a native implementation, these will be the same prototype
intrinsics['%ImmutableArrayBufferPrototype%'] = iabProto;
}

return intrinsics;
};
115 changes: 115 additions & 0 deletions packages/ses/src/immutable-array-buffer-shim.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import {
setPrototypeOf,
defineProperties,
arrayBufferSlice,
arrayBufferTransferToFixedLength,
arrayBufferPrototype,
getOwnPropertyDescriptors,
TypeError,
} from './commons.js';

if ('transferToImmutable' in arrayBufferPrototype) {
// If `transferToImmutable` already exists, it may be because the platform
// has implemented the proposal this shim emulates.
// See https://github.com/endojs/endo/pull/2311#discussion_r1632607527
// Conditional shims for non stage 4 features have been seen as problematic by
// the community as there is a risk that the program would start relying on a
// shim behavior that does not match what the final spec says, resulting in
// breakage when the engine starts implementing the feature.
// eslint-disable-next-line @endo/no-polymorphic-call
console.warn('Overriding existing transferToImmutable with shim');
}

/**
* This class only exists within the shim, as a convience for imperfectly
* emulating the proposal, which would not have this class. In the proposal,
* `transferToImmutable` makes a new `ArrayBuffer` that inherits from
* `ArrayBuffer.prototype` as you'd expect. In the shim, `transferToImmutable`
* makes a normal object that inherits from
* `ImmutableArrayBufferInternal.prototype`, which has been surgically
* altered to inherit from `ArrayBuffer.prototype`. The constructor is
* captured for use internal to this module, and is made otherwise inaccessible.
* Therefore, `ImmutableArrayBufferInternal.prototype` and all its methods
* and accessor functions effectively become hidden intrinsics.
*
* TODO handle them as hidden intrinsics, so they get hardened when they should.
*/
class ImmutableArrayBufferInternal {
/** @type {ArrayBuffer} */
#buffer;

constructor(buffer) {
// This also enforces that `buffer` is a genuine `ArrayBuffer`.
// This constructor is deleted from the prototype below.
this.#buffer = arrayBufferSlice(buffer, 0);
Comment on lines +42 to +44
Copy link
Contributor

@gibson042 gibson042 Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be slicing an ArrayBuffer that already came from detachment; full responsibility should fall on either transferToImmutable or ImmutableArrayBufferInternal but not both.

}

get byteLength() {
return this.#buffer.byteLength;
}

get detached() {
this.#buffer; // shim brand check
return false;
}

get maxByteLength() {
// Not underlying maxByteLength, which is irrelevant
return this.#buffer.byteLength;
}

get resizable() {
this.#buffer; // shim brand check
return false;
}

get immutable() {
this.#buffer; // shim brand check
return true;
}

slice(begin = 0, end = undefined) {
return arrayBufferSlice(this.#buffer, begin, end);
}

resize(_newByteLength = undefined) {
this.#buffer; // shim brand check
throw TypeError('Cannot resize an immutable ArrayBuffer');
}

transfer(_newLength = undefined) {
this.#buffer; // shim brand check
throw TypeError('Cannot detach an immutable ArrayBuffer');
}

transferToFixedLength(_newLength = undefined) {
this.#buffer; // shim brand check
throw TypeError('Cannot detach an immutable ArrayBuffer');
}

transferToImmutable(_newLength = undefined) {
this.#buffer; // shim brand check
throw TypeError('Cannot detach an immutable ArrayBuffer');
}
}

const ImmutableArrayBufferInternalPrototype =
ImmutableArrayBufferInternal.prototype;
// @ts-expect-error can only delete optionals
delete ImmutableArrayBufferInternalPrototype.constructor;

setPrototypeOf(ImmutableArrayBufferInternalPrototype, arrayBufferPrototype);

defineProperties(
arrayBufferPrototype,
getOwnPropertyDescriptors({
get immutable() {
return false;
},
transferToImmutable(newLength = undefined) {
return new ImmutableArrayBufferInternal(
arrayBufferTransferToFixedLength(this, newLength),
);
},
}),
);
1 change: 1 addition & 0 deletions packages/ses/src/lockdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import { makeCompartmentConstructor } from './compartment.js';
import { tameHarden } from './tame-harden.js';
import { tameSymbolConstructor } from './tame-symbol-constructor.js';
import { tameFauxDataProperties } from './tame-faux-data-properties.js';
import './immutable-array-buffer-shim.js';

/** @import {LockdownOptions} from '../types.js' */

Expand Down
25 changes: 25 additions & 0 deletions packages/ses/src/permits.js
Original file line number Diff line number Diff line change
Expand Up @@ -1283,6 +1283,31 @@ export const permitted = {
// https://github.com/tc39/proposal-arraybuffer-transfer
transferToFixedLength: fn,
detached: getter,
// https://github.com/endojs/endo/pull/2309#issuecomment-2155513240
// to be proposed
transferToImmutable: fn,
immutable: getter,
},

// If this exists, it is purely an artifact of how we currently shim
// `transferToImmutable`. As natively implemented, there would be no
// such extra prototype.
'%ImmutableArrayBufferPrototype%': {
'[[Proto]]': '%ArrayBufferPrototype%',
byteLength: getter,
slice: fn,
// See https://github.com/tc39/proposal-resizablearraybuffer
transfer: fn,
resize: fn,
resizable: getter,
maxByteLength: getter,
// https://github.com/tc39/proposal-arraybuffer-transfer
transferToFixedLength: fn,
detached: getter,
// https://github.com/endojs/endo/pull/2309#issuecomment-2155513240
// to be proposed
transferToImmutable: fn,
immutable: getter,
},

// SharedArrayBuffer Objects
Expand Down
91 changes: 91 additions & 0 deletions packages/ses/test/immutable-array-buffer.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import test from 'ava';
import '../index.js';

const { isFrozen, getPrototypeOf } = Object;

lockdown();

test('Immutable ArrayBuffer installed and hardened', t => {
const ab1 = new ArrayBuffer(0);
const iab = ab1.transferToImmutable();
const iabProto = getPrototypeOf(iab);
t.true(isFrozen(iabProto));
t.true(isFrozen(iabProto.slice));
});

test('Immutable ArrayBuffer ops', t => {
// Absent on Node <= 18
const canResize = 'maxByteLength' in ArrayBuffer.prototype;

const ab1 = new ArrayBuffer(2, { maxByteLength: 7 });
const ta1 = new Uint8Array(ab1);
ta1[0] = 3;
ta1[1] = 4;
const iab = ab1.transferToImmutable();
t.true(iab instanceof ArrayBuffer);
ta1[1] = 5;
const ab2 = iab.slice(0);
const ta2 = new Uint8Array(ab2);
t.is(ta1[1], undefined);
t.is(ta2[1], 4);
ta2[1] = 6;

const ab3 = iab.slice(0);
t.true(ab3 instanceof ArrayBuffer);

const ta3 = new Uint8Array(ab3);
t.is(ta1[1], undefined);
t.is(ta2[1], 6);
t.is(ta3[1], 4);

t.is(ab1.byteLength, 0);
t.is(iab.byteLength, 2);
t.is(ab2.byteLength, 2);

t.is(iab.maxByteLength, 2);
if (canResize) {
t.is(ab1.maxByteLength, 0);
t.is(ab2.maxByteLength, 2);
}

if ('detached' in ab1) {
t.true(ab1.detached);
t.false(ab2.detached);
t.false(ab3.detached);
}
t.false(iab.detached);
t.false(iab.resizable);
});

// This could have been written as a test.failing as compared to
// the immutable ArrayBuffer we'll propose. However, I'd rather test what
// the shim purposely does instead.
test('Immutable ArrayBuffer shim limitations', t => {
const ab1 = new ArrayBuffer(2);
const dv1 = new DataView(ab1);
t.is(dv1.buffer, ab1);
t.is(dv1.byteLength, 2);
const ta1 = new Uint8Array(ab1);
ta1[0] = 3;
ta1[1] = 4;
t.is(ta1.byteLength, 2);

t.throws(() => new DataView({}), { instanceOf: TypeError });
// Unfortutanely, calling a TypeArray constructor with an object that
// is not a TypeArray, ArrayBuffer, or Iterable just creates a useless
// empty TypedArray, rather than throwing.
const ta2 = new Uint8Array({});
t.is(ta2.byteLength, 0);

const iab = ab1.transferToImmutable();
t.throws(() => new DataView(iab), {
instanceOf: TypeError,
});
// Unfortunately, unlike the immutable ArrayBuffer to be proposed,
// calling a TypedArray constructor with the shim implementation of
// an immutable ArrayBuffer as argument treats it as an unrecognized object,
// rather than throwing an error.
t.is(iab.byteLength, 2);
const ta3 = new Uint8Array(iab);
t.is(ta3.byteLength, 0);
});
Loading