fixup! lib: refactor internal webidl converters

Author: panva

lib/internal/webidl.js

@@ -32,6 +32,7 @@ const {
 
 const { kEmptyObject } = require('internal/util');
 const {
+  isSharedArrayBuffer,
   isTypedArray,
 } = require('internal/util/types');
 
@@ -47,7 +48,8 @@ const converters = { __proto__: null };
  * @property {string} [code] Node.js error code to assign to TypeError.
  * @property {boolean} [enforceRange] Web IDL [EnforceRange] attribute.
  * @property {boolean} [clamp] Web IDL [Clamp] attribute.
- * @property {boolean} [allowShared] Web IDL [AllowShared] attribute.
+ * @property {boolean} [allowShared] Web IDL [AllowShared] attribute for
+ *   buffer view types.
  * @property {boolean} [allowResizable] Web IDL [AllowResizable] attribute.
  */
 
@@ -70,7 +72,7 @@ const converters = { __proto__: null };
  * @property {string} key Dictionary member identifier.
  * @property {Converter} converter Converter for the member type.
  * @property {boolean} [required] Whether the member is required.
- * @property {Function} [defaultValue] Function returning the default value.
+ * @property {() => any} [defaultValue] Function returning the default value.
  * @property {DictionaryMemberValidator} [validator] Optional Node.js
  *   extension point invoked after conversion and before storing the member.
  *   This is for early semantic validation of known unsupported IDL values,
@@ -827,7 +829,7 @@ function createInterfaceConverter(name, prototype) {
 /**
  * Returns the ArrayBuffer or SharedArrayBuffer viewed by a typed array or
  * DataView.
- * @param {ArrayBufferView} V ArrayBuffer view.
+ * @param {ArrayBufferView} V TypedArray or DataView.
  * @returns {ArrayBuffer|SharedArrayBuffer}
  */
 function getViewedArrayBuffer(V) {
@@ -848,7 +850,8 @@ function validateBufferSourceBacking(buffer, options) {
     // ArrayBuffer.prototype.resizable is an ArrayBuffer brand check and the
     // [AllowResizable] value we need. For SharedArrayBuffer-backed views it
     // throws, which lets this path avoid a separate IsSharedArrayBuffer check.
-    // Keep this in sync with the inlined BufferSource view branch below.
+    // BufferSource has separate inline logic because [AllowShared] cannot be
+    // used with that typedef.
     resizable = ArrayBufferPrototypeGetResizable(buffer);
   } catch {
     // ArrayBufferView conversion step 2: reject SharedArrayBuffer
@@ -936,32 +939,32 @@ converters.Uint8Array = (V, options = kEmptyObject) => {
  * @see https://webidl.spec.whatwg.org/#BufferSource
  * @param {any} V JavaScript value.
  * @param {ConversionOptions} [options] Conversion options.
- * @returns {ArrayBuffer|ArrayBufferView}
+ * @returns {ArrayBuffer|ArrayBufferView<ArrayBuffer>}
  */
 converters.BufferSource = (V, options = kEmptyObject) => {
   // BufferSource is a typedef for (ArrayBufferView or ArrayBuffer).
+  // [AllowShared] cannot be used with BufferSource because the ArrayBuffer
+  // union branch does not support it. Use AllowSharedBufferSource instead.
   if (ArrayBufferIsView(V)) {
     const buffer = getViewedArrayBuffer(V);
     // ArrayBufferView conversion steps 2-4: validate the viewed buffer
     // and return a reference to the same view.
     // Keep this logic inline instead of calling validateBufferSourceBacking().
     // BufferSource conversion is hot, and this avoids a helper call while still
     // using a primordial getter for the backing-buffer internal-slot check.
-    // Keep this in sync with validateBufferSourceBacking().
+    // Unlike validateBufferSourceBacking(), this intentionally ignores
+    // options.allowShared because [AllowShared] cannot be used with
+    // BufferSource.
     let resizable;
     try {
       // ArrayBuffer.prototype.resizable is both the ArrayBuffer brand check
-      // and the step 3 value. It throws for SharedArrayBuffer, which gives us
-      // the step 2 branch without an extra byteLength getter call.
+      // and the step 3 value. It throws for SharedArrayBuffer, which lets this
+      // path reject SAB-backed views without an extra byteLength getter call.
       resizable = ArrayBufferPrototypeGetResizable(buffer);
     } catch {
-      if (!options.allowShared) {
-        throw makeException(
-          'is a view on a SharedArrayBuffer, which is not allowed.',
-          options);
-      }
-      validateAllowGrowableSharedArrayBuffer(buffer, options);
-      return V;
+      throw makeException(
+        'is a view on a SharedArrayBuffer, which is not allowed.',
+        options);
     }
     if (resizable && !options.allowResizable) {
       throw makeException(
@@ -997,6 +1000,48 @@ converters.BufferSource = (V, options = kEmptyObject) => {
   return V;
 };
 
+/**
+ * Converts a JavaScript value to the IDL AllowSharedBufferSource typedef.
+ * @see https://webidl.spec.whatwg.org/#AllowSharedBufferSource
+ * @param {any} V JavaScript value.
+ * @param {ConversionOptions} [options] Conversion options.
+ * @returns {ArrayBuffer|SharedArrayBuffer|ArrayBufferView}
+ */
+converters.AllowSharedBufferSource = (V, options = kEmptyObject) => {
+  // AllowSharedBufferSource is a typedef for
+  // (ArrayBuffer or SharedArrayBuffer or [AllowShared] ArrayBufferView).
+  // The union branches are disjoint, so this keeps the hot view path first.
+  if (ArrayBufferIsView(V)) {
+    const buffer = getViewedArrayBuffer(V);
+    let resizable;
+    try {
+      resizable = ArrayBufferPrototypeGetResizable(buffer);
+    } catch {
+      validateAllowGrowableSharedArrayBuffer(buffer, options);
+      return V;
+    }
+    validateAllowResizableArrayBuffer(resizable, options);
+    return V;
+  }
+
+  let resizable;
+  try {
+    resizable = ArrayBufferPrototypeGetResizable(V);
+  } catch {
+    if (isSharedArrayBuffer(V)) {
+      validateAllowGrowableSharedArrayBuffer(V, options);
+      return V;
+    }
+    throw makeException(
+      'is not instance of ArrayBuffer, SharedArrayBuffer, Buffer, ' +
+      'TypedArray, or DataView.',
+      options);
+  }
+
+  validateAllowResizableArrayBuffer(resizable, options);
+  return V;
+};
+
 /**
  * Converts a JavaScript value to the IDL sequence<DOMString> type.
  * @see https://webidl.spec.whatwg.org/#es-sequence

test/parallel/test-internal-webidl-buffer-source.js

@@ -17,12 +17,17 @@ const TYPED_ARRAY_CTORS = [
 ];
 
 function createGrowableSharedArrayBufferView(Ctor) {
-  const buffer = new SharedArrayBuffer(0, { maxByteLength: 1 });
+  const buffer = createGrowableSharedArrayBuffer();
   const view = new Ctor(buffer);
-  assert.strictEqual(view.buffer.growable, true);
   return view;
 }
 
+function createGrowableSharedArrayBuffer() {
+  const buffer = new SharedArrayBuffer(0, { maxByteLength: 1 });
+  assert.strictEqual(buffer.growable, true);
+  return buffer;
+}
+
 test('BufferSource accepts ArrayBuffer', () => {
   const ab = new ArrayBuffer(8);
   assert.strictEqual(converters.BufferSource(ab), ab);
@@ -104,6 +109,10 @@ test('BufferSource rejects SAB-backed TypedArray', () => {
     () => converters.BufferSource(view),
     { code: 'ERR_INVALID_ARG_TYPE' },
   );
+  assert.throws(
+    () => converters.BufferSource(view, { allowShared: true }),
+    { code: 'ERR_INVALID_ARG_TYPE' },
+  );
 });
 
 test('BufferSource rejects SAB-backed DataView', () => {
@@ -112,6 +121,10 @@ test('BufferSource rejects SAB-backed DataView', () => {
     () => converters.BufferSource(dv),
     { code: 'ERR_INVALID_ARG_TYPE' },
   );
+  assert.throws(
+    () => converters.BufferSource(dv, { allowShared: true }),
+    { code: 'ERR_INVALID_ARG_TYPE' },
+  );
 });
 
 test('BufferSource rejects SAB view whose buffer prototype was reassigned', () => {
@@ -166,26 +179,96 @@ test('BufferSource handles resizable-backed views with explicit options', () =>
   }
 });
 
-test('BufferSource handles growable SAB-backed views with explicit options', () => {
+test('BufferSource rejects SAB-backed views with explicit options', () => {
   for (const Ctor of [DataView, ...TYPED_ARRAY_CTORS]) {
-    {
-      const view = createGrowableSharedArrayBufferView(Ctor);
-      assert.throws(
-        () => converters.BufferSource(view, {
-          allowShared: true,
-          allowResizable: false,
-        }),
-        { code: 'ERR_INVALID_ARG_TYPE' },
-      );
-    }
-
-    {
-      const view = createGrowableSharedArrayBufferView(Ctor);
-      assert.strictEqual(converters.BufferSource(view, {
+    const view = createGrowableSharedArrayBufferView(Ctor);
+    assert.throws(
+      () => converters.BufferSource(view, {
         allowShared: true,
         allowResizable: true,
-      }), view);
-    }
+      }),
+      { code: 'ERR_INVALID_ARG_TYPE' },
+    );
+  }
+});
+
+test('AllowSharedBufferSource accepts ArrayBuffer and SharedArrayBuffer', () => {
+  const ab = new ArrayBuffer(8);
+  const sab = new SharedArrayBuffer(8);
+
+  assert.strictEqual(converters.AllowSharedBufferSource(ab), ab);
+  assert.strictEqual(converters.AllowSharedBufferSource(sab), sab);
+});
+
+test('AllowSharedBufferSource accepts cross-realm buffers', () => {
+  const context = vm.createContext();
+  const ab = vm.runInContext('new ArrayBuffer(0)', context);
+  const sab = vm.runInContext('new SharedArrayBuffer(0)', context);
+
+  assert.strictEqual(converters.AllowSharedBufferSource(ab), ab);
+  assert.strictEqual(converters.AllowSharedBufferSource(sab), sab);
+});
+
+test('AllowSharedBufferSource accepts ArrayBuffer and SharedArrayBuffer views', () => {
+  const abView = new Uint8Array(new ArrayBuffer(8));
+  const sabView = new Uint8Array(new SharedArrayBuffer(8));
+  const abDataView = new DataView(new ArrayBuffer(8));
+  const sabDataView = new DataView(new SharedArrayBuffer(8));
+
+  assert.strictEqual(converters.AllowSharedBufferSource(abView), abView);
+  assert.strictEqual(converters.AllowSharedBufferSource(sabView), sabView);
+  assert.strictEqual(
+    converters.AllowSharedBufferSource(abDataView), abDataView);
+  assert.strictEqual(
+    converters.AllowSharedBufferSource(sabDataView), sabDataView);
+});
+
+test('AllowSharedBufferSource handles resizable buffers with explicit options', () => {
+  const ab = new ArrayBuffer(0, { maxByteLength: 1 });
+  const views = [new Uint8Array(ab), new DataView(ab)];
+
+  assert.throws(
+    () => converters.AllowSharedBufferSource(ab),
+    { code: 'ERR_INVALID_ARG_TYPE' },
+  );
+  for (const view of views) {
+    assert.throws(
+      () => converters.AllowSharedBufferSource(view),
+      { code: 'ERR_INVALID_ARG_TYPE' },
+    );
+  }
+  assert.strictEqual(converters.AllowSharedBufferSource(ab, {
+    allowResizable: true,
+  }), ab);
+  for (const view of views) {
+    assert.strictEqual(converters.AllowSharedBufferSource(view, {
+      allowResizable: true,
+    }), view);
+  }
+});
+
+test('AllowSharedBufferSource handles growable shared buffers with explicit ' +
+     'options', () => {
+  const sab = createGrowableSharedArrayBuffer();
+  const views = [new Uint8Array(sab), new DataView(sab)];
+
+  assert.throws(
+    () => converters.AllowSharedBufferSource(sab),
+    { code: 'ERR_INVALID_ARG_TYPE' },
+  );
+  for (const view of views) {
+    assert.throws(
+      () => converters.AllowSharedBufferSource(view),
+      { code: 'ERR_INVALID_ARG_TYPE' },
+    );
+  }
+  assert.strictEqual(converters.AllowSharedBufferSource(sab, {
+    allowResizable: true,
+  }), sab);
+  for (const view of views) {
+    assert.strictEqual(converters.AllowSharedBufferSource(view, {
+      allowResizable: true,
+    }), view);
   }
 });
 
@@ -205,4 +288,11 @@ for (const value of [null, undefined, 0, 1, 1n, '', 'x', true, Symbol('s'), [],
       { code: 'ERR_INVALID_ARG_TYPE' },
     );
   });
+
+  test(`AllowSharedBufferSource rejects ${typeof value} ${String(value)}`, () => {
+    assert.throws(
+      () => converters.AllowSharedBufferSource(value),
+      { code: 'ERR_INVALID_ARG_TYPE' },
+    );
+  });
 }

test/parallel/test-internal-webidl-converttoint.js

@@ -30,6 +30,10 @@ assert.strictEqual(convertToInt(1, 64), 1);
 assert.strictEqual(convertToInt(-0.5, 64), 0);
 assert.strictEqual(convertToInt(-0.5, 64, 'signed'), 0);
 assert.strictEqual(convertToInt(-1.5, 64, 'signed'), -1);
+assert.strictEqual(convertToInt(2 ** 12 + 1, 12), 1);
+assert.strictEqual(convertToInt(-1, 12), 2 ** 12 - 1);
+assert.strictEqual(convertToInt(2 ** 11, 12, 'signed'), -(2 ** 11));
+assert.strictEqual(convertToInt(-1, 12, 'signed'), -1);
 
 {
   const options = {
@@ -314,6 +318,23 @@ assert.throws(() => convertToInt(256, 8, 'unsigned', {
   assert.deepStrictEqual(calls, ['enforceRange', 'clamp']);
 }
 
+{
+  const calls = [];
+  const options = {
+    get enforceRange() {
+      calls.push('enforceRange');
+      return false;
+    },
+    get clamp() {
+      calls.push('clamp');
+      return true;
+    },
+  };
+
+  assert.strictEqual(convertToInt({ valueOf: () => 2.5 }, 8, 'unsigned', options), 2);
+  assert.deepStrictEqual(calls, ['enforceRange', 'clamp']);
+}
+
 // Out of range: clamp
 assert.strictEqual(convertToInt(NaN, 64, 'unsigned', { clamp: true }), 0);
 assert.strictEqual(convertToInt(Infinity, 64, 'unsigned', { clamp: true }), Number.MAX_SAFE_INTEGER);