From f6fd36dc5a5331cf01a9057265353925a60b7b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 22 Nov 2024 13:04:10 +0100 Subject: [PATCH 1/2] fix: Forbid usage of instanceof ArrayBuffer --- .../arraybuffer-no-instanceof.js | 29 +++++++++++++++++++ build/eslint-plugin-shaka-rules/index.js | 1 + lib/net/http_xhr_plugin.js | 1 + lib/util/buffer_utils.js | 18 +++++++----- lib/util/object_utils.js | 2 +- test/test/util/canned_idb.js | 1 + 6 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js diff --git a/build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js b/build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js new file mode 100644 index 0000000000..33b0456271 --- /dev/null +++ b/build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js @@ -0,0 +1,29 @@ +/*! @license + * Shaka Player + * Copyright 2016 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'Disallows usage of instanceof ArrayBuffer', + category: 'Best Practices', + recommended: false, + }, + schema: [], + }, + create: (ctx) => ({ + BinaryExpression: (node) => { + if (node.operator === 'instanceof' && node.right.type === 'Identifier' && + node.right.name === 'ArrayBuffer') { + ctx.report({ + node, + message: 'Do not use instanceof ArrayBuffer, consider using ' + + 'ArrayBuffer.isView() if possible', + }); + } + }, + }), +}; diff --git a/build/eslint-plugin-shaka-rules/index.js b/build/eslint-plugin-shaka-rules/index.js index 4fc4ac26aa..33482d11d4 100644 --- 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', + 'arraybuffer-no-instanceof', 'private', ]; for (const rule of RULES) { diff --git a/lib/net/http_xhr_plugin.js b/lib/net/http_xhr_plugin.js index 22e10411cd..a13716f75e 100644 --- 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/arraybuffer-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 index 55682752c1..cdb682657c 100644 --- 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 index c62eb73f98..762af37567 100644 --- 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/test/test/util/canned_idb.js b/test/test/util/canned_idb.js index 02b2772b44..02d9608e7e 100644 --- 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/arraybuffer-no-instanceof if (value instanceof ArrayBuffer) { /** @type {string} */ let data; From 03f6a24a74ea1cfd6e5c5ebe22dc974541256d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Mon, 25 Nov 2024 10:37:39 +0100 Subject: [PATCH 2/2] address DataView and TypedArrays --- ...nceof.js => buffersource-no-instanceof.js} | 19 ++++++++++++++++--- build/eslint-plugin-shaka-rules/index.js | 2 +- lib/cast/cast_utils.js | 6 ++++-- lib/net/http_xhr_plugin.js | 2 +- lib/util/uint8array_utils.js | 5 +++-- test/test/util/canned_idb.js | 2 +- 6 files changed, 26 insertions(+), 10 deletions(-) rename build/eslint-plugin-shaka-rules/{arraybuffer-no-instanceof.js => buffersource-no-instanceof.js} (51%) diff --git a/build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js b/build/eslint-plugin-shaka-rules/buffersource-no-instanceof.js similarity index 51% rename from build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js rename to build/eslint-plugin-shaka-rules/buffersource-no-instanceof.js index 33b0456271..1d39054a3e 100644 --- a/build/eslint-plugin-shaka-rules/arraybuffer-no-instanceof.js +++ b/build/eslint-plugin-shaka-rules/buffersource-no-instanceof.js @@ -8,7 +8,7 @@ module.exports = { meta: { type: 'suggestion', docs: { - description: 'Disallows usage of instanceof ArrayBuffer', + description: 'Disallows usage of instanceof on BufferSource objects', category: 'Best Practices', recommended: false, }, @@ -16,11 +16,24 @@ module.exports = { }, 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' && - node.right.name === 'ArrayBuffer') { + buffers.includes(node.right.name)) { ctx.report({ node, - message: 'Do not use instanceof ArrayBuffer, consider using ' + + 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 index 33482d11d4..3f5781cbcf 100644 --- a/build/eslint-plugin-shaka-rules/index.js +++ b/build/eslint-plugin-shaka-rules/index.js @@ -17,7 +17,7 @@ module.exports = { const RULES = [ 'arg-comment-spacing', 'array-no-instanceof', - 'arraybuffer-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 index 2557336474..3dbdee74d4 100644 --- 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 index a13716f75e..f92792052c 100644 --- a/lib/net/http_xhr_plugin.js +++ b/lib/net/http_xhr_plugin.js @@ -64,7 +64,7 @@ shaka.net.HttpXHRPlugin = class { }; xhr.onload = (event) => { const headers = shaka.net.HttpXHRPlugin.headersToGenericObject_(xhr); - // eslint-disable-next-line shaka-rules/arraybuffer-no-instanceof + // 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/uint8array_utils.js b/lib/util/uint8array_utils.js index 5e82e62d82..c2062249ff 100644 --- 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 index 02d9608e7e..f50a5a3280 100644 --- a/test/test/util/canned_idb.js +++ b/test/test/util/canned_idb.js @@ -119,7 +119,7 @@ shaka.test.CannedIDB = class { * @private */ static replacer_(dummyArrayBuffers, key, value) { - // eslint-disable-next-line shaka-rules/arraybuffer-no-instanceof + // eslint-disable-next-line shaka-rules/buffersource-no-instanceof if (value instanceof ArrayBuffer) { /** @type {string} */ let data;