From cf949293ba55e5d8193e26623c6e9201b14cd819 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 26 Mar 2016 03:29:13 +0100 Subject: [PATCH] assert: Check typed array view type in deepEqual Do not convert typed arrays to `Buffer` for deepEqual since their values may not be accurately represented by 8-bit ints. Instead perform binary comparison of underlying `ArrayBuffer`s, but only when the array types match. Never apply any kind of optimization for floating-point typed arrays since bit pattern equality is not the right kind of check for them. PR-URL: https://github.com/nodejs/node/pull/5910 Reviewed-By: Benjamin Gruenbaum Fixes: https://github.com/nodejs/node/issues/5907 --- lib/assert.js | 17 +++++++++++++---- .../test-assert-typedarray-deepequal.js | 16 ++++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 61aba557ec2e07..b4de551e28019f 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -29,6 +29,7 @@ const compare = process.binding('buffer').compare; const util = require('util'); const Buffer = require('buffer').Buffer; const pSlice = Array.prototype.slice; +const pToString = (obj) => Object.prototype.toString.call(obj); // 1. The assert module provides functions that throw // AssertionError's when particular conditions are not met. The @@ -170,10 +171,18 @@ function _deepEqual(actual, expected, strict) { (expected === null || typeof expected !== 'object')) { return strict ? actual === expected : actual == expected; - // If both values are instances of typed arrays, wrap them in - // a Buffer each to increase performance - } else if (ArrayBuffer.isView(actual) && ArrayBuffer.isView(expected)) { - return compare(Buffer.from(actual), Buffer.from(expected)) === 0; + // If both values are instances of typed arrays, wrap their underlying + // ArrayBuffers in a Buffer each to increase performance + // This optimization requires the arrays to have the same type as checked by + // Object.prototype.toString (aka pToString). Never perform binary + // comparisons for Float*Arrays, though, since e.g. +0 === -0 but their + // bit patterns are not identical. + } else if (ArrayBuffer.isView(actual) && ArrayBuffer.isView(expected) && + pToString(actual) === pToString(expected) && + !(actual instanceof Float32Array || + actual instanceof Float64Array)) { + return compare(Buffer.from(actual.buffer), + Buffer.from(expected.buffer)) === 0; // 7.5 For all other Object pairs, including Array objects, equivalence is // determined by having the same number of owned properties (as verified diff --git a/test/parallel/test-assert-typedarray-deepequal.js b/test/parallel/test-assert-typedarray-deepequal.js index 68edefdc175317..be4462de5dff77 100644 --- a/test/parallel/test-assert-typedarray-deepequal.js +++ b/test/parallel/test-assert-typedarray-deepequal.js @@ -20,13 +20,25 @@ const equalArrayPairs = [ [new Int16Array(1e5), new Int16Array(1e5)], [new Int32Array(1e5), new Int32Array(1e5)], [new Float32Array(1e5), new Float32Array(1e5)], - [new Float64Array(1e5), new Float64Array(1e5)] + [new Float64Array(1e5), new Float64Array(1e5)], + [new Int16Array(256), new Uint16Array(256)], + [new Int16Array([256]), new Uint16Array([256])], + [new Float32Array([+0.0]), new Float32Array([-0.0])], + [new Float64Array([+0.0]), new Float32Array([-0.0])], + [new Float64Array([+0.0]), new Float64Array([-0.0])] ]; const notEqualArrayPairs = [ [new Uint8Array(2), new Uint8Array(3)], [new Uint8Array([1, 2, 3]), new Uint8Array([4, 5, 6])], - [new Uint8ClampedArray([300, 2, 3]), new Uint8Array([300, 2, 3])] + [new Uint8ClampedArray([300, 2, 3]), new Uint8Array([300, 2, 3])], + [new Uint16Array([2]), new Uint16Array([3])], + [new Uint16Array([0]), new Uint16Array([256])], + [new Int16Array([0]), new Uint16Array([256])], + [new Int16Array([-256]), new Uint16Array([0xff00])], // same bits + [new Int32Array([-256]), new Uint32Array([0xffffff00])], // ditto + [new Float32Array([0.1]), new Float32Array([0.0])], + [new Float64Array([0.1]), new Float64Array([0.0])] ]; equalArrayPairs.forEach((arrayPair) => {