Improve buffer source type handling

* Properly prohibit SharedArrayBuffers by default
* Introduce the allowShared option to allow them
* Throw on detached ArrayBuffers

Note that the tests for detached ArrayBuffers are not run in Node v10, since there the worker_threads module is experimental and there is no way to create a detached ArrayBuffer.

Author: domenic

README.md

@@ -56,11 +56,11 @@ Conversions for all of the basic types from the Web IDL specification are implem
 - [`DOMString`](https://heycam.github.io/webidl/#es-DOMString), which can additionally be provided the boolean option `{ treatNullAsEmptyString }` as a second parameter
 - [`ByteString`](https://heycam.github.io/webidl/#es-ByteString), [`USVString`](https://heycam.github.io/webidl/#es-USVString)
 - [`object`](https://heycam.github.io/webidl/#es-object)
-- [Buffer source types](https://heycam.github.io/webidl/#es-buffer-source-types)
+- [Buffer source types](https://heycam.github.io/webidl/#es-buffer-source-types), which can additionally be provided with the boolean option `{ allowShared }` as a second parameter
 
 Additionally, for convenience, the following derived type definitions are implemented:
 
-- [`ArrayBufferView`](https://heycam.github.io/webidl/#ArrayBufferView)
+- [`ArrayBufferView`](https://heycam.github.io/webidl/#ArrayBufferView), which can additionally be provided with the boolean option `{ allowShared }` as a second parameter
 - [`BufferSource`](https://heycam.github.io/webidl/#BufferSource)
 - [`DOMTimeStamp`](https://heycam.github.io/webidl/#DOMTimeStamp)
 - [`Function`](https://heycam.github.io/webidl/#Function)
@@ -76,6 +76,10 @@ To mitigate this, we could return the raw BigInt value from the conversion funct
 
 On the other hand, `long long` conversion is always accurate, since the input value can never be more precise than the output value.
 
+### A note on `BufferSource` types
+
+All of the `BufferSource` types will throw when the relevant `ArrayBuffer` has been detached. This technically is not part of the [specified conversion algorithm](https://heycam.github.io/webidl/#es-buffer-source-types), but instead part of the [getting a reference/getting a copy](https://heycam.github.io/webidl/#ref-for-dfn-get-buffer-source-reference%E2%91%A0) algorithms. We've consolidated them here for convenience and ease of implementation, but if there is a need to separate them in the future, please open an issue so we can investigate.
+
 ## Background
 
 What's actually going on here, conceptually, is pretty weird. Let's try to explain.

lib/index.js

@@ -107,11 +107,7 @@ function createIntegerConversion(bitLength, typeOpts) {
     const twoToTheBitLength = Math.pow(2, bitLength);
     const twoToOneLessThanTheBitLength = Math.pow(2, bitLength - 1);
 
-    return (V, opts) => {
-        if (opts === undefined) {
-            opts = {};
-        }
-
+    return (V, opts = {}) => {
         let x = toNumber(V, opts);
         x = censorNegativeZero(x);
 
@@ -274,11 +270,7 @@ exports["unrestricted float"] = (V, opts) => {
     return Math.fround(x);
 };
 
-exports.DOMString = function (V, opts) {
-    if (opts === undefined) {
-        opts = {};
-    }
-
+exports.DOMString = function (V, opts = {}) {
     if (opts.treatNullAsEmptyString && V === null) {
         return "";
     }
@@ -352,33 +344,71 @@ function convertCallbackFunction(V, opts) {
 
 const abByteLengthGetter =
     Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, "byteLength").get;
+const sabByteLengthGetter =
+    Object.getOwnPropertyDescriptor(SharedArrayBuffer.prototype, "byteLength").get;
 
-function isArrayBuffer(V) {
+function isNonSharedArrayBuffer(V) {
     try {
+        // This will throw on SharedArrayBuffers, but not detached ArrayBuffers.
+        // (The spec says it should throw, but the spec conflicts with implementations: https://github.com/tc39/ecma262/issues/678)
         abByteLengthGetter.call(V);
+
         return true;
-    } catch (e) {
+    } catch {
+        return false;
+    }
+}
+
+function isSharedArrayBuffer(V) {
+    try {
+        sabByteLengthGetter.call(V);
+        return true;
+    } catch {
         return false;
     }
 }
 
-// I don't think we can reliably detect detached ArrayBuffers.
-exports.ArrayBuffer = (V, opts) => {
-    if (!isArrayBuffer(V)) {
-        throw makeException(TypeError, "is not a view on an ArrayBuffer object", opts);
+function isArrayBufferDetached(V) {
+    try {
+        // eslint-disable-next-line no-new
+        new Uint8Array(V);
+        return false;
+    } catch {
+        return true;
+    }
+}
+
+exports.ArrayBuffer = (V, opts = {}) => {
+    if (!isNonSharedArrayBuffer(V)) {
+        if (opts.allowShared && !isSharedArrayBuffer(V)) {
+            throw makeException(TypeError, "is not an ArrayBuffer or SharedArrayBuffer", opts);
+        }
+        throw makeException(TypeError, "is not an ArrayBuffer", opts);
     }
+    if (isArrayBufferDetached(V)) {
+        throw makeException(TypeError, "is a detached ArrayBuffer", opts);
+    }
+
     return V;
 };
 
 const dvByteLengthGetter =
     Object.getOwnPropertyDescriptor(DataView.prototype, "byteLength").get;
-exports.DataView = (V, opts) => {
+exports.DataView = (V, opts = {}) => {
     try {
         dvByteLengthGetter.call(V);
-        return V;
     } catch (e) {
-        throw makeException(TypeError, "is not a view on an DataView object", opts);
+        throw makeException(TypeError, "is not a DataView", opts);
+    }
+
+    if (!opts.allowShared && isSharedArrayBuffer(V.buffer)) {
+        throw makeException(TypeError, "is backed by a SharedArrayBuffer, which is not allowed", opts);
+    }
+    if (isArrayBufferDetached(V.buffer)) {
+        throw makeException(TypeError, "is backed by a detached ArrayBuffer", opts);
     }
+
+    return V;
 };
 
 // Returns the unforgeable `TypedArray` constructor name or `undefined`,
@@ -395,28 +425,58 @@ const typedArrayNameGetter = Object.getOwnPropertyDescriptor(
 ].forEach(func => {
     const name = func.name;
     const article = /^[AEIOU]/.test(name) ? "an" : "a";
-    exports[name] = (V, opts) => {
+    exports[name] = (V, opts = {}) => {
         if (!ArrayBuffer.isView(V) || typedArrayNameGetter.call(V) !== name) {
             throw makeException(TypeError, `is not ${article} ${name} object`, opts);
         }
+        if (!opts.allowShared && isSharedArrayBuffer(V.buffer)) {
+            throw makeException(TypeError, "is a view on a SharedArrayBuffer, which is not allowed", opts);
+        }
+        if (isArrayBufferDetached(V.buffer)) {
+            throw makeException(TypeError, "is a view on a detached ArrayBuffer", opts);
+        }
 
         return V;
     };
 });
 
 // Common definitions
 
-exports.ArrayBufferView = (V, opts) => {
+exports.ArrayBufferView = (V, opts = {}) => {
     if (!ArrayBuffer.isView(V)) {
-        throw makeException(TypeError, "is not a view on an ArrayBuffer object", opts);
+        throw makeException(TypeError, "is not a view on an ArrayBuffer or SharedArrayBuffer", opts);
     }
 
+    if (!opts.allowShared && isSharedArrayBuffer(V.buffer)) {
+        throw makeException(TypeError, "is a view on a SharedArrayBuffer, which is not allowed", opts);
+    }
+
+    if (isArrayBufferDetached(V.buffer)) {
+        throw makeException(TypeError, "is a view on a detached ArrayBuffer", opts);
+    }
     return V;
 };
 
-exports.BufferSource = (V, opts) => {
-    if (!ArrayBuffer.isView(V) && !isArrayBuffer(V)) {
-        throw makeException(TypeError, "is not an ArrayBuffer object or a view on one", opts);
+exports.BufferSource = (V, opts = {}) => {
+    if (ArrayBuffer.isView(V)) {
+        if (!opts.allowShared && isSharedArrayBuffer(V.buffer)) {
+            throw makeException(TypeError, "is a view on a SharedArrayBuffer, which is not allowed", opts);
+        }
+
+        if (isArrayBufferDetached(V.buffer)) {
+            throw makeException(TypeError, "is a view on a detached ArrayBuffer", opts);
+        }
+        return V;
+    }
+
+    if (!opts.allowShared && !isNonSharedArrayBuffer(V)) {
+        throw makeException(TypeError, "is not an ArrayBuffer or a view on one", opts);
+    }
+    if (opts.allowShared && !isSharedArrayBuffer(V) && !isNonSharedArrayBuffer(V)) {
+        throw makeException(TypeError, "is not an ArrayBuffer, SharedArrayBufer, or a view on one", opts);
+    }
+    if (isArrayBufferDetached(V)) {
+        throw makeException(TypeError, "is a detached ArrayBuffer", opts);
     }
 
     return V;