fix: Forbid usage of instanceof ArrayBuffer (#7653)

Fixes #6279
shaka-project · Nov 26, 2024 · 55b7bd0 · 55b7bd0
1 parent 79b1536
commit 55b7bd0
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 13 deletions.
diff --git a/build/eslint-plugin-shaka-rules/buffersource-no-instanceof.js b/build/eslint-plugin-shaka-rules/buffersource-no-instanceof.js
@@ -0,0 +1,42 @@
+/*! @license
+ * Shaka Player
+ * Copyright 2016 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+module.exports = {
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'Disallows usage of instanceof on BufferSource objects',
+      category: 'Best Practices',
+      recommended: false,
+    },
+    schema: [],
+  },
+  create: (ctx) => ({
+    BinaryExpression: (node) => {
+      const buffers = [
+        'ArrayBuffer',
+        'DataView',
+        'Uint8Array',
+        'Uint8ClampedArray',
+        'Int8Array',
+        'Uint16Array',
+        'Int16Array',
+        'Uint32Array',
+        'Int32Array',
+        'Float32Array',
+        'Float64Array',
+      ];
+      if (node.operator === 'instanceof' && node.right.type === 'Identifier' &&
+          buffers.includes(node.right.name)) {
+        ctx.report({
+          node,
+          message: `Do not use instanceof ${node.right.name}, consider using ` +
+            'ArrayBuffer.isView() if possible',
+        });
+      }
+    },
+  }),
+};
diff --git a/build/eslint-plugin-shaka-rules/index.js b/build/eslint-plugin-shaka-rules/index.js
@@ -17,6 +17,7 @@ module.exports = {
 const RULES = [
   'arg-comment-spacing',
   'array-no-instanceof',
+  'buffersource-no-instanceof',
   'private',
 ];
 for (const rule of RULES) {

diff --git a/lib/cast/cast_utils.js b/lib/cast/cast_utils.js
@@ -66,10 +66,12 @@ shaka.cast.CastUtils = class {
         return shaka.cast.CastUtils.unpackTimeRanges_(value);
       }
 
-      if (value instanceof Uint8Array) {
+      if (ArrayBuffer.isView(value) &&
+      /** @type {TypedArray} */(value).BYTES_PER_ELEMENT === 1) {
         // Some of our code cares about Uint8Arrays actually being Uint8Arrays,
         // so this gives them special treatment.
-        return shaka.cast.CastUtils.unpackUint8Array_(value);
+        return shaka.cast.CastUtils.unpackUint8Array_(
+            /** @type {!Uint8Array} */(value));
       }
 
       if (typeof value == 'number') {

diff --git a/lib/net/http_xhr_plugin.js b/lib/net/http_xhr_plugin.js
@@ -64,6 +64,7 @@ shaka.net.HttpXHRPlugin = class {
       };
       xhr.onload = (event) => {
         const headers = shaka.net.HttpXHRPlugin.headersToGenericObject_(xhr);
+        // eslint-disable-next-line shaka-rules/buffersource-no-instanceof
         goog.asserts.assert(xhr.response instanceof ArrayBuffer,
             'XHR should have a response by now!');
         const xhrResponse = xhr.response;

diff --git a/lib/util/buffer_utils.js b/lib/util/buffer_utils.js
@@ -62,10 +62,10 @@ shaka.util.BufferUtils = class {
    * @private
    */
   static unsafeGetArrayBuffer_(view) {
-    if (view instanceof ArrayBuffer) {
-      return view;
+    if (!ArrayBuffer.isView(view)) {
+      return /** @type {!ArrayBuffer} */(view);
     } else {
-      return view.buffer;
+      return /** @type {!ArrayBufferView} */(view).buffer;
     }
   }
 
@@ -79,17 +79,19 @@ shaka.util.BufferUtils = class {
    * @export
    */
   static toArrayBuffer(view) {
-    if (view instanceof ArrayBuffer) {
-      return view;
+    if (!ArrayBuffer.isView(view)) {
+      return /** @type {!ArrayBuffer} */(view);
     } else {
-      if (view.byteOffset == 0 && view.byteLength == view.buffer.byteLength) {
+      const aView = /** @type {!ArrayBufferView} */(view);
+      if (aView.byteOffset == 0 &&
+          aView.byteLength == aView.buffer.byteLength) {
         // This is a TypedArray over the whole buffer.
-        return view.buffer;
+        return aView.buffer;
       }
       // This is a "view" on the buffer.  Create a new buffer that only contains
       // the data.  Note that since this isn't an ArrayBuffer, the "new" call
       // will allocate a new buffer to hold the copy.
-      return new Uint8Array(view).buffer;
+      return new Uint8Array(aView).buffer;
     }
   }
 

diff --git a/lib/util/object_utils.js b/lib/util/object_utils.js
@@ -41,7 +41,7 @@ shaka.util.ObjectUtils = class {
 
           // This covers Uint8Array and friends, even without a TypedArray
           // base-class constructor.
-          const isTypedArray = val.buffer instanceof ArrayBuffer;
+          const isTypedArray = ArrayBuffer.isView(val);
           if (isTypedArray) {
             return val;
           }

diff --git a/lib/util/uint8array_utils.js b/lib/util/uint8array_utils.js
@@ -119,8 +119,9 @@ shaka.util.Uint8ArrayUtils = class {
 
     for (let i = 0; i < varArgs.length; ++i) {
       const value = varArgs[i];
-      if (value instanceof Uint8Array) {
-        result.set(value, offset);
+      if (ArrayBuffer.isView(value) &&
+      /** @type {TypedArray} */ (value).BYTES_PER_ELEMENT === 1) {
+        result.set(/** @type {!Uint8Array} */(value), offset);
       } else {
         result.set(BufferUtils.toUint8(value), offset);
       }

diff --git a/test/test/util/canned_idb.js b/test/test/util/canned_idb.js
@@ -119,6 +119,7 @@ shaka.test.CannedIDB = class {
    * @private
    */
   static replacer_(dummyArrayBuffers, key, value) {
+    // eslint-disable-next-line shaka-rules/buffersource-no-instanceof
     if (value instanceof ArrayBuffer) {
       /** @type {string} */
       let data;